Skip to content

Commit ed3707a

Browse files
authored
refactor(logging): scope log messages to specific LSP using topic headers. (#6526)
## Problem The log messages coming out of the LSP work do not make it clear which LSP its working with (Q Workspace or Q general). Follow up from: #6385 (comment) ## Solution - Use new `LogTopic` functionality to scope these to specific LSP. - Add more logging information from LSP cleanup process. --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 2008dbd commit ed3707a

File tree

6 files changed

+59
-25
lines changed

6 files changed

+59
-25
lines changed

packages/amazonq/src/lsp/lspInstaller.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
LspResolution,
1313
getNodeExecutableName,
1414
cleanLspDownloads,
15+
getLogger,
1516
} from 'aws-core-vscode/shared'
1617
import path from 'path'
1718
import { Range } from 'semver'
@@ -20,11 +21,13 @@ export const manifestURL = 'https://aws-toolkit-language-servers.amazonaws.com/c
2021
export const supportedLspServerVersions = new Range('^3.1.1', {
2122
includePrerelease: true,
2223
})
24+
const logger = getLogger('amazonqLsp')
2325

2426
export class AmazonQLSPResolver implements LspResolver {
2527
async resolve(): Promise<LspResolution> {
2628
const overrideLocation = process.env.AWS_LANGUAGE_SERVER_OVERRIDE
2729
if (overrideLocation) {
30+
logger.info(`Using language server override location: ${overrideLocation}`)
2831
void vscode.window.showInformationMessage(`Using language server override location: ${overrideLocation}`)
2932
return {
3033
assetDirectory: overrideLocation,
@@ -49,7 +52,12 @@ export class AmazonQLSPResolver implements LspResolver {
4952
const nodePath = path.join(installationResult.assetDirectory, `servers/${getNodeExecutableName()}`)
5053
await fs.chmod(nodePath, 0o755)
5154

52-
await cleanLspDownloads(manifest.versions, path.dirname(installationResult.assetDirectory))
55+
const deletedVersions = await cleanLspDownloads(
56+
manifest.versions,
57+
path.dirname(installationResult.assetDirectory)
58+
)
59+
logger.debug(`Cleaned up ${deletedVersions.length} old versions`)
60+
5361
return {
5462
...installationResult,
5563
resourcePaths: {

packages/core/src/amazonq/lsp/lspController.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export interface BuildIndexConfig {
3131
}
3232

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

4849
public static get instance() {
4950
return (this.#instance ??= new this())
@@ -83,18 +84,18 @@ export class LspController {
8384
return await LspClient.instance.queryInlineProjectContext(query, path, target)
8485
} catch (e) {
8586
if (e instanceof Error) {
86-
getLogger().error(`unexpected error while querying inline project context, e=${e.message}`)
87+
this.logger.error(`unexpected error while querying inline project context, e=${e.message}`)
8788
}
8889
return []
8990
}
9091
}
9192

9293
async buildIndex(buildIndexConfig: BuildIndexConfig) {
93-
getLogger().info(`LspController: Starting to build index of project`)
94+
this.logger.info(`LspController: Starting to build index of project`)
9495
const start = performance.now()
9596
const projPaths = (vscode.workspace.workspaceFolders ?? []).map((folder) => folder.uri.fsPath)
9697
if (projPaths.length === 0) {
97-
getLogger().info(`LspController: Skipping building index. No projects found in workspace`)
98+
this.logger.info(`LspController: Skipping building index. No projects found in workspace`)
9899
return
99100
}
100101
projPaths.sort()
@@ -111,12 +112,12 @@ export class LspController {
111112
(accumulator, currentFile) => accumulator + currentFile.fileSizeBytes,
112113
0
113114
)
114-
getLogger().info(`LspController: Found ${files.length} files in current project ${projPaths}`)
115+
this.logger.info(`LspController: Found ${files.length} files in current project ${projPaths}`)
115116
const config = buildIndexConfig.isVectorIndexEnabled ? 'all' : 'default'
116117
const r = files.map((f) => f.fileUri.fsPath)
117118
const resp = await LspClient.instance.buildIndex(r, projRoot, config)
118119
if (resp) {
119-
getLogger().debug(`LspController: Finish building index of project`)
120+
this.logger.debug(`LspController: Finish building index of project`)
120121
const usage = await LspClient.instance.getLspServerUsage()
121122
telemetry.amazonq_indexWorkspace.emit({
122123
duration: performance.now() - start,
@@ -128,7 +129,7 @@ export class LspController {
128129
credentialStartUrl: buildIndexConfig.startUrl,
129130
})
130131
} else {
131-
getLogger().error(`LspController: Failed to build index of project`)
132+
this.logger.error(`LspController: Failed to build index of project`)
132133
telemetry.amazonq_indexWorkspace.emit({
133134
duration: performance.now() - start,
134135
result: 'Failed',
@@ -139,7 +140,7 @@ export class LspController {
139140
}
140141
} catch (error) {
141142
// TODO: use telemetry.run()
142-
getLogger().error(`LspController: Failed to build index of project`)
143+
this.logger.error(`LspController: Failed to build index of project`)
143144
telemetry.amazonq_indexWorkspace.emit({
144145
duration: performance.now() - start,
145146
result: 'Failed',
@@ -155,7 +156,7 @@ export class LspController {
155156

156157
async trySetupLsp(context: vscode.ExtensionContext, buildIndexConfig: BuildIndexConfig) {
157158
if (isCloud9() || isWeb() || isAmazonInternalOs()) {
158-
getLogger().warn('LspController: Skipping LSP setup. LSP is not compatible with the current environment. ')
159+
this.logger.warn('LspController: Skipping LSP setup. LSP is not compatible with the current environment. ')
159160
// do not do anything if in Cloud9 or Web mode or in AL2 (AL2 does not support node v18+)
160161
return
161162
}
@@ -168,7 +169,7 @@ export class LspController {
168169
async () => {
169170
const usage = await LspClient.instance.getLspServerUsage()
170171
if (usage) {
171-
getLogger().info(
172+
this.logger.info(
172173
`LspController: LSP server CPU ${usage.cpuUsage}%, LSP server Memory ${
173174
usage.memoryUsage / (1024 * 1024)
174175
}MB `
@@ -178,7 +179,7 @@ export class LspController {
178179
30 * 60 * 1000
179180
)
180181
} catch (e) {
181-
getLogger().error(`LspController: LSP failed to activate ${e}`)
182+
this.logger.error(`LspController: LSP failed to activate ${e}`)
182183
}
183184
})
184185
}
@@ -187,7 +188,7 @@ export class LspController {
187188
await lspSetupStage('all', async () => {
188189
const installResult = await new WorkspaceLSPResolver().resolve()
189190
await lspSetupStage('launch', async () => activateLsp(context, installResult.resourcePaths))
190-
getLogger().info('LspController: LSP activated')
191+
this.logger.info('LspController: LSP activated')
191192
})
192193
}
193194
}

packages/core/src/amazonq/lsp/workspaceInstaller.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,12 @@ import { LanguageServerResolver } from '../../shared/lsp/lspResolver'
1010
import { Range } from 'semver'
1111
import { getNodeExecutableName } from '../../shared/lsp/utils/platform'
1212
import { fs } from '../../shared/fs/fs'
13-
import { cleanLspDownloads } from '../../shared'
13+
import { cleanLspDownloads, getLogger } from '../../shared'
1414

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

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

34-
await cleanLspDownloads(manifest.versions, path.basename(installationResult.assetDirectory))
35+
const deletedVersions = await cleanLspDownloads(
36+
manifest.versions,
37+
path.basename(installationResult.assetDirectory)
38+
)
39+
logger.debug(`cleaning old LSP versions deleted ${deletedVersions.length} versions`)
3540
return {
3641
...installationResult,
3742
resourcePaths: {

packages/core/src/shared/logger/logger.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,11 @@ export type LogTopic =
1111
| 'notifications'
1212
| 'test'
1313
| 'childProcess'
14-
| 'unknown'
15-
| 'chat'
1614
| 'lsp'
15+
| 'amazonqWorkspaceLsp'
16+
| 'amazonqLsp'
17+
| 'chat'
18+
| 'unknown'
1719

1820
class ErrorLog {
1921
constructor(

packages/core/src/shared/lsp/utils/cleanup.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,23 +19,30 @@ function isDelisted(manifestVersions: LspVersion[], targetVersion: string): bool
1919

2020
/**
2121
* Delete all delisted versions and keep the two newest versions that remain
22-
* @param manifest
22+
* @param manifestVersions
2323
* @param downloadDirectory
24+
* @returns array of deleted versions.
2425
*/
25-
export async function cleanLspDownloads(manifestVersions: LspVersion[], downloadDirectory: string): Promise<void> {
26+
export async function cleanLspDownloads(manifestVersions: LspVersion[], downloadDirectory: string): Promise<string[]> {
2627
const downloadedVersions = await getDownloadedVersions(downloadDirectory)
2728
const [delistedVersions, remainingVersions] = partition(downloadedVersions, (v: string) =>
2829
isDelisted(manifestVersions, v)
2930
)
31+
const deletedVersions: string[] = []
32+
3033
for (const v of delistedVersions) {
3134
await fs.delete(path.join(downloadDirectory, v), { force: true, recursive: true })
35+
deletedVersions.push(v)
3236
}
3337

3438
if (remainingVersions.length <= 2) {
35-
return
39+
return deletedVersions
3640
}
3741

3842
for (const v of sort(remainingVersions).slice(0, -2)) {
3943
await fs.delete(path.join(downloadDirectory, v), { force: true, recursive: true })
44+
deletedVersions.push(v)
4045
}
46+
47+
return deletedVersions
4148
}

packages/core/src/test/shared/lsp/utils/cleanup.test.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,27 +41,32 @@ describe('cleanLSPDownloads', function () {
4141

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

4646
const result = (await fs.readdir(installationDir.fsPath)).map(([filename, _filetype], _index) => filename)
4747
assert.strictEqual(result.length, 2)
4848
assert.ok(result.includes('2.1.1'))
4949
assert.ok(result.includes('1.1.1'))
50+
assert.strictEqual(deleted.length, 2)
5051
})
5152

5253
it('deletes delisted versions', async function () {
5354
await fakeInstallVersions(['1.0.0', '1.0.1', '1.1.1', '2.1.1'], installationDir.fsPath)
54-
await cleanLspDownloads([{ serverVersion: '1.1.1', isDelisted: true, targets: [] }], installationDir.fsPath)
55+
const deleted = await cleanLspDownloads(
56+
[{ serverVersion: '1.1.1', isDelisted: true, targets: [] }],
57+
installationDir.fsPath
58+
)
5559

5660
const result = (await fs.readdir(installationDir.fsPath)).map(([filename, _filetype], _index) => filename)
5761
assert.strictEqual(result.length, 2)
5862
assert.ok(result.includes('2.1.1'))
5963
assert.ok(result.includes('1.0.1'))
64+
assert.strictEqual(deleted.length, 2)
6065
})
6166

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

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

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

8693
it('does not install delisted version when no other option exists', async function () {
8794
await fakeInstallVersions(['1.0.0'], installationDir.fsPath)
88-
await cleanLspDownloads([{ serverVersion: '1.0.0', isDelisted: true, targets: [] }], installationDir.fsPath)
95+
const deleted = await cleanLspDownloads(
96+
[{ serverVersion: '1.0.0', isDelisted: true, targets: [] }],
97+
installationDir.fsPath
98+
)
8999

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

0 commit comments

Comments
 (0)