-
Notifications
You must be signed in to change notification settings - Fork 272
fix(amazonq): add metrics for flare/node resolution #5786
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,10 +77,12 @@ | |
import software.aws.toolkits.jetbrains.services.amazonq.lsp.util.WorkspaceFolderUtil.createWorkspaceFolders | ||
import software.aws.toolkits.jetbrains.services.amazonq.lsp.workspace.WorkspaceServiceHandler | ||
import software.aws.toolkits.jetbrains.services.amazonq.profile.QDefaultServiceConfig | ||
import software.aws.toolkits.jetbrains.services.cwc.controller.chat.telemetry.getStartUrl | ||
import software.aws.toolkits.jetbrains.services.telemetry.ClientMetadata | ||
import software.aws.toolkits.jetbrains.settings.LspSettings | ||
import software.aws.toolkits.jetbrains.utils.notifyInfo | ||
import software.aws.toolkits.resources.message | ||
import software.aws.toolkits.telemetry.Telemetry | ||
import java.io.IOException | ||
import java.io.OutputStreamWriter | ||
import java.io.PipedInputStream | ||
|
@@ -526,20 +528,36 @@ | |
* may fail to start in that case. The caller should handle potential runtime initialization failures. | ||
*/ | ||
private fun getNodeRuntimePath(nodePath: Path): Path { | ||
val resolveNodeMetric = { isBundled: Boolean, success: Boolean -> | ||
Telemetry.languageserver.setup.use { | ||
it.id("q") | ||
it.metadata("languageServerSetupStage", "resolveNode") | ||
it.metadata("credentialStartUrl", getStartUrl(project)) | ||
it.setAttribute("isBundledNode", isBundled) | ||
it.success(success) | ||
} | ||
Check warning on line 538 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/AmazonQLspService.kt
|
||
} | ||
|
||
if (Files.exists(nodePath) && Files.isExecutable(nodePath)) { | ||
resolveNodeMetric(true, true) | ||
Check warning on line 542 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/AmazonQLspService.kt
|
||
return nodePath | ||
} | ||
|
||
// use alternative node runtime if it is not found | ||
LOG.warn { "Node Runtime download failed. Fallback to user specified node runtime " } | ||
// attempt to use user provided node runtime path | ||
val nodeRuntime = LspSettings.getInstance().getNodeRuntimePath() | ||
if (!nodeRuntime.isNullOrEmpty()) { | ||
LOG.info { "Using node from $nodeRuntime " } | ||
|
||
resolveNodeMetric(false, true) | ||
Check warning on line 553 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/AmazonQLspService.kt
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: is it possible to add parameter names? |
||
return Path.of(nodeRuntime) | ||
} else { | ||
val localNode = locateNodeCommand() | ||
if (localNode != null) { | ||
LOG.info { "Using node from ${localNode.toAbsolutePath()}" } | ||
|
||
resolveNodeMetric(false, true) | ||
Check warning on line 560 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/AmazonQLspService.kt
|
||
return localNode | ||
} | ||
notifyInfo( | ||
|
@@ -557,6 +575,8 @@ | |
) { _, notification -> notification.expire() } | ||
) | ||
) | ||
|
||
resolveNodeMetric(false, false) | ||
Check warning on line 579 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/AmazonQLspService.kt
|
||
return nodePath | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,10 @@ | |
import software.aws.toolkits.core.utils.info | ||
import software.aws.toolkits.core.utils.warn | ||
import software.aws.toolkits.jetbrains.core.saveFileFromUrl | ||
import software.aws.toolkits.jetbrains.services.cwc.controller.chat.telemetry.getStartUrl | ||
import software.aws.toolkits.resources.AwsCoreBundle | ||
import software.aws.toolkits.telemetry.LanguageServerSetupStage | ||
import software.aws.toolkits.telemetry.Telemetry | ||
import java.nio.file.Files | ||
import java.nio.file.Path | ||
import java.nio.file.Paths | ||
|
@@ -106,33 +109,33 @@ | |
} | ||
|
||
suspend fun tryDownloadLspArtifacts(project: Project, targetVersion: Version, target: VersionTarget): Path? { | ||
val temporaryDownloadPath = Files.createTempDirectory("lsp-dl") | ||
val downloadPath = lspArtifactsPath.resolve(targetVersion.serverVersion.toString()) | ||
val destinationPath = lspArtifactsPath.resolve(targetVersion.serverVersion.toString()) | ||
Check warning on line 112 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt
|
||
|
||
while (currentAttempt.get() < maxDownloadAttempts) { | ||
currentAttempt.incrementAndGet() | ||
logger.info { "Attempt ${currentAttempt.get()} of $maxDownloadAttempts to download LSP artifacts" } | ||
val temporaryDownloadPath = Files.createTempDirectory("lsp-dl") | ||
Check warning on line 117 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt
|
||
|
||
try { | ||
return withBackgroundProgress( | ||
project, | ||
AwsCoreBundle.message("amazonqFeatureDev.placeholder.downloading_and_extracting_lsp_artifacts"), | ||
cancellable = true | ||
) { | ||
if (downloadLspArtifacts(temporaryDownloadPath, target) && !target.contents.isNullOrEmpty()) { | ||
moveFilesFromSourceToDestination(temporaryDownloadPath, downloadPath) | ||
if (downloadLspArtifacts(project, temporaryDownloadPath, target) && !target.contents.isNullOrEmpty()) { | ||
moveFilesFromSourceToDestination(temporaryDownloadPath, destinationPath) | ||
Check warning on line 126 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt
|
||
target.contents | ||
.mapNotNull { it.filename } | ||
.forEach { filename -> extractZipFile(downloadPath.resolve(filename), downloadPath) } | ||
logger.info { "Successfully downloaded and moved LSP artifacts to $downloadPath" } | ||
.forEach { filename -> extractZipFile(destinationPath.resolve(filename), destinationPath) } | ||
logger.info { "Successfully downloaded and moved LSP artifacts to $destinationPath" } | ||
Check warning on line 130 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt
|
||
|
||
val thirdPartyLicenses = targetVersion.thirdPartyLicenses | ||
logger.info { | ||
"Installing Amazon Q Language Server v${targetVersion.serverVersion} to: $downloadPath. " + | ||
"Installing Amazon Q Language Server v${targetVersion.serverVersion} to: $destinationPath. " + | ||
Check warning on line 134 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt
|
||
if (thirdPartyLicenses == null) "" else "Attribution notice can be found at $thirdPartyLicenses" | ||
} | ||
|
||
return@withBackgroundProgress downloadPath | ||
return@withBackgroundProgress destinationPath | ||
Check warning on line 138 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt
|
||
} | ||
|
||
return@withBackgroundProgress null | ||
|
@@ -146,15 +149,15 @@ | |
else -> { logger.error(e) { "Failed to download/move LSP artifacts on attempt ${currentAttempt.get()}" } } | ||
} | ||
temporaryDownloadPath.toFile().deleteRecursively() | ||
downloadPath.toFile().deleteRecursively() | ||
destinationPath.toFile().deleteRecursively() | ||
Check warning on line 152 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt
|
||
} | ||
} | ||
logger.error { "Failed to download LSP artifacts after $maxDownloadAttempts attempts" } | ||
return null | ||
} | ||
|
||
@VisibleForTesting | ||
internal fun downloadLspArtifacts(downloadPath: Path, target: VersionTarget?): Boolean { | ||
internal fun downloadLspArtifacts(project: Project, downloadPath: Path, target: VersionTarget?): Boolean { | ||
if (target == null || target.contents.isNullOrEmpty()) { | ||
logger.warn { "No target contents available for download" } | ||
return false | ||
|
@@ -171,7 +174,7 @@ | |
logger.warn { "No hash available for ${content.filename}" } | ||
return@forEach | ||
} | ||
downloadAndValidateFile(content.url, filePath, contentHash) | ||
downloadAndValidateFile(project, content.url, filePath, contentHash) | ||
Check warning on line 177 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt
|
||
} | ||
validateDownloadedFiles(downloadPath, target.contents) | ||
} catch (e: Exception) { | ||
|
@@ -182,18 +185,46 @@ | |
return true | ||
} | ||
|
||
private fun downloadAndValidateFile(url: String, filePath: Path, expectedHash: String) { | ||
private fun downloadAndValidateFile(project: Project, url: String, filePath: Path, expectedHash: String) { | ||
val recordDownload = { runnable: () -> Unit -> | ||
Telemetry.languageserver.setup.use { telemetry -> | ||
telemetry.id("q") | ||
telemetry.languageServerSetupStage(LanguageServerSetupStage.GetServer) | ||
telemetry.metadata("credentialStartUrl", getStartUrl(project)) | ||
telemetry.success(true) | ||
Check warning on line 194 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt
|
||
|
||
try { | ||
runnable() | ||
} catch (t: Throwable) { | ||
telemetry.success(false) | ||
telemetry.recordException(t) | ||
} | ||
} | ||
Check warning on line 202 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt
|
||
} | ||
|
||
try { | ||
if (!filePath.exists()) { | ||
logger.info { "Downloading file: ${filePath.fileName}" } | ||
saveFileFromUrl(url, filePath, ProgressManager.getInstance().progressIndicator) | ||
recordDownload { saveFileFromUrl(url, filePath, ProgressManager.getInstance().progressIndicator) } | ||
Check warning on line 208 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt
|
||
} | ||
if (!validateFileHash(filePath, expectedHash)) { | ||
logger.warn { "Hash mismatch for ${filePath.fileName}, re-downloading" } | ||
filePath.deleteIfExists() | ||
Comment on lines
206
to
212
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not related to this PR but it seems like we can download twice if the first downloaded one has hash mismatch due to antivirus changing content, but still acceptable |
||
saveFileFromUrl(url, filePath) | ||
if (!validateFileHash(filePath, expectedHash)) { | ||
throw LspException("Hash mismatch after re-download for ${filePath.fileName}", LspException.ErrorCode.HASH_MISMATCH) | ||
recordDownload { saveFileFromUrl(url, filePath) } | ||
Check warning on line 213 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt
|
||
|
||
Telemetry.languageserver.setup.use { | ||
it.id("q") | ||
it.languageServerSetupStage(LanguageServerSetupStage.Validate) | ||
it.metadata("credentialStartUrl", getStartUrl(project)) | ||
it.success(true) | ||
Check warning on line 219 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt
|
||
|
||
if (!validateFileHash(filePath, expectedHash)) { | ||
it.success(false) | ||
Check warning on line 222 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt
|
||
|
||
val exception = LspException("Hash mismatch after re-download for ${filePath.fileName}", LspException.ErrorCode.HASH_MISMATCH) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we run this through the util that removes username(scrubNames) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's |
||
it.recordException(exception) | ||
throw exception | ||
Check warning on line 226 in plugins/amazonq/shared/jetbrains-community/src/software/aws/toolkits/jetbrains/services/amazonq/lsp/artifacts/ArtifactHelper.kt
|
||
} | ||
} | ||
} | ||
} catch (e: Exception) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getStartUrl may be null if user has not login but I think it's fine