Skip to content

Commit 7f10b59

Browse files
feat: comprehensive logging overhaul for better diagnostics
- Add extension initialization progress logging (native finder, manager registration, env selection, setup complete with timings) - Add discovery START/END logging in InternalEnvironmentManager.refresh() - Fix wrong log levels: traceError→traceWarn for non-error conditions (uvPythonInstaller, nativePythonFinder), traceInfo→traceVerbose for clipboard operations - Add distinguishing context to terminal env var injector catch handlers - Add environment creation logging in envCommands - Promote setEnvironment logging to traceInfo and log unconditionally - Add pyenv discovery source logging for all lookup paths - Enhance discovery summary to include managers with 0 environments - Update tests to match traceError→traceWarn change Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com>
1 parent c5a1430 commit 7f10b59

10 files changed

Lines changed: 47 additions & 25 deletions

File tree

src/common/telemetry/helpers.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,11 +169,9 @@ export async function logDiscoverySummary(envManagers: EnvironmentManagers): Pro
169169
try {
170170
const envs = await manager.getEnvironments('all');
171171
totalEnvCount += envs.length;
172-
if (envs.length > 0) {
173-
managerSummaries.push(`${manager.displayName}: ${envs.length}`);
174-
}
172+
managerSummaries.push(`${manager.displayName}: ${envs.length}`);
175173
} catch {
176-
// Discovery errors are already logged by InternalEnvironmentManager.refresh()
174+
managerSummaries.push(`${manager.displayName}: error`);
177175
}
178176
}
179177

