From 5c08a7d3d434aadd81888222e31053a9678def5a Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Tue, 14 Apr 2026 14:11:05 -0700 Subject: [PATCH] feat: add global environment cache telemetry and improve cache management --- src/common/telemetry/constants.ts | 21 ++++ src/managers/builtin/cache.ts | 13 ++- src/managers/builtin/sysPythonManager.ts | 4 +- src/managers/common/fastPath.ts | 44 +++++++- .../managers/common/fastPath.unit.test.ts | 103 +++++++++++++++++- src/test/managers/fastPath.get.unit.test.ts | 1 + 6 files changed, 175 insertions(+), 11 deletions(-) diff --git a/src/common/telemetry/constants.ts b/src/common/telemetry/constants.ts index 29d7aa8a..706903af 100644 --- a/src/common/telemetry/constants.ts +++ b/src/common/telemetry/constants.ts @@ -113,6 +113,15 @@ export enum EventNames { * - errorType: string (classified error category, on failure only) */ MANAGER_LAZY_INIT = 'MANAGER.LAZY_INIT', + /** + * Telemetry event fired when a manager's fast path attempts to resolve + * a cached global environment (cross-session cache). + * Properties: + * - managerLabel: string (the manager's label, e.g. 'system') + * - result: 'hit' | 'miss' | 'stale' ('hit' = cached path resolved successfully, + * 'miss' = no cached path, 'stale' = cached path found but resolve failed) + */ + GLOBAL_ENV_CACHE = 'GLOBAL_ENV.CACHE', } // Map all events to their properties @@ -403,4 +412,16 @@ export interface IEventNamePropertyMapping { toolSource: string; errorType?: string; }; + + /* __GDPR__ + "global_env.cache": { + "managerLabel": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, + "result": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "eleanorjboyd" }, + "": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "eleanorjboyd" } + } + */ + [EventNames.GLOBAL_ENV_CACHE]: { + managerLabel: string; + result: 'hit' | 'miss' | 'stale'; + }; } diff --git a/src/managers/builtin/cache.ts b/src/managers/builtin/cache.ts index 08f88d0e..f6bf5ba0 100644 --- a/src/managers/builtin/cache.ts +++ b/src/managers/builtin/cache.ts @@ -1,13 +1,14 @@ import { ENVS_EXTENSION_ID } from '../../common/constants'; -import { getWorkspacePersistentState } from '../../common/persistentState'; +import { getGlobalPersistentState, getWorkspacePersistentState } from '../../common/persistentState'; export const SYSTEM_WORKSPACE_KEY = `${ENVS_EXTENSION_ID}:system:WORKSPACE_SELECTED`; export const SYSTEM_GLOBAL_KEY = `${ENVS_EXTENSION_ID}:system:GLOBAL_SELECTED`; export async function clearSystemEnvCache(): Promise { - const keys = [SYSTEM_WORKSPACE_KEY, SYSTEM_GLOBAL_KEY]; - const state = await getWorkspacePersistentState(); - await state.clear(keys); + const workspaceState = await getWorkspacePersistentState(); + await workspaceState.clear([SYSTEM_WORKSPACE_KEY]); + const globalState = await getGlobalPersistentState(); + await globalState.clear([SYSTEM_GLOBAL_KEY]); } export async function getSystemEnvForWorkspace(fsPath: string): Promise { @@ -48,11 +49,11 @@ export async function setSystemEnvForWorkspaces(fsPath: string[], envPath: strin } export async function getSystemEnvForGlobal(): Promise { - const state = await getWorkspacePersistentState(); + const state = await getGlobalPersistentState(); return await state.get(SYSTEM_GLOBAL_KEY); } export async function setSystemEnvForGlobal(envPath: string | undefined): Promise { - const state = await getWorkspacePersistentState(); + const state = await getGlobalPersistentState(); await state.set(SYSTEM_GLOBAL_KEY, envPath); } diff --git a/src/managers/builtin/sysPythonManager.ts b/src/managers/builtin/sysPythonManager.ts index 4a2a21f7..2b031ba3 100644 --- a/src/managers/builtin/sysPythonManager.ts +++ b/src/managers/builtin/sysPythonManager.ts @@ -115,7 +115,8 @@ export class SysPythonManager implements EnvironmentManager { const discard = this.collection.map((c) => c); // hit here is fine... - this.collection = (await refreshPythons(hardRefresh, this.nativeFinder, this.api, this.log, this)) ?? []; + this.collection = + (await refreshPythons(hardRefresh, this.nativeFinder, this.api, this.log, this)) ?? []; await this.loadEnvMap(); const args = [ @@ -155,6 +156,7 @@ export class SysPythonManager implements EnvironmentManager { label: 'system', getProjectFsPath: (s) => getProjectFsPathForScope(this.api, s), getPersistedPath: (fsPath) => getSystemEnvForWorkspace(fsPath), + getGlobalPersistedPath: () => getSystemEnvForGlobal(), resolve: (p) => resolveSystemPythonEnvironmentPath(p, this.nativeFinder, this.api, this), startBackgroundInit: () => this.internalRefresh(false, SysManagerStrings.sysManagerDiscovering), }); diff --git a/src/managers/common/fastPath.ts b/src/managers/common/fastPath.ts index a2a11a32..10d82eb1 100644 --- a/src/managers/common/fastPath.ts +++ b/src/managers/common/fastPath.ts @@ -4,6 +4,9 @@ import { Uri } from 'vscode'; import { GetEnvironmentScope, PythonEnvironment, PythonEnvironmentApi } from '../../api'; import { traceError, traceVerbose, traceWarn } from '../../common/logging'; +import { StopWatch } from '../../common/stopWatch'; +import { EventNames } from '../../common/telemetry/constants'; +import { sendTelemetryEvent } from '../../common/telemetry/sender'; import { createDeferred, Deferred } from '../../common/utils/deferred'; /** @@ -26,6 +29,8 @@ export interface FastPathOptions { resolve: (persistedPath: string) => Promise; /** Starts background initialization (full discovery). Returns a promise that completes when init is done. */ startBackgroundInit: () => Promise | Thenable; + /** Optional: reads the persisted env path for global scope (when scope is undefined). */ + getGlobalPersistedPath?: () => Promise; } /** @@ -52,7 +57,10 @@ export function getProjectFsPathForScope(api: Pick { - if (!(opts.scope instanceof Uri)) { + const isGlobalScope = !(opts.scope instanceof Uri); + + // Global scope is only supported when the caller provides getGlobalPersistedPath + if (isGlobalScope && !opts.getGlobalPersistedPath) { return undefined; } @@ -82,22 +90,52 @@ export async function tryFastPathGet(opts: FastPathOptions): Promise): FastPathTestOptions { } suite('tryFastPathGet', () => { + let sendTelemetryStub: sinon.SinonStub; + + setup(() => { + sendTelemetryStub = sinon.stub(telemetrySender, 'sendTelemetryEvent'); + }); + + teardown(() => { + sinon.restore(); + }); + test('returns resolved env when persisted path exists and init not started', async () => { const { opts } = createOpts(); const result = await tryFastPathGet(opts); assert.ok(result, 'Should return a result'); assert.strictEqual(result!.env.envId.id, 'test-env'); + assert.ok(sendTelemetryStub.notCalled, 'Should not emit global cache telemetry for workspace scope'); }); - test('returns undefined when scope is undefined', async () => { + test('returns undefined when scope is undefined and no getGlobalPersistedPath', async () => { const { opts } = createOpts({ scope: undefined }); const result = await tryFastPathGet(opts); @@ -63,6 +76,94 @@ suite('tryFastPathGet', () => { assert.ok((opts.getPersistedPath as sinon.SinonStub).notCalled); }); + test('returns resolved env for global scope when getGlobalPersistedPath returns a path', async () => { + const globalPath = path.resolve('usr', 'bin', 'python3'); + const resolve = sinon.stub().resolves(createMockEnv(globalPath)); + const { opts } = createOpts({ + scope: undefined, + getGlobalPersistedPath: sinon.stub().resolves(globalPath), + resolve, + }); + const result = await tryFastPathGet(opts); + + assert.ok(result, 'Should return a result for global scope'); + assert.strictEqual(result!.env.envId.id, 'test-env'); + assert.ok(resolve.calledOnceWith(globalPath), 'Should resolve the global persisted path'); + assert.ok((opts.getPersistedPath as sinon.SinonStub).notCalled, 'Should not call workspace getPersistedPath'); + + // Verify cache hit telemetry + assert.ok(sendTelemetryStub.calledOnce, 'Should send telemetry for global cache hit'); + const [eventName, , props] = sendTelemetryStub.firstCall.args; + assert.strictEqual(eventName, EventNames.GLOBAL_ENV_CACHE); + assert.strictEqual(props.result, 'hit'); + assert.strictEqual(props.managerLabel, 'test'); + }); + + test('returns undefined for global scope when getGlobalPersistedPath returns undefined', async () => { + const { opts } = createOpts({ + scope: undefined, + getGlobalPersistedPath: sinon.stub().resolves(undefined), + }); + const result = await tryFastPathGet(opts); + + assert.strictEqual(result, undefined); + + // Verify cache miss telemetry + assert.ok(sendTelemetryStub.calledOnce, 'Should send telemetry for global cache miss'); + const [eventName, , props] = sendTelemetryStub.firstCall.args; + assert.strictEqual(eventName, EventNames.GLOBAL_ENV_CACHE); + assert.strictEqual(props.result, 'miss'); + }); + + test('reports stale when global cached path resolves to undefined', async () => { + const globalPath = path.resolve('usr', 'bin', 'python3'); + const { opts } = createOpts({ + scope: undefined, + getGlobalPersistedPath: sinon.stub().resolves(globalPath), + resolve: sinon.stub().resolves(undefined), + }); + const result = await tryFastPathGet(opts); + + assert.strictEqual(result, undefined, 'Should fall through when cached env resolves to undefined'); + assert.ok(sendTelemetryStub.calledOnce, 'Should send telemetry for stale cache'); + const [eventName, , props] = sendTelemetryStub.firstCall.args; + assert.strictEqual(eventName, EventNames.GLOBAL_ENV_CACHE); + assert.strictEqual(props.result, 'stale'); + }); + + test('returns undefined for global scope when cached path resolve fails', async () => { + const globalPath = path.resolve('usr', 'bin', 'python3'); + const { opts } = createOpts({ + scope: undefined, + getGlobalPersistedPath: sinon.stub().resolves(globalPath), + resolve: sinon.stub().rejects(new Error('python was uninstalled')), + }); + const result = await tryFastPathGet(opts); + + assert.strictEqual(result, undefined, 'Should fall through when cached global env is stale'); + + // Verify cache stale telemetry + assert.ok(sendTelemetryStub.calledOnce, 'Should send telemetry for stale global cache'); + const [eventName, , props] = sendTelemetryStub.firstCall.args; + assert.strictEqual(eventName, EventNames.GLOBAL_ENV_CACHE); + assert.strictEqual(props.result, 'stale'); + }); + + test('global scope fast path starts background init when initialized is undefined', async () => { + const globalPath = path.resolve('usr', 'bin', 'python3'); + const startBackgroundInit = sinon.stub().resolves(); + const { opts, setInitialized } = createOpts({ + scope: undefined, + getGlobalPersistedPath: sinon.stub().resolves(globalPath), + startBackgroundInit, + }); + const result = await tryFastPathGet(opts); + + assert.ok(result, 'Should return fast-path result'); + assert.ok(startBackgroundInit.calledOnce, 'Should start background init for global scope'); + assert.ok(setInitialized.calledOnce, 'Should set initialized for global scope'); + }); + test('returns undefined when init is already completed', async () => { const deferred = createDeferred(); deferred.resolve(); diff --git a/src/test/managers/fastPath.get.unit.test.ts b/src/test/managers/fastPath.get.unit.test.ts index 3694449f..aed05e05 100644 --- a/src/test/managers/fastPath.get.unit.test.ts +++ b/src/test/managers/fastPath.get.unit.test.ts @@ -139,6 +139,7 @@ function createManagerCases(): ManagerCase[] { createContext: (sandbox: sinon.SinonSandbox) => { const getPersistedStub = sandbox.stub(sysCache, 'getSystemEnvForWorkspace'); const resolveStub = sandbox.stub(sysUtils, 'resolveSystemPythonEnvironmentPath'); + sandbox.stub(sysCache, 'getSystemEnvForGlobal').resolves(undefined); sandbox.stub(sysUtils, 'refreshPythons').resolves([]); const manager = new SysPythonManager(createMockNativeFinder(), createMockApi(testUri), createMockLog()); return { manager, getPersistedStub, resolveStub };