[10/?] feat(python-setup): setup attempt/result telemetry by rugpanov · Pull Request #2074 · databricks/databricks-vscode

rugpanov · 2026-08-04T08:31:38Z

Why

The VPEX Python-environment setup flow is uninstrumented: we cannot see how often setup runs, where it breaks, or how long it takes.

What

Two events emitted from the setup orchestrator, following the existing PackageManagerTelemetry / packageManagerExtensions split — module augmentation for the emit half, injected deps for collection:

python_env.setup.attempt — package manager, target type, serverless version, mode, is-greenfield. Recorded once the compute target resolves and a CLI run is about to start.
python_env.setup.result — outcome (ok / failed / cancelled / not_started), failure phase, error code, env key, disk-mutated, duration.

recordPythonSetupAttempt returns the result reporter, so the 1:1 attempt/result pairing is structural rather than a convention.

Together with the existing python_env.setup.detected (explicit_command) this gives a three-stage funnel: click → attempt → result.

Two requested fields are not taken from the CLI result

Because it has no producer for either:

Duration is measured in-extension. The CLI documents durationMs as reserved and always emits 0 (libs/localenv/result.go). Measuring here also captures the latency the user actually experiences, including process spawn and interpreter adoption.
The merge-conflict warning count is omitted. Nothing in the CLI ever appends to Result.Warnings (NewResult() seeds it to []; only the text renderer reads it), and merge conflicts are not a modelled concept there. The field would be a permanent 0 — a dashboard on it would read "merge quality is perfect" rather than "unmeasured". It can be added once the CLI has a producer.

Notes

adopt is added as a synthetic seventh failure phase for the extension-side interpreter-adoption step, which happens after the CLI exits and so cannot appear in its own phases[].
Categorical/enum data only — no cluster IDs/names, paths, or package names. Absent optionals are omitted rather than stringified to "undefined".
isGreenfield is only emitted for uv/unknown projects: for pip/conda users a missing pyproject.toml says nothing about greenfield-ness, so reporting it would inflate the rate.
Emission is fully best-effort — a detection, emit, or report failure can never break the setup run it measures.
mode is constant default until the --constraints-only picker ships; the field is in the schema now so no migration is needed then.
telemetry.json is generated from EventTypes (scripts/generateTelemetry.ts), so it needs no manual edit.

Schema and privacy docs: src/telemetry/PYTHON_SETUP_TELEMETRY.md.

Verification

yarn build — clean; yarn test:lint — 0 errors, prettier clean.
Unit tests: 461 passing, +14 new — attempt/result for each outcome path, no-attempt for each early return, the pairing invariant, greenfield conditionality, no-cluster-id, and telemetry-failure resilience.
The 6 failures in cli/CliWrapper.test.ts are pre-existing on the base commit (7 there, one flaky) and untouched by this change.
Regenerated telemetry.json and confirmed both events serialize.

Backward compatibility

Additive only: two new events, and new required fields on the internal PythonSetupSetupDeps / PythonSetupWiringDeps seams (both internal to the extension). No change to existing event schemas, settings, persisted state, or the CLI contract. Behaviour for users who have not opted into the feature flag is unchanged.

This pull request and its description were written by Isaac.

