Skip to content

Commit e37a89d

Browse files
edvilmeedvilme
authored
Merge branch 'main' into vscode-python-environments-package-refactor
2 parents b7408d2 + bac9000 commit e37a89d

6 files changed

Lines changed: 182 additions & 62 deletions

File tree

‎package-lock.json‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"name": "vscode-python-envs",
33
"displayName": "Python Environments",
44
"description": "Provides a unified python environment experience",
5-
"version": "1.33.0",
5+
"version": "1.35.0",
66
"publisher": "ms-python",
77
"preview": true,
88
"engines": {

‎src/common/telemetry/constants.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,9 @@ export interface IEventNamePropertyMapping {
593593
"breakdownGlobalVirtualEnvs": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true, "owner": "eleanorjboyd" },
594594
"breakdownWorkspaces": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "isMeasurement": true, "owner": "eleanorjboyd" },
595595
"locatorsJson": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" },
596+
"petVersion": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" },
597+
"petBuildId": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" },
598+
"petCommitSha": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" },
596599
"<duration>": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "eleanorjboyd" }
597600
}
598601
*/
@@ -615,6 +618,12 @@ export interface IEventNamePropertyMapping {
615618
breakdownWorkspaces?: number;
616619
/** JSON-serialized Record<locatorName, ms>. Parse with parse_json() in Kusto. */
617620
locatorsJson?: string;
621+
/** PET crate version reported by the `info` RPC. 'unknown' if the call failed or the PET binary doesn't implement it. */
622+
petVersion?: string;
623+
/** PET build identifier (CI build run ID) reported by the `info` RPC. 'unknown' if unavailable. */
624+
petBuildId?: string;
625+
/** PET source git commit SHA reported by the `info` RPC. 'unknown' if unavailable. */
626+
petCommitSha?: string;
618627
};
619628

620629
/* __GDPR__
@@ -639,6 +648,9 @@ export interface IEventNamePropertyMapping {
639648
"result": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" },
640649
"errorType": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" },
641650
"triggerReason": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" },
651+
"petVersion": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" },
652+
"petBuildId": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" },
653+
"petCommitSha": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" },
642654
"<duration>": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "eleanorjboyd" }
643655
}
644656
*/
@@ -654,18 +666,33 @@ export interface IEventNamePropertyMapping {
654666
* start_failed | unknown.
655667
*/
656668
triggerReason: string;
669+
/** PET crate version reported by the `info` RPC. 'unknown' if the call failed or the PET binary doesn't implement it. */
670+
petVersion?: string;
671+
/** PET build identifier (CI build run ID) reported by the `info` RPC. 'unknown' if unavailable. */
672+
petBuildId?: string;
673+
/** PET source git commit SHA reported by the `info` RPC. 'unknown' if unavailable. */
674+
petCommitSha?: string;
657675
};
658676

659677
/* __GDPR__
660678
"pet.resolve": {
661679
"result": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" },
662680
"errorType": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" },
681+
"petVersion": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" },
682+
"petBuildId": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" },
683+
"petCommitSha": { "classification": "SystemMetaData", "purpose": "PerformanceAndHealth", "owner": "eleanorjboyd" },
663684
"<duration>": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "eleanorjboyd" }
664685
}
665686
*/
666687
[EventNames.PET_RESOLVE]: {
667688
result: 'success' | 'timeout' | 'error';
668689
errorType?: string;
690+
/** PET crate version reported by the `info` RPC. 'unknown' if the call failed or the PET binary doesn't implement it. */
691+
petVersion?: string;
692+
/** PET build identifier (CI build run ID) reported by the `info` RPC. 'unknown' if unavailable. */
693+
petBuildId?: string;
694+
/** PET source git commit SHA reported by the `info` RPC. 'unknown' if unavailable. */
695+
petCommitSha?: string;
669696
};
670697

