Skip to content

Commit 0c711be

Browse files
eleanorjboydeleanorjboyd
committed
tightening comments
1 parent ffcc15e commit 0c711be

2 files changed

Lines changed: 83 additions & 47 deletions

File tree

‎src/common/telemetry/constants.ts‎

Lines changed: 46 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,12 @@ export enum EventNames {
6060
*/
6161
MANAGER_REGISTRATION_FAILED = 'MANAGER_REGISTRATION.FAILED',
6262
/**
63-
* Telemetry event fired when the setup block appears to be hung.
64-
* A watchdog timer fires after a deadline; if the setup completes normally,
65-
* the timer is cancelled and this event never fires.
63+
* Watchdog event fired when setup appears hung. Cancelled if setup completes normally.
6664
* Properties:
67-
* - failureStage: string (which phase was in progress when the watchdog fired)
65+
* - failureStage: which phase was in progress when the watchdog fired
66+
* - globalScopeDeferred: distinguishes a real foreground hang from the benign 120s
67+
* background global-scope scan. String union (not boolean) because the sender drops
68+
* undefined. Values: 'deferred' | 'not_deferred' | 'unknown'.
6869
* Measures:
6970
* - duration: total elapsed since activation
7071
* - stageDuration: elapsed in the current stage
@@ -87,12 +88,11 @@ export enum EventNames {
8788
*/
8889
PET_INIT_DURATION = 'PET.INIT_DURATION',
8990
/**
90-
* Telemetry event fired once per activation reporting the version of the bundled
91-
* PET (Python Environment Tools) binary in use. Used to slice other PET telemetry
92-
* by binary version when investigating regressions/improvements.
91+
* Fired once per activation. Lets us slice every other PET event by which binary
92+
* version was running — important since PET evolves independently of the extension.
9393
* Properties:
94-
* - version: string (e.g. '0.1.0' from `pet --version`; 'unknown' if the lookup failed)
95-
* - source: 'envs_extension' | 'python_extension' (which extension shipped the binary)
94+
* - version: output of `pet --version` (e.g. '0.1.0'), or 'unknown' on failure/timeout
95+
* - source: 'envs_extension' | 'python_extension' — which extension shipped the binary
9696
*/
9797
PET_VERSION = 'PET.VERSION',
9898
/**
@@ -155,12 +155,16 @@ export enum EventNames {
155155
* Telemetry event for a PET refresh attempt (the core discovery RPC call).
156156
* Properties:
157157
* - result: 'success' | 'timeout' | 'error'
158-
* - envCount: number (environments returned via notifications)
159-
* - unresolvedCount: number (envs that needed follow-up resolve calls)
160-
* - workspaceDirCount: number (workspace directories sent in configure)
161-
* - searchPathCount: number (extra search paths sent in configure)
162-
* - attempt: number (0 = first try, 1 = retry)
163-
* - errorType: string (classified error category, on failure only)
158+
* - envCount, unresolvedCount, workspaceDirCount, searchPathCount: number
159+
* - attempt: 0 = first try, 1 = retry
160+
* - errorType: classified error category, on failure only
161+
* - locatorsJson: JSON-serialized Record<locatorName, ms>. Locator set is platform-dependent
162+
* so a flat blob is more practical than a fixed schema. Parse with parse_json() in Kusto.
163+
* Measures (numeric; phases run in parallel so sum may exceed total wall-clock):
164+
* - breakdownLocators: ms in locator plugins
165+
* - breakdownPathEnv: ms scanning PATH env var entries (not a file path)
166+
* - breakdownGlobalVirtualEnvs: ms scanning global virtualenv dirs
167+
* - breakdownWorkspaces: ms scanning workspace dirs
164168
*/
165169
PET_REFRESH = 'PET.REFRESH',
166170
/**
@@ -175,9 +179,15 @@ export enum EventNames {
175179
/**
176180
* Telemetry event for PET process restart attempts.
177181
* Properties:
178-
* - attempt: number (1-based restart attempt number)
182+
* - attempt: 1-based restart attempt number
179183
* - result: 'success' | 'error'
180-
* - errorType: string (classified error category, on failure only)
184+
* - errorType: classified error category, on failure only
185+
* - triggerReason: why the restart was needed (lets us separate crashes from
186+
* timeout-induced kills; the most specific reason wins — rpc_* recorded before a
187+
* kill is not overwritten by the subsequent exit event). Values:
188+
* rpc_connection_error | rpc_resolve_timeout | rpc_refresh_timeout |
189+
* rpc_configure_timeout | process_exit:<code>:<signal> | process_error |
190+
* start_failed | unknown
181191
*/
182192
PET_PROCESS_RESTART = 'PET.PROCESS_RESTART',
183193
/**
@@ -408,11 +418,10 @@ export interface IEventNamePropertyMapping {
408418
[EventNames.SETUP_HANG_DETECTED]: {
409419
failureStage: string;
410420
/**
411-
* State of the global Python scope search at the time of hang:
412-
* - 'deferred': workspace env resolved and global scope was pushed to background.
413-
* - 'not_deferred': no workspace env resolved, global scope was awaited as primary fallback.
414-
* - 'unknown': hang fired before the env-selection stage reached the global-scope decision
415-
* (i.e. before/during env selection, prior to the workspace-resolution branch).
421+
* Distinguishes a real foreground hang from the benign 120s background global-scope scan.
422+
* - 'deferred': workspace env resolved; global scope ran in background.
423+
* - 'not_deferred': no workspace env; global scope was awaited as primary fallback.
424+
* - 'unknown': hang fired before env-selection reached the global-scope decision.
416425
*/
417426
globalScopeDeferred: 'deferred' | 'not_deferred' | 'unknown';
418427
};
@@ -547,7 +556,7 @@ export interface IEventNamePropertyMapping {
547556
"attempt": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "eleanorjboyd" },
548557
"errorType": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" },
549558
"breakdownLocators": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true, "owner": "eleanorjboyd" },
550-
"breakdownPath": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true, "owner": "eleanorjboyd" },
559+
"breakdownPathEnv": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true, "owner": "eleanorjboyd" },
551560
"breakdownGlobalVirtualEnvs": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true, "owner": "eleanorjboyd" },
552561
"breakdownWorkspaces": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true, "owner": "eleanorjboyd" },
553562
"locatorsJson": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" },
@@ -562,15 +571,16 @@ export interface IEventNamePropertyMapping {
562571
searchPathCount?: number;
563572
attempt: number;
564573
errorType?: string;
565-
/** ms spent in the Locators phase (runs locator plugins). From PET RefreshPerformance.breakdown. */
574+
// breakdown* fields go through the measures payload (numeric); listed here for GDPR only.
575+
/** ms in the Locators phase. */
566576
breakdownLocators?: number;
567-
/** ms spent walking PATH entries. From PET RefreshPerformance.breakdown. */
568-
breakdownPath?: number;
569-
/** ms spent scanning global virtual-env dirs. From PET RefreshPerformance.breakdown. */
577+
/** ms walking PATH env var entries (not a file path). */
578+
breakdownPathEnv?: number;
579+
/** ms scanning global virtual-env dirs. */
570580
breakdownGlobalVirtualEnvs?: number;
571-
/** ms spent scanning workspace dirs. From PET RefreshPerformance.breakdown. */
581+
/** ms scanning workspace dirs. */
572582
breakdownWorkspaces?: number;
573-
/** JSON-serialized Record<locatorName, ms>. Query with parse_json() in Kusto. From PET RefreshPerformance.locators. */
583+
/** JSON-serialized Record<locatorName, ms>. Parse with parse_json() in Kusto. */
574584
locatorsJson?: string;
575585
};
576586