*Why* The VPEX setup flow is uninstrumented: we cannot see how often setup runs, where it breaks, or how long it takes -- the numbers the ERD's Metrics section asks for. DECO-27787. *What* Adds two events emitted from the setup orchestrator, following the existing PackageManagerTelemetry/packageManagerExtensions split (module augmentation for the emit half, injected deps for collection): - python_env.setup.attempt: package manager, target type, serverless version, mode, is-greenfield. Recorded once the compute target resolves and a CLI run is about to start. - python_env.setup.result: outcome (ok/failed/cancelled/not_started), failure phase, error code, env key, disk-mutated, duration. recordPythonSetupAttempt returns the result reporter, so the 1:1 attempt/result pairing is structural rather than a convention. Two ERD fields are not taken from the CLI result, because it has no producer for either: - duration is measured in-extension (the CLI documents durationMs as reserved and always 0), which also captures the latency the user actually experiences, including spawn and interpreter adoption; - the merge-conflict warning count is omitted -- nothing in the CLI ever appends to Result.Warnings, so the field would be a permanent 0 and would read as "merge quality is perfect" rather than "unmeasured". `adopt` is added as a synthetic seventh failure phase for the extension-side interpreter adoption step, which happens after the CLI exits and so cannot appear in its own phases[]. Categorical/enum data only -- no cluster IDs/names, paths, or package names; absent optionals are omitted rather than stringified to "undefined". Emission is fully best-effort: a detection, emit, or report failure can never break the setup run it measures. Schema docs in src/telemetry/PYTHON_SETUP_TELEMETRY.md. telemetry.json is generated from EventTypes (scripts/generateTelemetry.ts), so it needs no manual edit. *Verification* - yarn build: clean - yarn test:lint: 0 errors, prettier clean - unit tests: 461 passing, +14 new (attempt/result per outcome path, no-attempt for each early return, pairing invariant, greenfield conditionality, no-cluster-id, telemetry-failure resilience). The 6 failures in cli/CliWrapper.test.ts are pre-existing on the base commit (7 there, one flaky) and untouched by this change. - regenerated telemetry.json and confirmed both events serialize. Co-authored-by: Isaac

rugpanov · 2026-08-04T08:36:29Z

🤖 Integration tests triggered for 8bf51268 — ⏳ running.
View run

*Why* Codex review of #2074 found three ways the events could misreport. All are latent on current paths, but each is cheap to make structurally impossible rather than relying on a convention holding. *What* - Never record `ok` for a run that then rejects. The success report moved after saveState/readiness, with a new `persist` failure phase for a throw there. It stays *before* showSuccess deliberately: that awaits the user dismissing a toast, so reporting after it would fold think-time into `duration` and wreck the setup-time metric. - Constrain `envKey` to the CLI's two documented shapes (serverless/serverless-v<N>, dbr/<sparkVersion>) before emission; anything else collapses to "other". The key comes from JSON the parser validates only minimally, and the DBR arm is a raw "dbr/" + sparkVersion concatenation -- so drift could otherwise put unbounded, potentially identifying, high-cardinality content into a field documented as categorical. - Make the result reporter once-only, so the documented 1:1 attempt/result pairing survives a future refactor that adds a terminal path. *Verification* - yarn build clean; test:lint 0 errors, prettier clean - unit tests: 466 passing, +5 (persist-phase reporting, report-before-toast ordering, once-only reporter, env-key passthrough for valid shapes, env-key collapse for 5 invalid/identifying ones). Same 6 pre-existing cli/CliWrapper.test.ts failures as the base commit. - PR CI on the parent commit: Linux + Windows unit tests and VSIX packaging all passed. Co-authored-by: Isaac

rugpanov · 2026-08-04T08:42:13Z

🤖 Integration tests ✅ all 35 test jobs passed for bef83830.
View run

misha-db · 2026-08-04T09:03:53Z

@@ -0,0 +1,178 @@
+# Telemetry: Python environment setup attempt / result


Why do we need this markdown files under src folder?

It aims to help understanding what exact telemetry are we collecting. Should it be README.md instead?

Ideally we should have separate docs folder (in project/package root?) for things like that. mixing source code and documentation makes navigation less straightforward

