Skip to content

Commit ad46274

Browse files
StellaHuang95Copilot
StellaHuang95
and
Copilot
committed
Correct inline script telemetry contracts
Report reuse dependency counts and preserve accurate final failure outcomes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6b12d843-8011-4bfc-9ba9-f75761eadee2
1 parent d9692b3 commit ad46274

3 files changed

Lines changed: 61 additions & 33 deletions

File tree

‎src/common/telemetry/constants.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,8 @@ export enum EventNames {
233233
/**
234234
* Telemetry event fired when inline-script environment creation validates
235235
* and reuses an existing cache entry without rebuilding it.
236+
* Measures:
237+
* - dependencyCount: number (normalized dependency count in the cache key)
236238
*/
237239
INLINE_SCRIPT_ENV_REUSE_HIT = 'inlineScript.envReuseHit',
238240
/**
@@ -259,6 +261,7 @@ export type InlineScriptEnvErrorCategory =
259261
| 'no-compatible-python'
260262
| 'package-install-cancelled'
261263
| 'install-failure'
264+
| 'setup-failure'
262265
| 'lock-timeout'
263266
| 'lock-unavailable';
264267

@@ -737,9 +740,14 @@ export interface IEventNamePropertyMapping {
737740
};
738741

739742
/* __GDPR__
740-
"inlineScript.envReuseHit": {"owner": "StellaHuang95" }
743+
"inlineScript.envReuseHit": {
744+
"dependencyCount": { "classification": "SystemMetaData", "purpose": "FeatureInsight", "isMeasurement": true, "owner": "StellaHuang95" }
745+
}
741746
*/
742-
[EventNames.INLINE_SCRIPT_ENV_REUSE_HIT]: never | undefined;
747+
[EventNames.INLINE_SCRIPT_ENV_REUSE_HIT]: {
748+
// Goes through the measures payload (numeric); listed here for GDPR only.
749+
dependencyCount?: number;
750+
};
743751

744752
/* __GDPR__
745753
"inlineScript.envError": {

‎src/managers/builtin/inlineScript/envManager.ts‎

Lines changed: 13 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
185185
}
186186
}
187187
} catch (error) {
188-
this.sendInlineScriptEnvErrorTelemetry('install-failure');
188+
this.sendInlineScriptEnvErrorTelemetry('setup-failure');
189189
this.log.error(`Failed to set up inline-script environment: ${getErrorMessage(error)}`);
190190
return undefined;
191191
}
@@ -257,7 +257,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
257257
}
258258
if (quickCreate) {
259259
return {
260-
errorCategory: this.getBaseInterpreterErrorCategory(selection.discoveryFailed, 'no-compatible-python'),
260+
errorCategory: selection.discoveryFailed ? 'discovery-failure' : 'no-compatible-python',
261261
};
262262
}
263263
return this.installAndSelectBaseInterpreter(metadata, selection.discoveryFailed);
@@ -940,20 +940,15 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
940940
'Cannot install a Python for this inline script because no compatible install version could be selected.',
941941
);
942942
return {
943-
errorCategory: this.getBaseInterpreterErrorCategory(
944-
discoveryFailed,
945-
versionSelection.errorCategory ?? 'no-compatible-python',
946-
),
943+
errorCategory: versionSelection.errorCategory ?? 'no-compatible-python',
947944
};
948945
}
949946

950947
const installResult = await this.installPythonAndRefresh(requiresPython, versionSelection.version);
951948
if (installResult.kind !== 'installed') {
952949
return {
953-
errorCategory: this.getBaseInterpreterErrorCategory(
954-
discoveryFailed,
950+
errorCategory:
955951
installResult.kind === 'declined' ? 'compatible-python-declined' : 'install-failure',
956-
),
957952
};
958953
}
959954
const installedPath = installResult.installedPath;
@@ -991,7 +986,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
991986
'Python was installed for an inline script, but no compatible base interpreter was discovered after refreshing environments.',
992987
);
993988
return {
994-
errorCategory: this.getBaseInterpreterErrorCategory(discoveryFailedAfterInstall, 'install-failure'),
989+
errorCategory: discoveryFailedAfterInstall ? 'discovery-failure' : 'setup-failure',
995990
};
996991
}
997992
return { selectedBase: selected };
@@ -1140,19 +1135,19 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
11401135

11411136
const cached = await this.inspectCacheEntry(cacheRoot, envDir, metadata, selectedBase);
11421137
if (cached.kind === 'reusable') {
1143-
this.sendInlineScriptEnvReuseHitTelemetry();
1138+
this.sendInlineScriptEnvReuseHitTelemetry(dependencyCount);
11441139
return cached.environment;
11451140
}
11461141
if (cached.kind === 'uncertain') {
11471142
this.log.warn(
11481143
`Preserving an inline-script cache entry that could not be safely inspected: ${envDir.fsPath}`,
11491144
);
1150-
this.sendInlineScriptEnvErrorTelemetry('install-failure');
1145+
this.sendInlineScriptEnvErrorTelemetry('setup-failure');
11511146
return undefined;
11521147
}
11531148
if (cached.kind === 'stale') {
11541149
if (!(await this.removeCacheEntry(envDir))) {
1155-
this.sendInlineScriptEnvErrorTelemetry('install-failure');
1150+
this.sendInlineScriptEnvErrorTelemetry('setup-failure');
11561151
return undefined;
11571152
}
11581153
}
@@ -1313,7 +1308,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
13131308
) {
13141309
this.log.error('Created inline-script environment does not match the requested cache entry.');
13151310
await this.removeCacheEntry(envDir);
1316-
return { errorCategory: 'install-failure' };
1311+
return { errorCategory: 'setup-failure' };
13171312
}
13181313

13191314
try {
@@ -1326,7 +1321,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
13261321
} catch (error) {
13271322
this.log.error(`Failed to record inline-script cache metadata: ${getErrorMessage(error)}`);
13281323
await this.removeCacheEntry(envDir);
1329-
return { errorCategory: 'install-failure' };
1324+
return { errorCategory: 'setup-failure' };
13301325
}
13311326

13321327
return { environment: result.environment };
@@ -1374,21 +1369,14 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
13741369
});
13751370
}
13761371

1377-
private sendInlineScriptEnvReuseHitTelemetry(): void {
1378-
sendTelemetryEvent(EventNames.INLINE_SCRIPT_ENV_REUSE_HIT);
1372+
private sendInlineScriptEnvReuseHitTelemetry(dependencyCount: number): void {
1373+
sendTelemetryEvent(EventNames.INLINE_SCRIPT_ENV_REUSE_HIT, { dependencyCount });
13791374
}
13801375

13811376
private sendInlineScriptEnvErrorTelemetry(category: InlineScriptEnvErrorCategory): void {
13821377
sendTelemetryEvent(EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category });
13831378
}
13841379

1385-
private getBaseInterpreterErrorCategory(
1386-
discoveryFailed: boolean,
1387-
fallbackCategory: InlineScriptEnvErrorCategory,
1388-
): InlineScriptEnvErrorCategory {
1389-
return discoveryFailed ? 'discovery-failure' : fallbackCategory;
1390-
}
1391-
13921380
private getCreateOrReuseErrorCategory(error: unknown): InlineScriptEnvErrorCategory {
13931381
if (
13941382
typeof error === 'object' &&
@@ -1398,7 +1386,7 @@ export class InlineScriptEnvManager implements EnvironmentManager, Disposable {
13981386
) {
13991387
return (error as NodeJS.ErrnoException).code === 'ELOCKED' ? 'lock-timeout' : 'lock-unavailable';
14001388
}
1401-
return 'install-failure';
1389+
return 'setup-failure';
14021390
}
14031391

14041392
dispose(): void {

‎src/test/managers/builtin/inlineScript/envManager.unit.test.ts‎

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1537,6 +1537,7 @@ suite('InlineScriptEnvManager', () => {
15371537
});
15381538

15391539
test('emits envReuseHit only for validated cache hits', async () => {
1540+
readMetadataStub.resolves({ ...VALID_METADATA, dependencies: ['Requests', 'requests'] });
15401541
await fs.ensureDir(envDir().fsPath);
15411542
setSidecar({
15421543
schemaVersion: cacheLayout.META_SCHEMA_VERSION,
@@ -1557,11 +1558,28 @@ suite('InlineScriptEnvManager', () => {
15571558

15581559
const reuseCalls = telemetryCalls(EventNames.INLINE_SCRIPT_ENV_REUSE_HIT);
15591560
assert.strictEqual(reuseCalls.length, 1);
1560-
assert.deepStrictEqual(reuseCalls[0].args, [EventNames.INLINE_SCRIPT_ENV_REUSE_HIT]);
1561+
assert.deepStrictEqual(reuseCalls[0].args, [
1562+
EventNames.INLINE_SCRIPT_ENV_REUSE_HIT,
1563+
{ dependencyCount: 1 },
1564+
]);
15611565
assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED).length, 0);
15621566
assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).length, 0);
15631567
});
15641568

1569+
test('emits setup-failure when cache inspection is unavailable', async () => {
1570+
await fs.ensureDir(envDir().fsPath);
1571+
inspectMetaStub.resolves({ kind: 'unavailable' });
1572+
1573+
assert.strictEqual(await manager.create(scriptUri()), undefined);
1574+
1575+
assert.deepStrictEqual(
1576+
telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).map((call) => call.args),
1577+
[[EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category: 'setup-failure' }]],
1578+
);
1579+
assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED).length, 0);
1580+
assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_REUSE_HIT).length, 0);
1581+
});
1582+
15651583
test('emits a single compatible-python-declined error for coalesced same-script requests', async () => {
15661584
const uri = scriptUri();
15671585
readMetadataStub.resolves({ ...VALID_METADATA, requiresPython: '>=3.13' });
@@ -1621,7 +1639,7 @@ suite('InlineScriptEnvManager', () => {
16211639
);
16221640
});
16231641

1624-
test('emits discovery-failure instead of compatible-python-declined when discovery is unavailable', async () => {
1642+
test('emits the final compatible-python-declined outcome when discovery was unavailable', async () => {
16251643
const uri = scriptUri();
16261644
readMetadataStub.resolves({ ...VALID_METADATA, requiresPython: '>=3.13' });
16271645
apiGetEnvironmentsStub.rejects(new Error('discovery unavailable'));
@@ -1631,11 +1649,25 @@ suite('InlineScriptEnvManager', () => {
16311649
assert.strictEqual(promptInstallPythonViaUvStub.callCount, 1);
16321650
assert.deepStrictEqual(
16331651
telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).map((call) => call.args),
1634-
[[EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category: 'discovery-failure' }]],
1652+
[[EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category: 'compatible-python-declined' }]],
1653+
);
1654+
});
1655+
1656+
test('emits the final install-failure outcome when discovery was unavailable', async () => {
1657+
readMetadataStub.resolves({ ...VALID_METADATA, requiresPython: '>=3.13' });
1658+
apiGetEnvironmentsStub.rejects(new Error('discovery unavailable'));
1659+
promptInstallPythonViaUvStub.resolves({ kind: 'failed' });
1660+
1661+
assert.strictEqual(await manager.create(scriptUri()), undefined);
1662+
1663+
assert.strictEqual(promptInstallPythonViaUvStub.callCount, 1);
1664+
assert.deepStrictEqual(
1665+
telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).map((call) => call.args),
1666+
[[EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category: 'install-failure' }]],
16351667
);
16361668
});
16371669

1638-
test('emits discovery-failure instead of install-failure when discovery never recovers', async () => {
1670+
test('emits discovery-failure when installed Python cannot be discovered or resolved', async () => {
16391671
const uvExecutable = path.join(tempRoot, 'uv-python', isWindows() ? 'python.exe' : 'python');
16401672
await fs.outputFile(uvExecutable, '');
16411673
readMetadataStub.resolves({ ...VALID_METADATA, requiresPython: '>=3.13' });
@@ -1775,14 +1807,14 @@ suite('InlineScriptEnvManager', () => {
17751807
assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED).length, 0);
17761808
});
17771809

1778-
test('emits install-failure when sidecar persistence rollback removes the new environment', async () => {
1810+
test('emits setup-failure when sidecar persistence rollback removes the new environment', async () => {
17791811
writeMetaStub.rejects(new Error('disk full'));
17801812

17811813
assert.strictEqual(await manager.create(scriptUri()), undefined);
17821814

17831815
assert.deepStrictEqual(
17841816
telemetryCalls(EventNames.INLINE_SCRIPT_ENV_ERROR).map((call) => call.args),
1785-
[[EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category: 'install-failure' }]],
1817+
[[EventNames.INLINE_SCRIPT_ENV_ERROR, undefined, { category: 'setup-failure' }]],
17861818
);
17871819
assert.strictEqual(telemetryCalls(EventNames.INLINE_SCRIPT_ENV_CREATED).length, 0);
17881820
});

0 commit comments

Comments
 (0)