chore: improve java upgrade telemetry tracking and extension installation flow#1030
Open
frankliu20 wants to merge 6 commits into
Open
Conversation
frankliu20
requested review from
chagong,
testforstephen and
wenytang-ms
as code owners
frankliu20
force-pushed
the
haital-microsoft/fix-upgrade-notification-telemetry
branch
5 times, most recently
from
faf0be4 to
447c880
Compare
frankliu20
changed the title
frankliu20
force-pushed
the
haital-microsoft/fix-upgrade-notification-telemetry
branch
from
27162b6 to
5eeef91
Compare
Follow-up to v0.27.5 telemetry analysis (devdiv-azure-service-dmitryr/azure-java-migration-copilot-vscode-extension#5979): - Add a source field (SOURCE_CVE / SOURCE_JAVA_UPGRADE) to the upgradeNotification.show event so NOTIFY-level data can be split by CVE vs upgrade and aligned with downstream funnel stages. - Emit an installSucceeded event (with the initial extensionState, so fresh installs and updates can be told apart) right after installExtension resolves. - Drop the post-install getExtensionState re-check that produced false activation-timeout results; the not-installed path now reports proceeded and returns true after install completes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
frankliu20
force-pushed
the
haital-microsoft/fix-upgrade-notification-telemetry
branch
from
5eeef91 to
a3b367f
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refines the Java upgrade notification → upgradeFlow telemetry funnel to improve attribution and reduce false “activation-timeout” outcomes observed in production telemetry.
Changes:
- Adds a NOTIFY-level
sourcedimension (SOURCE_CVEvsSOURCE_JAVA_UPGRADE) toupgradeNotification.showtelemetry. - Emits a new
upgradeFlow.installSucceededtelemetry event immediately afterinstallExtensionresolves. - Simplifies the
not-installedupgrade path to proceed after install (removing the post-installgetExtensionState()re-check).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/upgrade/utility.ts | Adds post-install telemetry and changes the post-install proceed/return behavior in the upgrade flow. |
| src/upgrade/display/notificationManager.ts | Adds a source dimension to the upgrade notification “show” telemetry event. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wenytang-ms
reviewed
Jun 9, 2026
Address review: avoid adding a new business operationName for the install step. Install success is already implied by the subsequent upgradeFlow.result event under the same operation ID. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address review: instead of a dedicated installSucceeded operationName, reuse the stable java.dependency.upgradeFlow.result event and mark the install step with an upgradeFlowStep dimension (plus extensionState so fresh installs and updates can be told apart). Easier to query and evolve. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address review: extensionState was the pre-install value, so tagging a
'succeeded' event with 'not-installed'/'outdated' was contradictory.
Replace it with installType ('installed' vs 'updated') derived from the
pre-install state, which is unambiguous and avoids an unreliable
post-install state recompute (the original activation-race bug).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
extensions.getExtension() returns undefined both when the extension is disabled and when it is freshly installed but not yet activated, so the 'seems disabled, enable it manually' prompt is misleading right after a successful install. Skip it on the upgrade path; the function and the modernization caller are left unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the fixed 2s wait after a fresh install with an event-driven wait: resolve immediately if the extension is already registered, otherwise listen on extensions.onDidChange and proceed as soon as it appears, with a 5s timeout fallback. Returns as early as possible while still bounding the wait. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
frankliu20
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Production telemetry from
vscode-java-dependencyv0.27.5 (7d) surfaced gaps in theupgrade notification ->
upgradeFlowfunnel (follow-up to #1022):upgradeNotification.showevent could not be split by CVE vs upgrade, soper-bucket click rates were diluted at the NOTIFY level.
not-installedflow starts were classified asactivation-timeout, but0 of those carried an install error. The post-install activation check was the
problem, not the install:
installExtensionresolves on install completion, notwhen the extension's
activate()has run, so a fixed wait plus an immediategetExtensionState()re-check frequently judged a healthy install as a timeout.What changed
1. NOTIFY-level dimension (
notificationManager.ts)Add a
sourcefield (SOURCE_CVE/SOURCE_JAVA_UPGRADE) to theupgradeNotification.showevent, reusing the same constants the click handleralready passes to the upgrade command so NOTIFY and downstream funnel stages align
on one dimension.
2. Observable install completion (
utility.ts)Emit a dedicated
upgradeFlow.installSucceededevent immediately afterinstallExtensionresolves, so install completion is measurable independent of thelater result/reload events.
3. Remove the unreliable activation check (
utility.ts)On the
not-installedpath, drop thegetExtensionState()re-check that producedthe false
activation-timeout. After install and the enable prompt, the flow nowreports
result: "proceeded"and returnstrue.Behavior change to review
The
not-installedpath previously returnedcanProceed(oftenfalsebecause thefreshly installed extension had not activated yet). It now returns
trueunconditionally after install. This matches the telemetry finding that the install
itself succeeds and the old re-check was a false negative. Reviewers should confirm
this return value is acceptable for the caller (
upgradeManager.ts).Testing
No existing tests cover this code. Verified with
tsc -p . --noEmitandtslint(both clean).
Fixes devdiv-azure-service-dmitryr/azure-java-migration-copilot-vscode-extension#5979