Skip to content
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

Telemetry emission for init stage and full process #246

Merged
merged 1 commit into from
Nov 21, 2024

Conversation

nborges-aws
Copy link
Contributor

Description of changes:
This PR includes the addition of telemetry metrics for the initialization of the amazon Q LSP server, as well as metrics for the full process of fetching and starting the LSP. Some additional QoL changes featured in this PR include:

  • abstracting logic for setting AmazonQ server into its own method
  • added logic to include error message in getManifest metric on failure case
  • included manifest & server location data in validate LSP step

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@nborges-aws nborges-aws marked this pull request as ready for review November 20, 2024 20:39
@nborges-aws nborges-aws force-pushed the nickdb/lspTelemetryv2 branch from 27475ef to 8452cce Compare November 21, 2024 15:47
@nborges-aws nborges-aws force-pushed the nickdb/lspTelemetryv2 branch from 8452cce to 3946d5e Compare November 21, 2024 17:05
@nborges-aws nborges-aws merged commit 633c897 into main Nov 21, 2024
1 check passed
@nborges-aws nborges-aws deleted the nickdb/lspTelemetryv2 branch November 21, 2024 17:11
}

public static void emitSetupValidate(final Result result, final RecordLspSetupArgs args) {
emitSetupMetric(result, args, LanguageServerSetupStage.VALIDATE);
if (result == Result.FAILED) {
emitSetupAll(Result.FAILED, args);
Copy link
Contributor

Choose a reason for hiding this comment

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

why not emit this at the end of the overall setup process in getlspinstallation instead of sprinkling it all across this class

Copy link
Contributor Author

@nborges-aws nborges-aws Nov 27, 2024

Choose a reason for hiding this comment

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

The logic here should've had and additional condition that location is not equal to override, which was fixed in PR-260. The logic is the same as this case. SetupAll should not be emitted if override fails validation, but the event should emit as telemetry wanted the granularity of knowing that override failed. This ensures that setupAll is only emitted when validation fails at a point that the entire language server setup process is halted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants