Skip to content

Commit d3e1e4f

Browse files
authored
fix(amazonq): update LSP client info name for sagemaker unified studio (#7786)
## Problem In order to set appropriate Origin Info on LSP side for SMUS CodeEditor, [link](https://github.com/aws/language-servers/blob/68adf18d7ec46a7ecf9c66fd9d52b1b8f7bc236e/server/aws-lsp-codewhisperer/src/shared/utils.ts#L377), clientInfo needs to be distinct and with current logic that uses ```vscode.env.appName``` whose value will be same for all sagemaker codeeditor instances ## Solution - To check if the environment is SageMaker and a Unified Studio instance and set corresponding clientInfo Name which is AmazonQ-For-SMUS-CE ## Testing - Built artefact locally using ```npm run compile && npm run package``` and tested on a SMUS CE space - LSP logs are attached to show the respective client Info details ``` [Trace - 9:55:46 PM] Sending request 'initialize - (0)'. Params: { "processId": 6395, "clientInfo": { "name": "vscode", "version": "1.90.1" }, .... "initializationOptions": { "aws": { "clientInfo": { "name": "AmazonQ-For-SMUS-CE", "version": "1.90.1", "extension": { "name": "AmazonQ-For-VSCode", ..... ``` - Tested the debug artefact in SMUS and SMAI spaces As observed below, the sign out was only disabled for SMUS case initially with [this](https://github.com/parameja1/aws-toolkit-vscode/blob/f5fa7989be44238d4d27b8c9e7fed967c05bc0e9/packages/core/src/codewhisperer/ui/statusBarMenu.ts#L96) change, a [CR](f5cf3bd) followed up which overrode the logic in isSageMaker and returned true for all cases irrespective of the appName passed SMUS ------ <img width="720" height="383" alt="image" src="https://github.com/user-attachments/assets/49504777-0922-49b8-942a-12efacfd4311" /> SMAI ----- <img width="720" height="383" alt="image" src="https://github.com/user-attachments/assets/e50c30df-f275-4f7b-bff0-11177a7fed0a" /> - Observing Q sendMessage failure in SMAI CE instance due to missing permissions, again unrelated to this change --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 6bfa34f commit d3e1e4f

File tree

6 files changed

+236
-8
lines changed

6 files changed

+236
-8
lines changed

packages/amazonq/src/lsp/client.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6-
import vscode, { env, version } from 'vscode'
6+
import vscode, { version } from 'vscode'
77
import * as nls from 'vscode-nls'
88
import { LanguageClient, LanguageClientOptions, RequestType, State } from 'vscode-languageclient'
99
import { InlineCompletionManager } from '../app/inline/completion'
@@ -38,6 +38,7 @@ import {
3838
getOptOutPreference,
3939
isAmazonLinux2,
4040
getClientId,
41+
getClientName,
4142
extensionVersion,
4243
isSageMaker,
4344
DevSettings,
@@ -163,7 +164,7 @@ export async function startLanguageServer(
163164
initializationOptions: {
164165
aws: {
165166
clientInfo: {
166-
name: env.appName,
167+
name: getClientName(),
167168
version: version,
168169
extension: {
169170
name: 'AmazonQ-For-VSCode',

packages/core/src/shared/extensionUtilities.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -150,14 +150,27 @@ function createCloud9Properties(company: string): IdeProperties {
150150
}
151151
}
152152

153-
function isSageMakerUnifiedStudio(): boolean {
153+
/**
154+
* export method - for testing purposes only
155+
* @internal
156+
*/
157+
export function isSageMakerUnifiedStudio(): boolean {
154158
if (serviceName === notInitialized) {
155159
serviceName = process.env.SERVICE_NAME ?? ''
156160
isSMUS = serviceName === sageMakerUnifiedStudio
157161
}
158162
return isSMUS
159163
}
160164

165+
/**
166+
* Reset cached SageMaker state - for testing purposes only
167+
* @internal
168+
*/
169+
export function resetSageMakerState(): void {
170+
serviceName = notInitialized
171+
isSMUS = false
172+
}
173+
161174
/**
162175
* Decides if the current system is (the specified flavor of) Cloud9.
163176
*/
@@ -177,17 +190,17 @@ export function isCloud9(flavor: 'classic' | 'codecatalyst' | 'any' = 'any'): bo
177190
*/
178191
export function isSageMaker(appName: 'SMAI' | 'SMUS' = 'SMAI'): boolean {
179192
// Check for SageMaker-specific environment variables first
193+
let hasSMEnvVars: boolean = false
180194
if (hasSageMakerEnvVars()) {
181195
getLogger().debug('SageMaker environment detected via environment variables')
182-
return true
196+
hasSMEnvVars = true
183197
}
184198

185-
// Fall back to app name checks
186199
switch (appName) {
187200
case 'SMAI':
188-
return vscode.env.appName === sageMakerAppname
201+
return vscode.env.appName === sageMakerAppname && hasSMEnvVars
189202
case 'SMUS':
190-
return vscode.env.appName === sageMakerAppname && isSageMakerUnifiedStudio()
203+
return vscode.env.appName === sageMakerAppname && isSageMakerUnifiedStudio() && hasSMEnvVars
191204
default:
192205
return false
193206
}

packages/core/src/shared/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export { Prompter } from './ui/prompter'
2727
export { VirtualFileSystem } from './virtualFilesystem'
2828
export { VirtualMemoryFile } from './virtualMemoryFile'
2929
export { AmazonqCreateUpload, Metric } from './telemetry/telemetry'
30-
export { getClientId, getOperatingSystem, getOptOutPreference } from './telemetry/util'
30+
export { getClientId, getClientName, getOperatingSystem, getOptOutPreference } from './telemetry/util'
3131
export { extensionVersion } from './vscode/env'
3232
export { cast } from './utilities/typeConstructors'
3333
export * as workspaceUtils from './utilities/workspaceUtils'

packages/core/src/shared/telemetry/util.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,3 +481,15 @@ export function withTelemetryContext(opts: TelemetryContextArgs) {
481481
})
482482
}
483483
}
484+
485+
/**
486+
* Used to identify the q client info and send the respective origin parameter from LSP to invoke Maestro service at CW API level
487+
*
488+
* Returns default value of vscode appName or AmazonQ-For-SMUS-CE in case of a sagemaker unified studio environment
489+
*/
490+
export function getClientName(): string {
491+
if (isSageMaker('SMUS')) {
492+
return 'AmazonQ-For-SMUS-CE'
493+
}
494+
return env.appName
495+
}

packages/core/src/test/shared/extensionUtilities.test.ts

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import globals from '../../shared/extensionGlobals'
1818
import { maybeShowMinVscodeWarning } from '../../shared/extensionStartup'
1919
import { getTestWindow } from './vscode/window'
2020
import { assertTelemetry } from '../testUtil'
21+
import { isSageMaker } from '../../shared/extensionUtilities'
22+
import { hasSageMakerEnvVars } from '../../shared/vscode/env'
2123

2224
describe('extensionUtilities', function () {
2325
it('maybeShowMinVscodeWarning', async () => {
@@ -361,3 +363,146 @@ describe('UserActivity', function () {
361363
return event.event
362364
}
363365
})
366+
367+
describe('isSageMaker', function () {
368+
let sandbox: sinon.SinonSandbox
369+
const env = require('../../shared/vscode/env')
370+
const utils = require('../../shared/extensionUtilities')
371+
372+
beforeEach(function () {
373+
sandbox = sinon.createSandbox()
374+
utils.resetSageMakerState()
375+
})
376+
377+
afterEach(function () {
378+
sandbox.restore()
379+
delete process.env.SERVICE_NAME
380+
})
381+
382+
describe('SMAI detection', function () {
383+
it('returns true when both app name and env vars match', function () {
384+
sandbox.stub(vscode.env, 'appName').value('SageMaker Code Editor')
385+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(true)
386+
387+
assert.strictEqual(isSageMaker('SMAI'), true)
388+
})
389+
390+
it('returns false when app name is different', function () {
391+
sandbox.stub(vscode.env, 'appName').value('Visual Studio Code')
392+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(true)
393+
394+
assert.strictEqual(isSageMaker('SMAI'), false)
395+
})
396+
397+
it('returns false when env vars are missing', function () {
398+
sandbox.stub(vscode.env, 'appName').value('SageMaker Code Editor')
399+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(false)
400+
401+
assert.strictEqual(isSageMaker('SMAI'), false)
402+
})
403+
404+
it('defaults to SMAI when no parameter provided', function () {
405+
sandbox.stub(vscode.env, 'appName').value('SageMaker Code Editor')
406+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(true)
407+
408+
assert.strictEqual(isSageMaker(), true)
409+
})
410+
})
411+
412+
describe('SMUS detection', function () {
413+
it('returns true when all conditions are met', function () {
414+
sandbox.stub(vscode.env, 'appName').value('SageMaker Code Editor')
415+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(true)
416+
process.env.SERVICE_NAME = 'SageMakerUnifiedStudio'
417+
418+
assert.strictEqual(isSageMaker('SMUS'), true)
419+
})
420+
421+
it('returns false when unified studio is missing', function () {
422+
sandbox.stub(vscode.env, 'appName').value('SageMaker Code Editor')
423+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(true)
424+
process.env.SERVICE_NAME = 'SomeOtherService'
425+
426+
assert.strictEqual(isSageMaker('SMUS'), false)
427+
})
428+
429+
it('returns false when env vars are missing', function () {
430+
sandbox.stub(vscode.env, 'appName').value('SageMaker Code Editor')
431+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(false)
432+
process.env.SERVICE_NAME = 'SageMakerUnifiedStudio'
433+
434+
assert.strictEqual(isSageMaker('SMUS'), false)
435+
})
436+
437+
it('returns false when app name is different', function () {
438+
sandbox.stub(vscode.env, 'appName').value('Visual Studio Code')
439+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(true)
440+
process.env.SERVICE_NAME = 'SageMakerUnifiedStudio'
441+
442+
assert.strictEqual(isSageMaker('SMUS'), false)
443+
})
444+
})
445+
446+
it('returns false for invalid appName parameter', function () {
447+
sandbox.stub(vscode.env, 'appName').value('SageMaker Code Editor')
448+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(true)
449+
450+
// @ts-ignore - Testing invalid input
451+
assert.strictEqual(isSageMaker('INVALID'), false)
452+
})
453+
})
454+
455+
describe('hasSageMakerEnvVars', function () {
456+
let originalEnv: NodeJS.ProcessEnv
457+
458+
beforeEach(function () {
459+
originalEnv = { ...process.env }
460+
// Clear all SageMaker-related env vars
461+
delete process.env.SAGEMAKER_APP_TYPE
462+
delete process.env.SAGEMAKER_INTERNAL_IMAGE_URI
463+
delete process.env.STUDIO_LOGGING_DIR
464+
delete process.env.SM_APP_TYPE
465+
delete process.env.SM_INTERNAL_IMAGE_URI
466+
delete process.env.SERVICE_NAME
467+
})
468+
469+
afterEach(function () {
470+
process.env = originalEnv
471+
})
472+
473+
const testCases = [
474+
{ env: 'SAGEMAKER_APP_TYPE', value: 'JupyterServer', expected: true },
475+
{ env: 'SAGEMAKER_INTERNAL_IMAGE_URI', value: 'some-uri', expected: true },
476+
{ env: 'STUDIO_LOGGING_DIR', value: '/var/log/studio/app.log', expected: true },
477+
{ env: 'STUDIO_LOGGING_DIR', value: '/var/log/other/app.log', expected: false },
478+
{ env: 'SM_APP_TYPE', value: 'JupyterServer', expected: true },
479+
{ env: 'SM_INTERNAL_IMAGE_URI', value: 'some-uri', expected: true },
480+
{ env: 'SERVICE_NAME', value: 'SageMakerUnifiedStudio', expected: true },
481+
{ env: 'SERVICE_NAME', value: 'SomeOtherService', expected: false },
482+
]
483+
484+
for (const { env, value, expected } of testCases) {
485+
it(`returns ${expected} when ${env} is set to "${value}"`, function () {
486+
process.env[env] = value
487+
488+
const result = hasSageMakerEnvVars()
489+
490+
assert.strictEqual(result, expected)
491+
})
492+
}
493+
494+
it('returns true when multiple SageMaker env vars are set', function () {
495+
process.env.SAGEMAKER_APP_TYPE = 'JupyterServer'
496+
process.env.SM_APP_TYPE = 'CodeEditor'
497+
498+
const result = hasSageMakerEnvVars()
499+
500+
assert.strictEqual(result, true)
501+
})
502+
503+
it('returns false when no SageMaker env vars are set', function () {
504+
const result = hasSageMakerEnvVars()
505+
506+
assert.strictEqual(result, false)
507+
})
508+
})

packages/core/src/test/shared/telemetry/util.test.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ import { randomUUID } from 'crypto'
2424
import { isUuid } from '../../../shared/crypto'
2525
import { MetricDatum } from '../../../shared/telemetry/clienttelemetry'
2626
import { assertLogsContain } from '../../globalSetup.test'
27+
import { getClientName } from '../../../shared/telemetry/util'
28+
import * as extensionUtilities from '../../../shared/extensionUtilities'
29+
import * as sinon from 'sinon'
30+
import * as vscode from 'vscode'
2731

2832
describe('TelemetryConfig', function () {
2933
const settingKey = 'aws.telemetry'
@@ -391,3 +395,56 @@ describe('validateMetricEvent', function () {
391395
assertLogsContain('invalid Metric', false, 'warn')
392396
})
393397
})
398+
399+
describe('getClientName', function () {
400+
let sandbox: sinon.SinonSandbox
401+
let isSageMakerStub: sinon.SinonStub
402+
403+
beforeEach(function () {
404+
sandbox = sinon.createSandbox()
405+
isSageMakerStub = sandbox.stub(extensionUtilities, 'isSageMaker')
406+
})
407+
408+
afterEach(function () {
409+
sandbox.restore()
410+
})
411+
412+
it('returns "AmazonQ-For-SMUS-CE" when in SMUS environment', function () {
413+
isSageMakerStub.withArgs('SMUS').returns(true)
414+
sandbox.stub(vscode.env, 'appName').value('SageMaker Code Editor')
415+
416+
const result = getClientName()
417+
418+
assert.strictEqual(result, 'AmazonQ-For-SMUS-CE')
419+
assert.ok(isSageMakerStub.calledOnceWith('SMUS'))
420+
})
421+
422+
it('returns vscode app name when not in SMUS environment', function () {
423+
const mockAppName = 'Visual Studio Code'
424+
isSageMakerStub.withArgs('SMUS').returns(false)
425+
sandbox.stub(vscode.env, 'appName').value(mockAppName)
426+
427+
const result = getClientName()
428+
429+
assert.strictEqual(result, mockAppName)
430+
assert.ok(isSageMakerStub.calledOnceWith('SMUS'))
431+
})
432+
433+
it('handles undefined app name gracefully', function () {
434+
isSageMakerStub.withArgs('SMUS').returns(false)
435+
sandbox.stub(vscode.env, 'appName').value(undefined)
436+
437+
const result = getClientName()
438+
439+
assert.strictEqual(result, undefined)
440+
})
441+
442+
it('prioritizes SMUS detection over app name', function () {
443+
isSageMakerStub.withArgs('SMUS').returns(true)
444+
sandbox.stub(vscode.env, 'appName').value('SageMaker Code Editor')
445+
446+
const result = getClientName()
447+
448+
assert.strictEqual(result, 'AmazonQ-For-SMUS-CE')
449+
})
450+
})

0 commit comments

Comments
 (0)