From 75a4d43a415bf2fe4631c7c032585e9b6b8a7df3 Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 16 Jan 2025 14:28:28 -0500 Subject: [PATCH 01/38] define metric within VSC telemetry --- .../src/shared/telemetry/vscodeTelemetry.json | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/packages/core/src/shared/telemetry/vscodeTelemetry.json b/packages/core/src/shared/telemetry/vscodeTelemetry.json index 5e32b249d4f..1da8d002f3b 100644 --- a/packages/core/src/shared/telemetry/vscodeTelemetry.json +++ b/packages/core/src/shared/telemetry/vscodeTelemetry.json @@ -1,5 +1,17 @@ { "types": [ + { + "name": "lspSetupStage", + "type": "string", + "allowedValues": ["fetchManifest", "serverCall", "validate", "activate", "final"], + "description": "The stage of the LSP setup process" + }, + { + "name": "lspSetupLocation", + "type": "string", + "allowedValues": ["cache", "remote", "fallback", "override"], + "description": "The location of the LSP server" + }, { "name": "amazonGenerateApproachLatency", "type": "double", @@ -358,6 +370,28 @@ } ], "metrics": [ + { + "name": "lsp_setup", + "description": "LSP setup event", + "metadata": [ + { + "type": "lspSetupStage", + "required": true + }, + { + "type": "lspSetupLocation", + "required": false + }, + { + "type": "duration", + "required": true + }, + { + "type": "result", + "required": true + } + ] + }, { "name": "ide_fileSystem", "description": "File System event on execution", From 191e0eaadddff4b04dd2c3cdad5f70d296bae6c2 Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 16 Jan 2025 15:22:38 -0500 Subject: [PATCH 02/38] implement telemetry for fetching stage --- .../core/src/shared/lsp/manifestResolver.ts | 23 ++++++--- .../test/shared/lsp/manifestResolver.test.ts | 49 +++++++++++++++++++ 2 files changed, 66 insertions(+), 6 deletions(-) create mode 100644 packages/core/src/test/shared/lsp/manifestResolver.test.ts diff --git a/packages/core/src/shared/lsp/manifestResolver.ts b/packages/core/src/shared/lsp/manifestResolver.ts index 36685890620..6c377b7c809 100644 --- a/packages/core/src/shared/lsp/manifestResolver.ts +++ b/packages/core/src/shared/lsp/manifestResolver.ts @@ -9,6 +9,7 @@ import { RetryableResourceFetcher } from '../resourcefetcher/httpResourceFetcher import { Timeout } from '../utilities/timeoutUtils' import globals from '../extensionGlobals' import { Manifest } from './types' +import { telemetry } from '../telemetry' const logger = getLogger('lsp') @@ -32,11 +33,22 @@ export class ManifestResolver { * Fetches the latest manifest, falling back to local cache on failure */ async resolve(): Promise { - try { - return await this.fetchRemoteManifest() - } catch (error) { - return await this.getLocalManifest() - } + return await telemetry.lsp_setup.run(async (span) => { + span.record({ lspSetupStage: 'fetchManifest' }) + const startTime = performance.now() + + try { + const result = await this.fetchRemoteManifest() + span.record({ lspSetupLocation: 'remote' }) + return result + } catch (error) { + span.record({ lspSetupLocation: 'cache' }) + const result = await this.getLocalManifest() + return result + } finally { + span.record({ duration: performance.now() - startTime }) + } + }) } private async fetchRemoteManifest(): Promise { @@ -55,7 +67,6 @@ export class ManifestResolver { const manifest = this.parseManifest(resp.content) await this.saveManifest(resp.eTag, resp.content) this.checkDeprecation(manifest) - return manifest } diff --git a/packages/core/src/test/shared/lsp/manifestResolver.test.ts b/packages/core/src/test/shared/lsp/manifestResolver.test.ts new file mode 100644 index 00000000000..ed694bacfbb --- /dev/null +++ b/packages/core/src/test/shared/lsp/manifestResolver.test.ts @@ -0,0 +1,49 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as sinon from 'sinon' +import { ManifestResolver } from '../../../shared' +import { assertTelemetry } from '../../testUtil' + +describe('manifestResolver telemetry', function () { + let resolver: ManifestResolver + let remoteFetchStub: sinon.SinonStub + let localFetchStub: sinon.SinonStub + + before(function () { + resolver = new ManifestResolver('https://example.com/manifest.json', 'test-server') + remoteFetchStub = sinon.stub(ManifestResolver.prototype, 'fetchRemoteManifest' as any) + localFetchStub = sinon.stub(ManifestResolver.prototype, 'getLocalManifest' as any) + }) + + after(function () { + remoteFetchStub.restore() + localFetchStub.restore() + }) + it('emits when fetching from remote', async function () { + remoteFetchStub.callsFake(() => { + return Promise.resolve({}) + }) + await resolver.resolve() + + assertTelemetry('lsp_setup', { lspSetupStage: 'fetchManifest', lspSetupLocation: 'remote' }) + + remoteFetchStub.restore() + }) + + it('emits when fetching from local cache', async function () { + remoteFetchStub.callsFake(() => { + throw new Error('nope') + }) + localFetchStub.callsFake(() => { + return Promise.resolve({}) + }) + await resolver.resolve() + + assertTelemetry('lsp_setup', { lspSetupStage: 'fetchManifest', lspSetupLocation: 'cache' }) + + localFetchStub.restore() + }) +}) From 0b4cec40d1addf44b9b2d2394bfa05694488f38d Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 16 Jan 2025 16:46:08 -0500 Subject: [PATCH 03/38] lift telemetry logic to top level --- .../src/amazonq/lsp/workspaceInstaller.ts | 32 +++++++++--- .../core/src/shared/lsp/manifestResolver.ts | 24 +++------ packages/core/src/shared/lsp/types.ts | 1 + .../src/shared/telemetry/vscodeTelemetry.json | 2 +- .../test/shared/lsp/manifestResolver.test.ts | 49 ------------------- 5 files changed, 35 insertions(+), 73 deletions(-) delete mode 100644 packages/core/src/test/shared/lsp/manifestResolver.test.ts diff --git a/packages/core/src/amazonq/lsp/workspaceInstaller.ts b/packages/core/src/amazonq/lsp/workspaceInstaller.ts index 6c5d869a70a..59afde7acb3 100644 --- a/packages/core/src/amazonq/lsp/workspaceInstaller.ts +++ b/packages/core/src/amazonq/lsp/workspaceInstaller.ts @@ -10,6 +10,7 @@ import { LanguageServerResolver } from '../../shared/lsp/lspResolver' import { Range } from 'semver' import { getNodeExecutableName } from '../../shared/lsp/utils/platform' import { fs } from '../../shared/fs/fs' +import { telemetry } from '../../shared/telemetry' 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 @@ -18,12 +19,31 @@ const supportedLspServerVersions = '0.1.32' export class WorkspaceLSPResolver implements LspResolver { async resolve(): Promise { const name = 'AmazonQ-Workspace' - const manifest = await new ManifestResolver(manifestUrl, name).resolve() - const installationResult = await new LanguageServerResolver( - manifest, - name, - new Range(supportedLspServerVersions) - ).resolve() + const manifest = await telemetry.lsp_setup.run(async (span) => { + const startTime = performance.now() + span.record({ lspSetupStage: 'fetchManifest' }) + const result = await new ManifestResolver(manifestUrl, name).resolve() + span.record({ + lspSetupLocation: result.location ?? 'unknown', + duration: performance.now() - startTime, + }) + return result + }) + + const installationResult = await telemetry.lsp_setup.run(async (span) => { + const startTime = performance.now() + span.record({ lspSetupStage: 'serverCall' }) + const result = await new LanguageServerResolver( + manifest, + name, + new Range(supportedLspServerVersions) + ).resolve() + span.record({ + lspSetupLocation: result.location ?? 'unknown', + duration: performance.now() - startTime, + }) + return result + }) const nodeName = process.platform === 'win32' ? getNodeExecutableName() : `node-${process.platform}-${process.arch}` diff --git a/packages/core/src/shared/lsp/manifestResolver.ts b/packages/core/src/shared/lsp/manifestResolver.ts index 6c377b7c809..07ea3b9ace2 100644 --- a/packages/core/src/shared/lsp/manifestResolver.ts +++ b/packages/core/src/shared/lsp/manifestResolver.ts @@ -9,7 +9,6 @@ import { RetryableResourceFetcher } from '../resourcefetcher/httpResourceFetcher import { Timeout } from '../utilities/timeoutUtils' import globals from '../extensionGlobals' import { Manifest } from './types' -import { telemetry } from '../telemetry' const logger = getLogger('lsp') @@ -33,22 +32,11 @@ export class ManifestResolver { * Fetches the latest manifest, falling back to local cache on failure */ async resolve(): Promise { - return await telemetry.lsp_setup.run(async (span) => { - span.record({ lspSetupStage: 'fetchManifest' }) - const startTime = performance.now() - - try { - const result = await this.fetchRemoteManifest() - span.record({ lspSetupLocation: 'remote' }) - return result - } catch (error) { - span.record({ lspSetupLocation: 'cache' }) - const result = await this.getLocalManifest() - return result - } finally { - span.record({ duration: performance.now() - startTime }) - } - }) + try { + return await this.fetchRemoteManifest() + } catch (error) { + return await this.getLocalManifest() + } } private async fetchRemoteManifest(): Promise { @@ -67,6 +55,7 @@ export class ManifestResolver { const manifest = this.parseManifest(resp.content) await this.saveManifest(resp.eTag, resp.content) this.checkDeprecation(manifest) + manifest.location = 'remote' return manifest } @@ -81,6 +70,7 @@ export class ManifestResolver { const manifest = this.parseManifest(manifestData.content) this.checkDeprecation(manifest) + manifest.location = 'cache' return manifest } diff --git a/packages/core/src/shared/lsp/types.ts b/packages/core/src/shared/lsp/types.ts index 4f5a3cf1c87..3ed009ffd7c 100644 --- a/packages/core/src/shared/lsp/types.ts +++ b/packages/core/src/shared/lsp/types.ts @@ -53,6 +53,7 @@ export interface Manifest { artifactDescription: string isManifestDeprecated: boolean versions: LspVersion[] + location?: Location } export interface VersionRange { diff --git a/packages/core/src/shared/telemetry/vscodeTelemetry.json b/packages/core/src/shared/telemetry/vscodeTelemetry.json index 1da8d002f3b..c9d5f007161 100644 --- a/packages/core/src/shared/telemetry/vscodeTelemetry.json +++ b/packages/core/src/shared/telemetry/vscodeTelemetry.json @@ -9,7 +9,7 @@ { "name": "lspSetupLocation", "type": "string", - "allowedValues": ["cache", "remote", "fallback", "override"], + "allowedValues": ["cache", "remote", "fallback", "override", "unknown"], "description": "The location of the LSP server" }, { diff --git a/packages/core/src/test/shared/lsp/manifestResolver.test.ts b/packages/core/src/test/shared/lsp/manifestResolver.test.ts deleted file mode 100644 index ed694bacfbb..00000000000 --- a/packages/core/src/test/shared/lsp/manifestResolver.test.ts +++ /dev/null @@ -1,49 +0,0 @@ -/*! - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -import * as sinon from 'sinon' -import { ManifestResolver } from '../../../shared' -import { assertTelemetry } from '../../testUtil' - -describe('manifestResolver telemetry', function () { - let resolver: ManifestResolver - let remoteFetchStub: sinon.SinonStub - let localFetchStub: sinon.SinonStub - - before(function () { - resolver = new ManifestResolver('https://example.com/manifest.json', 'test-server') - remoteFetchStub = sinon.stub(ManifestResolver.prototype, 'fetchRemoteManifest' as any) - localFetchStub = sinon.stub(ManifestResolver.prototype, 'getLocalManifest' as any) - }) - - after(function () { - remoteFetchStub.restore() - localFetchStub.restore() - }) - it('emits when fetching from remote', async function () { - remoteFetchStub.callsFake(() => { - return Promise.resolve({}) - }) - await resolver.resolve() - - assertTelemetry('lsp_setup', { lspSetupStage: 'fetchManifest', lspSetupLocation: 'remote' }) - - remoteFetchStub.restore() - }) - - it('emits when fetching from local cache', async function () { - remoteFetchStub.callsFake(() => { - throw new Error('nope') - }) - localFetchStub.callsFake(() => { - return Promise.resolve({}) - }) - await resolver.resolve() - - assertTelemetry('lsp_setup', { lspSetupStage: 'fetchManifest', lspSetupLocation: 'cache' }) - - localFetchStub.restore() - }) -}) From 3727ea9e07beeeb7b5f155ffcf9aec82c9bf0455 Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 16 Jan 2025 17:20:32 -0500 Subject: [PATCH 04/38] add telemetry for remaining stages --- packages/core/src/amazonq/lsp/lspClient.ts | 173 +++++++++--------- .../core/src/amazonq/lsp/lspController.ts | 11 +- 2 files changed, 97 insertions(+), 87 deletions(-) diff --git a/packages/core/src/amazonq/lsp/lspClient.ts b/packages/core/src/amazonq/lsp/lspClient.ts index f30aa5b681d..7d37bc4edc8 100644 --- a/packages/core/src/amazonq/lsp/lspClient.ts +++ b/packages/core/src/amazonq/lsp/lspClient.ts @@ -33,6 +33,7 @@ import { import { Writable } from 'stream' import { CodeWhispererSettings } from '../../codewhisperer/util/codewhispererSettings' import { ResourcePaths, fs, getLogger, globals } from '../../shared' +import { telemetry } from '../../shared/telemetry' const localize = nls.loadMessageBundle() @@ -173,104 +174,108 @@ export class LspClient { * This function assumes the LSP server has already been downloaded. */ export async function activate(extensionContext: ExtensionContext, resourcePaths: ResourcePaths) { - LspClient.instance - const toDispose = extensionContext.subscriptions + return await telemetry.lsp_setup.run(async (span) => { + span.record({ lspSetupStage: 'activate' }) + const startTime = performance.now() + LspClient.instance + const toDispose = extensionContext.subscriptions - let rangeFormatting: Disposable | undefined - // The debug options for the server - // --inspect=6009: runs the server in Node's Inspector mode so VS Code can attach to the server for debugging - const debugOptions = { execArgv: ['--nolazy', '--preserve-symlinks', '--stdio'] } + let rangeFormatting: Disposable | undefined + // The debug options for the server + // --inspect=6009: runs the server in Node's Inspector mode so VS Code can attach to the server for debugging + const debugOptions = { execArgv: ['--nolazy', '--preserve-symlinks', '--stdio'] } - const workerThreads = CodeWhispererSettings.instance.getIndexWorkerThreads() - const gpu = CodeWhispererSettings.instance.isLocalIndexGPUEnabled() + const workerThreads = CodeWhispererSettings.instance.getIndexWorkerThreads() + const gpu = CodeWhispererSettings.instance.isLocalIndexGPUEnabled() - if (gpu) { - process.env.Q_ENABLE_GPU = 'true' - } else { - delete process.env.Q_ENABLE_GPU - } - if (workerThreads > 0 && workerThreads < 100) { - process.env.Q_WORKER_THREADS = workerThreads.toString() - } else { - delete process.env.Q_WORKER_THREADS - } + if (gpu) { + process.env.Q_ENABLE_GPU = 'true' + } else { + delete process.env.Q_ENABLE_GPU + } + if (workerThreads > 0 && workerThreads < 100) { + process.env.Q_WORKER_THREADS = workerThreads.toString() + } else { + delete process.env.Q_WORKER_THREADS + } - const serverModule = resourcePaths.lsp + const serverModule = resourcePaths.lsp - const child = spawn(resourcePaths.node, [serverModule, ...debugOptions.execArgv]) - // share an encryption key using stdin - // follow same practice of DEXP LSP server - writeEncryptionInit(child.stdin) + const child = spawn(resourcePaths.node, [serverModule, ...debugOptions.execArgv]) + // share an encryption key using stdin + // follow same practice of DEXP LSP server + writeEncryptionInit(child.stdin) - // If the extension is launch in debug mode the debug server options are use - // Otherwise the run options are used - let serverOptions: ServerOptions = { - run: { module: serverModule, transport: TransportKind.ipc }, - debug: { module: serverModule, transport: TransportKind.ipc, options: debugOptions }, - } + // If the extension is launch in debug mode the debug server options are use + // Otherwise the run options are used + let serverOptions: ServerOptions = { + run: { module: serverModule, transport: TransportKind.ipc }, + debug: { module: serverModule, transport: TransportKind.ipc, options: debugOptions }, + } - serverOptions = () => Promise.resolve(child!) + serverOptions = () => Promise.resolve(child!) - const documentSelector = [{ scheme: 'file', language: '*' }] + const documentSelector = [{ scheme: 'file', language: '*' }] - // Options to control the language client - const clientOptions: LanguageClientOptions = { - // Register the server for json documents - documentSelector, - initializationOptions: { - handledSchemaProtocols: ['file', 'untitled'], // language server only loads file-URI. Fetching schemas with other protocols ('http'...) are made on the client. - provideFormatter: false, // tell the server to not provide formatting capability and ignore the `aws.stepfunctions.asl.format.enable` setting. - // this is used by LSP to determine index cache path, move to this folder so that when extension updates index is not deleted. - extensionPath: path.join(fs.getUserHomeDir(), '.aws', 'amazonq', 'cache'), - }, - // Log to the Amazon Q Logs so everything is in a single channel - // TODO: Add prefix to the language server logs so it is easier to search - outputChannel: globals.logOutputChannel, - } + // Options to control the language client + const clientOptions: LanguageClientOptions = { + // Register the server for json documents + documentSelector, + initializationOptions: { + handledSchemaProtocols: ['file', 'untitled'], // language server only loads file-URI. Fetching schemas with other protocols ('http'...) are made on the client. + provideFormatter: false, // tell the server to not provide formatting capability and ignore the `aws.stepfunctions.asl.format.enable` setting. + // this is used by LSP to determine index cache path, move to this folder so that when extension updates index is not deleted. + extensionPath: path.join(fs.getUserHomeDir(), '.aws', 'amazonq', 'cache'), + }, + // Log to the Amazon Q Logs so everything is in a single channel + // TODO: Add prefix to the language server logs so it is easier to search + outputChannel: globals.logOutputChannel, + } - // Create the language client and start the client. - LspClient.instance.client = new LanguageClient( - 'amazonq', - localize('amazonq.server.name', 'Amazon Q Language Server'), - serverOptions, - clientOptions - ) - LspClient.instance.client.registerProposedFeatures() + // Create the language client and start the client. + LspClient.instance.client = new LanguageClient( + 'amazonq', + localize('amazonq.server.name', 'Amazon Q Language Server'), + serverOptions, + clientOptions + ) + LspClient.instance.client.registerProposedFeatures() - const disposable = LspClient.instance.client.start() - toDispose.push(disposable) + const disposable = LspClient.instance.client.start() + toDispose.push(disposable) - let savedDocument: vscode.Uri | undefined = undefined + let savedDocument: vscode.Uri | undefined = undefined - toDispose.push( - vscode.workspace.onDidSaveTextDocument((document) => { - if (document.uri.scheme !== 'file') { - return - } - savedDocument = document.uri - }), - vscode.window.onDidChangeActiveTextEditor((editor) => { - if (savedDocument && editor && editor.document.uri.fsPath !== savedDocument.fsPath) { - void LspClient.instance.updateIndex([savedDocument.fsPath], 'update') - } - }), - vscode.workspace.onDidCreateFiles((e) => { - void LspClient.instance.updateIndex( - e.files.map((f) => f.fsPath), - 'add' - ) - }), - vscode.workspace.onDidDeleteFiles((e) => { - void LspClient.instance.updateIndex( - e.files.map((f) => f.fsPath), - 'remove' - ) + toDispose.push( + vscode.workspace.onDidSaveTextDocument((document) => { + if (document.uri.scheme !== 'file') { + return + } + savedDocument = document.uri + }), + vscode.window.onDidChangeActiveTextEditor((editor) => { + if (savedDocument && editor && editor.document.uri.fsPath !== savedDocument.fsPath) { + void LspClient.instance.updateIndex([savedDocument.fsPath], 'update') + } + }), + vscode.workspace.onDidCreateFiles((e) => { + void LspClient.instance.updateIndex( + e.files.map((f) => f.fsPath), + 'add' + ) + }), + vscode.workspace.onDidDeleteFiles((e) => { + void LspClient.instance.updateIndex( + e.files.map((f) => f.fsPath), + 'remove' + ) + }) + ) + void LspClient.instance.client.onReady().then(() => { + const disposableFunc = { dispose: () => rangeFormatting?.dispose() as void } + toDispose.push(disposableFunc) }) - ) - - return LspClient.instance.client.onReady().then(() => { - const disposableFunc = { dispose: () => rangeFormatting?.dispose() as void } - toDispose.push(disposableFunc) + span.record({ duration: performance.now() - startTime }) }) } diff --git a/packages/core/src/amazonq/lsp/lspController.ts b/packages/core/src/amazonq/lsp/lspController.ts index 3b38d9a17a2..ff31810b74d 100644 --- a/packages/core/src/amazonq/lsp/lspController.ts +++ b/packages/core/src/amazonq/lsp/lspController.ts @@ -160,9 +160,14 @@ export class LspController { } setImmediate(async () => { try { - const installResult = await new WorkspaceLSPResolver().resolve() - await activateLsp(context, installResult.resourcePaths) - getLogger().info('LspController: LSP activated') + await telemetry.lsp_setup.run(async (span) => { + const startTime = performance.now() + span.record({ lspSetupStage: 'final' }) + const installResult = await new WorkspaceLSPResolver().resolve() + await activateLsp(context, installResult.resourcePaths) + getLogger().info('LspController: LSP activated') + span.record({ duration: performance.now() - startTime }) + }) void LspController.instance.buildIndex(buildIndexConfig) // log the LSP server CPU and Memory usage per 30 minutes. globals.clock.setInterval( From 5d5a7d890c12c52323c10a2971f494b109a7d53f Mon Sep 17 00:00:00 2001 From: hkobew Date: Fri, 17 Jan 2025 09:50:58 -0500 Subject: [PATCH 05/38] seperate verification and downloading --- packages/core/src/shared/lsp/lspResolver.ts | 53 ++++++++++++++------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/packages/core/src/shared/lsp/lspResolver.ts b/packages/core/src/shared/lsp/lspResolver.ts index 7a6fc4102b0..8250014ce40 100644 --- a/packages/core/src/shared/lsp/lspResolver.ts +++ b/packages/core/src/shared/lsp/lspResolver.ts @@ -164,26 +164,43 @@ export class LanguageServerResolver { await fs.mkdir(downloadDirectory) } - const downloadTasks = contents.map(async (content) => { - // TODO This should be using the retryable http library but it doesn't seem to support zips right now - const res = await request.fetch('GET', content.url).response - if (!res.ok || !res.body) { - return false - } - - const arrBuffer = await res.arrayBuffer() - const data = Buffer.from(arrBuffer) + const fetchResults = await Promise.all( + contents.map(async (content) => { + return { + res: await request.fetch('GET', content.url).response, + hash: content.hashes[0], + filename: content.filename, + } + }) + ) + const filesToDownload = ( + await Promise.all( + fetchResults.flatMap(async (fetchResult) => { + if (!fetchResult.res.ok || !fetchResult.res.body) { + return [] + } + + const arrBuffer = await fetchResult.res.arrayBuffer() + const data = Buffer.from(arrBuffer) + + const hash = createHash('sha384', data) + if (hash === fetchResult.hash) { + return [{ filename: fetchResult.filename, data }] + } + return [] + }) + ) + ).flat() - const hash = createHash('sha384', data) - if (hash === content.hashes[0]) { - await fs.writeFile(`${downloadDirectory}/${content.filename}`, data) - return true - } + if (filesToDownload.length !== contents.length) { return false - }) - const downloadResults = await Promise.all(downloadTasks) - const downloadResult = downloadResults.every(Boolean) - return downloadResult && this.extractZipFilesFromRemote(downloadDirectory) + } + + for (const file of filesToDownload) { + await fs.writeFile(`${downloadDirectory}/${file.filename}`, file.data) + } + + return this.extractZipFilesFromRemote(downloadDirectory) } private async extractZipFilesFromRemote(downloadDirectory: string) { From a7b446d312f27306fefd0607c5ea89351849edf8 Mon Sep 17 00:00:00 2001 From: hkobew Date: Fri, 17 Jan 2025 10:05:34 -0500 Subject: [PATCH 06/38] add telemetry for validation step --- packages/core/src/shared/lsp/lspResolver.ts | 57 +++++++++++---------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/packages/core/src/shared/lsp/lspResolver.ts b/packages/core/src/shared/lsp/lspResolver.ts index 8250014ce40..83c5a7af68c 100644 --- a/packages/core/src/shared/lsp/lspResolver.ts +++ b/packages/core/src/shared/lsp/lspResolver.ts @@ -13,6 +13,7 @@ import { TargetContent, logger, LspResult, LspVersion, Manifest } from './types' import { getApplicationSupportFolder } from '../vscode/env' import { createHash } from '../crypto' import request from '../request' +import { telemetry } from '../telemetry' export class LanguageServerResolver { constructor( @@ -164,33 +165,35 @@ export class LanguageServerResolver { await fs.mkdir(downloadDirectory) } - const fetchResults = await Promise.all( - contents.map(async (content) => { - return { - res: await request.fetch('GET', content.url).response, - hash: content.hashes[0], - filename: content.filename, - } - }) - ) - const filesToDownload = ( - await Promise.all( - fetchResults.flatMap(async (fetchResult) => { - if (!fetchResult.res.ok || !fetchResult.res.body) { - return [] - } - - const arrBuffer = await fetchResult.res.arrayBuffer() - const data = Buffer.from(arrBuffer) - - const hash = createHash('sha384', data) - if (hash === fetchResult.hash) { - return [{ filename: fetchResult.filename, data }] - } - return [] - }) - ) - ).flat() + const fetchTasks = contents.map(async (content) => { + return { + res: await request.fetch('GET', content.url).response, + hash: content.hashes[0], + filename: content.filename, + } + }) + const fetchResults = await Promise.all(fetchTasks) + const verifyTasks = fetchResults.flatMap(async (fetchResult) => { + if (!fetchResult.res.ok || !fetchResult.res.body) { + return [] + } + + const arrBuffer = await fetchResult.res.arrayBuffer() + const data = Buffer.from(arrBuffer) + + const hash = createHash('sha384', data) + if (hash === fetchResult.hash) { + return [{ filename: fetchResult.filename, data }] + } + return [] + }) + const filesToDownload = await telemetry.lsp_setup.run(async (span) => { + span.record({ lspSetupStage: 'validate' }) + const startTime = performance.now() + const result = (await Promise.all(verifyTasks)).flat() + span.record({ duration: performance.now() - startTime }) + return result + }) if (filesToDownload.length !== contents.length) { return false From 3304e9914adc4a574080645b90f0beb7bf869a4f Mon Sep 17 00:00:00 2001 From: hkobew Date: Fri, 17 Jan 2025 10:47:47 -0500 Subject: [PATCH 07/38] rename metrics and types --- packages/core/src/amazonq/lsp/lspClient.ts | 4 +-- .../core/src/amazonq/lsp/lspController.ts | 4 +-- .../src/amazonq/lsp/workspaceInstaller.ts | 16 ++++++----- packages/core/src/shared/lsp/lspResolver.ts | 4 +-- .../src/shared/telemetry/vscodeTelemetry.json | 27 ++++++++++--------- 5 files changed, 30 insertions(+), 25 deletions(-) diff --git a/packages/core/src/amazonq/lsp/lspClient.ts b/packages/core/src/amazonq/lsp/lspClient.ts index 7d37bc4edc8..2114fb879cc 100644 --- a/packages/core/src/amazonq/lsp/lspClient.ts +++ b/packages/core/src/amazonq/lsp/lspClient.ts @@ -174,8 +174,8 @@ export class LspClient { * This function assumes the LSP server has already been downloaded. */ export async function activate(extensionContext: ExtensionContext, resourcePaths: ResourcePaths) { - return await telemetry.lsp_setup.run(async (span) => { - span.record({ lspSetupStage: 'activate' }) + return await telemetry.languageServer_setup.run(async (span) => { + span.record({ languageServerSetupStage: 'launch' }) const startTime = performance.now() LspClient.instance const toDispose = extensionContext.subscriptions diff --git a/packages/core/src/amazonq/lsp/lspController.ts b/packages/core/src/amazonq/lsp/lspController.ts index ff31810b74d..4554c3218e9 100644 --- a/packages/core/src/amazonq/lsp/lspController.ts +++ b/packages/core/src/amazonq/lsp/lspController.ts @@ -160,9 +160,9 @@ export class LspController { } setImmediate(async () => { try { - await telemetry.lsp_setup.run(async (span) => { + await telemetry.languageServer_setup.run(async (span) => { const startTime = performance.now() - span.record({ lspSetupStage: 'final' }) + span.record({ languageServerSetupStage: 'final' }) const installResult = await new WorkspaceLSPResolver().resolve() await activateLsp(context, installResult.resourcePaths) getLogger().info('LspController: LSP activated') diff --git a/packages/core/src/amazonq/lsp/workspaceInstaller.ts b/packages/core/src/amazonq/lsp/workspaceInstaller.ts index 59afde7acb3..d435a8859e8 100644 --- a/packages/core/src/amazonq/lsp/workspaceInstaller.ts +++ b/packages/core/src/amazonq/lsp/workspaceInstaller.ts @@ -19,27 +19,31 @@ const supportedLspServerVersions = '0.1.32' export class WorkspaceLSPResolver implements LspResolver { async resolve(): Promise { const name = 'AmazonQ-Workspace' - const manifest = await telemetry.lsp_setup.run(async (span) => { + const manifest = await telemetry.languageServer_setup.run(async (span) => { const startTime = performance.now() - span.record({ lspSetupStage: 'fetchManifest' }) + span.record({ languageServerSetupStage: 'getManifest' }) const result = await new ManifestResolver(manifestUrl, name).resolve() span.record({ - lspSetupLocation: result.location ?? 'unknown', + languageServerResourceLocation: result.location ?? 'unknown', + manifestVersion: result.manifestSchemaVersion, duration: performance.now() - startTime, }) return result }) + telemetry.record({ + manifestVersion: manifest.manifestSchemaVersion, + }) - const installationResult = await telemetry.lsp_setup.run(async (span) => { + const installationResult = await telemetry.languageServer_setup.run(async (span) => { const startTime = performance.now() - span.record({ lspSetupStage: 'serverCall' }) + span.record({ languageServerSetupStage: 'getServer' }) const result = await new LanguageServerResolver( manifest, name, new Range(supportedLspServerVersions) ).resolve() span.record({ - lspSetupLocation: result.location ?? 'unknown', + languageServerResourceLocation: result.location ?? 'unknown', duration: performance.now() - startTime, }) return result diff --git a/packages/core/src/shared/lsp/lspResolver.ts b/packages/core/src/shared/lsp/lspResolver.ts index 83c5a7af68c..44c9b27f663 100644 --- a/packages/core/src/shared/lsp/lspResolver.ts +++ b/packages/core/src/shared/lsp/lspResolver.ts @@ -187,8 +187,8 @@ export class LanguageServerResolver { } return [] }) - const filesToDownload = await telemetry.lsp_setup.run(async (span) => { - span.record({ lspSetupStage: 'validate' }) + const filesToDownload = await telemetry.languageServer_setup.run(async (span) => { + span.record({ languageServerSetupStage: 'validate' }) const startTime = performance.now() const result = (await Promise.all(verifyTasks)).flat() span.record({ duration: performance.now() - startTime }) diff --git a/packages/core/src/shared/telemetry/vscodeTelemetry.json b/packages/core/src/shared/telemetry/vscodeTelemetry.json index c9d5f007161..3a7af9a6f1c 100644 --- a/packages/core/src/shared/telemetry/vscodeTelemetry.json +++ b/packages/core/src/shared/telemetry/vscodeTelemetry.json @@ -1,16 +1,21 @@ { "types": [ { - "name": "lspSetupStage", + "name": "languageServerSetupStage", "type": "string", - "allowedValues": ["fetchManifest", "serverCall", "validate", "activate", "final"], + "allowedValues": ["getManifest", "getServer", "validate", "launch", "final"], "description": "The stage of the LSP setup process" }, { - "name": "lspSetupLocation", + "name": "languageServerResourceLocation", "type": "string", "allowedValues": ["cache", "remote", "fallback", "override", "unknown"], - "description": "The location of the LSP server" + "description": "The location of the LSP resource" + }, + { + "name": "manifestVersion", + "type": "string", + "description": "The version of the manifest file" }, { "name": "amazonGenerateApproachLatency", @@ -371,24 +376,20 @@ ], "metrics": [ { - "name": "lsp_setup", + "name": "languageServer_setup", "description": "LSP setup event", "metadata": [ { - "type": "lspSetupStage", + "type": "languageServerSetupStage", "required": true }, { - "type": "lspSetupLocation", + "type": "languageServerResourceLocation", "required": false }, { - "type": "duration", - "required": true - }, - { - "type": "result", - "required": true + "type": "manifestVersion", + "required": false } ] }, From 45b023699f1b74abd5604decb2378ea9c9d51621 Mon Sep 17 00:00:00 2001 From: hkobew Date: Fri, 17 Jan 2025 11:06:25 -0500 Subject: [PATCH 08/38] reduce duplication across stage implementations --- packages/core/src/amazonq/lsp/lspClient.ts | 172 +++++++++--------- .../core/src/amazonq/lsp/lspController.ts | 8 +- packages/core/src/amazonq/lsp/util.ts | 16 ++ .../src/amazonq/lsp/workspaceInstaller.ts | 18 +- packages/core/src/shared/lsp/lspResolver.ts | 10 +- 5 files changed, 110 insertions(+), 114 deletions(-) create mode 100644 packages/core/src/amazonq/lsp/util.ts diff --git a/packages/core/src/amazonq/lsp/lspClient.ts b/packages/core/src/amazonq/lsp/lspClient.ts index 2114fb879cc..f4db4fafbcd 100644 --- a/packages/core/src/amazonq/lsp/lspClient.ts +++ b/packages/core/src/amazonq/lsp/lspClient.ts @@ -33,7 +33,6 @@ import { import { Writable } from 'stream' import { CodeWhispererSettings } from '../../codewhisperer/util/codewhispererSettings' import { ResourcePaths, fs, getLogger, globals } from '../../shared' -import { telemetry } from '../../shared/telemetry' const localize = nls.loadMessageBundle() @@ -174,108 +173,103 @@ export class LspClient { * This function assumes the LSP server has already been downloaded. */ export async function activate(extensionContext: ExtensionContext, resourcePaths: ResourcePaths) { - return await telemetry.languageServer_setup.run(async (span) => { - span.record({ languageServerSetupStage: 'launch' }) - const startTime = performance.now() - LspClient.instance - const toDispose = extensionContext.subscriptions + LspClient.instance + const toDispose = extensionContext.subscriptions - let rangeFormatting: Disposable | undefined - // The debug options for the server - // --inspect=6009: runs the server in Node's Inspector mode so VS Code can attach to the server for debugging - const debugOptions = { execArgv: ['--nolazy', '--preserve-symlinks', '--stdio'] } + let rangeFormatting: Disposable | undefined + // The debug options for the server + // --inspect=6009: runs the server in Node's Inspector mode so VS Code can attach to the server for debugging + const debugOptions = { execArgv: ['--nolazy', '--preserve-symlinks', '--stdio'] } - const workerThreads = CodeWhispererSettings.instance.getIndexWorkerThreads() - const gpu = CodeWhispererSettings.instance.isLocalIndexGPUEnabled() + const workerThreads = CodeWhispererSettings.instance.getIndexWorkerThreads() + const gpu = CodeWhispererSettings.instance.isLocalIndexGPUEnabled() - if (gpu) { - process.env.Q_ENABLE_GPU = 'true' - } else { - delete process.env.Q_ENABLE_GPU - } - if (workerThreads > 0 && workerThreads < 100) { - process.env.Q_WORKER_THREADS = workerThreads.toString() - } else { - delete process.env.Q_WORKER_THREADS - } + if (gpu) { + process.env.Q_ENABLE_GPU = 'true' + } else { + delete process.env.Q_ENABLE_GPU + } + if (workerThreads > 0 && workerThreads < 100) { + process.env.Q_WORKER_THREADS = workerThreads.toString() + } else { + delete process.env.Q_WORKER_THREADS + } - const serverModule = resourcePaths.lsp + const serverModule = resourcePaths.lsp - const child = spawn(resourcePaths.node, [serverModule, ...debugOptions.execArgv]) - // share an encryption key using stdin - // follow same practice of DEXP LSP server - writeEncryptionInit(child.stdin) + const child = spawn(resourcePaths.node, [serverModule, ...debugOptions.execArgv]) + // share an encryption key using stdin + // follow same practice of DEXP LSP server + writeEncryptionInit(child.stdin) - // If the extension is launch in debug mode the debug server options are use - // Otherwise the run options are used - let serverOptions: ServerOptions = { - run: { module: serverModule, transport: TransportKind.ipc }, - debug: { module: serverModule, transport: TransportKind.ipc, options: debugOptions }, - } + // If the extension is launch in debug mode the debug server options are use + // Otherwise the run options are used + let serverOptions: ServerOptions = { + run: { module: serverModule, transport: TransportKind.ipc }, + debug: { module: serverModule, transport: TransportKind.ipc, options: debugOptions }, + } - serverOptions = () => Promise.resolve(child!) + serverOptions = () => Promise.resolve(child!) - const documentSelector = [{ scheme: 'file', language: '*' }] + const documentSelector = [{ scheme: 'file', language: '*' }] - // Options to control the language client - const clientOptions: LanguageClientOptions = { - // Register the server for json documents - documentSelector, - initializationOptions: { - handledSchemaProtocols: ['file', 'untitled'], // language server only loads file-URI. Fetching schemas with other protocols ('http'...) are made on the client. - provideFormatter: false, // tell the server to not provide formatting capability and ignore the `aws.stepfunctions.asl.format.enable` setting. - // this is used by LSP to determine index cache path, move to this folder so that when extension updates index is not deleted. - extensionPath: path.join(fs.getUserHomeDir(), '.aws', 'amazonq', 'cache'), - }, - // Log to the Amazon Q Logs so everything is in a single channel - // TODO: Add prefix to the language server logs so it is easier to search - outputChannel: globals.logOutputChannel, - } + // Options to control the language client + const clientOptions: LanguageClientOptions = { + // Register the server for json documents + documentSelector, + initializationOptions: { + handledSchemaProtocols: ['file', 'untitled'], // language server only loads file-URI. Fetching schemas with other protocols ('http'...) are made on the client. + provideFormatter: false, // tell the server to not provide formatting capability and ignore the `aws.stepfunctions.asl.format.enable` setting. + // this is used by LSP to determine index cache path, move to this folder so that when extension updates index is not deleted. + extensionPath: path.join(fs.getUserHomeDir(), '.aws', 'amazonq', 'cache'), + }, + // Log to the Amazon Q Logs so everything is in a single channel + // TODO: Add prefix to the language server logs so it is easier to search + outputChannel: globals.logOutputChannel, + } - // Create the language client and start the client. - LspClient.instance.client = new LanguageClient( - 'amazonq', - localize('amazonq.server.name', 'Amazon Q Language Server'), - serverOptions, - clientOptions - ) - LspClient.instance.client.registerProposedFeatures() + // Create the language client and start the client. + LspClient.instance.client = new LanguageClient( + 'amazonq', + localize('amazonq.server.name', 'Amazon Q Language Server'), + serverOptions, + clientOptions + ) + LspClient.instance.client.registerProposedFeatures() - const disposable = LspClient.instance.client.start() - toDispose.push(disposable) + const disposable = LspClient.instance.client.start() + toDispose.push(disposable) - let savedDocument: vscode.Uri | undefined = undefined + let savedDocument: vscode.Uri | undefined = undefined - toDispose.push( - vscode.workspace.onDidSaveTextDocument((document) => { - if (document.uri.scheme !== 'file') { - return - } - savedDocument = document.uri - }), - vscode.window.onDidChangeActiveTextEditor((editor) => { - if (savedDocument && editor && editor.document.uri.fsPath !== savedDocument.fsPath) { - void LspClient.instance.updateIndex([savedDocument.fsPath], 'update') - } - }), - vscode.workspace.onDidCreateFiles((e) => { - void LspClient.instance.updateIndex( - e.files.map((f) => f.fsPath), - 'add' - ) - }), - vscode.workspace.onDidDeleteFiles((e) => { - void LspClient.instance.updateIndex( - e.files.map((f) => f.fsPath), - 'remove' - ) - }) - ) - void LspClient.instance.client.onReady().then(() => { - const disposableFunc = { dispose: () => rangeFormatting?.dispose() as void } - toDispose.push(disposableFunc) + toDispose.push( + vscode.workspace.onDidSaveTextDocument((document) => { + if (document.uri.scheme !== 'file') { + return + } + savedDocument = document.uri + }), + vscode.window.onDidChangeActiveTextEditor((editor) => { + if (savedDocument && editor && editor.document.uri.fsPath !== savedDocument.fsPath) { + void LspClient.instance.updateIndex([savedDocument.fsPath], 'update') + } + }), + vscode.workspace.onDidCreateFiles((e) => { + void LspClient.instance.updateIndex( + e.files.map((f) => f.fsPath), + 'add' + ) + }), + vscode.workspace.onDidDeleteFiles((e) => { + void LspClient.instance.updateIndex( + e.files.map((f) => f.fsPath), + 'remove' + ) }) - span.record({ duration: performance.now() - startTime }) + ) + void LspClient.instance.client.onReady().then(() => { + const disposableFunc = { dispose: () => rangeFormatting?.dispose() as void } + toDispose.push(disposableFunc) }) } diff --git a/packages/core/src/amazonq/lsp/lspController.ts b/packages/core/src/amazonq/lsp/lspController.ts index 4554c3218e9..74d13aec882 100644 --- a/packages/core/src/amazonq/lsp/lspController.ts +++ b/packages/core/src/amazonq/lsp/lspController.ts @@ -15,6 +15,7 @@ import { isCloud9 } from '../../shared/extensionUtilities' import globals, { isWeb } from '../../shared/extensionGlobals' import { isAmazonInternalOs } from '../../shared/vscode/env' import { WorkspaceLSPResolver } from './workspaceInstaller' +import { lspSetupStage } from './util' export interface Chunk { readonly filePath: string @@ -160,13 +161,10 @@ export class LspController { } setImmediate(async () => { try { - await telemetry.languageServer_setup.run(async (span) => { - const startTime = performance.now() - span.record({ languageServerSetupStage: 'final' }) + await lspSetupStage('final', async () => { const installResult = await new WorkspaceLSPResolver().resolve() - await activateLsp(context, installResult.resourcePaths) + await lspSetupStage('launch', async () => activateLsp(context, installResult.resourcePaths)) getLogger().info('LspController: LSP activated') - span.record({ duration: performance.now() - startTime }) }) void LspController.instance.buildIndex(buildIndexConfig) // log the LSP server CPU and Memory usage per 30 minutes. diff --git a/packages/core/src/amazonq/lsp/util.ts b/packages/core/src/amazonq/lsp/util.ts new file mode 100644 index 00000000000..bfc24827aec --- /dev/null +++ b/packages/core/src/amazonq/lsp/util.ts @@ -0,0 +1,16 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import { LanguageServerSetupStage, telemetry } from '../../shared/telemetry' + +export async function lspSetupStage(stageName: LanguageServerSetupStage, stage: () => Promise) { + return await telemetry.languageServer_setup.run(async (span) => { + const startTime = performance.now() + const result = await stage() + span.record({ languageServerSetupStage: stageName }) + span.record({ duration: performance.now() - startTime }) + return result + }) +} diff --git a/packages/core/src/amazonq/lsp/workspaceInstaller.ts b/packages/core/src/amazonq/lsp/workspaceInstaller.ts index d435a8859e8..9cca1603551 100644 --- a/packages/core/src/amazonq/lsp/workspaceInstaller.ts +++ b/packages/core/src/amazonq/lsp/workspaceInstaller.ts @@ -11,6 +11,7 @@ import { Range } from 'semver' import { getNodeExecutableName } from '../../shared/lsp/utils/platform' import { fs } from '../../shared/fs/fs' import { telemetry } from '../../shared/telemetry' +import { lspSetupStage } from './util' 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 @@ -19,32 +20,25 @@ const supportedLspServerVersions = '0.1.32' export class WorkspaceLSPResolver implements LspResolver { async resolve(): Promise { const name = 'AmazonQ-Workspace' - const manifest = await telemetry.languageServer_setup.run(async (span) => { - const startTime = performance.now() - span.record({ languageServerSetupStage: 'getManifest' }) + const manifest = await lspSetupStage('getManifest', async () => { const result = await new ManifestResolver(manifestUrl, name).resolve() - span.record({ - languageServerResourceLocation: result.location ?? 'unknown', + telemetry.record({ manifestVersion: result.manifestSchemaVersion, - duration: performance.now() - startTime, + languageServerResourceLocation: result.location ?? 'unknown', }) return result }) telemetry.record({ manifestVersion: manifest.manifestSchemaVersion, }) - - const installationResult = await telemetry.languageServer_setup.run(async (span) => { - const startTime = performance.now() - span.record({ languageServerSetupStage: 'getServer' }) + const installationResult = await lspSetupStage('getServer', async () => { const result = await new LanguageServerResolver( manifest, name, new Range(supportedLspServerVersions) ).resolve() - span.record({ + telemetry.record({ languageServerResourceLocation: result.location ?? 'unknown', - duration: performance.now() - startTime, }) return result }) diff --git a/packages/core/src/shared/lsp/lspResolver.ts b/packages/core/src/shared/lsp/lspResolver.ts index 44c9b27f663..c789c591580 100644 --- a/packages/core/src/shared/lsp/lspResolver.ts +++ b/packages/core/src/shared/lsp/lspResolver.ts @@ -13,7 +13,7 @@ import { TargetContent, logger, LspResult, LspVersion, Manifest } from './types' import { getApplicationSupportFolder } from '../vscode/env' import { createHash } from '../crypto' import request from '../request' -import { telemetry } from '../telemetry' +import { lspSetupStage } from '../../amazonq/lsp/util' export class LanguageServerResolver { constructor( @@ -187,13 +187,7 @@ export class LanguageServerResolver { } return [] }) - const filesToDownload = await telemetry.languageServer_setup.run(async (span) => { - span.record({ languageServerSetupStage: 'validate' }) - const startTime = performance.now() - const result = (await Promise.all(verifyTasks)).flat() - span.record({ duration: performance.now() - startTime }) - return result - }) + const filesToDownload = await lspSetupStage('validate', async () => (await Promise.all(verifyTasks)).flat()) if (filesToDownload.length !== contents.length) { return false From c2596cfa35a1a4608862de917372456925d0e438 Mon Sep 17 00:00:00 2001 From: hkobew Date: Fri, 17 Jan 2025 11:20:56 -0500 Subject: [PATCH 09/38] undo unnecessary change --- packages/core/src/amazonq/lsp/lspClient.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/src/amazonq/lsp/lspClient.ts b/packages/core/src/amazonq/lsp/lspClient.ts index f4db4fafbcd..f30aa5b681d 100644 --- a/packages/core/src/amazonq/lsp/lspClient.ts +++ b/packages/core/src/amazonq/lsp/lspClient.ts @@ -267,7 +267,8 @@ export async function activate(extensionContext: ExtensionContext, resourcePaths ) }) ) - void LspClient.instance.client.onReady().then(() => { + + return LspClient.instance.client.onReady().then(() => { const disposableFunc = { dispose: () => rangeFormatting?.dispose() as void } toDispose.push(disposableFunc) }) From 6ef6633ea481f482b61ff1e4ce6ee8b01e1ce6a8 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 21 Jan 2025 11:00:59 -0500 Subject: [PATCH 10/38] switch to fallback instead of cache when remote option fails --- packages/core/src/shared/lsp/manifestResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/shared/lsp/manifestResolver.ts b/packages/core/src/shared/lsp/manifestResolver.ts index 07ea3b9ace2..162bb1af1cf 100644 --- a/packages/core/src/shared/lsp/manifestResolver.ts +++ b/packages/core/src/shared/lsp/manifestResolver.ts @@ -70,7 +70,7 @@ export class ManifestResolver { const manifest = this.parseManifest(manifestData.content) this.checkDeprecation(manifest) - manifest.location = 'cache' + manifest.location = 'fallback' return manifest } From 3b3b39bafafe0d3a5d4835a43a1e98b7a034ad8d Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 23 Jan 2025 15:17:41 -0500 Subject: [PATCH 11/38] fix telemetry metadata field --- packages/core/src/shared/lsp/manifestResolver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/shared/lsp/manifestResolver.ts b/packages/core/src/shared/lsp/manifestResolver.ts index 162bb1af1cf..07ea3b9ace2 100644 --- a/packages/core/src/shared/lsp/manifestResolver.ts +++ b/packages/core/src/shared/lsp/manifestResolver.ts @@ -70,7 +70,7 @@ export class ManifestResolver { const manifest = this.parseManifest(manifestData.content) this.checkDeprecation(manifest) - manifest.location = 'fallback' + manifest.location = 'cache' return manifest } From abdb5fd04023d71ebad52db61a26ac65b6283e2b Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 23 Jan 2025 15:20:17 -0500 Subject: [PATCH 12/38] port metrics down from commons until release --- .../src/shared/telemetry/vscodeTelemetry.json | 48 ++++++++++++++----- 1 file changed, 37 insertions(+), 11 deletions(-) diff --git a/packages/core/src/shared/telemetry/vscodeTelemetry.json b/packages/core/src/shared/telemetry/vscodeTelemetry.json index 3a7af9a6f1c..50b00fca4bd 100644 --- a/packages/core/src/shared/telemetry/vscodeTelemetry.json +++ b/packages/core/src/shared/telemetry/vscodeTelemetry.json @@ -1,21 +1,32 @@ { "types": [ + { + "name": "languageServerLocation", + "type": "string", + "allowedValues": ["cache", "remote", "fallback", "override"], + "description": "The location of the language server" + }, { "name": "languageServerSetupStage", "type": "string", - "allowedValues": ["getManifest", "getServer", "validate", "launch", "final"], + "allowedValues": ["getManifest", "getServer", "validate", "launch", "handshake", "all"], "description": "The stage of the LSP setup process" }, { - "name": "languageServerResourceLocation", + "name": "languageServerVersion", + "type": "string", + "description": "The version of the language server" + }, + { + "name": "manifestLocation", "type": "string", - "allowedValues": ["cache", "remote", "fallback", "override", "unknown"], - "description": "The location of the LSP resource" + "allowedValues": ["cache", "remote", "override"], + "description": "The location of the manifest" }, { - "name": "manifestVersion", + "name": "manifestSchemaVersion", "type": "string", - "description": "The version of the manifest file" + "description": "The version of the manifest schema file" }, { "name": "amazonGenerateApproachLatency", @@ -377,19 +388,34 @@ "metrics": [ { "name": "languageServer_setup", - "description": "LSP setup event", + "description": "Sets up a language server", + "passive": true, + "unit": "Milliseconds", "metadata": [ { - "type": "languageServerSetupStage", - "required": true + "type": "id" }, { - "type": "languageServerResourceLocation", + "type": "languageServerLocation", "required": false }, { - "type": "manifestVersion", + "type": "languageServerSetupStage" + }, + { + "type": "languageServerVersion", "required": false + }, + { + "type": "manifestLocation", + "required": false + }, + { + "type": "manifestSchemaVersion", + "required": false + }, + { + "type": "result" } ] }, From 0aadddfe0d48eed59ceab54a7efd4cdc196f19b9 Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 23 Jan 2025 16:01:58 -0500 Subject: [PATCH 13/38] implement logic for manifest telemetry --- .../core/src/amazonq/lsp/lspController.ts | 14 ++++--- packages/core/src/amazonq/lsp/util.ts | 39 ++++++++++++++++--- .../src/amazonq/lsp/workspaceInstaller.ts | 30 +++----------- .../core/src/shared/lsp/manifestResolver.ts | 16 ++++++-- 4 files changed, 61 insertions(+), 38 deletions(-) diff --git a/packages/core/src/amazonq/lsp/lspController.ts b/packages/core/src/amazonq/lsp/lspController.ts index 74d13aec882..7356399b304 100644 --- a/packages/core/src/amazonq/lsp/lspController.ts +++ b/packages/core/src/amazonq/lsp/lspController.ts @@ -161,11 +161,7 @@ export class LspController { } setImmediate(async () => { try { - await lspSetupStage('final', async () => { - const installResult = await new WorkspaceLSPResolver().resolve() - await lspSetupStage('launch', async () => activateLsp(context, installResult.resourcePaths)) - getLogger().info('LspController: LSP activated') - }) + await this.setupLsp(context) void LspController.instance.buildIndex(buildIndexConfig) // log the LSP server CPU and Memory usage per 30 minutes. globals.clock.setInterval( @@ -186,4 +182,12 @@ export class LspController { } }) } + + private async setupLsp(context: vscode.ExtensionContext) { + await lspSetupStage('all', async () => { + const installResult = await new WorkspaceLSPResolver().resolve() + await lspSetupStage('launch', async () => activateLsp(context, installResult.resourcePaths)) + getLogger().info('LspController: LSP activated') + }) + } } diff --git a/packages/core/src/amazonq/lsp/util.ts b/packages/core/src/amazonq/lsp/util.ts index bfc24827aec..3829f643e79 100644 --- a/packages/core/src/amazonq/lsp/util.ts +++ b/packages/core/src/amazonq/lsp/util.ts @@ -3,14 +3,43 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { LanguageServerSetupStage, telemetry } from '../../shared/telemetry' +import { LanguageServerSetup, LanguageServerSetupStage, telemetry } from '../../shared/telemetry' -export async function lspSetupStage(stageName: LanguageServerSetupStage, stage: () => Promise) { +/** + * Runs the designated stage within a telemetry span and optionally uses the getMetadata extractor to record metadata from the result of the stage. + * @param stageName name of stage for telemetry. + * @param runStage stage to be run. + * @param getMetadata metadata extracter to be applied to result. + * @returns result of stage + */ +export async function lspSetupStage( + stageName: LanguageServerSetupStage, + runStage: () => Promise, + getMetadata?: (result: T) => Partial +) { return await telemetry.languageServer_setup.run(async (span) => { - const startTime = performance.now() - const result = await stage() + const result = await runStage() span.record({ languageServerSetupStage: stageName }) - span.record({ duration: performance.now() - startTime }) + if (getMetadata) { + span.record(getMetadata(result)) + } return result }) } + +/** + * Try Functions in the order presented and return the first returned result. If none return, throw the final error. + * @param functions non-empty list of functions to try. + * @returns + */ +export async function tryFunctions(functions: (() => Promise)[]): Promise { + let currentError: Error = new Error('No functions provided') + for (const func of functions) { + try { + return await func() + } catch (e) { + currentError = e as Error + } + } + throw currentError +} diff --git a/packages/core/src/amazonq/lsp/workspaceInstaller.ts b/packages/core/src/amazonq/lsp/workspaceInstaller.ts index 9cca1603551..6c5d869a70a 100644 --- a/packages/core/src/amazonq/lsp/workspaceInstaller.ts +++ b/packages/core/src/amazonq/lsp/workspaceInstaller.ts @@ -10,8 +10,6 @@ import { LanguageServerResolver } from '../../shared/lsp/lspResolver' import { Range } from 'semver' import { getNodeExecutableName } from '../../shared/lsp/utils/platform' import { fs } from '../../shared/fs/fs' -import { telemetry } from '../../shared/telemetry' -import { lspSetupStage } from './util' 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 @@ -20,28 +18,12 @@ const supportedLspServerVersions = '0.1.32' export class WorkspaceLSPResolver implements LspResolver { async resolve(): Promise { const name = 'AmazonQ-Workspace' - const manifest = await lspSetupStage('getManifest', async () => { - const result = await new ManifestResolver(manifestUrl, name).resolve() - telemetry.record({ - manifestVersion: result.manifestSchemaVersion, - languageServerResourceLocation: result.location ?? 'unknown', - }) - return result - }) - telemetry.record({ - manifestVersion: manifest.manifestSchemaVersion, - }) - const installationResult = await lspSetupStage('getServer', async () => { - const result = await new LanguageServerResolver( - manifest, - name, - new Range(supportedLspServerVersions) - ).resolve() - telemetry.record({ - languageServerResourceLocation: result.location ?? 'unknown', - }) - return result - }) + const manifest = await new ManifestResolver(manifestUrl, name).resolve() + const installationResult = await new LanguageServerResolver( + manifest, + name, + new Range(supportedLspServerVersions) + ).resolve() const nodeName = process.platform === 'win32' ? getNodeExecutableName() : `node-${process.platform}-${process.arch}` diff --git a/packages/core/src/shared/lsp/manifestResolver.ts b/packages/core/src/shared/lsp/manifestResolver.ts index 07ea3b9ace2..f3a0a3cfcda 100644 --- a/packages/core/src/shared/lsp/manifestResolver.ts +++ b/packages/core/src/shared/lsp/manifestResolver.ts @@ -9,6 +9,7 @@ import { RetryableResourceFetcher } from '../resourcefetcher/httpResourceFetcher import { Timeout } from '../utilities/timeoutUtils' import globals from '../extensionGlobals' import { Manifest } from './types' +import { lspSetupStage, tryFunctions } from '../../amazonq/lsp/util' const logger = getLogger('lsp') @@ -32,10 +33,17 @@ export class ManifestResolver { * Fetches the latest manifest, falling back to local cache on failure */ async resolve(): Promise { - try { - return await this.fetchRemoteManifest() - } catch (error) { - return await this.getLocalManifest() + return await tryFunctions([ + async () => await resolveManifestWith(async () => await this.fetchRemoteManifest()), + async () => await resolveManifestWith(async () => await this.getLocalManifest()), + ]) + + async function resolveManifestWith(resolver: () => Promise) { + return await lspSetupStage('getManifest', async () => await resolver(), extractVersionFromResult) + } + + function extractVersionFromResult(r: Manifest) { + return { manifestSchemaVersion: r.manifestSchemaVersion } } } From 7d05c682c4e25b97ce7b57b9fbd066862fa28fb3 Mon Sep 17 00:00:00 2001 From: hkobew Date: Fri, 24 Jan 2025 11:59:32 -0500 Subject: [PATCH 14/38] telemetry: implement logic for server resolution --- packages/core/src/shared/lsp/lspResolver.ts | 109 +++++++++++------- .../core/src/shared/lsp/manifestResolver.ts | 23 ++-- packages/core/src/shared/lsp/types.ts | 7 +- 3 files changed, 84 insertions(+), 55 deletions(-) diff --git a/packages/core/src/shared/lsp/lspResolver.ts b/packages/core/src/shared/lsp/lspResolver.ts index c789c591580..5903aa1bc6c 100644 --- a/packages/core/src/shared/lsp/lspResolver.ts +++ b/packages/core/src/shared/lsp/lspResolver.ts @@ -13,7 +13,7 @@ import { TargetContent, logger, LspResult, LspVersion, Manifest } from './types' import { getApplicationSupportFolder } from '../vscode/env' import { createHash } from '../crypto' import request from '../request' -import { lspSetupStage } from '../../amazonq/lsp/util' +import { lspSetupStage, tryFunctions } from '../../amazonq/lsp/util' export class LanguageServerResolver { constructor( @@ -31,51 +31,38 @@ export class LanguageServerResolver { * @throws ToolkitError if no compatible version can be found */ async resolve() { - const result: LspResult = { - location: 'unknown', - version: '', - assetDirectory: '', - } - const latestVersion = this.latestCompatibleLspVersion() const targetContents = this.getLSPTargetContents(latestVersion) const cacheDirectory = this.getDownloadDirectory(latestVersion.serverVersion) - if (await this.hasValidLocalCache(cacheDirectory, targetContents)) { - result.location = 'cache' - result.version = latestVersion.serverVersion - result.assetDirectory = cacheDirectory - return result - } else { - // Delete the cached directory since it's invalid - if (await fs.existsDir(cacheDirectory)) { - await fs.delete(cacheDirectory, { - recursive: true, - }) - } - } - - if (await this.downloadRemoteTargetContent(targetContents, latestVersion.serverVersion)) { - result.location = 'remote' - result.version = latestVersion.serverVersion - result.assetDirectory = cacheDirectory - return result - } else { - // clean up any leftover content that may have been downloaded - if (await fs.existsDir(cacheDirectory)) { - await fs.delete(cacheDirectory, { - recursive: true, - }) - } + const serverResolvers = [ + async () => await this.getLocalServer(cacheDirectory, latestVersion, targetContents), + async () => await this.fetchRemoteServer(cacheDirectory, latestVersion, targetContents), + async () => await this.getFallbackServer(latestVersion), + ].map((resolver) => async () => await resolveServerWith(resolver)) + + return await tryFunctions(serverResolvers) + + async function resolveServerWith(resolver: () => Promise) { + return await lspSetupStage( + 'getServer', + async () => await resolver(), + (result) => { + return { + languageServerLocation: result.location, + languageServerVersion: result.version, + } + } + ) } + } - logger.info( - `Unable to download language server version ${latestVersion.serverVersion}. Attempting to fetch from fallback location` - ) - + async getFallbackServer(latestVersion: LspVersion): Promise { const fallbackDirectory = await this.getFallbackDir(latestVersion.serverVersion) if (!fallbackDirectory) { - throw new ToolkitError('Unable to find a compatible version of the Language Server') + throw new ToolkitError('Unable to find a compatible version of the Language Server', { + code: 'IncompatibleVersion', + }) } const version = path.basename(fallbackDirectory) @@ -83,11 +70,49 @@ export class LanguageServerResolver { `Unable to install ${this.lsName} language server v${latestVersion.serverVersion}. Launching a previous version from ${fallbackDirectory}` ) - result.location = 'fallback' - result.version = version - result.assetDirectory = fallbackDirectory + return { + location: 'fallback', + version: version, + assetDirectory: fallbackDirectory, + } + } + + async fetchRemoteServer( + cacheDirectory: string, + latestVersion: LspVersion, + targetContents: TargetContent[] + ): Promise { + if (await this.downloadRemoteTargetContent(targetContents, latestVersion.serverVersion)) { + return { + location: 'remote', + version: latestVersion.serverVersion, + assetDirectory: cacheDirectory, + } + } else { + throw new ToolkitError('Failed to download server from remote', { code: 'RemoteDownloadFailed' }) + } + } - return result + async getLocalServer( + cacheDirectory: string, + latestVersion: LspVersion, + targetContents: TargetContent[] + ): Promise { + if (await this.hasValidLocalCache(cacheDirectory, targetContents)) { + return { + location: 'cache', + version: latestVersion.serverVersion, + assetDirectory: cacheDirectory, + } + } else { + // Delete the cached directory since it's invalid + if (await fs.existsDir(cacheDirectory)) { + await fs.delete(cacheDirectory, { + recursive: true, + }) + } + throw new ToolkitError('Failed to retrieve server from cache', { code: 'InvalidCache' }) + } } /** diff --git a/packages/core/src/shared/lsp/manifestResolver.ts b/packages/core/src/shared/lsp/manifestResolver.ts index f3a0a3cfcda..1cc8ed647c8 100644 --- a/packages/core/src/shared/lsp/manifestResolver.ts +++ b/packages/core/src/shared/lsp/manifestResolver.ts @@ -33,17 +33,22 @@ export class ManifestResolver { * Fetches the latest manifest, falling back to local cache on failure */ async resolve(): Promise { - return await tryFunctions([ - async () => await resolveManifestWith(async () => await this.fetchRemoteManifest()), - async () => await resolveManifestWith(async () => await this.getLocalManifest()), - ]) + const resolvers = [async () => await this.fetchRemoteManifest(), async () => await this.getLocalManifest()].map( + (resolver) => async () => await resolveManifestWith(resolver) + ) + return await tryFunctions(resolvers) async function resolveManifestWith(resolver: () => Promise) { - return await lspSetupStage('getManifest', async () => await resolver(), extractVersionFromResult) - } - - function extractVersionFromResult(r: Manifest) { - return { manifestSchemaVersion: r.manifestSchemaVersion } + return await lspSetupStage( + 'getManifest', + async () => await resolver(), + (result) => { + return { + manifestSchemaVersion: result.manifestSchemaVersion, + manifestLocation: result.location, + } + } + ) } } diff --git a/packages/core/src/shared/lsp/types.ts b/packages/core/src/shared/lsp/types.ts index 3ed009ffd7c..ef7518db69a 100644 --- a/packages/core/src/shared/lsp/types.ts +++ b/packages/core/src/shared/lsp/types.ts @@ -4,13 +4,12 @@ */ import { getLogger } from '../logger/logger' +import { LanguageServerLocation, ManifestLocation } from '../telemetry' export const logger = getLogger('lsp') -type Location = 'remote' | 'cache' | 'override' | 'fallback' | 'unknown' - export interface LspResult { - location: Location + location: LanguageServerLocation version: string assetDirectory: string } @@ -53,7 +52,7 @@ export interface Manifest { artifactDescription: string isManifestDeprecated: boolean versions: LspVersion[] - location?: Location + location?: ManifestLocation } export interface VersionRange { From c7b97e2c0da5a5a5b2d0f0c039325786e01532f8 Mon Sep 17 00:00:00 2001 From: hkobew Date: Fri, 24 Jan 2025 12:32:49 -0500 Subject: [PATCH 15/38] refactor: move tryFunctions to core and include tests --- packages/core/src/amazonq/lsp/util.ts | 17 ----------- packages/core/src/shared/lsp/lspResolver.ts | 3 +- .../core/src/shared/lsp/manifestResolver.ts | 3 +- packages/core/src/shared/utilities/tsUtils.ts | 17 +++++++++++ .../src/test/shared/utilities/tsUtils.test.ts | 28 +++++++++++++++++++ 5 files changed, 49 insertions(+), 19 deletions(-) create mode 100644 packages/core/src/test/shared/utilities/tsUtils.test.ts diff --git a/packages/core/src/amazonq/lsp/util.ts b/packages/core/src/amazonq/lsp/util.ts index 3829f643e79..f0a384db115 100644 --- a/packages/core/src/amazonq/lsp/util.ts +++ b/packages/core/src/amazonq/lsp/util.ts @@ -26,20 +26,3 @@ export async function lspSetupStage( return result }) } - -/** - * Try Functions in the order presented and return the first returned result. If none return, throw the final error. - * @param functions non-empty list of functions to try. - * @returns - */ -export async function tryFunctions(functions: (() => Promise)[]): Promise { - let currentError: Error = new Error('No functions provided') - for (const func of functions) { - try { - return await func() - } catch (e) { - currentError = e as Error - } - } - throw currentError -} diff --git a/packages/core/src/shared/lsp/lspResolver.ts b/packages/core/src/shared/lsp/lspResolver.ts index 5903aa1bc6c..26043eb1d25 100644 --- a/packages/core/src/shared/lsp/lspResolver.ts +++ b/packages/core/src/shared/lsp/lspResolver.ts @@ -13,7 +13,8 @@ import { TargetContent, logger, LspResult, LspVersion, Manifest } from './types' import { getApplicationSupportFolder } from '../vscode/env' import { createHash } from '../crypto' import request from '../request' -import { lspSetupStage, tryFunctions } from '../../amazonq/lsp/util' +import { lspSetupStage } from '../../amazonq/lsp/util' +import { tryFunctions } from '../utilities/tsUtils' export class LanguageServerResolver { constructor( diff --git a/packages/core/src/shared/lsp/manifestResolver.ts b/packages/core/src/shared/lsp/manifestResolver.ts index 1cc8ed647c8..82e90596507 100644 --- a/packages/core/src/shared/lsp/manifestResolver.ts +++ b/packages/core/src/shared/lsp/manifestResolver.ts @@ -9,7 +9,8 @@ import { RetryableResourceFetcher } from '../resourcefetcher/httpResourceFetcher import { Timeout } from '../utilities/timeoutUtils' import globals from '../extensionGlobals' import { Manifest } from './types' -import { lspSetupStage, tryFunctions } from '../../amazonq/lsp/util' +import { lspSetupStage } from '../../amazonq/lsp/util' +import { tryFunctions } from '../utilities/tsUtils' const logger = getLogger('lsp') diff --git a/packages/core/src/shared/utilities/tsUtils.ts b/packages/core/src/shared/utilities/tsUtils.ts index e4fbf5a2b3f..80d7736f7e0 100644 --- a/packages/core/src/shared/utilities/tsUtils.ts +++ b/packages/core/src/shared/utilities/tsUtils.ts @@ -94,6 +94,23 @@ export function createFactoryFunction any>(cto return (...args) => new ctor(...args) } +/** + * Try Functions in the order presented and return the first returned result. If none return, throw the final error. + * @param functions non-empty list of functions to try. + * @returns + */ +export async function tryFunctions(functions: (() => Promise)[]): Promise { + let currentError: Error = new Error('No functions provided') + for (const func of functions) { + try { + return await func() + } catch (e) { + currentError = e as Error + } + } + throw currentError +} + type NoSymbols = { [Property in keyof T]: Property extends symbol ? never : Property }[keyof T] export type InterfaceNoSymbol = Pick> /** diff --git a/packages/core/src/test/shared/utilities/tsUtils.test.ts b/packages/core/src/test/shared/utilities/tsUtils.test.ts new file mode 100644 index 00000000000..bb75a8c436e --- /dev/null +++ b/packages/core/src/test/shared/utilities/tsUtils.test.ts @@ -0,0 +1,28 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import { tryFunctions } from '../../../shared/utilities/tsUtils' +import assert from 'assert' + +describe('tryFunctions', function () { + it('should return the result of the first function that returns', async function () { + const f1 = () => Promise.reject('f1') + const f2 = () => Promise.resolve('f2') + const f3 = () => Promise.reject('f3') + + assert.strictEqual(await tryFunctions([f1, f2, f3]), 'f2') + }) + + it('if all reject, then should throw final error', async function () { + const f1 = () => Promise.reject('f1') + const f2 = () => Promise.reject('f2') + const f3 = () => Promise.reject('f3') + + await assert.rejects( + async () => await tryFunctions([f1, f2, f3]), + (e) => e === 'f3' + ) + }) +}) From 5284763155cfe49f1a1561166bee08938f0bb5d7 Mon Sep 17 00:00:00 2001 From: hkobew Date: Fri, 24 Jan 2025 13:18:48 -0500 Subject: [PATCH 16/38] telemtry: implement telemetry for amazonQLSP --- packages/amazonq/src/lsp/activation.ts | 7 +++++-- packages/core/src/amazonq/index.ts | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/amazonq/src/lsp/activation.ts b/packages/amazonq/src/lsp/activation.ts index bdf4a59ab1a..2361da768f3 100644 --- a/packages/amazonq/src/lsp/activation.ts +++ b/packages/amazonq/src/lsp/activation.ts @@ -7,11 +7,14 @@ import vscode from 'vscode' import { startLanguageServer } from './client' import { AmazonQLSPResolver } from './lspInstaller' import { ToolkitError } from 'aws-core-vscode/shared' +import { lspSetupStage } from 'aws-core-vscode/amazonq' export async function activate(ctx: vscode.ExtensionContext): Promise { try { - const installResult = await new AmazonQLSPResolver().resolve() - await startLanguageServer(ctx, installResult.resourcePaths) + await lspSetupStage('all', async () => { + const installResult = await new AmazonQLSPResolver().resolve() + await lspSetupStage('launch', async () => await startLanguageServer(ctx, installResult.resourcePaths)) + }) } catch (err) { const e = err as ToolkitError void vscode.window.showInformationMessage(`Unable to launch amazonq language server: ${e.message}`) diff --git a/packages/core/src/amazonq/index.ts b/packages/core/src/amazonq/index.ts index c5abbf7658e..0c1b1803995 100644 --- a/packages/core/src/amazonq/index.ts +++ b/packages/core/src/amazonq/index.ts @@ -17,6 +17,7 @@ export { } from './onboardingPage/walkthrough' export { LspController } from './lsp/lspController' export { LspClient } from './lsp/lspClient' +export { lspSetupStage } from './lsp/util' export { api } from './extApi' export { AmazonQChatViewProvider } from './webview/webView' export { init as cwChatAppInit } from '../codewhispererChat/app' From 7dfefe68b8c97aa4773cae5e46df11fe81f9bb00 Mon Sep 17 00:00:00 2001 From: hkobew Date: Fri, 24 Jan 2025 16:18:56 -0500 Subject: [PATCH 17/38] test: implement tests for manifestResolver --- packages/core/src/amazonq/lsp/util.ts | 32 ++++++- .../core/src/shared/lsp/manifestResolver.ts | 36 +++---- .../test/shared/lsp/manifestResolver.test.ts | 95 +++++++++++++++++++ 3 files changed, 143 insertions(+), 20 deletions(-) create mode 100644 packages/core/src/test/shared/lsp/manifestResolver.test.ts diff --git a/packages/core/src/amazonq/lsp/util.ts b/packages/core/src/amazonq/lsp/util.ts index f0a384db115..b0ed397f3c0 100644 --- a/packages/core/src/amazonq/lsp/util.ts +++ b/packages/core/src/amazonq/lsp/util.ts @@ -4,6 +4,7 @@ */ import { LanguageServerSetup, LanguageServerSetupStage, telemetry } from '../../shared/telemetry' +import { tryFunctions } from '../../shared/utilities/tsUtils' /** * Runs the designated stage within a telemetry span and optionally uses the getMetadata extractor to record metadata from the result of the stage. @@ -15,14 +16,41 @@ import { LanguageServerSetup, LanguageServerSetupStage, telemetry } from '../../ export async function lspSetupStage( stageName: LanguageServerSetupStage, runStage: () => Promise, - getMetadata?: (result: T) => Partial + getMetadata?: MetadataExtracter ) { return await telemetry.languageServer_setup.run(async (span) => { - const result = await runStage() span.record({ languageServerSetupStage: stageName }) + const result = await runStage() if (getMetadata) { span.record(getMetadata(result)) } return result }) } + +export async function tryResolvers( + stageName: LanguageServerSetupStage, + resolvers: StageResolver[], + getMetadata: MetadataExtracter +) { + const fs = resolvers.map((resolver) => async () => { + return await lspSetupStage( + stageName, + async () => { + telemetry.record(resolver.telemetryMetadata) + const result = await resolver.resolve() + return result + }, + getMetadata + ) + }) + + return await tryFunctions(fs) +} + +export interface StageResolver { + resolve: () => Promise + telemetryMetadata: Partial +} + +type MetadataExtracter = (r: R) => Partial diff --git a/packages/core/src/shared/lsp/manifestResolver.ts b/packages/core/src/shared/lsp/manifestResolver.ts index 82e90596507..b8b46c0333a 100644 --- a/packages/core/src/shared/lsp/manifestResolver.ts +++ b/packages/core/src/shared/lsp/manifestResolver.ts @@ -9,8 +9,7 @@ import { RetryableResourceFetcher } from '../resourcefetcher/httpResourceFetcher import { Timeout } from '../utilities/timeoutUtils' import globals from '../extensionGlobals' import { Manifest } from './types' -import { lspSetupStage } from '../../amazonq/lsp/util' -import { tryFunctions } from '../utilities/tsUtils' +import { StageResolver, tryResolvers } from '../../amazonq/lsp/util' const logger = getLogger('lsp') @@ -34,22 +33,23 @@ export class ManifestResolver { * Fetches the latest manifest, falling back to local cache on failure */ async resolve(): Promise { - const resolvers = [async () => await this.fetchRemoteManifest(), async () => await this.getLocalManifest()].map( - (resolver) => async () => await resolveManifestWith(resolver) - ) - return await tryFunctions(resolvers) - - async function resolveManifestWith(resolver: () => Promise) { - return await lspSetupStage( - 'getManifest', - async () => await resolver(), - (result) => { - return { - manifestSchemaVersion: result.manifestSchemaVersion, - manifestLocation: result.location, - } - } - ) + const resolvers: StageResolver[] = [ + { + resolve: async () => await this.fetchRemoteManifest(), + telemetryMetadata: { id: this.lsName, manifestLocation: 'remote' }, + }, + { + resolve: async () => await this.getLocalManifest(), + telemetryMetadata: { id: this.lsName, manifestLocation: 'cache' }, + }, + ] + + return await tryResolvers('getManifest', resolvers, extractMetadata) + + function extractMetadata(r: Manifest) { + return { + manifestSchemaVersion: r.manifestSchemaVersion, + } } } diff --git a/packages/core/src/test/shared/lsp/manifestResolver.test.ts b/packages/core/src/test/shared/lsp/manifestResolver.test.ts new file mode 100644 index 00000000000..018f8a0af48 --- /dev/null +++ b/packages/core/src/test/shared/lsp/manifestResolver.test.ts @@ -0,0 +1,95 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import assert from 'assert' +import sinon from 'sinon' +import { ManifestResolver } from '../../../shared' +import { assertTelemetry } from '../../testUtil' + +describe('manifestResolver', function () { + let remoteStub: sinon.SinonStub + let localStub: sinon.SinonStub + + before(function () { + remoteStub = sinon.stub(ManifestResolver.prototype, 'fetchRemoteManifest' as any) + localStub = sinon.stub(ManifestResolver.prototype, 'getLocalManifest' as any) + }) + + after(function () { + sinon.restore() + }) + + it('attempts to fetch from remote first', async function () { + remoteStub.resolves({ + manifestSchemaVersion: '1.0.0', + artifactId: 'artifact-id', + artifactDescription: 'artifact-description', + isManifestDeprecated: false, + versions: [], + location: 'remote', + }) + + const r = await new ManifestResolver('remote-manifest.com', 'myLS').resolve() + assert.strictEqual(r.location, 'remote') + assertTelemetry('languageServer_setup', { + manifestLocation: 'remote', + manifestSchemaVersion: '1.0.0', + languageServerSetupStage: 'getManifest', + id: 'myLS', + result: 'Succeeded', + }) + }) + + it('uses local cache when remote fails', async function () { + remoteStub.rejects(new Error('failed to fetch')) + localStub.resolves({ + manifestSchemaVersion: '1.0.0', + artifactId: 'artifact-id', + artifactDescription: 'artifact-description', + isManifestDeprecated: false, + versions: [], + location: 'cache', + }) + + const r = await new ManifestResolver('remote-manifest.com', 'myLS').resolve() + assert.strictEqual(r.location, 'cache') + assertTelemetry('languageServer_setup', [ + { + manifestLocation: 'remote', + languageServerSetupStage: 'getManifest', + id: 'myLS', + result: 'Failed', + }, + { + manifestLocation: 'cache', + manifestSchemaVersion: '1.0.0', + languageServerSetupStage: 'getManifest', + id: 'myLS', + result: 'Succeeded', + }, + ]) + }) + + it('fails if both local and remote fail', async function () { + remoteStub.rejects(new Error('failed to fetch')) + localStub.rejects(new Error('failed to fetch')) + + await assert.rejects(new ManifestResolver('remote-manifest.com', 'myLS').resolve(), /failed to fetch/) + assertTelemetry('languageServer_setup', [ + { + manifestLocation: 'remote', + languageServerSetupStage: 'getManifest', + id: 'myLS', + result: 'Failed', + }, + { + manifestLocation: 'cache', + languageServerSetupStage: 'getManifest', + id: 'myLS', + result: 'Failed', + }, + ]) + }) +}) From 4b1e90ad45fedab8a981cbe80c28f96fd9ecbd63 Mon Sep 17 00:00:00 2001 From: hkobew Date: Fri, 24 Jan 2025 16:44:39 -0500 Subject: [PATCH 18/38] refactor: simplify lspResolver implementation --- packages/core/src/amazonq/lsp/util.ts | 12 +++++- packages/core/src/shared/lsp/lspResolver.ts | 43 +++++++++++---------- 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/packages/core/src/amazonq/lsp/util.ts b/packages/core/src/amazonq/lsp/util.ts index b0ed397f3c0..287ca2ebf9b 100644 --- a/packages/core/src/amazonq/lsp/util.ts +++ b/packages/core/src/amazonq/lsp/util.ts @@ -27,7 +27,14 @@ export async function lspSetupStage( return result }) } - +/** + * Tries to resolve the result of a stage using the resolvers provided in order. The first one to succceed + * has its result succeeded, but all intermediate will emit telemetry. + * @param stageName name of stage to resolve. + * @param resolvers stage resolvers to try IN ORDER + * @param getMetadata function to be applied to result to extract necessary metadata for telemetry. + * @returns result of the first succesful resolver. + */ export async function tryResolvers( stageName: LanguageServerSetupStage, resolvers: StageResolver[], @@ -48,6 +55,9 @@ export async function tryResolvers( return await tryFunctions(fs) } +/** + * A method that returns the result of a stage along with the default telemetry metadata to attach to the stage metric. + */ export interface StageResolver { resolve: () => Promise telemetryMetadata: Partial diff --git a/packages/core/src/shared/lsp/lspResolver.ts b/packages/core/src/shared/lsp/lspResolver.ts index 26043eb1d25..b4d72e97d8d 100644 --- a/packages/core/src/shared/lsp/lspResolver.ts +++ b/packages/core/src/shared/lsp/lspResolver.ts @@ -13,8 +13,7 @@ import { TargetContent, logger, LspResult, LspVersion, Manifest } from './types' import { getApplicationSupportFolder } from '../vscode/env' import { createHash } from '../crypto' import request from '../request' -import { lspSetupStage } from '../../amazonq/lsp/util' -import { tryFunctions } from '../utilities/tsUtils' +import { lspSetupStage, StageResolver, tryResolvers } from '../../amazonq/lsp/util' export class LanguageServerResolver { constructor( @@ -36,25 +35,27 @@ export class LanguageServerResolver { const targetContents = this.getLSPTargetContents(latestVersion) const cacheDirectory = this.getDownloadDirectory(latestVersion.serverVersion) - const serverResolvers = [ - async () => await this.getLocalServer(cacheDirectory, latestVersion, targetContents), - async () => await this.fetchRemoteServer(cacheDirectory, latestVersion, targetContents), - async () => await this.getFallbackServer(latestVersion), - ].map((resolver) => async () => await resolveServerWith(resolver)) - - return await tryFunctions(serverResolvers) - - async function resolveServerWith(resolver: () => Promise) { - return await lspSetupStage( - 'getServer', - async () => await resolver(), - (result) => { - return { - languageServerLocation: result.location, - languageServerVersion: result.version, - } - } - ) + const serverResolvers: StageResolver[] = [ + { + resolve: async () => await this.getLocalServer(cacheDirectory, latestVersion, targetContents), + telemetryMetadata: { id: this.lsName, languageServerLocation: 'cache' }, + }, + { + resolve: async () => await this.fetchRemoteServer(cacheDirectory, latestVersion, targetContents), + telemetryMetadata: { id: this.lsName, languageServerLocation: 'remote' }, + }, + { + resolve: async () => await this.getFallbackServer(latestVersion), + telemetryMetadata: { id: this.lsName, languageServerLocation: 'fallback' }, + }, + ] + + return await tryResolvers('getServer', serverResolvers, getServerVersion) + + function getServerVersion(result: LspResult) { + return { + languageServerVersion: result.version, + } } } From 45901e7dbda1dddb29b352e7690eb839fee61711 Mon Sep 17 00:00:00 2001 From: hkobew Date: Fri, 24 Jan 2025 17:53:53 -0500 Subject: [PATCH 19/38] test: add tests for lsp resolver (WIP) --- packages/core/src/test/shared/lsp/lspResolver.test.ts | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 packages/core/src/test/shared/lsp/lspResolver.test.ts diff --git a/packages/core/src/test/shared/lsp/lspResolver.test.ts b/packages/core/src/test/shared/lsp/lspResolver.test.ts new file mode 100644 index 00000000000..383dfee0afd --- /dev/null +++ b/packages/core/src/test/shared/lsp/lspResolver.test.ts @@ -0,0 +1,9 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import assert from 'assert' +import sinon from 'sinon' + +describe('lspResolver', function () {}) From 2ee4792cf13281432fbbc1279b7d636fdb05955e Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 27 Jan 2025 10:47:30 -0500 Subject: [PATCH 20/38] test: add tests for lspResolver --- packages/core/src/shared/lsp/lspResolver.ts | 6 +- .../src/test/shared/lsp/lspResolver.test.ts | 234 +++++++++++++++++- .../test/shared/lsp/manifestResolver.test.ts | 4 +- 3 files changed, 238 insertions(+), 6 deletions(-) diff --git a/packages/core/src/shared/lsp/lspResolver.ts b/packages/core/src/shared/lsp/lspResolver.ts index b4d72e97d8d..d9ecd941e66 100644 --- a/packages/core/src/shared/lsp/lspResolver.ts +++ b/packages/core/src/shared/lsp/lspResolver.ts @@ -59,7 +59,7 @@ export class LanguageServerResolver { } } - async getFallbackServer(latestVersion: LspVersion): Promise { + private async getFallbackServer(latestVersion: LspVersion): Promise { const fallbackDirectory = await this.getFallbackDir(latestVersion.serverVersion) if (!fallbackDirectory) { throw new ToolkitError('Unable to find a compatible version of the Language Server', { @@ -79,7 +79,7 @@ export class LanguageServerResolver { } } - async fetchRemoteServer( + private async fetchRemoteServer( cacheDirectory: string, latestVersion: LspVersion, targetContents: TargetContent[] @@ -95,7 +95,7 @@ export class LanguageServerResolver { } } - async getLocalServer( + private async getLocalServer( cacheDirectory: string, latestVersion: LspVersion, targetContents: TargetContent[] diff --git a/packages/core/src/test/shared/lsp/lspResolver.test.ts b/packages/core/src/test/shared/lsp/lspResolver.test.ts index 383dfee0afd..f774957d491 100644 --- a/packages/core/src/test/shared/lsp/lspResolver.test.ts +++ b/packages/core/src/test/shared/lsp/lspResolver.test.ts @@ -5,5 +5,237 @@ import assert from 'assert' import sinon from 'sinon' +import { LanguageServerResolver, LspResult, Manifest } from '../../../shared' +import { Range } from 'semver' +import { assertTelemetry } from '../../testUtil' -describe('lspResolver', function () {}) +describe('lspResolver', function () { + let remoteStub: sinon.SinonStub + let localStub: sinon.SinonStub + let fallbackStub: sinon.SinonStub + let manifest: Manifest + let versionRange: Range + + before(function () { + remoteStub = sinon.stub(LanguageServerResolver.prototype, 'fetchRemoteServer' as any) + localStub = sinon.stub(LanguageServerResolver.prototype, 'getLocalServer' as any) + fallbackStub = sinon.stub(LanguageServerResolver.prototype, 'getFallbackServer' as any) + manifest = { + manifestSchemaVersion: '1.0.0', + artifactId: 'artifact-id', + artifactDescription: 'artifact-description', + isManifestDeprecated: false, + versions: [ + { + serverVersion: '1.0.0', + isDelisted: false, + targets: [ + { + platform: 'linux', + arch: 'x64', + contents: [ + { + filename: 'myLS-linux-x64.zip', + url: 'https://example.com/qserver-linux-x64.zip', + hashes: ['sha384:thisisahash'], + bytes: 100, + }, + { + filename: 'node-linux-x64', + url: 'https://example.com/temp-assets/node-linux-x64', + hashes: ['sha384:thisisanotherhash'], + bytes: 200, + }, + ], + }, + { + platform: 'linux', + arch: 'arm64', + contents: [ + { + filename: 'myLS-linux-arm64.zip', + url: 'https://example.com/qserver-linux-arm64.zip', + hashes: ['sha384:thisisahash'], + bytes: 100, + }, + { + filename: 'node-linux-arm64', + url: 'https://example.com/temp-assets/node-linux-arm64', + hashes: ['sha384:thisisanotherhash'], + bytes: 200, + }, + ], + }, + { + platform: 'darwin', + arch: 'x64', + contents: [ + { + filename: 'myLS-darwin-x64.zip', + url: 'https://example.com/qserver-darwin-x64.zip', + hashes: ['sha384:thisisahash'], + bytes: 100, + }, + { + filename: 'node-linux-x64', + url: 'https://example.com/temp-assets/node-darwin-x64', + hashes: ['sha384:thisisanotherhash'], + bytes: 200, + }, + ], + }, + { + platform: 'darwin', + arch: 'arm64', + contents: [ + { + filename: 'myLS-darwin-arm64.zip', + url: 'https://example.com/qserver-darwin-arm64.zip', + hashes: ['sha384:thisisahash'], + bytes: 100, + }, + { + filename: 'node-linux-arm64', + url: 'https://example.com/temp-assets/node-darwin-arm64', + hashes: ['sha384:thisisanotherhash'], + bytes: 200, + }, + ], + }, + { + platform: 'windows', + arch: 'x64', + contents: [ + { + filename: 'myLS-windows-x64.zip', + url: 'https://example.com/qserver-windows-x64.zip', + hashes: ['sha384:thisisahash'], + bytes: 100, + }, + { + filename: 'node-linux-x64', + url: 'https://example.com/temp-assets/node-windows-x64', + hashes: ['sha384:thisisanotherhash'], + bytes: 200, + }, + ], + }, + ], + }, + ], + location: 'remote', + } + versionRange = new Range('>=1.0.0') + }) + + after(function () { + sinon.restore() + }) + + it('tries local cache first', async function () { + localStub.resolves({ + location: 'cache', + version: '1.0.0', + assetDirectory: 'path/to/assets', + } satisfies LspResult) + + const r = await new LanguageServerResolver(manifest, 'myLS', versionRange).resolve() + assert.strictEqual(r.location, 'cache') + assertTelemetry('languageServer_setup', { + languageServerSetupStage: 'getServer', + id: 'myLS', + languageServerLocation: 'cache', + languageServerVersion: '1.0.0', + result: 'Succeeded', + }) + }) + + it('tries fetching remote if cache fails', async function () { + localStub.rejects(new Error('not found')) + remoteStub.resolves({ + location: 'remote', + version: '1.0.0', + assetDirectory: 'path/to/assets', + } satisfies LspResult) + + const r = await new LanguageServerResolver(manifest, 'myLS', versionRange).resolve() + assert.strictEqual(r.location, 'remote') + assertTelemetry('languageServer_setup', [ + { + languageServerSetupStage: 'getServer', + id: 'myLS', + languageServerLocation: 'cache', + result: 'Failed', + }, + { + languageServerSetupStage: 'getServer', + id: 'myLS', + languageServerLocation: 'remote', + languageServerVersion: '1.0.0', + result: 'Succeeded', + }, + ]) + }) + + it('tries fallback version if both remote and cache fail', async function () { + localStub.rejects(new Error('not found')) + remoteStub.rejects(new Error('not found')) + fallbackStub.resolves({ + location: 'fallback', + version: '1.0.0', + assetDirectory: 'path/to/assets', + } satisfies LspResult) + + const r = await new LanguageServerResolver(manifest, 'myLS', versionRange).resolve() + assert.strictEqual(r.location, 'fallback') + assertTelemetry('languageServer_setup', [ + { + languageServerSetupStage: 'getServer', + id: 'myLS', + languageServerLocation: 'cache', + result: 'Failed', + }, + { + languageServerSetupStage: 'getServer', + id: 'myLS', + languageServerLocation: 'remote', + result: 'Failed', + }, + { + languageServerSetupStage: 'getServer', + id: 'myLS', + languageServerLocation: 'fallback', + languageServerVersion: '1.0.0', + result: 'Succeeded', + }, + ]) + }) + + it('rejects if local, remote, and fallback all reject', async function () { + localStub.rejects(new Error('not found')) + remoteStub.rejects(new Error('not found')) + fallbackStub.rejects(new Error('not found')) + + await assert.rejects(new LanguageServerResolver(manifest, 'myLS', versionRange).resolve(), /not found/) + assertTelemetry('languageServer_setup', [ + { + languageServerSetupStage: 'getServer', + id: 'myLS', + languageServerLocation: 'cache', + result: 'Failed', + }, + { + languageServerSetupStage: 'getServer', + id: 'myLS', + languageServerLocation: 'remote', + result: 'Failed', + }, + { + languageServerSetupStage: 'getServer', + id: 'myLS', + languageServerLocation: 'fallback', + result: 'Failed', + }, + ]) + }) +}) diff --git a/packages/core/src/test/shared/lsp/manifestResolver.test.ts b/packages/core/src/test/shared/lsp/manifestResolver.test.ts index 018f8a0af48..46288a6927c 100644 --- a/packages/core/src/test/shared/lsp/manifestResolver.test.ts +++ b/packages/core/src/test/shared/lsp/manifestResolver.test.ts @@ -5,7 +5,7 @@ import assert from 'assert' import sinon from 'sinon' -import { ManifestResolver } from '../../../shared' +import { Manifest, ManifestResolver } from '../../../shared' import { assertTelemetry } from '../../testUtil' describe('manifestResolver', function () { @@ -29,7 +29,7 @@ describe('manifestResolver', function () { isManifestDeprecated: false, versions: [], location: 'remote', - }) + } satisfies Manifest) const r = await new ManifestResolver('remote-manifest.com', 'myLS').resolve() assert.strictEqual(r.location, 'remote') From 3a6bad220db09c4f93344d512c27d48ac8f8a9cd Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 27 Jan 2025 11:06:23 -0500 Subject: [PATCH 21/38] test: remove duplicate data in tests --- .../src/test/shared/lsp/lspResolver.test.ts | 198 +++++++----------- .../test/shared/lsp/manifestResolver.test.ts | 58 ++--- 2 files changed, 104 insertions(+), 152 deletions(-) diff --git a/packages/core/src/test/shared/lsp/lspResolver.test.ts b/packages/core/src/test/shared/lsp/lspResolver.test.ts index f774957d491..3b7ec5be0d5 100644 --- a/packages/core/src/test/shared/lsp/lspResolver.test.ts +++ b/packages/core/src/test/shared/lsp/lspResolver.test.ts @@ -5,9 +5,53 @@ import assert from 'assert' import sinon from 'sinon' -import { LanguageServerResolver, LspResult, Manifest } from '../../../shared' +import { LanguageServerResolver, LspResult, Manifest, Target } from '../../../shared' import { Range } from 'semver' import { assertTelemetry } from '../../testUtil' +import { LanguageServerLocation } from '../../../shared/telemetry' + +const serverVersion = '1.0.0' +const assetDirectory = 'path/to/assets' +const serverName = 'myLS' + +/** + * Helper function for generating valid manifest for tests. + * @param platform + * @param arch + * @returns + */ +function manifestTarget(platform: 'linux' | 'darwin' | 'windows', arch: 'x64' | 'arm64'): Target { + return { + platform, + arch, + contents: [ + { + filename: `${serverName}-${platform}-${arch}.zip`, + url: `https://example.com/lsp-${platform}-${arch}.zip`, + hashes: ['sha384:thisisahash'], + bytes: 100, + }, + { + filename: `node-${platform}-${arch}`, + url: `https://example.com/temp-assets/node-${platform}-${arch}`, + hashes: ['sha384:thisisanotherhash'], + bytes: 200, + }, + ], + } +} +/** + * Helper function for generating valid LspResult for tests. + * @param location + * @returns + */ +function lspResult(location: LanguageServerLocation): LspResult { + return { + location, + version: serverVersion, + assetDirectory: assetDirectory, + } +} describe('lspResolver', function () { let remoteStub: sinon.SinonStub @@ -21,111 +65,25 @@ describe('lspResolver', function () { localStub = sinon.stub(LanguageServerResolver.prototype, 'getLocalServer' as any) fallbackStub = sinon.stub(LanguageServerResolver.prototype, 'getFallbackServer' as any) manifest = { - manifestSchemaVersion: '1.0.0', + manifestSchemaVersion: '2.0.0', artifactId: 'artifact-id', artifactDescription: 'artifact-description', isManifestDeprecated: false, versions: [ { - serverVersion: '1.0.0', + serverVersion: serverVersion, isDelisted: false, targets: [ - { - platform: 'linux', - arch: 'x64', - contents: [ - { - filename: 'myLS-linux-x64.zip', - url: 'https://example.com/qserver-linux-x64.zip', - hashes: ['sha384:thisisahash'], - bytes: 100, - }, - { - filename: 'node-linux-x64', - url: 'https://example.com/temp-assets/node-linux-x64', - hashes: ['sha384:thisisanotherhash'], - bytes: 200, - }, - ], - }, - { - platform: 'linux', - arch: 'arm64', - contents: [ - { - filename: 'myLS-linux-arm64.zip', - url: 'https://example.com/qserver-linux-arm64.zip', - hashes: ['sha384:thisisahash'], - bytes: 100, - }, - { - filename: 'node-linux-arm64', - url: 'https://example.com/temp-assets/node-linux-arm64', - hashes: ['sha384:thisisanotherhash'], - bytes: 200, - }, - ], - }, - { - platform: 'darwin', - arch: 'x64', - contents: [ - { - filename: 'myLS-darwin-x64.zip', - url: 'https://example.com/qserver-darwin-x64.zip', - hashes: ['sha384:thisisahash'], - bytes: 100, - }, - { - filename: 'node-linux-x64', - url: 'https://example.com/temp-assets/node-darwin-x64', - hashes: ['sha384:thisisanotherhash'], - bytes: 200, - }, - ], - }, - { - platform: 'darwin', - arch: 'arm64', - contents: [ - { - filename: 'myLS-darwin-arm64.zip', - url: 'https://example.com/qserver-darwin-arm64.zip', - hashes: ['sha384:thisisahash'], - bytes: 100, - }, - { - filename: 'node-linux-arm64', - url: 'https://example.com/temp-assets/node-darwin-arm64', - hashes: ['sha384:thisisanotherhash'], - bytes: 200, - }, - ], - }, - { - platform: 'windows', - arch: 'x64', - contents: [ - { - filename: 'myLS-windows-x64.zip', - url: 'https://example.com/qserver-windows-x64.zip', - hashes: ['sha384:thisisahash'], - bytes: 100, - }, - { - filename: 'node-linux-x64', - url: 'https://example.com/temp-assets/node-windows-x64', - hashes: ['sha384:thisisanotherhash'], - bytes: 200, - }, - ], - }, + manifestTarget('linux', 'x64'), + manifestTarget('linux', 'arm64'), + manifestTarget('darwin', 'x64'), + manifestTarget('darwin', 'arm64'), + manifestTarget('windows', 'x64'), ], }, ], - location: 'remote', } - versionRange = new Range('>=1.0.0') + versionRange = new Range(`>=${serverVersion}`) }) after(function () { @@ -133,45 +91,37 @@ describe('lspResolver', function () { }) it('tries local cache first', async function () { - localStub.resolves({ - location: 'cache', - version: '1.0.0', - assetDirectory: 'path/to/assets', - } satisfies LspResult) + localStub.resolves(lspResult('cache')) - const r = await new LanguageServerResolver(manifest, 'myLS', versionRange).resolve() + const r = await new LanguageServerResolver(manifest, serverName, versionRange).resolve() assert.strictEqual(r.location, 'cache') assertTelemetry('languageServer_setup', { languageServerSetupStage: 'getServer', - id: 'myLS', + id: serverName, languageServerLocation: 'cache', - languageServerVersion: '1.0.0', + languageServerVersion: serverVersion, result: 'Succeeded', }) }) it('tries fetching remote if cache fails', async function () { localStub.rejects(new Error('not found')) - remoteStub.resolves({ - location: 'remote', - version: '1.0.0', - assetDirectory: 'path/to/assets', - } satisfies LspResult) + remoteStub.resolves(lspResult('remote')) - const r = await new LanguageServerResolver(manifest, 'myLS', versionRange).resolve() + const r = await new LanguageServerResolver(manifest, serverName, versionRange).resolve() assert.strictEqual(r.location, 'remote') assertTelemetry('languageServer_setup', [ { languageServerSetupStage: 'getServer', - id: 'myLS', + id: serverName, languageServerLocation: 'cache', result: 'Failed', }, { languageServerSetupStage: 'getServer', - id: 'myLS', + id: serverName, languageServerLocation: 'remote', - languageServerVersion: '1.0.0', + languageServerVersion: serverVersion, result: 'Succeeded', }, ]) @@ -180,32 +130,28 @@ describe('lspResolver', function () { it('tries fallback version if both remote and cache fail', async function () { localStub.rejects(new Error('not found')) remoteStub.rejects(new Error('not found')) - fallbackStub.resolves({ - location: 'fallback', - version: '1.0.0', - assetDirectory: 'path/to/assets', - } satisfies LspResult) + fallbackStub.resolves(lspResult('fallback')) - const r = await new LanguageServerResolver(manifest, 'myLS', versionRange).resolve() + const r = await new LanguageServerResolver(manifest, serverName, versionRange).resolve() assert.strictEqual(r.location, 'fallback') assertTelemetry('languageServer_setup', [ { languageServerSetupStage: 'getServer', - id: 'myLS', + id: serverName, languageServerLocation: 'cache', result: 'Failed', }, { languageServerSetupStage: 'getServer', - id: 'myLS', + id: serverName, languageServerLocation: 'remote', result: 'Failed', }, { languageServerSetupStage: 'getServer', - id: 'myLS', + id: serverName, languageServerLocation: 'fallback', - languageServerVersion: '1.0.0', + languageServerVersion: serverVersion, result: 'Succeeded', }, ]) @@ -216,23 +162,23 @@ describe('lspResolver', function () { remoteStub.rejects(new Error('not found')) fallbackStub.rejects(new Error('not found')) - await assert.rejects(new LanguageServerResolver(manifest, 'myLS', versionRange).resolve(), /not found/) + await assert.rejects(new LanguageServerResolver(manifest, serverName, versionRange).resolve(), /not found/) assertTelemetry('languageServer_setup', [ { languageServerSetupStage: 'getServer', - id: 'myLS', + id: serverName, languageServerLocation: 'cache', result: 'Failed', }, { languageServerSetupStage: 'getServer', - id: 'myLS', + id: serverName, languageServerLocation: 'remote', result: 'Failed', }, { languageServerSetupStage: 'getServer', - id: 'myLS', + id: serverName, languageServerLocation: 'fallback', result: 'Failed', }, diff --git a/packages/core/src/test/shared/lsp/manifestResolver.test.ts b/packages/core/src/test/shared/lsp/manifestResolver.test.ts index 46288a6927c..eb5f8e90893 100644 --- a/packages/core/src/test/shared/lsp/manifestResolver.test.ts +++ b/packages/core/src/test/shared/lsp/manifestResolver.test.ts @@ -7,6 +7,26 @@ import assert from 'assert' import sinon from 'sinon' import { Manifest, ManifestResolver } from '../../../shared' import { assertTelemetry } from '../../testUtil' +import { ManifestLocation } from '../../../shared/telemetry' + +const manifestSchemaVersion = '1.0.0' +const serverName = 'myLS' + +/** + * Helper function generating valid manifest results for tests. + * @param location + * @returns + */ +function manifestResult(location: ManifestLocation): Manifest { + return { + location, + manifestSchemaVersion, + artifactId: 'artifact-id', + artifactDescription: 'artifact-description', + isManifestDeprecated: false, + versions: [], + } +} describe('manifestResolver', function () { let remoteStub: sinon.SinonStub @@ -22,51 +42,37 @@ describe('manifestResolver', function () { }) it('attempts to fetch from remote first', async function () { - remoteStub.resolves({ - manifestSchemaVersion: '1.0.0', - artifactId: 'artifact-id', - artifactDescription: 'artifact-description', - isManifestDeprecated: false, - versions: [], - location: 'remote', - } satisfies Manifest) + remoteStub.resolves(manifestResult('remote')) - const r = await new ManifestResolver('remote-manifest.com', 'myLS').resolve() + const r = await new ManifestResolver('remote-manifest.com', serverName).resolve() assert.strictEqual(r.location, 'remote') assertTelemetry('languageServer_setup', { manifestLocation: 'remote', - manifestSchemaVersion: '1.0.0', + manifestSchemaVersion, languageServerSetupStage: 'getManifest', - id: 'myLS', + id: serverName, result: 'Succeeded', }) }) it('uses local cache when remote fails', async function () { remoteStub.rejects(new Error('failed to fetch')) - localStub.resolves({ - manifestSchemaVersion: '1.0.0', - artifactId: 'artifact-id', - artifactDescription: 'artifact-description', - isManifestDeprecated: false, - versions: [], - location: 'cache', - }) + localStub.resolves(manifestResult('cache')) - const r = await new ManifestResolver('remote-manifest.com', 'myLS').resolve() + const r = await new ManifestResolver('remote-manifest.com', serverName).resolve() assert.strictEqual(r.location, 'cache') assertTelemetry('languageServer_setup', [ { manifestLocation: 'remote', languageServerSetupStage: 'getManifest', - id: 'myLS', + id: serverName, result: 'Failed', }, { manifestLocation: 'cache', - manifestSchemaVersion: '1.0.0', + manifestSchemaVersion, languageServerSetupStage: 'getManifest', - id: 'myLS', + id: serverName, result: 'Succeeded', }, ]) @@ -76,18 +82,18 @@ describe('manifestResolver', function () { remoteStub.rejects(new Error('failed to fetch')) localStub.rejects(new Error('failed to fetch')) - await assert.rejects(new ManifestResolver('remote-manifest.com', 'myLS').resolve(), /failed to fetch/) + await assert.rejects(new ManifestResolver('remote-manifest.com', serverName).resolve(), /failed to fetch/) assertTelemetry('languageServer_setup', [ { manifestLocation: 'remote', languageServerSetupStage: 'getManifest', - id: 'myLS', + id: serverName, result: 'Failed', }, { manifestLocation: 'cache', languageServerSetupStage: 'getManifest', - id: 'myLS', + id: serverName, result: 'Failed', }, ]) From 9e708fd33d61e587d6e721cc22f308902d44639a Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 27 Jan 2025 11:12:13 -0500 Subject: [PATCH 22/38] refactor: move telemetry work to more general location --- packages/amazonq/src/lsp/activation.ts | 2 +- packages/core/src/amazonq/index.ts | 1 - packages/core/src/amazonq/lsp/lspController.ts | 2 +- packages/core/src/shared/index.ts | 1 + packages/core/src/shared/lsp/lspResolver.ts | 4 ++-- .../lsp/util.ts => shared/lsp/utils/stage.ts} | 14 +++++++------- 6 files changed, 12 insertions(+), 12 deletions(-) rename packages/core/src/{amazonq/lsp/util.ts => shared/lsp/utils/stage.ts} (85%) diff --git a/packages/amazonq/src/lsp/activation.ts b/packages/amazonq/src/lsp/activation.ts index 2361da768f3..cc04f235a94 100644 --- a/packages/amazonq/src/lsp/activation.ts +++ b/packages/amazonq/src/lsp/activation.ts @@ -7,7 +7,7 @@ import vscode from 'vscode' import { startLanguageServer } from './client' import { AmazonQLSPResolver } from './lspInstaller' import { ToolkitError } from 'aws-core-vscode/shared' -import { lspSetupStage } from 'aws-core-vscode/amazonq' +import { lspSetupStage } from 'aws-core-vscode/shared' export async function activate(ctx: vscode.ExtensionContext): Promise { try { diff --git a/packages/core/src/amazonq/index.ts b/packages/core/src/amazonq/index.ts index 0c1b1803995..c5abbf7658e 100644 --- a/packages/core/src/amazonq/index.ts +++ b/packages/core/src/amazonq/index.ts @@ -17,7 +17,6 @@ export { } from './onboardingPage/walkthrough' export { LspController } from './lsp/lspController' export { LspClient } from './lsp/lspClient' -export { lspSetupStage } from './lsp/util' export { api } from './extApi' export { AmazonQChatViewProvider } from './webview/webView' export { init as cwChatAppInit } from '../codewhispererChat/app' diff --git a/packages/core/src/amazonq/lsp/lspController.ts b/packages/core/src/amazonq/lsp/lspController.ts index 7356399b304..f556a070775 100644 --- a/packages/core/src/amazonq/lsp/lspController.ts +++ b/packages/core/src/amazonq/lsp/lspController.ts @@ -15,7 +15,7 @@ import { isCloud9 } from '../../shared/extensionUtilities' import globals, { isWeb } from '../../shared/extensionGlobals' import { isAmazonInternalOs } from '../../shared/vscode/env' import { WorkspaceLSPResolver } from './workspaceInstaller' -import { lspSetupStage } from './util' +import { lspSetupStage } from '../../shared' export interface Chunk { readonly filePath: string diff --git a/packages/core/src/shared/index.ts b/packages/core/src/shared/index.ts index 7cdf3ad12f0..3cf69bc2305 100644 --- a/packages/core/src/shared/index.ts +++ b/packages/core/src/shared/index.ts @@ -63,6 +63,7 @@ export { TabTypeDataMap } from '../amazonq/webview/ui/tabs/constants' export * from './lsp/manifestResolver' export * from './lsp/lspResolver' export * from './lsp/types' +export * from './lsp/utils/stage' export { default as request } from './request' export * from './lsp/utils/platform' export * as processUtils from './utilities/processUtils' diff --git a/packages/core/src/shared/lsp/lspResolver.ts b/packages/core/src/shared/lsp/lspResolver.ts index d9ecd941e66..8851efb3dc3 100644 --- a/packages/core/src/shared/lsp/lspResolver.ts +++ b/packages/core/src/shared/lsp/lspResolver.ts @@ -13,7 +13,7 @@ import { TargetContent, logger, LspResult, LspVersion, Manifest } from './types' import { getApplicationSupportFolder } from '../vscode/env' import { createHash } from '../crypto' import request from '../request' -import { lspSetupStage, StageResolver, tryResolvers } from '../../amazonq/lsp/util' +import { lspSetupStage, StageResolver, tryStageResolvers } from './utils/stage' export class LanguageServerResolver { constructor( @@ -50,7 +50,7 @@ export class LanguageServerResolver { }, ] - return await tryResolvers('getServer', serverResolvers, getServerVersion) + return await tryStageResolvers('getServer', serverResolvers, getServerVersion) function getServerVersion(result: LspResult) { return { diff --git a/packages/core/src/amazonq/lsp/util.ts b/packages/core/src/shared/lsp/utils/stage.ts similarity index 85% rename from packages/core/src/amazonq/lsp/util.ts rename to packages/core/src/shared/lsp/utils/stage.ts index 287ca2ebf9b..18c37eaa814 100644 --- a/packages/core/src/amazonq/lsp/util.ts +++ b/packages/core/src/shared/lsp/utils/stage.ts @@ -3,8 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { LanguageServerSetup, LanguageServerSetupStage, telemetry } from '../../shared/telemetry' -import { tryFunctions } from '../../shared/utilities/tsUtils' +import { LanguageServerSetup, LanguageServerSetupStage, telemetry } from '../../../shared/telemetry' +import { tryFunctions } from '../../../shared/utilities/tsUtils' /** * Runs the designated stage within a telemetry span and optionally uses the getMetadata extractor to record metadata from the result of the stage. @@ -13,10 +13,10 @@ import { tryFunctions } from '../../shared/utilities/tsUtils' * @param getMetadata metadata extracter to be applied to result. * @returns result of stage */ -export async function lspSetupStage( +export async function lspSetupStage( stageName: LanguageServerSetupStage, - runStage: () => Promise, - getMetadata?: MetadataExtracter + runStage: () => Promise, + getMetadata?: MetadataExtracter ) { return await telemetry.languageServer_setup.run(async (span) => { span.record({ languageServerSetupStage: stageName }) @@ -29,13 +29,13 @@ export async function lspSetupStage( } /** * Tries to resolve the result of a stage using the resolvers provided in order. The first one to succceed - * has its result succeeded, but all intermediate will emit telemetry. + * has its result returned, but all intermediate will emit telemetry. * @param stageName name of stage to resolve. * @param resolvers stage resolvers to try IN ORDER * @param getMetadata function to be applied to result to extract necessary metadata for telemetry. * @returns result of the first succesful resolver. */ -export async function tryResolvers( +export async function tryStageResolvers( stageName: LanguageServerSetupStage, resolvers: StageResolver[], getMetadata: MetadataExtracter From 0b4da9d97fb5dc66a4c2c63ca99d38e8169206d0 Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 27 Jan 2025 11:24:18 -0500 Subject: [PATCH 23/38] fix: change outdated reference --- packages/core/src/shared/lsp/manifestResolver.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/shared/lsp/manifestResolver.ts b/packages/core/src/shared/lsp/manifestResolver.ts index b8b46c0333a..6d0f72b2c6d 100644 --- a/packages/core/src/shared/lsp/manifestResolver.ts +++ b/packages/core/src/shared/lsp/manifestResolver.ts @@ -9,7 +9,7 @@ import { RetryableResourceFetcher } from '../resourcefetcher/httpResourceFetcher import { Timeout } from '../utilities/timeoutUtils' import globals from '../extensionGlobals' import { Manifest } from './types' -import { StageResolver, tryResolvers } from '../../amazonq/lsp/util' +import { StageResolver, tryStageResolvers } from './utils/stage' const logger = getLogger('lsp') @@ -44,7 +44,7 @@ export class ManifestResolver { }, ] - return await tryResolvers('getManifest', resolvers, extractMetadata) + return await tryStageResolvers('getManifest', resolvers, extractMetadata) function extractMetadata(r: Manifest) { return { From 9cee3e79af4edebedb0342c2ba76eddd8df01600 Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 27 Jan 2025 11:34:51 -0500 Subject: [PATCH 24/38] refactor: rename stage -> setupStage --- packages/core/src/shared/index.ts | 2 +- packages/core/src/shared/lsp/lspResolver.ts | 2 +- packages/core/src/shared/lsp/manifestResolver.ts | 2 +- .../core/src/shared/lsp/utils/{stage.ts => setupStage.ts} | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) rename packages/core/src/shared/lsp/utils/{stage.ts => setupStage.ts} (95%) diff --git a/packages/core/src/shared/index.ts b/packages/core/src/shared/index.ts index 3cf69bc2305..a6534a8e780 100644 --- a/packages/core/src/shared/index.ts +++ b/packages/core/src/shared/index.ts @@ -63,7 +63,7 @@ export { TabTypeDataMap } from '../amazonq/webview/ui/tabs/constants' export * from './lsp/manifestResolver' export * from './lsp/lspResolver' export * from './lsp/types' -export * from './lsp/utils/stage' +export * from './lsp/utils/setupStage' export { default as request } from './request' export * from './lsp/utils/platform' export * as processUtils from './utilities/processUtils' diff --git a/packages/core/src/shared/lsp/lspResolver.ts b/packages/core/src/shared/lsp/lspResolver.ts index 7a96e8f5814..161611dbf6f 100644 --- a/packages/core/src/shared/lsp/lspResolver.ts +++ b/packages/core/src/shared/lsp/lspResolver.ts @@ -12,7 +12,7 @@ import AdmZip from 'adm-zip' import { TargetContent, logger, LspResult, LspVersion, Manifest } from './types' import { getApplicationSupportFolder } from '../vscode/env' import { createHash } from '../crypto' -import { lspSetupStage, StageResolver, tryStageResolvers } from './utils/stage' +import { lspSetupStage, StageResolver, tryStageResolvers } from './utils/setupStage' import { HttpResourceFetcher } from '../resourcefetcher/httpResourceFetcher' export class LanguageServerResolver { diff --git a/packages/core/src/shared/lsp/manifestResolver.ts b/packages/core/src/shared/lsp/manifestResolver.ts index af56dc3c0c3..1e4e5d7a777 100644 --- a/packages/core/src/shared/lsp/manifestResolver.ts +++ b/packages/core/src/shared/lsp/manifestResolver.ts @@ -8,7 +8,7 @@ import { ToolkitError } from '../errors' import { Timeout } from '../utilities/timeoutUtils' import globals from '../extensionGlobals' import { Manifest } from './types' -import { StageResolver, tryStageResolvers } from './utils/stage' +import { StageResolver, tryStageResolvers } from './utils/setupStage' import { HttpResourceFetcher } from '../resourcefetcher/httpResourceFetcher' const logger = getLogger('lsp') diff --git a/packages/core/src/shared/lsp/utils/stage.ts b/packages/core/src/shared/lsp/utils/setupStage.ts similarity index 95% rename from packages/core/src/shared/lsp/utils/stage.ts rename to packages/core/src/shared/lsp/utils/setupStage.ts index 18c37eaa814..0c16964b1d5 100644 --- a/packages/core/src/shared/lsp/utils/stage.ts +++ b/packages/core/src/shared/lsp/utils/setupStage.ts @@ -3,8 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { LanguageServerSetup, LanguageServerSetupStage, telemetry } from '../../../shared/telemetry' -import { tryFunctions } from '../../../shared/utilities/tsUtils' +import { LanguageServerSetup, LanguageServerSetupStage, telemetry } from '../../telemetry' +import { tryFunctions } from '../../utilities/tsUtils' /** * Runs the designated stage within a telemetry span and optionally uses the getMetadata extractor to record metadata from the result of the stage. From 05f3a2b317d620d0ecf35b133dd82b648d168368 Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 27 Jan 2025 12:33:46 -0500 Subject: [PATCH 25/38] fix: add workaround for windows tests --- packages/core/src/shared/lsp/lspResolver.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/core/src/shared/lsp/lspResolver.ts b/packages/core/src/shared/lsp/lspResolver.ts index 161611dbf6f..34b2bee7f72 100644 --- a/packages/core/src/shared/lsp/lspResolver.ts +++ b/packages/core/src/shared/lsp/lspResolver.ts @@ -375,7 +375,9 @@ export class LanguageServerResolver { private getCompatibleLspTarget(version: LspVersion) { // TODO make this web friendly // TODO make this fully support windows - const platform = process.platform + + // Workaround: Manifest platform field is `windows`, whereas node returns win32 + const platform = process.platform === 'win32' ? 'windows' : process.platform const arch = process.arch return version.targets.find((x) => x.arch === arch && x.platform === platform) } From 80fa5d6e0fe750043e05375663bb0e5ea017f8dc Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 27 Jan 2025 12:56:23 -0500 Subject: [PATCH 26/38] test: disable lspResolver test on nonmac, add techdebt test --- .../core/src/test/shared/lsp/lspResolver.test.ts | 14 ++++++++++++++ packages/core/src/test/techdebt.test.ts | 7 +++++++ 2 files changed, 21 insertions(+) diff --git a/packages/core/src/test/shared/lsp/lspResolver.test.ts b/packages/core/src/test/shared/lsp/lspResolver.test.ts index 3b7ec5be0d5..587cf03b863 100644 --- a/packages/core/src/test/shared/lsp/lspResolver.test.ts +++ b/packages/core/src/test/shared/lsp/lspResolver.test.ts @@ -59,8 +59,10 @@ describe('lspResolver', function () { let fallbackStub: sinon.SinonStub let manifest: Manifest let versionRange: Range + let isMac: boolean // TODO: remove this check once LSPResolver supports other OS. before(function () { + isMac = process.platform === 'darwin' remoteStub = sinon.stub(LanguageServerResolver.prototype, 'fetchRemoteServer' as any) localStub = sinon.stub(LanguageServerResolver.prototype, 'getLocalServer' as any) fallbackStub = sinon.stub(LanguageServerResolver.prototype, 'getFallbackServer' as any) @@ -91,6 +93,9 @@ describe('lspResolver', function () { }) it('tries local cache first', async function () { + if (!isMac) { + this.skip() + } localStub.resolves(lspResult('cache')) const r = await new LanguageServerResolver(manifest, serverName, versionRange).resolve() @@ -105,6 +110,9 @@ describe('lspResolver', function () { }) it('tries fetching remote if cache fails', async function () { + if (!isMac) { + this.skip() + } localStub.rejects(new Error('not found')) remoteStub.resolves(lspResult('remote')) @@ -128,6 +136,9 @@ describe('lspResolver', function () { }) it('tries fallback version if both remote and cache fail', async function () { + if (!isMac) { + this.skip() + } localStub.rejects(new Error('not found')) remoteStub.rejects(new Error('not found')) fallbackStub.resolves(lspResult('fallback')) @@ -158,6 +169,9 @@ describe('lspResolver', function () { }) it('rejects if local, remote, and fallback all reject', async function () { + if (!isMac) { + this.skip() + } localStub.rejects(new Error('not found')) remoteStub.rejects(new Error('not found')) fallbackStub.rejects(new Error('not found')) diff --git a/packages/core/src/test/techdebt.test.ts b/packages/core/src/test/techdebt.test.ts index 69e557100f7..f55f220dd8d 100644 --- a/packages/core/src/test/techdebt.test.ts +++ b/packages/core/src/test/techdebt.test.ts @@ -39,4 +39,11 @@ describe('tech debt', function () { // This is relevant for the use of `fs.cpSync` in the copyFiles scripts. assert.ok(semver.lt(minNodejs, '18.0.0'), 'with node18+, we can remove the dependency on @types/node@18') }) + + it('allow lspResolver tests to run on all platforms', function () { + fixByDate( + '03-01-2025', + 'allow lspResolver tests to run on all platforms once it supports it. Remove the `isMac` check in core/src/test/shared/lsp/lspResolver.test.ts' + ) + }) }) From 8ebcc9761b153398fb82eb3313004ef973f3b9ee Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 27 Jan 2025 13:07:46 -0500 Subject: [PATCH 27/38] jscpd: ignore verbose telemetry checks --- packages/core/src/test/shared/lsp/lspResolver.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/core/src/test/shared/lsp/lspResolver.test.ts b/packages/core/src/test/shared/lsp/lspResolver.test.ts index 587cf03b863..690af722367 100644 --- a/packages/core/src/test/shared/lsp/lspResolver.test.ts +++ b/packages/core/src/test/shared/lsp/lspResolver.test.ts @@ -145,6 +145,7 @@ describe('lspResolver', function () { const r = await new LanguageServerResolver(manifest, serverName, versionRange).resolve() assert.strictEqual(r.location, 'fallback') + // jscpd:ignore-start assertTelemetry('languageServer_setup', [ { languageServerSetupStage: 'getServer', @@ -166,6 +167,7 @@ describe('lspResolver', function () { result: 'Succeeded', }, ]) + // jscpd:ignore-end }) it('rejects if local, remote, and fallback all reject', async function () { @@ -177,6 +179,7 @@ describe('lspResolver', function () { fallbackStub.rejects(new Error('not found')) await assert.rejects(new LanguageServerResolver(manifest, serverName, versionRange).resolve(), /not found/) + // jscpd:ignore-start assertTelemetry('languageServer_setup', [ { languageServerSetupStage: 'getServer', @@ -197,5 +200,6 @@ describe('lspResolver', function () { result: 'Failed', }, ]) + // jscpd:ignore-end }) }) From 3cf254a85ab1c4e1bafe56f6be654c7084b8b8be Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 27 Jan 2025 13:21:10 -0500 Subject: [PATCH 28/38] refactor: avoid duplicate import line --- packages/amazonq/src/lsp/activation.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/amazonq/src/lsp/activation.ts b/packages/amazonq/src/lsp/activation.ts index cc04f235a94..09871c67920 100644 --- a/packages/amazonq/src/lsp/activation.ts +++ b/packages/amazonq/src/lsp/activation.ts @@ -6,8 +6,7 @@ import vscode from 'vscode' import { startLanguageServer } from './client' import { AmazonQLSPResolver } from './lspInstaller' -import { ToolkitError } from 'aws-core-vscode/shared' -import { lspSetupStage } from 'aws-core-vscode/shared' +import { ToolkitError, lspSetupStage } from 'aws-core-vscode/shared' export async function activate(ctx: vscode.ExtensionContext): Promise { try { From 9b9443bf1ece6c85877b72036bb3d2e7877e5289 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 28 Jan 2025 11:12:11 -0500 Subject: [PATCH 29/38] deps: bump telemetry version and remove local changes --- package-lock.json | 9 +-- package.json | 2 +- .../src/shared/telemetry/vscodeTelemetry.json | 61 ------------------- 3 files changed, 6 insertions(+), 66 deletions(-) diff --git a/package-lock.json b/package-lock.json index d3cda5c4334..9db3ac69dd9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20,7 +20,7 @@ "vscode-nls-dev": "^4.0.4" }, "devDependencies": { - "@aws-toolkits/telemetry": "^1.0.293", + "@aws-toolkits/telemetry": "^1.0.296", "@playwright/browser-chromium": "^1.43.1", "@stylistic/eslint-plugin": "^2.11.0", "@types/he": "^1.2.3", @@ -6047,10 +6047,11 @@ } }, "node_modules/@aws-toolkits/telemetry": { - "version": "1.0.294", - "resolved": "https://registry.npmjs.org/@aws-toolkits/telemetry/-/telemetry-1.0.294.tgz", - "integrity": "sha512-Hy/yj93pFuHhKCqAA9FgNjdJHRi4huUnyl13dZLzzICDlFVl/AHlm9iWmm9LR22KOuXUyu3uX40VtXLdExIHqw==", + "version": "1.0.296", + "resolved": "https://registry.npmjs.org/@aws-toolkits/telemetry/-/telemetry-1.0.296.tgz", + "integrity": "sha512-laPLYzImOCyAyZWWrmEXWWXZv2xeS5PKatVksGuCxjSohI+20fuCAUjhwMtyq7bRUPkOTkG9pzVYB7Rg49QHFg==", "dev": true, + "license": "Apache-2.0", "dependencies": { "ajv": "^6.12.6", "cross-spawn": "^7.0.6", diff --git a/package.json b/package.json index e14641a45c4..f44992706ef 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "mergeReports": "ts-node ./scripts/mergeReports.ts" }, "devDependencies": { - "@aws-toolkits/telemetry": "^1.0.293", + "@aws-toolkits/telemetry": "^1.0.296", "@playwright/browser-chromium": "^1.43.1", "@stylistic/eslint-plugin": "^2.11.0", "@types/he": "^1.2.3", diff --git a/packages/core/src/shared/telemetry/vscodeTelemetry.json b/packages/core/src/shared/telemetry/vscodeTelemetry.json index 50b00fca4bd..5e32b249d4f 100644 --- a/packages/core/src/shared/telemetry/vscodeTelemetry.json +++ b/packages/core/src/shared/telemetry/vscodeTelemetry.json @@ -1,33 +1,5 @@ { "types": [ - { - "name": "languageServerLocation", - "type": "string", - "allowedValues": ["cache", "remote", "fallback", "override"], - "description": "The location of the language server" - }, - { - "name": "languageServerSetupStage", - "type": "string", - "allowedValues": ["getManifest", "getServer", "validate", "launch", "handshake", "all"], - "description": "The stage of the LSP setup process" - }, - { - "name": "languageServerVersion", - "type": "string", - "description": "The version of the language server" - }, - { - "name": "manifestLocation", - "type": "string", - "allowedValues": ["cache", "remote", "override"], - "description": "The location of the manifest" - }, - { - "name": "manifestSchemaVersion", - "type": "string", - "description": "The version of the manifest schema file" - }, { "name": "amazonGenerateApproachLatency", "type": "double", @@ -386,39 +358,6 @@ } ], "metrics": [ - { - "name": "languageServer_setup", - "description": "Sets up a language server", - "passive": true, - "unit": "Milliseconds", - "metadata": [ - { - "type": "id" - }, - { - "type": "languageServerLocation", - "required": false - }, - { - "type": "languageServerSetupStage" - }, - { - "type": "languageServerVersion", - "required": false - }, - { - "type": "manifestLocation", - "required": false - }, - { - "type": "manifestSchemaVersion", - "required": false - }, - { - "type": "result" - } - ] - }, { "name": "ide_fileSystem", "description": "File System event on execution", From 077e4028f66fe4d057f138c40fd18649c1bd6fb1 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 28 Jan 2025 11:16:45 -0500 Subject: [PATCH 30/38] refactor: cleanup minor details --- packages/core/src/shared/utilities/tsUtils.ts | 2 +- packages/core/src/test/techdebt.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/src/shared/utilities/tsUtils.ts b/packages/core/src/shared/utilities/tsUtils.ts index 55b34e7ef8d..09a9b276280 100644 --- a/packages/core/src/shared/utilities/tsUtils.ts +++ b/packages/core/src/shared/utilities/tsUtils.ts @@ -95,7 +95,7 @@ export function createFactoryFunction any>(cto } /** - * Try Functions in the order presented and return the first returned result. If none return, throw the final error. + * Try functions in the order presented and return the first returned result. If none return, throw the final error. * @param functions non-empty list of functions to try. * @returns */ diff --git a/packages/core/src/test/techdebt.test.ts b/packages/core/src/test/techdebt.test.ts index f55f220dd8d..1058f5e846b 100644 --- a/packages/core/src/test/techdebt.test.ts +++ b/packages/core/src/test/techdebt.test.ts @@ -42,7 +42,7 @@ describe('tech debt', function () { it('allow lspResolver tests to run on all platforms', function () { fixByDate( - '03-01-2025', + '04-01-2025', 'allow lspResolver tests to run on all platforms once it supports it. Remove the `isMac` check in core/src/test/shared/lsp/lspResolver.test.ts' ) }) From b0a37f35a2261fea1df05b31243cf2c538bfc99b Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 3 Feb 2025 11:12:47 -0500 Subject: [PATCH 31/38] refactor: fix spelling mistake --- packages/core/src/shared/lsp/utils/setupStage.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/core/src/shared/lsp/utils/setupStage.ts b/packages/core/src/shared/lsp/utils/setupStage.ts index 0c16964b1d5..f8932f0b2ef 100644 --- a/packages/core/src/shared/lsp/utils/setupStage.ts +++ b/packages/core/src/shared/lsp/utils/setupStage.ts @@ -10,13 +10,13 @@ import { tryFunctions } from '../../utilities/tsUtils' * Runs the designated stage within a telemetry span and optionally uses the getMetadata extractor to record metadata from the result of the stage. * @param stageName name of stage for telemetry. * @param runStage stage to be run. - * @param getMetadata metadata extracter to be applied to result. + * @param getMetadata metadata extractor to be applied to result. * @returns result of stage */ export async function lspSetupStage( stageName: LanguageServerSetupStage, runStage: () => Promise, - getMetadata?: MetadataExtracter + getMetadata?: MetadataExtractor ) { return await telemetry.languageServer_setup.run(async (span) => { span.record({ languageServerSetupStage: stageName }) @@ -38,7 +38,7 @@ export async function lspSetupStage( export async function tryStageResolvers( stageName: LanguageServerSetupStage, resolvers: StageResolver[], - getMetadata: MetadataExtracter + getMetadata: MetadataExtractor ) { const fs = resolvers.map((resolver) => async () => { return await lspSetupStage( @@ -63,4 +63,4 @@ export interface StageResolver { telemetryMetadata: Partial } -type MetadataExtracter = (r: R) => Partial +type MetadataExtractor = (r: R) => Partial From 42ed7458f49a8cfa8ae6ed2310179c0aa389b2fd Mon Sep 17 00:00:00 2001 From: hkobew Date: Mon, 3 Feb 2025 13:54:33 -0500 Subject: [PATCH 32/38] fix: remove accidental space From d286070d9e703f28e74937c6e430fb1704b121c1 Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 4 Feb 2025 12:38:59 -0500 Subject: [PATCH 33/38] test: add telemetry to e2e tests --- .../amazonq/test/e2e/lsp/lspInstaller.test.ts | 89 +++++++++++++++++++ packages/core/src/shared/lsp/lspResolver.ts | 30 +++---- 2 files changed, 103 insertions(+), 16 deletions(-) diff --git a/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts b/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts index 0bf2edafa3f..19a3065ce39 100644 --- a/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts +++ b/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts @@ -14,6 +14,8 @@ import { request, } from 'aws-core-vscode/shared' import * as semver from 'semver' +import { assertTelemetry } from 'aws-core-vscode/test' +import { LspController } from 'aws-core-vscode/amazonq' function createVersion(version: string) { return { @@ -46,6 +48,8 @@ describe('AmazonQLSPInstaller', () => { resolver = new AmazonQLSPResolver() tempDir = await makeTemporaryToolkitFolder() sandbox.stub(LanguageServerResolver.prototype, 'defaultDownloadFolder').returns(tempDir) + // Called on extension activation and can contaminate telemetry. + sandbox.stub(LspController.prototype, 'trySetupLsp') }) afterEach(async () => { @@ -117,6 +121,91 @@ describe('AmazonQLSPInstaller', () => { assert.ok(fallback.assetDirectory.startsWith(tempDir)) assert.deepStrictEqual(fallback.location, 'fallback') assert.ok(semver.satisfies(fallback.version, supportedLspServerVersions)) + + // Exclude version numbers so that this test doesn't have to be updated on each update. + assertTelemetry('languageServer_setup', [ + /* First Try Telemetry + getManifest: remote fails, then cache succeeds. + getServer: cache fails then remote succeeds. + validate: succeeds. + */ + { + id: 'AmazonQ', + manifestLocation: 'remote', + languageServerSetupStage: 'getManifest', + result: 'Failed', + }, + { + id: 'AmazonQ', + manifestLocation: 'cache', + languageServerSetupStage: 'getManifest', + result: 'Succeeded', + }, + { + id: 'AmazonQ', + languageServerLocation: 'cache', + languageServerSetupStage: 'getServer', + result: 'Failed', + }, + { + id: 'AmazonQ', + languageServerLocation: 'remote', + languageServerSetupStage: 'validate', + result: 'Succeeded', + }, + { + id: 'AmazonQ', + languageServerLocation: 'remote', + languageServerSetupStage: 'getServer', + result: 'Succeeded', + }, + /* Second Try Telemetry + getManifest: remote fails, then cache succeeds. + getServer: cache succeeds + validate: doesn't run since its cached. + */ + { + id: 'AmazonQ', + manifestLocation: 'remote', + languageServerSetupStage: 'getManifest', + result: 'Failed', + }, + { + id: 'AmazonQ', + manifestLocation: 'cache', + languageServerSetupStage: 'getManifest', + result: 'Succeeded', + }, + { + id: 'AmazonQ', + languageServerLocation: 'cache', + languageServerSetupStage: 'getServer', + result: 'Succeeded', + }, + /* Third Try Telemetry + getManifest: (stubbed to fail, no telemetry) + getServer: remote and cache fail + validate: no validation since not remote. + */ + { + id: 'AmazonQ', + languageServerLocation: 'cache', + languageServerSetupStage: 'getServer', + result: 'Failed', + }, + { + id: 'AmazonQ', + languageServerLocation: 'remote', + languageServerSetupStage: 'getServer', + result: 'Failed', + }, + { + id: 'AmazonQ', + languageServerLocation: 'fallback', + languageServerSetupStage: 'getServer', + result: 'Succeeded', + }, + ]) }) }) }) diff --git a/packages/core/src/shared/lsp/lspResolver.ts b/packages/core/src/shared/lsp/lspResolver.ts index 1d683c4380e..265de137ed9 100644 --- a/packages/core/src/shared/lsp/lspResolver.ts +++ b/packages/core/src/shared/lsp/lspResolver.ts @@ -200,26 +200,24 @@ export class LanguageServerResolver { } }) const fetchResults = await Promise.all(fetchTasks) - const verifyTasks = fetchResults.flatMap(async (fetchResult) => { - if (!(fetchResult.res && fetchResult.res.ok && fetchResult.res.body)) { + const verifyTasks = fetchResults + .filter((fetchResult) => fetchResult.res && fetchResult.res.ok && fetchResult.res.body) + .flatMap(async (fetchResult) => { + const arrBuffer = await fetchResult.res!.arrayBuffer() + const data = Buffer.from(arrBuffer) + + const hash = createHash('sha384', data) + if (hash === fetchResult.hash) { + return [{ filename: fetchResult.filename, data }] + } return [] - } - - const arrBuffer = await fetchResult.res.arrayBuffer() - const data = Buffer.from(arrBuffer) - - const hash = createHash('sha384', data) - if (hash === fetchResult.hash) { - return [{ filename: fetchResult.filename, data }] - } - return [] - }) - const filesToDownload = await lspSetupStage('validate', async () => (await Promise.all(verifyTasks)).flat()) - - if (filesToDownload.length !== contents.length) { + }) + if (verifyTasks.length !== contents.length) { return false } + const filesToDownload = await lspSetupStage('validate', async () => (await Promise.all(verifyTasks)).flat()) + for (const file of filesToDownload) { await fs.writeFile(`${downloadDirectory}/${file.filename}`, file.data) } From 3888ac599ae25f71055899d2eaf6c0017d4f827b Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 4 Feb 2025 16:05:37 -0500 Subject: [PATCH 34/38] test: adjust telemetry to mirror CI reality --- .../amazonq/test/e2e/lsp/lspInstaller.test.ts | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts b/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts index 19a3065ce39..1bdb92a9869 100644 --- a/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts +++ b/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts @@ -123,9 +123,10 @@ describe('AmazonQLSPInstaller', () => { assert.ok(semver.satisfies(fallback.version, supportedLspServerVersions)) // Exclude version numbers so that this test doesn't have to be updated on each update. + // Fetching manifest fails locally, but passes in CI. Therefore, this test will fail locally. assertTelemetry('languageServer_setup', [ /* First Try Telemetry - getManifest: remote fails, then cache succeeds. + getManifest: remote succeeds getServer: cache fails then remote succeeds. validate: succeeds. */ @@ -133,12 +134,6 @@ describe('AmazonQLSPInstaller', () => { id: 'AmazonQ', manifestLocation: 'remote', languageServerSetupStage: 'getManifest', - result: 'Failed', - }, - { - id: 'AmazonQ', - manifestLocation: 'cache', - languageServerSetupStage: 'getManifest', result: 'Succeeded', }, { @@ -168,12 +163,6 @@ describe('AmazonQLSPInstaller', () => { id: 'AmazonQ', manifestLocation: 'remote', languageServerSetupStage: 'getManifest', - result: 'Failed', - }, - { - id: 'AmazonQ', - manifestLocation: 'cache', - languageServerSetupStage: 'getManifest', result: 'Succeeded', }, { From 4819d49a1b45aa45154f8e1c3d1d492b4fe23a2a Mon Sep 17 00:00:00 2001 From: hkobew Date: Tue, 4 Feb 2025 16:32:46 -0500 Subject: [PATCH 35/38] test: adjust results to mirror ci --- packages/amazonq/test/e2e/lsp/lspInstaller.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts b/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts index 1bdb92a9869..fac1a50bd73 100644 --- a/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts +++ b/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts @@ -163,6 +163,12 @@ describe('AmazonQLSPInstaller', () => { id: 'AmazonQ', manifestLocation: 'remote', languageServerSetupStage: 'getManifest', + result: 'Failed', + }, + { + id: 'AmazonQ', + manifestLocation: 'cache', + languageServerSetupStage: 'getManifest', result: 'Succeeded', }, { From 3aa2670c3fdbfbe0990a427ff718b8c2361fea23 Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 5 Feb 2025 09:43:12 -0500 Subject: [PATCH 36/38] test: delete duplicate coverage test case --- .../src/test/shared/lsp/lspResolver.test.ts | 205 ------------------ packages/core/src/test/techdebt.test.ts | 7 - 2 files changed, 212 deletions(-) delete mode 100644 packages/core/src/test/shared/lsp/lspResolver.test.ts diff --git a/packages/core/src/test/shared/lsp/lspResolver.test.ts b/packages/core/src/test/shared/lsp/lspResolver.test.ts deleted file mode 100644 index 690af722367..00000000000 --- a/packages/core/src/test/shared/lsp/lspResolver.test.ts +++ /dev/null @@ -1,205 +0,0 @@ -/*! - * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -import assert from 'assert' -import sinon from 'sinon' -import { LanguageServerResolver, LspResult, Manifest, Target } from '../../../shared' -import { Range } from 'semver' -import { assertTelemetry } from '../../testUtil' -import { LanguageServerLocation } from '../../../shared/telemetry' - -const serverVersion = '1.0.0' -const assetDirectory = 'path/to/assets' -const serverName = 'myLS' - -/** - * Helper function for generating valid manifest for tests. - * @param platform - * @param arch - * @returns - */ -function manifestTarget(platform: 'linux' | 'darwin' | 'windows', arch: 'x64' | 'arm64'): Target { - return { - platform, - arch, - contents: [ - { - filename: `${serverName}-${platform}-${arch}.zip`, - url: `https://example.com/lsp-${platform}-${arch}.zip`, - hashes: ['sha384:thisisahash'], - bytes: 100, - }, - { - filename: `node-${platform}-${arch}`, - url: `https://example.com/temp-assets/node-${platform}-${arch}`, - hashes: ['sha384:thisisanotherhash'], - bytes: 200, - }, - ], - } -} -/** - * Helper function for generating valid LspResult for tests. - * @param location - * @returns - */ -function lspResult(location: LanguageServerLocation): LspResult { - return { - location, - version: serverVersion, - assetDirectory: assetDirectory, - } -} - -describe('lspResolver', function () { - let remoteStub: sinon.SinonStub - let localStub: sinon.SinonStub - let fallbackStub: sinon.SinonStub - let manifest: Manifest - let versionRange: Range - let isMac: boolean // TODO: remove this check once LSPResolver supports other OS. - - before(function () { - isMac = process.platform === 'darwin' - remoteStub = sinon.stub(LanguageServerResolver.prototype, 'fetchRemoteServer' as any) - localStub = sinon.stub(LanguageServerResolver.prototype, 'getLocalServer' as any) - fallbackStub = sinon.stub(LanguageServerResolver.prototype, 'getFallbackServer' as any) - manifest = { - manifestSchemaVersion: '2.0.0', - artifactId: 'artifact-id', - artifactDescription: 'artifact-description', - isManifestDeprecated: false, - versions: [ - { - serverVersion: serverVersion, - isDelisted: false, - targets: [ - manifestTarget('linux', 'x64'), - manifestTarget('linux', 'arm64'), - manifestTarget('darwin', 'x64'), - manifestTarget('darwin', 'arm64'), - manifestTarget('windows', 'x64'), - ], - }, - ], - } - versionRange = new Range(`>=${serverVersion}`) - }) - - after(function () { - sinon.restore() - }) - - it('tries local cache first', async function () { - if (!isMac) { - this.skip() - } - localStub.resolves(lspResult('cache')) - - const r = await new LanguageServerResolver(manifest, serverName, versionRange).resolve() - assert.strictEqual(r.location, 'cache') - assertTelemetry('languageServer_setup', { - languageServerSetupStage: 'getServer', - id: serverName, - languageServerLocation: 'cache', - languageServerVersion: serverVersion, - result: 'Succeeded', - }) - }) - - it('tries fetching remote if cache fails', async function () { - if (!isMac) { - this.skip() - } - localStub.rejects(new Error('not found')) - remoteStub.resolves(lspResult('remote')) - - const r = await new LanguageServerResolver(manifest, serverName, versionRange).resolve() - assert.strictEqual(r.location, 'remote') - assertTelemetry('languageServer_setup', [ - { - languageServerSetupStage: 'getServer', - id: serverName, - languageServerLocation: 'cache', - result: 'Failed', - }, - { - languageServerSetupStage: 'getServer', - id: serverName, - languageServerLocation: 'remote', - languageServerVersion: serverVersion, - result: 'Succeeded', - }, - ]) - }) - - it('tries fallback version if both remote and cache fail', async function () { - if (!isMac) { - this.skip() - } - localStub.rejects(new Error('not found')) - remoteStub.rejects(new Error('not found')) - fallbackStub.resolves(lspResult('fallback')) - - const r = await new LanguageServerResolver(manifest, serverName, versionRange).resolve() - assert.strictEqual(r.location, 'fallback') - // jscpd:ignore-start - assertTelemetry('languageServer_setup', [ - { - languageServerSetupStage: 'getServer', - id: serverName, - languageServerLocation: 'cache', - result: 'Failed', - }, - { - languageServerSetupStage: 'getServer', - id: serverName, - languageServerLocation: 'remote', - result: 'Failed', - }, - { - languageServerSetupStage: 'getServer', - id: serverName, - languageServerLocation: 'fallback', - languageServerVersion: serverVersion, - result: 'Succeeded', - }, - ]) - // jscpd:ignore-end - }) - - it('rejects if local, remote, and fallback all reject', async function () { - if (!isMac) { - this.skip() - } - localStub.rejects(new Error('not found')) - remoteStub.rejects(new Error('not found')) - fallbackStub.rejects(new Error('not found')) - - await assert.rejects(new LanguageServerResolver(manifest, serverName, versionRange).resolve(), /not found/) - // jscpd:ignore-start - assertTelemetry('languageServer_setup', [ - { - languageServerSetupStage: 'getServer', - id: serverName, - languageServerLocation: 'cache', - result: 'Failed', - }, - { - languageServerSetupStage: 'getServer', - id: serverName, - languageServerLocation: 'remote', - result: 'Failed', - }, - { - languageServerSetupStage: 'getServer', - id: serverName, - languageServerLocation: 'fallback', - result: 'Failed', - }, - ]) - // jscpd:ignore-end - }) -}) diff --git a/packages/core/src/test/techdebt.test.ts b/packages/core/src/test/techdebt.test.ts index 1058f5e846b..69e557100f7 100644 --- a/packages/core/src/test/techdebt.test.ts +++ b/packages/core/src/test/techdebt.test.ts @@ -39,11 +39,4 @@ describe('tech debt', function () { // This is relevant for the use of `fs.cpSync` in the copyFiles scripts. assert.ok(semver.lt(minNodejs, '18.0.0'), 'with node18+, we can remove the dependency on @types/node@18') }) - - it('allow lspResolver tests to run on all platforms', function () { - fixByDate( - '04-01-2025', - 'allow lspResolver tests to run on all platforms once it supports it. Remove the `isMac` check in core/src/test/shared/lsp/lspResolver.test.ts' - ) - }) }) From e55dd7f6c049a7e05bd2385584dd10e4e58630b3 Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 5 Feb 2025 10:38:46 -0500 Subject: [PATCH 37/38] test: avoid bleedthrough of globalstate --- .../amazonq/test/e2e/lsp/lspInstaller.test.ts | 41 ++++++++++++++----- .../core/src/shared/lsp/manifestResolver.ts | 2 +- 2 files changed, 32 insertions(+), 11 deletions(-) diff --git a/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts b/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts index fac1a50bd73..46f76d64215 100644 --- a/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts +++ b/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts @@ -8,14 +8,17 @@ import sinon from 'sinon' import { AmazonQLSPResolver, supportedLspServerVersions } from '../../../src/lsp/lspInstaller' import { fs, + globals, LanguageServerResolver, makeTemporaryToolkitFolder, ManifestResolver, + manifestStorageKey, request, } from 'aws-core-vscode/shared' import * as semver from 'semver' import { assertTelemetry } from 'aws-core-vscode/test' import { LspController } from 'aws-core-vscode/amazonq' +import { LanguageServerSetup } from 'aws-core-vscode/telemetry' function createVersion(version: string) { return { @@ -42,6 +45,11 @@ describe('AmazonQLSPInstaller', () => { let resolver: AmazonQLSPResolver let sandbox: sinon.SinonSandbox let tempDir: string + let manifestStorage: { [key: string]: any } + + before(async () => { + manifestStorage = globals.globalState.get(manifestStorageKey) || {} + }) beforeEach(async () => { sandbox = sinon.createSandbox() @@ -50,6 +58,7 @@ describe('AmazonQLSPInstaller', () => { sandbox.stub(LanguageServerResolver.prototype, 'defaultDownloadFolder').returns(tempDir) // Called on extension activation and can contaminate telemetry. sandbox.stub(LspController.prototype, 'trySetupLsp') + await globals.globalState.update(manifestStorageKey, {}) }) afterEach(async () => { @@ -60,6 +69,10 @@ describe('AmazonQLSPInstaller', () => { }) }) + after(async () => { + await globals.globalState.update(manifestStorageKey, manifestStorage) + }) + describe('resolve()', () => { it('uses AWS_LANGUAGE_SERVER_OVERRIDE', async () => { const overridePath = '/custom/path/to/lsp' @@ -122,14 +135,12 @@ describe('AmazonQLSPInstaller', () => { assert.deepStrictEqual(fallback.location, 'fallback') assert.ok(semver.satisfies(fallback.version, supportedLspServerVersions)) - // Exclude version numbers so that this test doesn't have to be updated on each update. - // Fetching manifest fails locally, but passes in CI. Therefore, this test will fail locally. - assertTelemetry('languageServer_setup', [ - /* First Try Telemetry + /* First Try Telemetry getManifest: remote succeeds getServer: cache fails then remote succeeds. validate: succeeds. - */ + */ + const firstTryTelemetry: Partial[] = [ { id: 'AmazonQ', manifestLocation: 'remote', @@ -154,11 +165,14 @@ describe('AmazonQLSPInstaller', () => { languageServerSetupStage: 'getServer', result: 'Succeeded', }, - /* Second Try Telemetry + ] + + /* Second Try Telemetry getManifest: remote fails, then cache succeeds. getServer: cache succeeds validate: doesn't run since its cached. - */ + */ + const secondTryTelemetry: Partial[] = [ { id: 'AmazonQ', manifestLocation: 'remote', @@ -177,11 +191,14 @@ describe('AmazonQLSPInstaller', () => { languageServerSetupStage: 'getServer', result: 'Succeeded', }, - /* Third Try Telemetry + ] + + /* Third Try Telemetry getManifest: (stubbed to fail, no telemetry) getServer: remote and cache fail validate: no validation since not remote. - */ + */ + const thirdTryTelemetry: Partial[] = [ { id: 'AmazonQ', languageServerLocation: 'cache', @@ -200,7 +217,11 @@ describe('AmazonQLSPInstaller', () => { languageServerSetupStage: 'getServer', result: 'Succeeded', }, - ]) + ] + + const expectedTelemetry = firstTryTelemetry.concat(secondTryTelemetry, thirdTryTelemetry) + + assertTelemetry('languageServer_setup', expectedTelemetry) }) }) }) diff --git a/packages/core/src/shared/lsp/manifestResolver.ts b/packages/core/src/shared/lsp/manifestResolver.ts index 1e4e5d7a777..e19dcb0ced1 100644 --- a/packages/core/src/shared/lsp/manifestResolver.ts +++ b/packages/core/src/shared/lsp/manifestResolver.ts @@ -20,7 +20,7 @@ interface StorageManifest { type ManifestStorage = Record -const manifestStorageKey = 'aws.toolkit.lsp.manifest' +export const manifestStorageKey = 'aws.toolkit.lsp.manifest' const manifestTimeoutMs = 15000 export class ManifestResolver { From 8bc7ccc285b67389e0b9a0e4aeb4d14e3877ed0b Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 5 Feb 2025 11:05:49 -0500 Subject: [PATCH 38/38] test: ensure consistent global storage state to ensure consistent behavior --- packages/amazonq/test/e2e/lsp/lspInstaller.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts b/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts index 46f76d64215..5e1d3f68d47 100644 --- a/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts +++ b/packages/amazonq/test/e2e/lsp/lspInstaller.test.ts @@ -45,6 +45,8 @@ describe('AmazonQLSPInstaller', () => { let resolver: AmazonQLSPResolver let sandbox: sinon.SinonSandbox let tempDir: string + // If globalState contains an ETag that is up to date with remote, we won't fetch it resulting in inconsistent behavior. + // Therefore, we clear it temporarily for these tests to ensure consistent behavior. let manifestStorage: { [key: string]: any } before(async () => {