Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(logging): scope log messages to specific LSP using topic headers. #6526

Open
wants to merge 7 commits into
base: feature/amazonqLSP
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions packages/amazonq/src/lsp/lspInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,18 @@ import {
LspResolution,
getNodeExecutableName,
cleanLspDownloads,
getLogger,
} from 'aws-core-vscode/shared'
import path from 'path'

const manifestURL = 'https://aws-toolkit-language-servers.amazonaws.com/codewhisperer/0/manifest.json'
export const supportedLspServerVersions = '^2.3.0'

const logger = getLogger('QLSP')
export class AmazonQLSPResolver implements LspResolver {
async resolve(): Promise<LspResolution> {
const overrideLocation = process.env.AWS_LANGUAGE_SERVER_OVERRIDE
if (overrideLocation) {
logger.info(`Using language server override location: ${overrideLocation}`)
void vscode.window.showInformationMessage(`Using language server override location: ${overrideLocation}`)
return {
assetDirectory: overrideLocation,
Expand All @@ -47,7 +49,12 @@ export class AmazonQLSPResolver implements LspResolver {
const nodePath = path.join(installationResult.assetDirectory, `servers/${getNodeExecutableName()}`)
await fs.chmod(nodePath, 0o755)

await cleanLspDownloads(manifest.versions, path.dirname(installationResult.assetDirectory))
const deletedVersions = await cleanLspDownloads(
manifest.versions,
path.dirname(installationResult.assetDirectory)
)
logger.debug(`cleaning old LSP versions deleted ${deletedVersions.length} versions`)

return {
...installationResult,
resourcePaths: {
Expand Down
25 changes: 13 additions & 12 deletions packages/core/src/amazonq/lsp/lspController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export interface BuildIndexConfig {
}

/*
* LSP Controller manages the status of Amazon Q LSP:
* LSP Controller manages the status of Amazon Q Workspace Indexing LSP:
* 1. Downloading, verifying and installing LSP using DEXP LSP manifest and CDN.
* 2. Managing the LSP states. There are a couple of possible LSP states:
* Not installed. Installed. Running. Indexing. Indexing Done.
Expand All @@ -44,6 +44,7 @@ export interface BuildIndexConfig {
export class LspController {
static #instance: LspController
private _isIndexingInProgress = false
private logger = getLogger('QWorkspaceLsp')

public static get instance() {
return (this.#instance ??= new this())
Expand Down Expand Up @@ -83,18 +84,18 @@ export class LspController {
return await LspClient.instance.queryInlineProjectContext(query, path, target)
} catch (e) {
if (e instanceof Error) {
getLogger().error(`unexpected error while querying inline project context, e=${e.message}`)
this.logger.error(`unexpected error while querying inline project context, e=${e.message}`)
}
return []
}
}

async buildIndex(buildIndexConfig: BuildIndexConfig) {
getLogger().info(`LspController: Starting to build index of project`)
this.logger.info(`LspController: Starting to build index of project`)
const start = performance.now()
const projPaths = (vscode.workspace.workspaceFolders ?? []).map((folder) => folder.uri.fsPath)
if (projPaths.length === 0) {
getLogger().info(`LspController: Skipping building index. No projects found in workspace`)
this.logger.info(`LspController: Skipping building index. No projects found in workspace`)
return
}
projPaths.sort()
Expand All @@ -111,12 +112,12 @@ export class LspController {
(accumulator, currentFile) => accumulator + currentFile.fileSizeBytes,
0
)
getLogger().info(`LspController: Found ${files.length} files in current project ${projPaths}`)
this.logger.info(`LspController: Found ${files.length} files in current project ${projPaths}`)
const config = buildIndexConfig.isVectorIndexEnabled ? 'all' : 'default'
const r = files.map((f) => f.fileUri.fsPath)
const resp = await LspClient.instance.buildIndex(r, projRoot, config)
if (resp) {
getLogger().debug(`LspController: Finish building index of project`)
this.logger.debug(`LspController: Finish building index of project`)
const usage = await LspClient.instance.getLspServerUsage()
telemetry.amazonq_indexWorkspace.emit({
duration: performance.now() - start,
Expand All @@ -128,7 +129,7 @@ export class LspController {
credentialStartUrl: buildIndexConfig.startUrl,
})
} else {
getLogger().error(`LspController: Failed to build index of project`)
this.logger.error(`LspController: Failed to build index of project`)
telemetry.amazonq_indexWorkspace.emit({
duration: performance.now() - start,
result: 'Failed',
Expand All @@ -139,7 +140,7 @@ export class LspController {
}
} catch (error) {
// TODO: use telemetry.run()
getLogger().error(`LspController: Failed to build index of project`)
this.logger.error(`LspController: Failed to build index of project`)
telemetry.amazonq_indexWorkspace.emit({
duration: performance.now() - start,
result: 'Failed',
Expand All @@ -155,7 +156,7 @@ export class LspController {

async trySetupLsp(context: vscode.ExtensionContext, buildIndexConfig: BuildIndexConfig) {
if (isCloud9() || isWeb() || isAmazonInternalOs()) {
getLogger().warn('LspController: Skipping LSP setup. LSP is not compatible with the current environment. ')
this.logger.warn('LspController: Skipping LSP setup. LSP is not compatible with the current environment. ')
// do not do anything if in Cloud9 or Web mode or in AL2 (AL2 does not support node v18+)
return
}
Expand All @@ -168,7 +169,7 @@ export class LspController {
async () => {
const usage = await LspClient.instance.getLspServerUsage()
if (usage) {
getLogger().info(
this.logger.info(
`LspController: LSP server CPU ${usage.cpuUsage}%, LSP server Memory ${
usage.memoryUsage / (1024 * 1024)
}MB `
Expand All @@ -178,7 +179,7 @@ export class LspController {
30 * 60 * 1000
)
} catch (e) {
getLogger().error(`LspController: LSP failed to activate ${e}`)
this.logger.error(`LspController: LSP failed to activate ${e}`)
}
})
}
Expand All @@ -187,7 +188,7 @@ export class LspController {
await lspSetupStage('all', async () => {
const installResult = await new WorkspaceLSPResolver().resolve()
await lspSetupStage('launch', async () => activateLsp(context, installResult.resourcePaths))
getLogger().info('LspController: LSP activated')
this.logger.info('LspController: LSP activated')
})
}
}
9 changes: 7 additions & 2 deletions packages/core/src/amazonq/lsp/workspaceInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ import { LanguageServerResolver } from '../../shared/lsp/lspResolver'
import { Range } from 'semver'
import { getNodeExecutableName } from '../../shared/lsp/utils/platform'
import { fs } from '../../shared/fs/fs'
import { cleanLspDownloads } from '../../shared'
import { cleanLspDownloads, getLogger } from '../../shared'

const manifestUrl = 'https://aws-toolkit-language-servers.amazonaws.com/q-context/manifest.json'
// this LSP client in Q extension is only going to work with these LSP server versions
const supportedLspServerVersions = '0.1.35'
const logger = getLogger('QWorkspaceLsp')

export class WorkspaceLSPResolver implements LspResolver {
async resolve(): Promise<LspResolution> {
Expand All @@ -31,7 +32,11 @@ export class WorkspaceLSPResolver implements LspResolver {
const nodePath = path.join(installationResult.assetDirectory, nodeName)
await fs.chmod(nodePath, 0o755)

await cleanLspDownloads(manifest.versions, path.basename(installationResult.assetDirectory))
const deletedVersions = await cleanLspDownloads(
manifest.versions,
path.basename(installationResult.assetDirectory)
)
logger.debug(`cleaning old LSP versions deleted ${deletedVersions.length} versions`)
Hweinstock marked this conversation as resolved.
Show resolved Hide resolved
return {
...installationResult,
resourcePaths: {
Expand Down
11 changes: 10 additions & 1 deletion packages/core/src/shared/logger/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,16 @@

import * as vscode from 'vscode'

export type LogTopic = 'crashMonitoring' | 'dev/beta' | 'notifications' | 'test' | 'childProcess' | 'lsp' | 'unknown'
export type LogTopic =
| 'crashMonitoring'
| 'dev/beta'
| 'notifications'
| 'test'
| 'childProcess'
| 'lsp'
| 'QWorkspaceLsp'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should these be amazonqWorkspaceLSP or is that too long? I feel like we need to have conventions on whether we use q vs amazonq since both are used in the codebase 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That feels more consistent. Better to start verbose and look for opportunities to shorten later.

| 'QLSP'
| 'unknown'

class ErrorLog {
constructor(
Expand Down
13 changes: 10 additions & 3 deletions packages/core/src/shared/lsp/utils/cleanup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,23 +19,30 @@ function isDelisted(manifestVersions: LspVersion[], targetVersion: string): bool

/**
* Delete all delisted versions and keep the two newest versions that remain
* @param manifest
* @param manifestVersions
* @param downloadDirectory
* @returns array of deleted versions.
*/
export async function cleanLspDownloads(manifestVersions: LspVersion[], downloadDirectory: string): Promise<void> {
export async function cleanLspDownloads(manifestVersions: LspVersion[], downloadDirectory: string): Promise<string[]> {
const downloadedVersions = await getDownloadedVersions(downloadDirectory)
const [delistedVersions, remainingVersions] = partition(downloadedVersions, (v: string) =>
isDelisted(manifestVersions, v)
)
const deletedVersions: string[] = []

for (const v of delistedVersions) {
await fs.delete(path.join(downloadDirectory, v), { force: true, recursive: true })
deletedVersions.push(v)
}

if (remainingVersions.length <= 2) {
return
return deletedVersions
}

for (const v of sort(remainingVersions).slice(0, -2)) {
await fs.delete(path.join(downloadDirectory, v), { force: true, recursive: true })
deletedVersions.push(v)
}

return deletedVersions
}
21 changes: 16 additions & 5 deletions packages/core/src/test/shared/lsp/utils/cleanup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,27 +41,32 @@ describe('cleanLSPDownloads', function () {

it('keeps two newest versions', async function () {
await fakeInstallVersions(['1.0.0', '1.0.1', '1.1.1', '2.1.1'], installationDir.fsPath)
await cleanLspDownloads([], installationDir.fsPath)
const deleted = await cleanLspDownloads([], installationDir.fsPath)

const result = (await fs.readdir(installationDir.fsPath)).map(([filename, _filetype], _index) => filename)
assert.strictEqual(result.length, 2)
assert.ok(result.includes('2.1.1'))
assert.ok(result.includes('1.1.1'))
assert.strictEqual(deleted.length, 2)
})

it('deletes delisted versions', async function () {
await fakeInstallVersions(['1.0.0', '1.0.1', '1.1.1', '2.1.1'], installationDir.fsPath)
await cleanLspDownloads([{ serverVersion: '1.1.1', isDelisted: true, targets: [] }], installationDir.fsPath)
const deleted = await cleanLspDownloads(
[{ serverVersion: '1.1.1', isDelisted: true, targets: [] }],
installationDir.fsPath
)

const result = (await fs.readdir(installationDir.fsPath)).map(([filename, _filetype], _index) => filename)
assert.strictEqual(result.length, 2)
assert.ok(result.includes('2.1.1'))
assert.ok(result.includes('1.0.1'))
assert.strictEqual(deleted.length, 2)
})

it('handles case where less than 2 versions are not delisted', async function () {
await fakeInstallVersions(['1.0.0', '1.0.1', '1.1.1', '2.1.1'], installationDir.fsPath)
await cleanLspDownloads(
const deleted = await cleanLspDownloads(
[
{ serverVersion: '1.1.1', isDelisted: true, targets: [] },
{ serverVersion: '2.1.1', isDelisted: true, targets: [] },
Expand All @@ -73,21 +78,27 @@ describe('cleanLSPDownloads', function () {
const result = (await fs.readdir(installationDir.fsPath)).map(([filename, _filetype], _index) => filename)
assert.strictEqual(result.length, 1)
assert.ok(result.includes('1.0.1'))
assert.strictEqual(deleted.length, 3)
})

it('handles case where less than 2 versions exist', async function () {
await fakeInstallVersions(['1.0.0'], installationDir.fsPath)
await cleanLspDownloads([], installationDir.fsPath)
const deleted = await cleanLspDownloads([], installationDir.fsPath)

const result = (await fs.readdir(installationDir.fsPath)).map(([filename, _filetype], _index) => filename)
assert.strictEqual(result.length, 1)
assert.strictEqual(deleted.length, 0)
})

it('does not install delisted version when no other option exists', async function () {
await fakeInstallVersions(['1.0.0'], installationDir.fsPath)
await cleanLspDownloads([{ serverVersion: '1.0.0', isDelisted: true, targets: [] }], installationDir.fsPath)
const deleted = await cleanLspDownloads(
[{ serverVersion: '1.0.0', isDelisted: true, targets: [] }],
installationDir.fsPath
)

const result = (await fs.readdir(installationDir.fsPath)).map(([filename, _filetype], _index) => filename)
assert.strictEqual(result.length, 0)
assert.strictEqual(deleted.length, 1)
})
})