Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "Bug Fix",
"description": "Improved Amazon Linux 2 support with better SageMaker environment detection"
}
11 changes: 7 additions & 4 deletions packages/amazonq/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,15 @@ export async function activateAmazonQCommon(context: vscode.ExtensionContext, is

// This contains every lsp agnostic things (auth, security scan, code scan)
await activateCodeWhisperer(extContext as ExtContext)
if (

// Always activate LSP for SageMaker environments
const shouldActivateLsp =
(Experiments.instance.get('amazonqLSP', true) || Auth.instance.isInternalAmazonUser()) &&
(!isAmazonLinux2() || hasGlibcPatch())
) {
(isSageMaker() || !isAmazonLinux2() || (await hasGlibcPatch()))
Copy link
Contributor

Choose a reason for hiding this comment

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

it appears that the isAmazonLinux2 check is incorrectly flagging ubuntu containers under AL2 hosts. i do not think we need to special case sagemaker for now

Copy link
Contributor

Choose a reason for hiding this comment

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

what is the exact setup for SageMaker AL2 instances?


if (shouldActivateLsp) {
// start the Amazon Q LSP for internal users first
// for AL2, start LSP if glibc patch is found
// for AL2, start LSP if glibc patch is found or if it's a SageMaker environment
await activateAmazonqLsp(context)
}
if (!Experiments.instance.get('amazonqLSPInline', true)) {
Expand Down
45 changes: 37 additions & 8 deletions packages/amazonq/src/lsp/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,11 @@ import {
undefinedIfEmpty,
getOptOutPreference,
isAmazonLinux2,
fs,
getClientId,
extensionVersion,
} from 'aws-core-vscode/shared'
import { processUtils } from 'aws-core-vscode/shared'
import { processUtils, isSageMaker } from 'aws-core-vscode/shared'
import { activate } from './chat/activation'
import { AmazonQResourcePaths } from './lspInstaller'
import { ConfigSection, isValidConfigSection, pushConfigUpdate, toAmazonQLSPLogLevel } from './config'
Expand All @@ -53,11 +54,24 @@ import { InlineChatTutorialAnnotation } from '../app/inline/tutorials/inlineChat
const localize = nls.loadMessageBundle()
const logger = getLogger('amazonqLsp.lspClient')

export const glibcLinker: string = process.env.VSCODE_SERVER_CUSTOM_GLIBC_LINKER || ''
export const glibcPath: string = process.env.VSCODE_SERVER_CUSTOM_GLIBC_PATH || ''
export async function hasGlibcPatch(): Promise<boolean> {
// Skip GLIBC patching for SageMaker environments
if (isSageMaker()) {
getLogger('amazonqLsp').info('SageMaker environment detected in hasGlibcPatch, skipping GLIBC patching')
return false // Return false to ensure SageMaker doesn't try to use GLIBC patching
}

// Check for environment variables (for CDM)
const glibcLinker = process.env.VSCODE_SERVER_CUSTOM_GLIBC_LINKER || ''
const glibcPath = process.env.VSCODE_SERVER_CUSTOM_GLIBC_PATH || ''

if (glibcLinker.length > 0 && glibcPath.length > 0) {
getLogger('amazonqLsp').info('GLIBC patching environment variables detected')
return true
}

export function hasGlibcPatch(): boolean {
return glibcLinker.length > 0 && glibcPath.length > 0
// No environment variables, no patching needed
return false
}

export async function startLanguageServer(
Expand All @@ -82,9 +96,24 @@ export async function startLanguageServer(
const traceServerEnabled = Settings.instance.isSet(`${clientId}.trace.server`)
let executable: string[] = []
// apply the GLIBC 2.28 path to node js runtime binary
if (isAmazonLinux2() && hasGlibcPatch()) {
executable = [glibcLinker, '--library-path', glibcPath, resourcePaths.node]
getLogger('amazonqLsp').info(`Patched node runtime with GLIBC to ${executable}`)
if (isSageMaker()) {
// SageMaker doesn't need GLIBC patching
getLogger('amazonqLsp').info('SageMaker environment detected, skipping GLIBC patching')
executable = [resourcePaths.node]
} else if (isAmazonLinux2() && (await hasGlibcPatch())) {
// Use environment variables if available (for CDM)
if (process.env.VSCODE_SERVER_CUSTOM_GLIBC_LINKER && process.env.VSCODE_SERVER_CUSTOM_GLIBC_PATH) {
executable = [
process.env.VSCODE_SERVER_CUSTOM_GLIBC_LINKER,
'--library-path',
process.env.VSCODE_SERVER_CUSTOM_GLIBC_PATH,
resourcePaths.node,
]
getLogger('amazonqLsp').info(`Patched node runtime with GLIBC using env vars to ${executable}`)
} else {
// No environment variables, use the node executable directly
executable = [resourcePaths.node]
}
} else {
executable = [resourcePaths.node]
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@
"@aws-sdk/s3-request-presigner": "<3.731.0",
"@aws-sdk/smithy-client": "<3.731.0",
"@aws-sdk/util-arn-parser": "<3.731.0",
"@aws/mynah-ui": "^4.35.4",
"@aws/mynah-ui": "^4.35.6",
Copy link
Contributor

Choose a reason for hiding this comment

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

mynah update should not be in this focused change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry..previous commit sneaked in...let me correct and raise a new one

"@gerhobbelt/gitignore-parser": "^0.2.0-9",
"@iarna/toml": "^2.2.5",
"@smithy/fetch-http-handler": "^5.0.1",
Expand Down
12 changes: 12 additions & 0 deletions packages/core/src/shared/extensionUtilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,18 @@ export function isCloud9(flavor: 'classic' | 'codecatalyst' | 'any' = 'any'): bo
* @returns true if the current system is SageMaker(SMAI or SMUS)
*/
export function isSageMaker(appName: 'SMAI' | 'SMUS' = 'SMAI'): boolean {
// Check for SageMaker-specific environment variables first
Copy link
Contributor

Choose a reason for hiding this comment

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

FYI, the check for Unified Studio i.e process.env.SERVICE_NAME === sageMakerUnifiedStudio, is already handled in isSageMakerUnifiedStudio()
And with this logic, it will bypass the check for Sagemaker AI or Unified Studio cases right? Can we extract these env variable checks omitting this process.env.SERVICE_NAME === sageMakerUnifiedStudiointo a boolean and use it in case statements for better readability?

if (
process.env.SAGEMAKER_APP_TYPE !== undefined ||
process.env.SERVICE_NAME === sageMakerUnifiedStudio ||
process.env.SAGEMAKER_INTERNAL_IMAGE_URI !== undefined ||
process.env.STUDIO_LOGGING_DIR?.includes('/var/log/studio') === true
) {
getLogger().debug('SageMaker environment detected via environment variables')
return true
}

// Fall back to app name checks
switch (appName) {
case 'SMAI':
return vscode.env.appName === sageMakerAppname
Expand Down
Loading