*Why* A second review pass (Claude subagent) found two P1s in the previous commits. Both were demonstrated empirically, not just argued. *What* 1. The emit half no longer spreads the caller's object. Spreading a *variable* disables TypeScript's excess-property check, so any field later added to PythonSetupAttempt / PythonSetupOutcomeReport -- or any wider object passed through the seam -- was emitted automatically, with objects JSON-stringified by recordEvent. The reviewer proved it: adding `leakedCompute?: {clusterId: string}` compiled with exit 0 and put the cluster ID on the wire. There was no live leak (the orchestrator builds an explicit literal), but nothing made the privacy claim enforceable. Both payloads now name every field, so the schema is a compiler-checked allowlist. 2. The documented three-stage funnel did not exist. The design justified recording nothing for early-abort clicks on the grounds that python_env.setup.detected covers them -- but its `explicit_command` trigger fires only from the *legacy* databricks.environment.setup command, while this entry dispatches setupPythonEnv, and the config view renders the two mutually exclusively. So the VPEX cohort never emitted that event, and the no-compute dead end (a real user-facing one, reached via NO_COMPUTE_TARGET_MESSAGE) was invisible. Adds an `outcome: "no_compute"` result, emitted without an attempt, and corrects the docs to stop claiming coverage that does not hold. `duration` is now optional and omitted there: nothing ran, and a 0 would drag the setup-time percentiles down. Also from that pass: - Tighten the envKey DBR arm to the spark-version grammar with a length bound. The previous pattern admitted cluster *names*, which are user-chosen and often contain a person's name (`dbr/janes-dev-cluster` passed). Four such cases added to the tests. - Split the two attempt probes into independent try blocks, so a failing pyproject.toml probe no longer discards a successfully detected package manager (which biased the distribution toward `unknown`). *Verification* - yarn build clean; test:lint 0 errors, prettier clean - unit tests: 471 passing, +5. New: schema-allowlist (extra fields with a cluster id / path / email are not emitted, exact key sets asserted), no_compute emitted alone with no duration, no_compute resilience, manager preserved when only the greenfield probe fails, cluster-name env keys collapse to "other". - One existing test's assertion updated: with independent try blocks a detection failure now still runs the greenfield probe, so isGreenfield is false rather than undefined. New behaviour is the intended one. - Same 6 pre-existing cli/CliWrapper.test.ts failures as the base commit. - Isaac Review: clean pass, 0 findings (its earlier run aborted on context thrashing and was rerun). Co-authored-by: Isaac

rugpanov · 2026-08-04T09:25:21Z

🤖 Integration tests ✅ all 35 test jobs passed for e4236807.
View run

*Why* Review feedback: a feature-specific PYTHON_SETUP_TELEMETRY.md dropped into the shared src/telemetry/ folder is undiscoverable. Nothing linked to it -- the same is true of the PACKAGE_MANAGER_DETECTION.md it was modelled on -- so the next person adds a third file instead of extending one. It also restated the event schema in prose tables, giving a second source of truth that goes stale silently while the generated telemetry.json stays correct. *What* - Rename to src/telemetry/README.md, the name tooling and humans already look for and what GitHub renders when browsing the folder. Links out to the existing PACKAGE_MANAGER_DETECTION.md rather than absorbing it. - Trim 203 -> 101 lines by dropping every field table. EventTypes in constants.ts owns the schema (telemetry.json is generated from it); the doc now keeps only the reasoning that has nowhere to live in a field comment: why duration is measured in-extension, why the merge-conflict count is omitted, why the emit half enumerates fields, why envKey is pattern-checked, why no_compute breaks the attempt/result pairing. - Document the convention in CODE_CONVENTIONS.md (new section 4a) so this is a repo rule and not one PR's taste, with the survey that informed it: vscode colocates ~29 architecture docs under src/ and has no top-level docs/; react and rust colocate README.md per package/crate; vscode-pull-request-github, kubernetes and eslint centralize. We colocate because docs/ is gitignored here, and standardize on README.md rather than vscode's descriptive-filename style because that only stays navigable when a folder is one subsystem -- src/telemetry/ serves every feature. - Also note in section 4a and the telemetry bullet that EventTypes is the only place a schema may be maintained. *Verification* - yarn build clean; test:lint 0 errors, prettier clean - unit tests: 471 passing, same 6 pre-existing cli/CliWrapper.test.ts failures - grepped for references before renaming; the one code comment pointing at the old filename now points at the README Co-authored-by: Isaac

rugpanov · 2026-08-04T10:13:10Z

🤖 Integration tests triggered for a87085a6 — ⏳ running.
View run

*Why* The CODE_CONVENTIONS.md change is a repo-wide rule that deserves its own review by whoever owns the conventions -- it should not ride in on a telemetry feature PR. *What* Reverts CODE_CONVENTIONS.md to its state before a87085a. The telemetry README rename/trim stays here, since it is part of this feature. The new section 4a ("Where module documentation goes") moves to a dedicated PR. *Verification* CODE_CONVENTIONS.md is byte-identical to e423680's version (git diff empty); no source files touched. Co-authored-by: Isaac

github-actions · 2026-08-04T10:16:32Z

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/vscode

Inputs:

PR number: 2074
Commit SHA: cc87c3106d8f6cb250a0d829179ba9ad5297af5c

