fix(vscode): Fix log subscriptions telemetry#8858
fix(vscode): Fix log subscriptions telemetry#8858andrew-eldridge wants to merge 3 commits intomainfrom
Conversation
🤖 AI PR Validation ReportPR Review ResultsThank you for your submission! Here's detailed feedback on your PR title and body compliance:✅ PR Title
✅ Commit Type
✅ Risk Level
✅ What & Why
✅ Impact of Change
✅ Test Plan
✅ Contributors
✅ Screenshots/Videos
Summary Table
Final message Thank you for the clear PR — small focused change and well-documented.Last updated: Tue, 03 Mar 2026 18:32:58 GMT |
🤖 AI PR Validation ReportPR Review ResultsThank you for your submission! Here's detailed feedback on your PR title and body compliance:✅ PR Title
❌ Commit Type
❌ Risk Level
|
| Section | Status | Recommendation |
|---|---|---|
| Title | ✅ | Minor wording suggestion (optional) |
| Commit Type | ❌ | Select fix in the PR body |
| Risk Level | ❌ | Select Low in PR body to match label risk:low |
| What & Why | Expand to explain before/after and why it matters | |
| Impact of Change | Clarify users/developers/system impact; remove ambiguous phrasing | |
| Test Plan | ❌ | Add unit/E2E tests or provide a detailed manual test plan and check appropriate boxes |
| Contributors | @andrew-eldridge present — add others if needed |
|
| Screenshots/Videos | ✅ | Not applicable |
Final message:
This PR does not pass the PR body/template checks. The code change is small and risk appears low (matches the risk:low label), but the PR body is missing required selections (Commit Type and Risk Level) and does not include tests or an adequate test justification. Please update the PR body to:
- Select exactly one Commit Type — check
fix. - Select exactly one Risk Level — check
Lowto match therisk:lowlabel. - Expand the "What & Why" with a short before/after description (see the recommended phrasing above).
- Clarify the "Impact of Change" using explicit bullets for Users/Developers/System.
- Update the Test Plan:
- Add unit or E2E tests that cover the behavior OR
- If not possible, add a clear manual test plan with reproduction steps and observed results, then check
Manual testing completed.
Once those changes are made, re-request review. The advised risk level based on the diff is low and matches the PR label, so no change to the risk label is required.
Thank you for the clear, small diff — once the PR body is updated and tests or justification are added this should be good to merge.
Last updated: Thu, 26 Feb 2026 17:14:27 GMT
📊 Coverage CheckThe following changed files need attention: ❌
Please add tests for the uncovered files before merging. |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the async logSubscriptions function was called without being awaited during VS Code extension activation, potentially causing subscription telemetry to not be captured before the activation event completed.
Changes:
- Converted fire-and-forget
logSubscriptionscall to a promise pattern that starts early and awaits at the end of activation - Removed unnecessary
asynckeyword fromlogExtensionSettingsfunction (it never usedawaitinternally)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| apps/vs-code-designer/src/main.ts | Starts logSubscriptions promise early (line 115) and awaits it at end of activation (line 163) to ensure telemetry is captured |
| apps/vs-code-designer/src/app/utils/telemetry.ts | Removes async keyword from logExtensionSettings since it's synchronous |
Commit Type
Risk Level
What & Why
Unawaited async logSubscriptions function doesn't log telemetry before startup completes
Impact of Change
Test Plan
Minor change to context for subscription telemetry, does not require additional tests
Contributors
@andrew-eldridge