Merge branch 'main' into copilot/overhaul-logging-for-debugging

eleanorjboyd · web-flow · commit 7b777438fe37 · 2026-05-06T10:54:51.000-07:00
diff --git a/api/src/main.ts b/api/src/main.ts
@@ -896,9 +896,8 @@ export interface PythonEnvironmentManagerRegistrationApi {
      *
      * @param manager Environment Manager implementation to register.
      * @param options Optional registration options.
-     * @param options.extensionId The extension ID of the calling extension. This is used as a fallback when
-     * automatic extension detection fails, such as during F5 debugging where the extension's file path
-     * does not contain its marketplace ID. If automatic detection succeeds, this value is ignored.
+     * @param options.extensionId The extension ID of the calling extension. When this is not specified,
+     * or when the specified extension cannot be found, the extension ID will be automatically detected.
      * @returns A disposable that can be used to unregister the environment manager.
      * @see {@link EnvironmentManager}
      */
@@ -1005,9 +1004,8 @@ export interface PythonPackageManagerRegistrationApi {
      *
      * @param manager Package Manager implementation to register.
      * @param options Optional registration options.
-     * @param options.extensionId The extension ID of the calling extension. This is used as a fallback when
-     * automatic extension detection fails, such as during F5 debugging where the extension's file path
-     * does not contain its marketplace ID. If automatic detection succeeds, this value is ignored.
+     * @param options.extensionId The extension ID of the calling extension. When this is not specified,
+     * or when the specified extension cannot be found, the extension ID will be automatically detected.
      * @returns A disposable that can be used to unregister the package manager.
      * @see {@link PackageManager}
      */
diff --git a/src/api.ts b/src/api.ts
@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License.
 
-import {
+import type {
     Disposable,
     Event,
     FileChangeType,
@@ -890,9 +890,8 @@ export interface PythonEnvironmentManagerRegistrationApi {
      *
      * @param manager Environment Manager implementation to register.
      * @param options Optional registration options.
-     * @param options.extensionId The extension ID of the calling extension. This is used as a fallback when
-     * automatic extension detection fails, such as during F5 debugging where the extension's file path
-     * does not contain its marketplace ID. If automatic detection succeeds, this value is ignored.
+     * @param options.extensionId The extension ID of the calling extension. When this is not specified,
+     * or when the specified extension cannot be found, the extension ID will be automatically detected.
      * @returns A disposable that can be used to unregister the environment manager.
      * @see {@link EnvironmentManager}
      */
@@ -999,9 +998,8 @@ export interface PythonPackageManagerRegistrationApi {
      *
      * @param manager Package Manager implementation to register.
      * @param options Optional registration options.
-     * @param options.extensionId The extension ID of the calling extension. This is used as a fallback when
-     * automatic extension detection fails, such as during F5 debugging where the extension's file path
-     * does not contain its marketplace ID. If automatic detection succeeds, this value is ignored.
+     * @param options.extensionId The extension ID of the calling extension. When this is not specified,
+     * or when the specified extension cannot be found, the extension ID will be automatically detected.
      * @returns A disposable that can be used to unregister the package manager.
      * @see {@link PackageManager}
      */
diff --git a/src/common/telemetry/constants.ts b/src/common/telemetry/constants.ts
@@ -124,6 +124,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',
     /**
      * Telemetry event fired when the JSON CLI fallback is used for environment discovery.
      * Triggered when the PET JSON-RPC server mode is exhausted after all restart attempts.
@@ -473,6 +482,18 @@ export interface IEventNamePropertyMapping {
         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';
+    };
+
     /* __GDPR__
         "pet.json_cli_fallback": {
             "operation": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "StellaHuang95" },
diff --git a/src/common/utils/frameUtils.ts b/src/common/utils/frameUtils.ts
@@ -22,15 +22,28 @@ function getFrameData(): FrameData[] {
 }
 
 function getPathFromFrame(frame: FrameData): string {
-    if (frame.filePath && frame.filePath.startsWith('file://')) {
+    if (frame.filePath?.startsWith('file://')) {
         return Uri.parse(frame.filePath).fsPath;
     }
     return frame.filePath;
 }
 
-export function getCallingExtension(extensionIdHint?: string): string {
+export function getCallingExtension(extensionId?: string): string {
     const pythonExts = [ENVS_EXTENSION_ID, PYTHON_EXTENSION_ID];
     const extensions = allExtensions();
+    
+    // Use the provided extensionId when available.
+    // Only accept if it matches an actually loaded extension so we can always return something.
+    if (extensionId) {
+        const hintExt = extensions.find((ext) => ext.id === extensionId);
+        if (hintExt) {
+            traceVerbose(`Using provided extensionId: ${extensionId}`);
+            return extensionId;
+        }
+        traceWarn(`Provided extensionId '${extensionId}' not found in loaded extensions, ignoring`);
+    }
+
+    // Search the stack as a fallback when no extensionId is provided
     const otherExts = extensions.filter((ext) => !pythonExts.includes(ext.id));
     const frames = getFrameData();
 
@@ -94,19 +107,6 @@ export function getCallingExtension(extensionIdHint?: string): string {
         }
     }
 
-    // Use the provided extensionId hint as a fallback (e.g., during F5 debugging where
-    // stack-based detection fails because the file path doesn't contain the extension ID).
-    // Only accept the hint if it matches an actually loaded extension for safety.
-    if (extensionIdHint) {
-        const hintExt = extensions.find((ext) => ext.id === extensionIdHint);
-        if (hintExt) {
-            traceVerbose(`Using provided extensionId hint: ${extensionIdHint}`);
-            extensionIdCache.set(cacheKey, extensionIdHint);
-            return extensionIdHint;
-        }
-        traceWarn(`Provided extensionId hint '${extensionIdHint}' not found in loaded extensions, ignoring`);
-    }
-
     // Fallback - we're likely being called from Python extension or built-in managers
     traceWarn(
         `Could not determine calling extension from stack frames. ` +
diff --git 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<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> {
@@ -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);
 }
diff --git 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
@@ -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<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>;
 }
 
 /**
@@ -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;
     }
 
@@ -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();
+
+        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;
diff --git a/src/test/managers/common/fastPath.unit.test.ts b/src/test/managers/common/fastPath.unit.test.ts
diff --git a/src/test/managers/fastPath.get.unit.test.ts b/src/test/managers/fastPath.get.unit.test.ts