Checks will be approved automatically on success.

rugpanov · 2026-08-04T10:18:45Z

🤖 Integration tests ❌ 1 of 35 test jobs failed for cc87c310 (34 passed).
View run

*Why* We had no rule, and the result drifts: src/telemetry/ picked up a feature-specific PACKAGE_MANAGER_DETECTION.md that nothing links to, and a second such file was about to land beside it. A folder that accumulates one markdown file per feature is a folder nobody reads. *What* Adds CODE_CONVENTIONS.md section 4a with three rules: - When a module needs a prose doc, it is README.md in the module's folder -- the name tooling and humans already look for, and what GitHub renders when browsing. Not a per-feature descriptive filename. - Document the why, not the what. Schemas, field lists and signatures belong in the code or in generated output; prose that restates them becomes a second source of truth that drifts. - No top-level docs/ -- it is gitignored in this repo, so anything committed lives beside the code or in an existing root file. Also states explicitly (here and in the section 4 telemetry bullet) that EventTypes in telemetry/constants.ts is the only place an event schema may be maintained, since telemetry.json is generated from it. The section records the survey behind the choice rather than asserting it, so the premise can be argued with: vscode colocates ~29 architecture docs under src/ and has no top-level docs/; react and rust colocate README.md per package/crate; vscode-pull-request-github, kubernetes and eslint centralize. Both approaches are well-established -- we colocate because docs/ is gitignored here, and use README.md rather than vscode's descriptive-filename style because that only stays navigable when a folder is one subsystem, which src/telemetry/ is not. Notes that vscode documents telemetry inline in TypeScript with no per-event markdown, which is what our EventTypes comments already do. Split out of #2074 on review feedback: a repo-wide rule should not ride in on a feature PR. *Verification* Docs only, no code touched. The diff is 62 insertions and 0 deletions -- no existing line is reformatted. (An earlier revision ran prettier --write over the whole file, which retyped every table's padding and re-indented every bullet for ~64 lines of unrelated churn. That was gratuitous: the file is not prettier-clean on main and is outside the lint scope, since test:lint runs `prettier . -c` from packages/databricks-vscode/ and no workflow checks the repo root.) Verified every path and claim the section cites exists on main (PACKAGE_MANAGER_DETECTION.md, python-setup/README.md, scripts/generateTelemetry.ts, docs/ in .gitignore), and that all seven link-reference definitions resolve. Co-authored-by: Isaac

## Why We have no rule for where a module's prose documentation lives, and the result drifts. `src/telemetry/` picked up a feature-specific `PACKAGE_MANAGER_DETECTION.md` that nothing links to, and a second such file was about to land beside it. A folder that accumulates one markdown file per feature is a folder nobody reads. ## What Adds `CODE_CONVENTIONS.md` section 4a with three rules: - **When a module needs a prose doc, it is `README.md` in the module's folder** — the name tooling and humans already look for, and what GitHub renders when browsing. Not a per-feature descriptive filename. - **Document the *why*, not the *what*.** Schemas, field lists and signatures belong in the code or in generated output; prose that restates them becomes a second source of truth that drifts. - **No top-level `docs/`** — it's gitignored in this repo, so anything committed lives beside the code or in an existing root file. Also states explicitly (here and in the section 4 telemetry bullet) that `EventTypes` in `telemetry/constants.ts` is the only place an event schema may be maintained, since `telemetry.json` is generated from it. ## The survey behind it The section records the evidence rather than just asserting a preference, so the premise can be argued with: | Project | Practice | |---|---| | microsoft/vscode | Colocated docs, **no top-level `docs/`** — ~29 architecture docs under `src/` (e.g. `src/vs/sessions/LAYOUT.md`) | | facebook/react | Colocated `README.md` per package | | rust-lang/rust | Colocated `README.md` per crate, pointing at the external dev guide | | vscode-pull-request-github | Centralized `documentation/` | | kubernetes, eslint | Centralized `docs/` + external enhancement proposals | Both colocation and centralization are well-established; there is no universal answer. We colocate because `docs/` is gitignored here, and standardize on `README.md` rather than VS Code's descriptive-filename style because the latter only stays navigable when a folder is one subsystem — `src/telemetry/` serves every feature. Worth noting for telemetry specifically: VS Code documents events *inline in TypeScript* (GDPR classification and comment in a type beside the emit call), with no per-event markdown. Our `EventTypes` `comment` fields are the same idea, which is why prose docs must not duplicate them. ## Notes - Split out of #2074 on review feedback — a repo-wide rule shouldn't ride in on a feature PR. That PR keeps only its own `src/telemetry/README.md`. - The existing `PACKAGE_MANAGER_DETECTION.md` is left in place; the section says to fold such files in when next touched, rather than churning a file this PR has no other reason to open. - This documents a convention, it doesn't enforce one. No lint rule. ## Verification Docs only, no code touched. I verified every path and claim the section cites exists on `main` (`PACKAGE_MANAGER_DETECTION.md`, `python-setup/README.md`, `scripts/generateTelemetry.ts`, `docs/` in `.gitignore`) and that the external references resolve. Prettier clean with the repo-pinned 3.1.1. This pull request and its description were written by Isaac.

