Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/common/telemetry/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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" },
"<duration>": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "eleanorjboyd" }
}
*/
[EventNames.GLOBAL_ENV_CACHE]: {
managerLabel: string;
result: 'hit' | 'miss' | 'stale';
};
}
13 changes: 7 additions & 6 deletions src/managers/builtin/cache.ts
Original file line number Diff line number Diff line change
@@ -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<void> {
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<string | undefined> {
Expand Down Expand Up @@ -48,11 +49,11 @@ export async function setSystemEnvForWorkspaces(fsPath: string[], envPath: strin
}

export async function getSystemEnvForGlobal(): Promise<string | undefined> {
const state = await getWorkspacePersistentState();
const state = await getGlobalPersistentState();
return await state.get(SYSTEM_GLOBAL_KEY);
}

export async function setSystemEnvForGlobal(envPath: string | undefined): Promise<void> {
const state = await getWorkspacePersistentState();
const state = await getGlobalPersistentState();
await state.set(SYSTEM_GLOBAL_KEY, envPath);
}
4 changes: 3 additions & 1 deletion src/managers/builtin/sysPythonManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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),
});
Expand Down
79 changes: 65 additions & 14 deletions src/managers/common/fastPath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -26,6 +29,8 @@ export interface FastPathOptions {
resolve: (persistedPath: string) => Promise<PythonEnvironment | undefined>;
/** Starts background initialization (full discovery). Returns a promise that completes when init is done. */
startBackgroundInit: () => Promise<void> | Thenable<void>;
/** Optional: reads the persisted env path for global scope (when scope is undefined). */
getGlobalPersistedPath?: () => Promise<string | undefined>;
}

/**
Expand All @@ -52,7 +57,10 @@ export function getProjectFsPathForScope(api: Pick<PythonEnvironmentApi, 'getPyt
* to fall through to the normal init path.
*/
export async function tryFastPathGet(opts: FastPathOptions): Promise<FastPathResult | undefined> {
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;
}

Expand Down Expand Up @@ -82,23 +90,66 @@ export async function tryFastPathGet(opts: FastPathOptions): Promise<FastPathRes
}
}

const fsPath = opts.getProjectFsPath(opts.scope);
const persistedPath = await opts.getPersistedPath(fsPath);
// Look up the persisted path — either from workspace cache or global cache
if (isGlobalScope) {
// Safe: guarded by the early return above
const getGlobalPersistedPath = opts.getGlobalPersistedPath as () => Promise<string | undefined>;

if (persistedPath) {
try {
const resolved = await opts.resolve(persistedPath);
if (resolved) {
return { env: resolved };
// Track end-to-end cross-session cache performance for global scope, including persisted-path lookup.
const cacheStopWatch = new StopWatch();
const persistedPath = await getGlobalPersistedPath();

Comment thread
eleanorjboyd marked this conversation as resolved.
if (persistedPath) {
try {
const resolved = await opts.resolve(persistedPath);
if (resolved) {
sendTelemetryEvent(EventNames.GLOBAL_ENV_CACHE, cacheStopWatch.elapsedTime, {
managerLabel: opts.label,
result: 'hit',
});
return { env: resolved };
}
// Cached path found but resolve returned undefined (e.g., Python was uninstalled)
sendTelemetryEvent(EventNames.GLOBAL_ENV_CACHE, cacheStopWatch.elapsedTime, {
managerLabel: opts.label,
result: 'stale',
});
} catch (err) {
sendTelemetryEvent(EventNames.GLOBAL_ENV_CACHE, cacheStopWatch.elapsedTime, {
managerLabel: opts.label,
result: 'stale',
});
traceWarn(
`[${opts.label}] Fast path resolve failed for '${persistedPath}', falling back to full init:`,
err,
);
}
} catch (err) {
traceWarn(
`[${opts.label}] Fast path resolve failed for '${persistedPath}', falling back to full init:`,
err,
);
} else {
sendTelemetryEvent(EventNames.GLOBAL_ENV_CACHE, cacheStopWatch.elapsedTime, {
managerLabel: opts.label,
result: 'miss',
});
traceVerbose(`[${opts.label}] Fast path: no persisted path, falling through to slow path`);
}
} else {
traceVerbose(`[${opts.label}] Fast path: no persisted path, falling through to slow path`);
const scope = opts.scope as Uri;
const persistedPath = await opts.getPersistedPath(opts.getProjectFsPath(scope));

if (persistedPath) {
try {
const resolved = await opts.resolve(persistedPath);
if (resolved) {
return { env: resolved };
}
} catch (err) {
traceWarn(
`[${opts.label}] Fast path resolve failed for '${persistedPath}', falling back to full init:`,
err,
);
}
} else {
traceVerbose(`[${opts.label}] Fast path: no persisted path, falling through to slow path`);
}
}

return undefined;
Expand Down
103 changes: 102 additions & 1 deletion src/test/managers/common/fastPath.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import * as path from 'path';
import * as sinon from 'sinon';
import { Uri } from 'vscode';
import { PythonEnvironment } from '../../../api';
import { EventNames } from '../../../common/telemetry/constants';
import * as telemetrySender from '../../../common/telemetry/sender';
import { createDeferred } from '../../../common/utils/deferred';
import { FastPathOptions, tryFastPathGet } from '../../../managers/common/fastPath';

Expand Down Expand Up @@ -47,22 +49,121 @@ function createOpts(overrides?: Partial<FastPathOptions>): 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);

assert.strictEqual(result, undefined);
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<void>();
deferred.resolve();
Expand Down
10 changes: 9 additions & 1 deletion src/test/managers/fastPath.get.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ interface ManagerCase {
name: string;
managerId: string;
persistedPath: string;
supportsGlobalScope?: boolean;
createContext: (sandbox: sinon.SinonSandbox) => ManagerCaseContext;
}

Expand Down Expand Up @@ -136,9 +137,11 @@ function createManagerCases(): ManagerCase[] {
name: 'SysPythonManager',
managerId: 'ms-python.python:system',
persistedPath: path.resolve('test', 'bin', 'python3'),
supportsGlobalScope: true,
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 };
Expand Down Expand Up @@ -217,7 +220,12 @@ function runSharedFastPathTests(managerCase: ManagerCase, getSandbox: () => sino

await manager.get(undefined);

assert.ok(getPersistedStub.notCalled);
// For managers that support global scope, the workspace getPersistedPath may be
// called by background init (loadEnvMap), so we only assert notCalled for managers
// that skip the fast path entirely when scope is undefined.
if (!managerCase.supportsGlobalScope) {
assert.ok(getPersistedStub.notCalled);
}
});

test('skip fast path: already initialized', async () => {
Expand Down