Skip to content

Commit 7f230ca

Browse files
author
Kartik Raj
authored
Ensure we correctly evaluate Unknown type before sending startup telemetry (#17362)
* Identify globally installed envs correctly * Add to cache * Add comments * News entry
1 parent 7ee43ec commit 7f230ca

File tree

5 files changed

+64
-3
lines changed

5 files changed

+64
-3
lines changed

news/2 Fixes/17362.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Ensure we correctly evaluate Unknown type before sending startup telemetry.

src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,11 @@ export class EnvsCollectionService extends PythonEnvsWatcher<PythonEnvCollection
6060
if (cachedEnv && this.refreshPromises.size === 0) {
6161
return cachedEnv;
6262
}
63-
return this.locator.resolveEnv(executablePath);
63+
const resolved = await this.locator.resolveEnv(executablePath);
64+
if (resolved) {
65+
this.cache.addEnv(resolved);
66+
}
67+
return resolved;
6468
}
6569

6670
public getEnvs(query?: PythonLocatorQuery): PythonEnvInfo[] {

src/client/pythonEnvironments/common/environmentIdentifier.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import { PythonEnvKind } from '../base/info';
55
import { getPrioritizedEnvKinds } from '../base/info/envKind';
66
import { isCondaEnvironment } from './environmentManagers/conda';
7+
import { isGloballyInstalledEnv } from './environmentManagers/globalInstalledEnvs';
78
import { isPipenvEnvironment } from './environmentManagers/pipenv';
89
import { isPoetryEnvironment } from './environmentManagers/poetry';
910
import { isPyenvEnvironment } from './environmentManagers/pyenv';
@@ -31,6 +32,7 @@ function getIdentifiers(): Map<PythonEnvKind, (path: string) => Promise<boolean>
3132
identifier.set(PythonEnvKind.VirtualEnvWrapper, isVirtualEnvWrapperEnvironment);
3233
identifier.set(PythonEnvKind.VirtualEnv, isVirtualEnvEnvironment);
3334
identifier.set(PythonEnvKind.Unknown, defaultTrue);
35+
identifier.set(PythonEnvKind.OtherGlobal, isGloballyInstalledEnv);
3436
return identifier;
3537
}
3638

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import { getSearchPathEntries } from '../../../common/utils/exec';
5+
import { getOSType, OSType } from '../../../common/utils/platform';
6+
import { isParentPath } from '../externalDependencies';
7+
import { commonPosixBinPaths } from '../posixUtils';
8+
import { isPyenvShimDir } from './pyenv';
9+
10+
/**
11+
* Checks if the given interpreter belongs to known globally installed types. If an global
12+
* executable is discoverable, we consider it as global type.
13+
* @param {string} interpreterPath: Absolute path to the python interpreter.
14+
* @returns {boolean} : Returns true if the interpreter belongs to a venv environment.
15+
*/
16+
export async function isGloballyInstalledEnv(executablePath: string): Promise<boolean> {
17+
// Identifying this type is not important, as the extension treats `Global` and `Unknown`
18+
// types the same way. This is only required for telemetry. As windows registry is known
19+
// to be slow, we do not want to unnecessarily block on that by default, hence skip this
20+
// step.
21+
// if (getOSType() === OSType.Windows) {
22+
// if (await isFoundInWindowsRegistry(executablePath)) {
23+
// return true;
24+
// }
25+
// }
26+
return isFoundInPathEnvVar(executablePath);
27+
}
28+
29+
async function isFoundInPathEnvVar(executablePath: string): Promise<boolean> {
30+
let searchPathEntries: string[] = [];
31+
if (getOSType() === OSType.Windows) {
32+
searchPathEntries = getSearchPathEntries();
33+
} else {
34+
searchPathEntries = await commonPosixBinPaths();
35+
}
36+
// Filter out pyenv shims. They are not actual python binaries, they are used to launch
37+
// the binaries specified in .python-version file in the cwd. We should not be reporting
38+
// those binaries as environments.
39+
searchPathEntries = searchPathEntries.filter((dirname) => !isPyenvShimDir(dirname));
40+
for (const searchPath of searchPathEntries) {
41+
if (isParentPath(executablePath, searchPath)) {
42+
return true;
43+
}
44+
}
45+
return false;
46+
}

src/client/startupTelemetry.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,15 +105,23 @@ async function getActivationTelemetryProps(serviceContainer: IServiceContainer):
105105
? workspaceService.workspaceFolders![0].uri
106106
: undefined;
107107
const settings = configurationService.getSettings(mainWorkspaceUri);
108-
const [condaVersion, interpreter, hasPython3] = await Promise.all([
108+
const [condaVersion, hasPython3] = await Promise.all([
109109
condaLocator
110110
.getCondaVersion()
111111
.then((ver) => (ver ? ver.raw : ''))
112112
.catch<string>(() => ''),
113-
interpreterService.getActiveInterpreter().catch<PythonEnvironment | undefined>(() => undefined),
114113
interpreterService.hasInterpreters(async (item) => item.version?.major === 3),
115114
]);
116115
const workspaceFolderCount = workspaceService.hasWorkspaceFolders ? workspaceService.workspaceFolders!.length : 0;
116+
// If an unknown type environment can be found from windows registry or path env var,
117+
// consider them as global type instead of unknown. Such types can only be known after
118+
// windows registry is queried. So wait for the refresh of windows registry locator to
119+
// finish. API getActiveInterpreter() does not block on windows registry by default as
120+
// it is slow.
121+
await interpreterService.refreshPromise;
122+
const interpreter = await interpreterService
123+
.getActiveInterpreter()
124+
.catch<PythonEnvironment | undefined>(() => undefined);
117125
const pythonVersion = interpreter && interpreter.version ? interpreter.version.raw : undefined;
118126
const interpreterType = interpreter ? interpreter.envType : undefined;
119127
const usingUserDefinedInterpreter = hasUserDefinedPythonPath(mainWorkspaceUri, serviceContainer);

0 commit comments

Comments
 (0)