@@ -603,7 +613,13 @@ export interface IEventNamePropertyMapping {
603613
attempt: number;
604614
result: 'success' | 'error';
605615
errorType?: string;
606-
/** Why the PET process needed restarting: process_exit:<code>:<signal>, process_error, rpc_connection_error, rpc_refresh_timeout, rpc_resolve_timeout, rpc_configure_timeout, start_failed, or unknown */
616+
/**
617+
* Why the restart was needed. The most specific reason wins (an rpc_* value recorded
618+
* before the kill is not overwritten by the subsequent exit/error event).
619+
* Values: rpc_connection_error | rpc_resolve_timeout | rpc_refresh_timeout |
620+
* rpc_configure_timeout | process_exit:<code>:<signal> | process_error |
621+
* start_failed | unknown.
622+
*/
607623
triggerReason: string;
608624
};
609625

‎src/managers/common/nativePythonFinder.ts‎

Lines changed: 37 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,16 @@ export async function getNativePythonToolsVersion(toolPath: string, timeoutMs: n
134134
const timer = setTimeout(() => {
135135
try {
136136
proc.kill('SIGTERM');
137+
// Force kill after a short grace period if still running.
138+
setTimeout(() => {
139+
if (proc.exitCode === null) {
140+
try {
141+
proc.kill('SIGKILL');
142+
} catch {
143+
// ignore
144+
}
145+
}
146+
}, 500);
137147
} catch {
138148
// ignore
139149
}
@@ -805,23 +815,33 @@ class NativePythonFinderImpl implements NativePythonFinder {
805815
this.outputChannel.info(`[pet] Refresh succeeded on retry attempt ${attempt + 1}`);
806816
}
807817

808-
sendTelemetryEvent(EventNames.PET_REFRESH, sw.elapsedTime, {
809-
result: 'success',
810-
envCount: nativeInfo.filter((e) => isNativeEnvInfo(e)).length,
811-
unresolvedCount,
812-
workspaceDirCount,
813-
searchPathCount,
814-
attempt,
815-
// Per-phase breakdown from PET's RefreshPerformance notification.
816-
// Breakdown phases run in parallel so their sum may exceed total (wall-clock).
817-
breakdownLocators: refreshPerf?.breakdown['Locators'],
818-
breakdownPath: refreshPerf?.breakdown['Path'],
819-
breakdownGlobalVirtualEnvs: refreshPerf?.breakdown['GlobalVirtualEnvs'],
820-
breakdownWorkspaces: refreshPerf?.breakdown['Workspaces'],
821-
// Per-locator timing serialized as JSON; platform-dependent keys (e.g. WindowsRegistry, Conda).
822-
// Query in Kusto with: parse_json(Properties.locatorsJson)
823-
locatorsJson: refreshPerf ? JSON.stringify(refreshPerf.locators) : undefined,
824-
});
818+
sendTelemetryEvent(
819+
EventNames.PET_REFRESH,
820+
{
821+
duration: sw.elapsedTime,
822+
...(refreshPerf?.breakdown['Locators'] !== undefined && {
823+
breakdownLocators: refreshPerf.breakdown['Locators'],
824+
}),
825+
...(refreshPerf?.breakdown['Path'] !== undefined && {
826+
breakdownPathEnv: refreshPerf.breakdown['Path'],
827+
}),
828+
...(refreshPerf?.breakdown['GlobalVirtualEnvs'] !== undefined && {
829+
breakdownGlobalVirtualEnvs: refreshPerf.breakdown['GlobalVirtualEnvs'],
830+
}),
831+
...(refreshPerf?.breakdown['Workspaces'] !== undefined && {
832+
breakdownWorkspaces: refreshPerf.breakdown['Workspaces'],
833+
}),
834+
},
835+
{
836+
result: 'success',
837+
envCount: nativeInfo.filter((e) => isNativeEnvInfo(e)).length,
838+
unresolvedCount,
839+
workspaceDirCount,
840+
searchPathCount,
841+
attempt,
842+
locatorsJson: refreshPerf ? JSON.stringify(refreshPerf.locators) : undefined,
843+
},
844+
);
825845
} catch (ex) {
826846
const errorType = classifyError(ex);
827847
sendTelemetryEvent(

0 commit comments

Comments
 (0)