rugpanov temporarily deployed to test-trigger-is August 4, 2026 08:31 — with GitHub Actions Inactive

rugpanov requested a deployment to test-trigger-is August 4, 2026 08:32 — with GitHub Actions Queued

rugpanov temporarily deployed to test-trigger-is August 4, 2026 08:38 — with GitHub Actions Inactive

rugpanov requested a deployment to test-trigger-is August 4, 2026 08:39 — with GitHub Actions Queued

misha-db reviewed Aug 4, 2026

View reviewed changes

rugpanov temporarily deployed to test-trigger-is August 4, 2026 09:20 — with GitHub Actions Inactive

rugpanov requested a deployment to test-trigger-is August 4, 2026 09:20 — with GitHub Actions Queued

rugpanov temporarily deployed to test-trigger-is August 4, 2026 10:09 — with GitHub Actions Inactive

rugpanov requested a deployment to test-trigger-is August 4, 2026 10:09 — with GitHub Actions Queued

rugpanov temporarily deployed to test-trigger-is August 4, 2026 10:15 — with GitHub Actions Inactive

rugpanov temporarily deployed to test-trigger-is August 4, 2026 10:16 — with GitHub Actions Inactive

rugpanov requested a deployment to test-trigger-is August 4, 2026 10:17 — with GitHub Actions Queued

rugpanov mentioned this pull request Aug 4, 2026

docs: add a convention for where module documentation goes #2075

Merged

misha-db approved these changes Aug 4, 2026

View reviewed changes

Navigation Menu

Uh oh!

[10/?] feat(python-setup): setup attempt/result telemetry - #2074

[10/?] feat(python-setup): setup attempt/result telemetry#2074
rugpanov wants to merge 5 commits into
mainfrom
rugpanov/vpex-setup-telemetry

rugpanov commented Aug 4, 2026

Uh oh!

rugpanov commented Aug 4, 2026

Uh oh!

rugpanov commented Aug 4, 2026 •

edited

Loading

Uh oh!

misha-db Aug 4, 2026

Uh oh!

rugpanov Aug 4, 2026

Uh oh!

misha-db Aug 4, 2026 •

edited

Loading

Uh oh!

rugpanov commented Aug 4, 2026 •

edited

Loading

Uh oh!

rugpanov commented Aug 4, 2026

Uh oh!

github-actions Bot commented Aug 4, 2026

Uh oh!

rugpanov commented Aug 4, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -0,0 +1,178 @@
		# Telemetry: Python environment setup attempt / result

Uh oh!

Conversation

rugpanov commented Aug 4, 2026

Why

What

Two requested fields are not taken from the CLI result

Notes

Verification

Backward compatibility

Uh oh!

rugpanov commented Aug 4, 2026

Uh oh!

rugpanov commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

misha-db Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

rugpanov Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

misha-db Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rugpanov commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rugpanov commented Aug 4, 2026

Uh oh!

github-actions Bot commented Aug 4, 2026

Uh oh!

rugpanov commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rugpanov commented Aug 4, 2026 •

edited

Loading

misha-db Aug 4, 2026 •

edited

Loading

rugpanov commented Aug 4, 2026 •

edited

Loading

rugpanov commented Aug 4, 2026 •

edited

Loading