src/extension.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -573,10 +573,12 @@ export async function activate(context: ExtensionContext): Promise<PythonEnviron
573573
context.subscriptions.push({ dispose: clearHangWatchdog });
574574
try {
575575
// This is the finder that is used by all the built in environment managers
576+
traceInfo('Initializing Python environment tool (native finder)...');
576577
const petStart = new StopWatch();
577578
let nativeFinder: NativePythonFinder;
578579
try {
579580
nativeFinder = await createNativePythonFinder(outputChannel, api, context);
581+
traceInfo(`Python environment tool initialized (${(petStart.elapsedTime / 1000).toFixed(1)}s)`);
580582
sendTelemetryEvent(EventNames.PET_INIT_DURATION, petStart.elapsedTime, { result: 'success' });
581583
} catch (petError) {
582584
sendTelemetryEvent(
@@ -592,6 +594,7 @@ export async function activate(context: ExtensionContext): Promise<PythonEnviron
592594
sysPythonManager.resolve(sysMgr);
593595
// Each manager registers independently — one failure must not block the others.
594596
failureStage = 'managerRegistration';
597+
traceInfo('Registering environment managers...');
595598
await Promise.all([
596599
safeRegister(
597600
'system',
@@ -609,9 +612,12 @@ export async function activate(context: ExtensionContext): Promise<PythonEnviron
609612
),
610613
safeRegister('shellStartupVars', shellStartupVarsMgr.initialize()),
611614
]);
615+
traceInfo('Environment managers registered.');
612616

613617
failureStage = 'envSelection';
618+
traceInfo('Applying initial environment selection...');
614619
await applyInitialEnvironmentSelection(envManagers, projectManager, nativeFinder, api, start.elapsedTime);
620+
traceInfo('Initial environment selection applied.');
615621

616622
// Register manager-agnostic terminal watcher for package-modifying commands
617623
failureStage = 'terminalWatcher';
@@ -627,6 +633,7 @@ export async function activate(context: ExtensionContext): Promise<PythonEnviron
627633
result: 'success',
628634
});
629635
clearHangWatchdog();
636+
traceInfo(`Extension setup complete (${(start.elapsedTime / 1000).toFixed(1)}s total)`);
630637
try {
631638
await terminalManager.initialize(api);
632639
sendManagerSelectionTelemetry(projectManager);

src/features/envCommands.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ export async function createEnvironmentCommand(
153153
if (projects.length === 0) {
154154
const env = await manager.create('global', undefined);
155155
if (env) {
156+
traceInfo(`Environment created: ${env.displayName}`);
156157
await em.setEnvironments('global', env);
157158
}
158159
return env;
@@ -162,6 +163,7 @@ export async function createEnvironmentCommand(
162163
const scope = selected.length === 0 ? 'global' : selected.map((p) => p.uri);
163164
const env = await manager.create(scope, undefined);
164165
if (env) {
166+
traceInfo(`Environment created: ${env.displayName}`);
165167
await em.setEnvironmentsIfUnset(scope, env);
166168
}
167169
return env;
@@ -216,6 +218,9 @@ export async function createAnyEnvironmentCommand(
216218
const manager = em.managers.find((m) => m.id === managerId);
217219
if (manager) {
218220
const env = await manager.create('global', { ...options });
221+
if (env) {
222+
traceInfo(`Environment created: ${env.displayName}`);
223+
}
219224
if (select && env) {
220225
await manager.set(undefined, env);
221226
}
@@ -270,6 +275,9 @@ export async function createAnyEnvironmentCommand(
270275
selected.map((p) => p.uri),
271276
{ ...options, quickCreate },
272277
);
278+
if (env) {
279+
traceInfo(`Environment created: ${env.displayName}`);
280+
}
273281
if (select && env) {
274282
await em.setEnvironments(
275283
selected.map((p) => p.uri),
@@ -739,12 +747,12 @@ export async function copyPathToClipboard(item: unknown): Promise<void> {
739747
if (item instanceof ProjectItem) {
740748
const projectPath = item.project.uri.fsPath;
741749
await clipboardWriteText(projectPath);
742-
traceInfo(`Copied project path to clipboard: ${projectPath}`);
750+
traceVerbose(`Copied project path to clipboard: ${projectPath}`);
743751
} else if (item instanceof ProjectEnvironment || item instanceof PythonEnvTreeItem) {
744752
const run = item.environment.execInfo.run;
745753
const envPath = run.executable;
746754
await clipboardWriteText(envPath);
747-
traceInfo(`Copied environment path to clipboard: ${envPath}`);
755+
traceVerbose(`Copied environment path to clipboard: ${envPath}`);
748756
} else {
749757
traceVerbose(`Invalid context for copy path to clipboard: ${item}`);
750758
}

src/features/envManagers.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {
1515
EnvironmentManagerAlreadyRegisteredError,
1616
PackageManagerAlreadyRegisteredError,
1717
} from '../common/errors/AlreadyRegisteredError';
18-
import { traceError, traceVerbose } from '../common/logging';
18+
import { traceError, traceInfo, traceVerbose } from '../common/logging';
1919
import { StopWatch } from '../common/stopWatch';
2020
import { EventNames } from '../common/telemetry/constants';
2121
import { sendTelemetryEvent } from '../common/telemetry/sender';
@@ -315,6 +315,10 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
315315
return;
316316
}
317317
await manager.set(scope, environment);
318+
traceInfo(
319+
`[setEnvironment] env=${environment?.displayName ?? 'none'} (${environment?.envId?.id ?? 'undefined'}), ` +
320+
`scope=${scope instanceof Uri ? scope.fsPath : scope ?? 'global'}, manager=${manager.id}`,
321+
);
318322

319323
const project = scope ? this.pm.get(scope) : undefined;
320324
// Only persist to settings when explicitly requested
@@ -330,11 +334,8 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
330334
]);
331335
}
332336
traceVerbose(
333-
`[setEnvironment] scope=${scope instanceof Uri ? scope.fsPath : scope}, ` +
334-
`env=${environment?.envId?.id ?? 'undefined'}, manager=${manager.id}, ` +
335-
`project=${project?.uri?.toString() ?? 'none'}, ` +
336-
`packageManager=${this.getPackageManager(environment)?.id ?? 'UNDEFINED'}, ` +
337-
`settingsPersisted=${!!(project && this.getPackageManager(environment))}`,
337+
`[setEnvironment] Settings persisted: project=${project?.uri?.toString() ?? 'none'}, ` +
338+
`packageManager=${this.getPackageManager(environment)?.id ?? 'UNDEFINED'}`,
338339
);
339340
}
340341

src/features/terminal/terminalEnvVarInjector.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export class TerminalEnvVarInjector implements Disposable {
4848
if (!args.uri) {
4949
// No specific URI, reload all workspaces
5050
this.updateEnvironmentVariables().catch((error) => {
51-
traceError('Failed to update environment variables:', error);
51+
traceError('Failed to update environment variables (global reload):', error);
5252
});
5353
return;
5454
}
@@ -57,7 +57,7 @@ export class TerminalEnvVarInjector implements Disposable {
5757
if (!affectedWorkspace) {
5858
// No workspace folder found for this URI, reloading all workspaces
5959
this.updateEnvironmentVariables().catch((error) => {
60-
traceError('Failed to update environment variables:', error);
60+
traceError('Failed to update environment variables (no workspace folder for URI):', error);
6161
});
6262
return;
6363
}
@@ -79,7 +79,7 @@ export class TerminalEnvVarInjector implements Disposable {
7979
this.clearWorkspaceVariables(affectedWorkspace);
8080
} else {
8181
this.updateEnvironmentVariables(affectedWorkspace).catch((error) => {
82-
traceError('Failed to update environment variables:', error);
82+
traceError(`Failed to update environment variables for workspace: ${affectedWorkspace?.name}`, error);
8383
});
8484
}
8585
}),
@@ -93,7 +93,7 @@ export class TerminalEnvVarInjector implements Disposable {
9393
'TerminalEnvVarInjector: python.envFile or python.terminal.useEnvFile setting changed, updating environment variables',
9494
);
9595
this.updateEnvironmentVariables().catch((error) => {
96-
traceError('Failed to update environment variables:', error);
96+
traceError('Failed to update environment variables (settings change):', error);
9797
});
9898
}
9999
}),

src/internal.api.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import {
2929
} from './api';
3030
import { ISSUES_URL } from './common/constants';
3131
import { CreateEnvironmentNotSupported, RemoveEnvironmentNotSupported } from './common/errors/NotSupportedError';
32-
import { traceWarn } from './common/logging';
32+
import { traceInfo, traceWarn } from './common/logging';
3333
import { StopWatch } from './common/stopWatch';
3434
import { EventNames } from './common/telemetry/constants';
3535
import { sendTelemetryEvent } from './common/telemetry/sender';
@@ -206,12 +206,14 @@ export class InternalEnvironmentManager implements EnvironmentManager {
206206
}
207207

208208
async refresh(options: RefreshEnvironmentsScope): Promise<void> {
209+
traceInfo(`[${this.displayName}] Discovering environments...`);
209210
const sw = new StopWatch();
210211
const SLOW_DISCOVERY_THRESHOLD_MS = 15000;
211212
try {
212213
await this.manager.refresh(options);
213214
const envs = await this.manager.getEnvironments('all').catch(() => []);
214215
const duration = sw.elapsedTime;
216+
traceInfo(`[${this.displayName}] Discovery complete: ${envs.length} environments found (${(duration / 1000).toFixed(1)}s)`);
215217
sendTelemetryEvent(EventNames.ENVIRONMENT_DISCOVERY, duration, {
216218
managerId: this.id,
217219
result: 'success',

src/managers/builtin/uvPythonInstaller.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
} from 'vscode';
1111
import { spawnProcess } from '../../common/childProcess.apis';
1212
import { Common, UvInstallStrings } from '../../common/localize';
13-
import { traceError, traceInfo, traceLog } from '../../common/logging';
13+
import { traceError, traceInfo, traceLog, traceWarn } from '../../common/logging';
1414
import { getGlobalPersistentState } from '../../common/persistentState';
1515
import { executeTask, onDidEndTaskProcess } from '../../common/tasks.apis';
1616
import { EventNames } from '../../common/telemetry/constants';
@@ -82,7 +82,7 @@ async function getUvInstallCommand(): Promise<{ executable: string; args: string
8282
}
8383

8484
// Default to curl and let it fail with a clear error if neither is available
85-
traceError('Neither curl nor wget found, attempting curl anyway');
85+
traceWarn('Neither curl nor wget found, attempting curl anyway');
8686
return {
8787
executable: 'sh',
8888
args: ['-c', 'curl -LsSf https://astral.sh/uv/install.sh | sh'],

src/managers/common/nativePythonFinder.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1384,7 +1384,7 @@ function getWorkspaceSearchPaths(scope?: Uri): string[] {
13841384

13851385
if (inspection?.globalValue && !workspaceSearchPathsGlobalWarningShown) {
13861386
workspaceSearchPathsGlobalWarningShown = true;
1387-
traceError(
1387+
traceWarn(
13881388
'python-envs.workspaceSearchPaths is set at the user/global level, but this setting can only be set at the workspace or workspace folder level.',
13891389
);
13901390
}

src/managers/pyenv/pyenvUtils.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ export async function getPyenv(): Promise<string | undefined> {
138138
if (pyenvRoot) {
139139
const pyenvPath = path.join(pyenvRoot, 'bin', pyenvBin);
140140
if (await fs.exists(pyenvPath)) {
141+
traceInfo(`Using pyenv from PYENV_ROOT: ${pyenvPath}`);
141142
return pyenvPath;
142143
}
143144
}
@@ -146,22 +147,26 @@ export async function getPyenv(): Promise<string | undefined> {
146147
if (home) {
147148
const pyenvPath = path.join(home, '.pyenv', 'bin', pyenvBin);
148149
if (await fs.exists(pyenvPath)) {
150+
traceInfo(`Using pyenv from home directory: ${pyenvPath}`);
149151
return pyenvPath;
150152
}
151153

152154
if (isWindows()) {
153155
const pyenvPathWin = path.join(home, '.pyenv', 'pyenv-win', 'bin', pyenvBin);
154156
if (await fs.exists(pyenvPathWin)) {
157+
traceInfo(`Using pyenv-win from home directory: ${pyenvPathWin}`);
155158
return pyenvPathWin;
156159
}
157160
}
158161
}
159162

160163
pyenvPath = await findPyenv();
161164
if (pyenvPath) {
165+
traceInfo(`Using pyenv from PATH: ${pyenvPath}`);
162166
return pyenvPath;
163167
}
164168

169+
traceInfo('pyenv not found in PYENV_ROOT, home directory, or PATH');
165170
return undefined;
166171
} catch (ex) {
167172
const err = ex instanceof Error ? ex : new Error(String(ex));
@@ -268,6 +273,7 @@ export async function refreshPyenv(
268273
}
269274
});
270275

276+
traceInfo(`Pyenv discovery complete: ${collection.length} environments found`);
271277
return sortEnvironments(collection);
272278
}
273279

src/test/managers/common/nativePythonFinder.getAllExtraSearchPaths.unit.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -331,8 +331,8 @@ suite('getAllExtraSearchPaths Integration Tests', () => {
331331
assert.deepStrictEqual(result, []);
332332
// Check that error was logged with key terms - don't be brittle about exact wording
333333
assert(
334-
mockTraceError.calledWith(sinon.match(/workspaceSearchPaths.*global.*level/i)),
335-
'Should log error about incorrect setting level',
334+
mockTraceWarn.calledWith(sinon.match(/workspaceSearchPaths.*global.*level/i)),
335+
'Should log warning about incorrect setting level',
336336
);
337337
});
338338

@@ -350,16 +350,16 @@ suite('getAllExtraSearchPaths Integration Tests', () => {
350350
await getAllExtraSearchPaths();
351351
await getAllExtraSearchPaths();
352352

353-
// Assert - error should only be logged once, not three times
354-
const matchingCalls = mockTraceError
353+
// Assert - warning should only be logged once, not three times
354+
const matchingCalls = mockTraceWarn
355355
.getCalls()
356356
.filter((call: sinon.SinonSpyCall) =>
357357
/workspaceSearchPaths.*global.*level/i.test(String(call.args[0])),
358358
);
359359
assert.strictEqual(
360360
matchingCalls.length,
361361
1,
362-
`Expected exactly 1 error about workspaceSearchPaths global level, got ${matchingCalls.length}`,
362+
`Expected exactly 1 warning about workspaceSearchPaths global level, got ${matchingCalls.length}`,
363363
);
364364
});
365365

0 commit comments

Comments
 (0)