feat: add telemetry for PET process failures and exits#1306
feat: add telemetry for PET process failures and exits#1306eleanorjboyd wants to merge 2 commits intomicrosoft:mainfrom
Conversation
src/extension.ts
Outdated
| arch: process.arch, | ||
| }); | ||
|
|
||
| window.showErrorMessage( |
There was a problem hiding this comment.
@karthiknadig if we cannot find the binary, is that a game-over scenario ? how should we present / prompt the user? I guess they could enter a interpreter path themselves so maybe its a notification instead?
There was a problem hiding this comment.
Pull request overview
Adds telemetry and diagnostics around PET (Python Environment Tools) startup failures and process exits to improve failure attribution and troubleshooting during extension activation.
Changes:
- Emit
PET_START_FAILEDtelemetry (with platform/arch/error info) when PET cannot be started during activation. - Emit
PET_PROCESS_EXITtelemetry when the PET process exits, tracking restart/dispose vs crash. - Add verbose diagnostics for PET binary path resolution and split activation error handling to isolate PET vs manager-registration failures.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/managers/common/nativePythonFinder.ts | Adds PET path resolution diagnostics, tracks disposal state, and emits process-exit telemetry. |
| src/extension.ts | Splits activation into PET startup vs manager registration and emits PET startup-failure telemetry. |
| src/common/telemetry/constants.ts | Introduces new telemetry event names and GDPR property mappings for PET events. |
| const errnoError = error as NodeJS.ErrnoException; | ||
| // Plain Error (no .code) = binary not found by getNativePythonToolsPath. | ||
| // Errno error (has .code) = spawn failed (ENOENT, EACCES, EPERM, etc.). | ||
| const reason = errnoError.code ? 'spawn_failed' : 'binary_not_found'; | ||
| sendTelemetryEvent(EventNames.PET_START_FAILED, undefined, { | ||
| errorCode: errnoError.code ?? 'UNKNOWN', | ||
| reason, | ||
| platform: process.platform, | ||
| arch: process.arch, | ||
| }); |
There was a problem hiding this comment.
The PET_START_FAILED reason classification assumes that any error without an errnoError.code must be "binary_not_found", but createNativePythonFinder() can also throw plain Errors after the binary is found/spawned (e.g., missing stdio streams, JSON-RPC setup failures). This will misattribute failures in telemetry. Consider setting reason to 'unknown' by default and only using 'binary_not_found' for the specific errors thrown by getNativePythonToolsPath() (e.g., by throwing a dedicated error type or attaching a discriminator).
| const wasExpected = this.isRestarting || this.isDisposed; | ||
| if (code !== 0) { | ||
| this.outputChannel.error( | ||
| `[pet] Python Environment Tools exited unexpectedly with code ${code}, signal ${signal}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
The exit handler logs an "unexpected" error whenever code !== 0, but code can be null (terminated by signal) and exits during restart/dispose are expected (wasExpected === true). This can generate noisy error logs on normal shutdown/restart. Consider gating the error log on !wasExpected && code !== 0 (and treating null as non-error unless !wasExpected).
Uh oh!
There was an error while loading. Please reload this page.