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(lsp): Integrate language server/manifest resolver telemetry #6385

Open
wants to merge 36 commits into
base: feature/amazonqLSP
Choose a base branch
from

Conversation

Hweinstock
Copy link
Contributor

@Hweinstock Hweinstock commented Jan 16, 2025

Problem

LSP downloading processes does not emit any telemetry.

Solution

Refactoring

  • separate verification from core downloading steps so that we can capture it as its own telemetry event.
  • Note: this refactor means that if all the downloaded content cannot be verified, no files will be written to disk.
  • Introduce abstraction of StageResolver to make telemetry instrumentation more natural.

Metric Behavior

Testing

  • adds basic tests for ManifestResolver in packages/core/src/test/shared/lsp/manifestResolver.test.ts.
  • adds basic tests for LanguageServerResolver in packages/core/src/test/shared/lsp/lspResolver.test.ts.
  • Currently LanguageServerResolver only supports mac, so its tests skip on non-mac platforms. There is a techdebt test to address this.

  • 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.
  • License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link

  • This pull request modifies code in src/* but no tests were added/updated.
    • Confirm whether tests should be added or ensure the PR description explains why tests are not required.

@justinmk3 justinmk3 changed the title telemetry(lsp): Integrate langauge server/manifest resolver telemetry telemetry(lsp): Integrate language server/manifest resolver telemetry Jan 17, 2025
@Hweinstock Hweinstock force-pushed the telemetry/setup branch 2 times, most recently from 94902b9 to 87913ca Compare January 17, 2025 16:07
Hweinstock added a commit to aws/aws-toolkit-common that referenced this pull request Jan 23, 2025
## Problem
Relevant PR: aws/aws-toolkit-vscode#6385

## Solution
- standardize the metrics across the toolkits. 
<!---
    REMINDER:
    - Read CONTRIBUTING.md first.
    - Add test coverage for your changes.
    - Link to related issues/commits.
    - Testing: how did you test your changes?
    - Screenshots if applicable
-->

## License

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

---------

Co-authored-by: Shruti Sinha <[email protected]>
@Hweinstock Hweinstock changed the title telemetry(lsp): Integrate language server/manifest resolver telemetry telemetry(lsp): Integrate language server/manifest resolver telemetry (WIP) Jan 23, 2025
@Hweinstock Hweinstock changed the title telemetry(lsp): Integrate language server/manifest resolver telemetry (WIP) telemetry(lsp): Integrate language server/manifest resolver telemetry Jan 28, 2025
@Hweinstock Hweinstock marked this pull request as ready for review January 28, 2025 16:31
@Hweinstock Hweinstock requested review from a team as code owners January 28, 2025 16:31
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