Skip to content

Commit 71a4aef

Browse files
feat(amazonq): update lsp clientname to support sagemaker unified studio case (#7817)
## 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 - The original PR was reverted [here](https://github.com/aws/aws-toolkit-vscode/pull/7813/commits) due to failing unit tests - Unit test failure was due to env variable ```SERVICE_NAME``` being modified directly from test code causing failure in other tests which identified the environment to be SMUS without clean isolation - Fixed the test failure by using sinon stub instead of modifying env vars in node process ## 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 - Ran ```npm run test -w packages/toolkit``` which succeeded - 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. --------- Co-authored-by: Laxman Reddy <[email protected]>
1 parent 781f89e commit 71a4aef

File tree

6 files changed

+233
-8
lines changed

6 files changed

+233
-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: 142 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,143 @@ 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+
})
380+
381+
describe('SMAI detection', function () {
382+
it('returns true when both app name and env vars match', function () {
383+
sandbox.stub(vscode.env, 'appName').value('SageMaker Code Editor')
384+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(true)
385+
386+
assert.strictEqual(isSageMaker('SMAI'), true)
387+
})
388+
389+
it('returns false when app name is different', function () {
390+
sandbox.stub(vscode.env, 'appName').value('Visual Studio Code')
391+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(true)
392+
393+
assert.strictEqual(isSageMaker('SMAI'), false)
394+
})
395+
396+
it('returns false when env vars are missing', function () {
397+
sandbox.stub(vscode.env, 'appName').value('SageMaker Code Editor')
398+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(false)
399+
400+
assert.strictEqual(isSageMaker('SMAI'), false)
401+
})
402+
403+
it('defaults to SMAI when no parameter provided', function () {
404+
sandbox.stub(vscode.env, 'appName').value('SageMaker Code Editor')
405+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(true)
406+
407+
assert.strictEqual(isSageMaker(), true)
408+
})
409+
})
410+
411+
describe('SMUS detection', function () {
412+
it('returns true when all conditions are met', function () {
413+
sandbox.stub(vscode.env, 'appName').value('SageMaker Code Editor')
414+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(true)
415+
sandbox.stub(process, 'env').value({ SERVICE_NAME: 'SageMakerUnifiedStudio' })
416+
utils.resetSageMakerState()
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+
sandbox.stub(process, 'env').value({ SERVICE_NAME: 'SomeOtherService' })
425+
utils.resetSageMakerState()
426+
427+
assert.strictEqual(isSageMaker('SMUS'), false)
428+
})
429+
430+
it('returns false when env vars are missing', function () {
431+
sandbox.stub(vscode.env, 'appName').value('SageMaker Code Editor')
432+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(false)
433+
sandbox.stub(process, 'env').value({ SERVICE_NAME: 'SageMakerUnifiedStudio' })
434+
utils.resetSageMakerState()
435+
436+
assert.strictEqual(isSageMaker('SMUS'), false)
437+
})
438+
439+
it('returns false when app name is different', function () {
440+
sandbox.stub(vscode.env, 'appName').value('Visual Studio Code')
441+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(true)
442+
sandbox.stub(process, 'env').value({ SERVICE_NAME: 'SageMakerUnifiedStudio' })
443+
utils.resetSageMakerState()
444+
445+
assert.strictEqual(isSageMaker('SMUS'), false)
446+
})
447+
})
448+
449+
it('returns false for invalid appName parameter', function () {
450+
sandbox.stub(vscode.env, 'appName').value('SageMaker Code Editor')
451+
sandbox.stub(env, 'hasSageMakerEnvVars').returns(true)
452+
453+
// @ts-ignore - Testing invalid input
454+
assert.strictEqual(isSageMaker('INVALID'), false)
455+
})
456+
})
457+
458+
describe('hasSageMakerEnvVars', function () {
459+
let sandbox: sinon.SinonSandbox
460+
461+
beforeEach(function () {
462+
sandbox = sinon.createSandbox()
463+
})
464+
465+
afterEach(function () {
466+
sandbox.restore()
467+
})
468+
469+
it('detects SageMaker environment variables', function () {
470+
// Test SAGEMAKER_ prefix
471+
sandbox.stub(process, 'env').value({ SAGEMAKER_APP_TYPE: 'JupyterServer' })
472+
assert.strictEqual(hasSageMakerEnvVars(), true)
473+
474+
// Test SM_ prefix
475+
sandbox.stub(process, 'env').value({ SM_APP_TYPE: 'CodeEditor' })
476+
assert.strictEqual(hasSageMakerEnvVars(), true)
477+
478+
// Test SERVICE_NAME with correct value
479+
sandbox.stub(process, 'env').value({ SERVICE_NAME: 'SageMakerUnifiedStudio' })
480+
assert.strictEqual(hasSageMakerEnvVars(), true)
481+
482+
// Test STUDIO_LOGGING_DIR with correct path
483+
sandbox.stub(process, 'env').value({ STUDIO_LOGGING_DIR: '/var/log/studio/app.log' })
484+
assert.strictEqual(hasSageMakerEnvVars(), true)
485+
486+
// Test invalid SERVICE_NAME
487+
sandbox.stub(process, 'env').value({ SERVICE_NAME: 'SomeOtherService' })
488+
assert.strictEqual(hasSageMakerEnvVars(), false)
489+
490+
// Test invalid STUDIO_LOGGING_DIR
491+
sandbox.stub(process, 'env').value({ STUDIO_LOGGING_DIR: '/var/log/other/app.log' })
492+
assert.strictEqual(hasSageMakerEnvVars(), false)
493+
494+
// Test multiple env vars
495+
sandbox.stub(process, 'env').value({
496+
SAGEMAKER_APP_TYPE: 'JupyterServer',
497+
SM_APP_TYPE: 'CodeEditor',
498+
})
499+
assert.strictEqual(hasSageMakerEnvVars(), true)
500+
501+
// Test no env vars
502+
sandbox.stub(process, 'env').value({})
503+
assert.strictEqual(hasSageMakerEnvVars(), false)
504+
})
505+
})

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)