671698
/* __GDPR__

‎src/features/envManagers.ts‎

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -363,9 +363,20 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
363363
const oldEnv = this._activeSelection.get(key);
364364
if (oldEnv?.envId.id !== environment?.envId.id) {
365365
this._activeSelection.set(key, environment);
366-
setImmediate(() =>
367-
this._onDidChangeActiveEnvironment.fire({ uri: project?.uri, new: environment, old: oldEnv }),
368-
);
366+
await new Promise<void>((resolve, reject) => {
367+
setImmediate(() => {
368+
try {
369+
this._onDidChangeActiveEnvironment.fire({
370+
uri: project?.uri,
371+
new: environment,
372+
old: oldEnv,
373+
});
374+
resolve();
375+
} catch (err) {
376+
reject(err);
377+
}
378+
});
379+
});
369380
}
370381
}
371382

@@ -443,7 +454,18 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
443454
if (shouldPersistSettings) {
444455
await setAllManagerSettings(settings);
445456
}
446-
setImmediate(() => events.forEach((e) => this._onDidChangeActiveEnvironment.fire(e)));
457+
if (events.length > 0) {
458+
await new Promise<void>((resolve, reject) => {
459+
setImmediate(() => {
460+
try {
461+
events.forEach((e) => this._onDidChangeActiveEnvironment.fire(e));
462+
resolve();
463+
} catch (err) {
464+
reject(err);
465+
}
466+
});
467+
});
468+
}
447469
} else {
448470
const promises: Promise<void>[] = [];
449471
const events: DidChangeEnvironmentEventArgs[] = [];
@@ -488,7 +510,18 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
488510
}
489511
}
490512
await Promise.all(promises);
491-
setImmediate(() => events.forEach((e) => this._onDidChangeActiveEnvironment.fire(e)));
513+
if (events.length > 0) {
514+
await new Promise<void>((resolve, reject) => {
515+
setImmediate(() => {
516+
try {
517+
events.forEach((e) => this._onDidChangeActiveEnvironment.fire(e));
518+
resolve();
519+
} catch (err) {
520+
reject(err);
521+
}
522+
});
523+
});
524+
}
492525
}
493526
}
494527

‎src/managers/common/nativePythonFinder.ts‎

Lines changed: 79 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const CONFIGURE_TIMEOUT_MS = 30_000; // 30 seconds for configuration
2424
const MAX_CONFIGURE_TIMEOUT_MS = 60_000; // Max configure timeout after retries (60s)
2525
const REFRESH_TIMEOUT_MS = 30_000; // 30 seconds for full refresh (with 1 retry = 60s max)
2626
const RESOLVE_TIMEOUT_MS = 30_000; // 30 seconds for single resolve
27+
const INFO_TIMEOUT_MS = 2_000; // `info` is a const lookup on PET; 2s is generous
2728

2829
// CLI fallback timeout: generous budget since it's a full process spawn doing a full scan
2930
const CLI_FALLBACK_TIMEOUT_MS = 120_000; // 2 minutes
@@ -265,6 +266,17 @@ interface PetTelemetryNotification {
265266
};
266267
}
267268

269+
/**
270+
* Response shape of the PET `info` JSON-RPC request.
271+
* `buildId` / `commitSha` are populated only when the PET binary was built by CI
272+
* with the appropriate env vars set; local dev builds omit them.
273+
*/
274+
interface NativePetInfo {
275+
petVersion: string;
276+
buildId?: string;
277+
commitSha?: string;
278+
}
279+
268280
/**
269281
* Error thrown when a JSON-RPC request times out.
270282
*/
@@ -322,6 +334,13 @@ class NativePythonFinderImpl implements NativePythonFinder {
322334
private isRestarting: boolean = false;
323335
private processExitReason: string | undefined = undefined;
324336
private readonly configureRetry = new ConfigureRetryState();
337+
/**
338+
* Cached PET `info` response for the current connection. Reset to undefined on every
339+
* `start()` and re-populated asynchronously by `kickoffInfoFetch()`. Telemetry callers
340+
* read this via `getPetInfoProperties()`; if the fetch hasn't finished yet (or the PET
341+
* binary is too old to implement `info`), telemetry reports 'unknown'.
342+
*/
343+
private petInfo: NativePetInfo | undefined;
325344

326345
constructor(
327346
private readonly outputChannel: LogOutputChannel,
@@ -353,7 +372,10 @@ class NativePythonFinderImpl implements NativePythonFinder {
353372
this.outputChannel.info(`Resolved Python Environment ${environment.executable}`);
354373
// Reset restart attempts on successful request
355374
this.restartAttempts = 0;
356-
sendTelemetryEvent(EventNames.PET_RESOLVE, sw.elapsedTime, { result: 'success' });
375+
sendTelemetryEvent(EventNames.PET_RESOLVE, sw.elapsedTime, {
376+
result: 'success',
377+
...this.getPetInfoProperties(),
378+
});
357379
return environment;
358380
} catch (ex) {
359381
// On resolve timeout or connection error (not configure — configure handles its own timeout),
@@ -376,6 +398,7 @@ class NativePythonFinderImpl implements NativePythonFinder {
376398
{
377399
result: errorType === 'spawn_timeout' ? 'timeout' : 'error',
378400
errorType,
401+
...this.getPetInfoProperties(),
379402
},
380403
ex instanceof Error ? ex : undefined,
381404
);
@@ -460,6 +483,7 @@ class NativePythonFinderImpl implements NativePythonFinder {
460483
attempt,
461484
result: 'success',
462485
triggerReason,
486+
...this.getPetInfoProperties(),
463487
});
464488

465489
// Reset restart attempts on successful start (process didn't immediately fail)
@@ -468,7 +492,13 @@ class NativePythonFinderImpl implements NativePythonFinder {
468492
sendTelemetryEvent(
469493
EventNames.PET_PROCESS_RESTART,
470494
sw.elapsedTime,
471-
{ attempt, result: 'error', errorType: classifyError(ex), triggerReason },
495+
{
496+
attempt,
497+
result: 'error',
498+
errorType: classifyError(ex),
499+
triggerReason,
500+
...this.getPetInfoProperties(),
501+
},
472502
ex instanceof Error ? ex : undefined,
473503
);
474504
this.outputChannel.error('[pet] Failed to restart Python Environment Tools:', ex);
@@ -701,9 +731,54 @@ class NativePythonFinderImpl implements NativePythonFinder {
701731
);
702732

703733
connection.listen();
734+
735+
// Stamp PET telemetry with version/buildId/commitSha. Fire-and-forget — must not block refresh.
736+
this.petInfo = undefined;
737+
this.kickoffInfoFetch(connection);
738+
704739
return connection;
705740
}
706741

742+
/**
743+
* Asks the PET server for its build metadata (version + optional buildId + optional commitSha)
744+
* and caches it in `this.petInfo` for downstream telemetry. Runs once per `start()` call.
745+
*
746+
* Fire-and-forget by design — the response is not awaited so refresh/resolve callers are
747+
* never blocked. The 2 s timeout caps the worst case if PET is misbehaving. If a newer
748+
* connection has replaced `this.connection` by the time the response arrives, the response
749+
* is dropped to avoid clobbering the cache for the newer process.
750+
*/
751+
private kickoffInfoFetch(connection: rpc.MessageConnection): void {
752+
sendRequestWithTimeout<NativePetInfo>(connection, 'info', {}, INFO_TIMEOUT_MS)
753+
.then((result) => {
754+
if (connection !== this.connection) {
755+
return;
756+
}
757+
this.petInfo = result;
758+
this.outputChannel.debug('[pet] info:', result);
759+
})
760+
.catch((ex) => {
761+
if (connection !== this.connection) {
762+
return;
763+
}
764+
// Older PET binaries don't implement `info`; leave petInfo undefined so telemetry reports 'unknown'.
765+
this.outputChannel.debug('[pet] info request failed:', ex);
766+
});
767+
}
768+
769+
/**
770+
* Builds the petVersion/petBuildId/petCommitSha properties for PET telemetry events.
771+
* Always returns concrete strings (defaulting to 'unknown') so Kusto can group by them
772+
* without dealing with nulls.
773+
*/
774+
private getPetInfoProperties(): { petVersion: string; petBuildId: string; petCommitSha: string } {
775+
return {
776+
petVersion: this.petInfo?.petVersion ?? 'unknown',
777+
petBuildId: this.petInfo?.buildId ?? 'unknown',
778+
petCommitSha: this.petInfo?.commitSha ?? 'unknown',
779+
};
780+
}
781+
707782
private async doRefresh(options?: NativePythonEnvironmentKind | Uri[]): Promise<NativeInfo[]> {
708783
let lastError: unknown;
709784

@@ -840,6 +915,7 @@ class NativePythonFinderImpl implements NativePythonFinder {
840915
searchPathCount,
841916
attempt,
842917
locatorsJson: refreshPerf ? JSON.stringify(refreshPerf.locators) : undefined,
918+
...this.getPetInfoProperties(),
843919
},
844920
);
845921
} catch (ex) {
@@ -853,6 +929,7 @@ class NativePythonFinderImpl implements NativePythonFinder {
853929
unresolvedCount,
854930
attempt,
855931
errorType,
932+
...this.getPetInfoProperties(),
856933
},
857934
ex instanceof Error ? ex : undefined,
858935
);

0 commit comments

Comments
 (0)