Skip to content

Commit 7db97ed

Browse files
eleanorjboydCopilot
andcommitted
Fix testing perf telemetry: send measurements via measures arg
The discovery/run telemetry emitted the numeric performance fields (totalDurationMs, testCount, durationMs, requestedCount) in the properties bag instead of the measures argument of sendTelemetryEvent. These fields are annotated `isMeasurement: true`, and telemetry ingestion drops measurement-annotated fields that arrive as properties. As a result, across ~46M unittest.discovery.done events none carried a duration or test-count value, while the string categoricals (mode, trigger, failureCategory) came through fine. This meant there was no duration signal to evaluate the recent pytest discovery performance work (#25974, #25982). Move the numeric fields to the measures (2nd) argument at all five emit sites so they land in the Measures column, matching the established NATIVE_FINDER_PERF pattern. Also drop these fields from the event property type definitions (leaving the __GDPR__ annotations intact) and add notes so they are not reintroduced as properties. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 66ff9a4 commit 7db97ed

6 files changed

Lines changed: 56 additions & 48 deletions

File tree

src/client/telemetry/index.ts

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2209,14 +2209,10 @@ export interface IEventNamePropertyMapping {
22092209
| 'env-resolution'
22102210
| 'cancelled'
22112211
| 'unknown';
2212-
/**
2213-
* Wall-clock duration of the discovery cycle in milliseconds.
2214-
*/
2215-
totalDurationMs?: number;
2216-
/**
2217-
* Number of test items discovered (leaf nodes).
2218-
*/
2219-
testCount?: number;
2212+
// NOTE: `totalDurationMs` and `testCount` are measurements (see the __GDPR__ block
2213+
// above) and MUST be passed via the `measures` argument of sendTelemetryEvent, not
2214+
// as properties. Fields annotated `isMeasurement: true` are dropped by telemetry
2215+
// ingestion when sent in the properties bag.
22202216
};
22212217
/**
22222218
* Telemetry event sent when cancelling discovering tests
@@ -2286,14 +2282,10 @@ export interface IEventNamePropertyMapping {
22862282
* Coarse failure category when `failed` is true.
22872283
*/
22882284
failureCategory?: UnitTestRunFailureCategory;
2289-
/**
2290-
* Wall-clock duration of the run in milliseconds.
2291-
*/
2292-
durationMs?: number;
2293-
/**
2294-
* Number of test items the user asked to run.
2295-
*/
2296-
requestedCount?: number;
2285+
// NOTE: `durationMs` and `requestedCount` are measurements (see the __GDPR__ block
2286+
// above) and MUST be passed via the `measures` argument of sendTelemetryEvent, not
2287+
// as properties. Fields annotated `isMeasurement: true` are dropped by telemetry
2288+
// ingestion when sent in the properties bag.
22972289
};
22982290
/**
22992291
* Telemetry event sent when testing is disabled for a workspace.

src/client/testing/testController/common/projectTestExecution.ts

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -85,15 +85,17 @@ export async function executeTestsForProjects(
8585
traceError(`[test-by-project] Execution failed for project ${project.projectName}:`, error);
8686
}
8787
} finally {
88-
sendTelemetryEvent(EventName.UNITTEST_RUN_DONE, undefined, {
89-
tool: project.testProvider,
90-
debugging: isDebugMode,
91-
mode: 'project',
92-
failed,
93-
failureCategory,
94-
durationMs: stopWatch.elapsedTime,
95-
requestedCount: items.length,
96-
});
88+
sendTelemetryEvent(
89+
EventName.UNITTEST_RUN_DONE,
90+
{ durationMs: stopWatch.elapsedTime, requestedCount: items.length },
91+
{
92+
tool: project.testProvider,
93+
debugging: isDebugMode,
94+
mode: 'project',
95+
failed,
96+
failureCategory,
97+
},
98+
);
9799
}
98100
});
99101

src/client/testing/testController/common/resultResolver.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,14 +93,20 @@ export class PythonResultResolver implements ITestResultResolver {
9393
const cycle = this.discoveryTelemetry.complete();
9494
const mode = cycle?.mode ?? this.discoveryTelemetry.defaultMode;
9595
const failed = payload?.status === 'error';
96-
sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, {
96+
// Numeric fields must be sent as measures (2nd arg), not properties. Fields
97+
// annotated `isMeasurement: true` are dropped by telemetry ingestion when they
98+
// arrive in the properties bag, so totalDurationMs/testCount would otherwise never
99+
// be recorded.
100+
const measures: Record<string, number> = { testCount };
101+
if (cycle?.stopWatch.elapsedTime !== undefined) {
102+
measures.totalDurationMs = cycle.stopWatch.elapsedTime;
103+
}
104+
sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, measures, {
97105
tool: this.testProvider,
98106
failed,
99107
mode,
100108
trigger: cycle?.trigger,
101109
failureCategory: failed ? (token?.isCancellationRequested ? 'cancelled' : 'unknown') : undefined,
102-
totalDurationMs: cycle?.stopWatch.elapsedTime,
103-
testCount,
104110
});
105111
}
106112

src/client/testing/testController/controller.ts

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -716,13 +716,16 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
716716
// Individual project failures don't block others
717717
projectsCompleted.add(project.projectUri.toString()); // Still mark as completed
718718
const cycle = (project.resultResolver as Partial<PythonResultResolver>).discoveryTelemetry?.complete();
719-
sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, {
719+
const measures: Record<string, number> = {};
720+
if (cycle?.stopWatch.elapsedTime !== undefined) {
721+
measures.totalDurationMs = cycle.stopWatch.elapsedTime;
722+
}
723+
sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, measures, {
720724
tool: project.testProvider,
721725
failed: true,
722726
mode: 'project',
723727
trigger: cycle?.trigger ?? this.currentDiscoveryTrigger,
724728
failureCategory: this.refreshCancellation.token.isCancellationRequested ? 'cancelled' : 'unknown',
725-
totalDurationMs: cycle?.stopWatch.elapsedTime,
726729
});
727730
} finally {
728731
project.isDiscovering = false;
@@ -1048,15 +1051,17 @@ export class PythonTestController implements ITestController, IExtensionSingleAc
10481051
failureCategory = token.isCancellationRequested ? 'cancelled' : 'unknown';
10491052
throw ex;
10501053
} finally {
1051-
sendTelemetryEvent(EventName.UNITTEST_RUN_DONE, undefined, {
1052-
tool: provider,
1053-
debugging,
1054-
mode: 'legacy',
1055-
failed,
1056-
failureCategory,
1057-
durationMs: stopWatch.elapsedTime,
1058-
requestedCount: testItems.length,
1059-
});
1054+
sendTelemetryEvent(
1055+
EventName.UNITTEST_RUN_DONE,
1056+
{ durationMs: stopWatch.elapsedTime, requestedCount: testItems.length },
1057+
{
1058+
tool: provider,
1059+
debugging,
1060+
mode: 'legacy',
1061+
failed,
1062+
failureCategory,
1063+
},
1064+
);
10601065
}
10611066
}
10621067

src/client/testing/testController/workspaceTestAdapter.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -153,14 +153,17 @@ export class WorkspaceTestAdapter {
153153
deferred.resolve();
154154
} catch (ex) {
155155
const cycle = (this.resultResolver as Partial<PythonResultResolver>).discoveryTelemetry?.complete();
156-
sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, {
157-
tool: this.testProvider,
158-
failed: true,
159-
mode: 'legacy',
160-
trigger: cycle?.trigger ?? trigger,
161-
failureCategory: token?.isCancellationRequested ? 'cancelled' : 'unknown',
162-
totalDurationMs: cycle?.stopWatch.elapsedTime ?? stopWatch.elapsedTime,
163-
});
156+
sendTelemetryEvent(
157+
EventName.UNITTEST_DISCOVERY_DONE,
158+
{ totalDurationMs: cycle?.stopWatch.elapsedTime ?? stopWatch.elapsedTime },
159+
{
160+
tool: this.testProvider,
161+
failed: true,
162+
mode: 'legacy',
163+
trigger: cycle?.trigger ?? trigger,
164+
failureCategory: token?.isCancellationRequested ? 'cancelled' : 'unknown',
165+
},
166+
);
164167

165168
let cancel = token?.isCancellationRequested
166169
? Testing.cancelUnittestDiscovery

src/test/testing/testController/resultResolver.unit.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ suite('Result Resolver tests', () => {
155155

156156
resultResolver.resolveDiscovery(payload, cancelationToken);
157157

158-
sinon.assert.calledWithMatch(sendTelemetryStub, EventName.UNITTEST_DISCOVERY_DONE, undefined, {
158+
sinon.assert.calledWithMatch(sendTelemetryStub, EventName.UNITTEST_DISCOVERY_DONE, sinon.match.any, {
159159
failed: true,
160160
failureCategory: 'unknown',
161161
});

0 commit comments

Comments
 (0)