From 7db97edf39936bd0de05af1b2da7ef87fff67181 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Thu, 2 Jul 2026 15:50:32 -0700 Subject: [PATCH] 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> --- src/client/telemetry/index.ts | 24 ++++++----------- .../common/projectTestExecution.ts | 20 +++++++------- .../testController/common/resultResolver.ts | 12 ++++++--- .../testing/testController/controller.ts | 27 +++++++++++-------- .../testController/workspaceTestAdapter.ts | 19 +++++++------ .../resultResolver.unit.test.ts | 2 +- 6 files changed, 56 insertions(+), 48 deletions(-) diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index ffb237eb032a..4a2be37165a1 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -2209,14 +2209,10 @@ export interface IEventNamePropertyMapping { | 'env-resolution' | 'cancelled' | 'unknown'; - /** - * Wall-clock duration of the discovery cycle in milliseconds. - */ - totalDurationMs?: number; - /** - * Number of test items discovered (leaf nodes). - */ - testCount?: number; + // NOTE: `totalDurationMs` and `testCount` are measurements (see the __GDPR__ block + // above) and MUST be passed via the `measures` argument of sendTelemetryEvent, not + // as properties. Fields annotated `isMeasurement: true` are dropped by telemetry + // ingestion when sent in the properties bag. }; /** * Telemetry event sent when cancelling discovering tests @@ -2286,14 +2282,10 @@ export interface IEventNamePropertyMapping { * Coarse failure category when `failed` is true. */ failureCategory?: UnitTestRunFailureCategory; - /** - * Wall-clock duration of the run in milliseconds. - */ - durationMs?: number; - /** - * Number of test items the user asked to run. - */ - requestedCount?: number; + // NOTE: `durationMs` and `requestedCount` are measurements (see the __GDPR__ block + // above) and MUST be passed via the `measures` argument of sendTelemetryEvent, not + // as properties. Fields annotated `isMeasurement: true` are dropped by telemetry + // ingestion when sent in the properties bag. }; /** * Telemetry event sent when testing is disabled for a workspace. diff --git a/src/client/testing/testController/common/projectTestExecution.ts b/src/client/testing/testController/common/projectTestExecution.ts index 9e87fde89407..8d4f62b508ac 100644 --- a/src/client/testing/testController/common/projectTestExecution.ts +++ b/src/client/testing/testController/common/projectTestExecution.ts @@ -85,15 +85,17 @@ export async function executeTestsForProjects( traceError(`[test-by-project] Execution failed for project ${project.projectName}:`, error); } } finally { - sendTelemetryEvent(EventName.UNITTEST_RUN_DONE, undefined, { - tool: project.testProvider, - debugging: isDebugMode, - mode: 'project', - failed, - failureCategory, - durationMs: stopWatch.elapsedTime, - requestedCount: items.length, - }); + sendTelemetryEvent( + EventName.UNITTEST_RUN_DONE, + { durationMs: stopWatch.elapsedTime, requestedCount: items.length }, + { + tool: project.testProvider, + debugging: isDebugMode, + mode: 'project', + failed, + failureCategory, + }, + ); } }); diff --git a/src/client/testing/testController/common/resultResolver.ts b/src/client/testing/testController/common/resultResolver.ts index 871fe136f321..1df43211ccc8 100644 --- a/src/client/testing/testController/common/resultResolver.ts +++ b/src/client/testing/testController/common/resultResolver.ts @@ -93,14 +93,20 @@ export class PythonResultResolver implements ITestResultResolver { const cycle = this.discoveryTelemetry.complete(); const mode = cycle?.mode ?? this.discoveryTelemetry.defaultMode; const failed = payload?.status === 'error'; - sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { + // Numeric fields must be sent as measures (2nd arg), not properties. Fields + // annotated `isMeasurement: true` are dropped by telemetry ingestion when they + // arrive in the properties bag, so totalDurationMs/testCount would otherwise never + // be recorded. + const measures: Record = { testCount }; + if (cycle?.stopWatch.elapsedTime !== undefined) { + measures.totalDurationMs = cycle.stopWatch.elapsedTime; + } + sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, measures, { tool: this.testProvider, failed, mode, trigger: cycle?.trigger, failureCategory: failed ? (token?.isCancellationRequested ? 'cancelled' : 'unknown') : undefined, - totalDurationMs: cycle?.stopWatch.elapsedTime, - testCount, }); } diff --git a/src/client/testing/testController/controller.ts b/src/client/testing/testController/controller.ts index f1e679f95dd9..4eebf3da368d 100644 --- a/src/client/testing/testController/controller.ts +++ b/src/client/testing/testController/controller.ts @@ -716,13 +716,16 @@ export class PythonTestController implements ITestController, IExtensionSingleAc // Individual project failures don't block others projectsCompleted.add(project.projectUri.toString()); // Still mark as completed const cycle = (project.resultResolver as Partial).discoveryTelemetry?.complete(); - sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { + const measures: Record = {}; + if (cycle?.stopWatch.elapsedTime !== undefined) { + measures.totalDurationMs = cycle.stopWatch.elapsedTime; + } + sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, measures, { tool: project.testProvider, failed: true, mode: 'project', trigger: cycle?.trigger ?? this.currentDiscoveryTrigger, failureCategory: this.refreshCancellation.token.isCancellationRequested ? 'cancelled' : 'unknown', - totalDurationMs: cycle?.stopWatch.elapsedTime, }); } finally { project.isDiscovering = false; @@ -1048,15 +1051,17 @@ export class PythonTestController implements ITestController, IExtensionSingleAc failureCategory = token.isCancellationRequested ? 'cancelled' : 'unknown'; throw ex; } finally { - sendTelemetryEvent(EventName.UNITTEST_RUN_DONE, undefined, { - tool: provider, - debugging, - mode: 'legacy', - failed, - failureCategory, - durationMs: stopWatch.elapsedTime, - requestedCount: testItems.length, - }); + sendTelemetryEvent( + EventName.UNITTEST_RUN_DONE, + { durationMs: stopWatch.elapsedTime, requestedCount: testItems.length }, + { + tool: provider, + debugging, + mode: 'legacy', + failed, + failureCategory, + }, + ); } } diff --git a/src/client/testing/testController/workspaceTestAdapter.ts b/src/client/testing/testController/workspaceTestAdapter.ts index d43e58fd2aaf..f81a5b4a7da7 100644 --- a/src/client/testing/testController/workspaceTestAdapter.ts +++ b/src/client/testing/testController/workspaceTestAdapter.ts @@ -153,14 +153,17 @@ export class WorkspaceTestAdapter { deferred.resolve(); } catch (ex) { const cycle = (this.resultResolver as Partial).discoveryTelemetry?.complete(); - sendTelemetryEvent(EventName.UNITTEST_DISCOVERY_DONE, undefined, { - tool: this.testProvider, - failed: true, - mode: 'legacy', - trigger: cycle?.trigger ?? trigger, - failureCategory: token?.isCancellationRequested ? 'cancelled' : 'unknown', - totalDurationMs: cycle?.stopWatch.elapsedTime ?? stopWatch.elapsedTime, - }); + sendTelemetryEvent( + EventName.UNITTEST_DISCOVERY_DONE, + { totalDurationMs: cycle?.stopWatch.elapsedTime ?? stopWatch.elapsedTime }, + { + tool: this.testProvider, + failed: true, + mode: 'legacy', + trigger: cycle?.trigger ?? trigger, + failureCategory: token?.isCancellationRequested ? 'cancelled' : 'unknown', + }, + ); let cancel = token?.isCancellationRequested ? Testing.cancelUnittestDiscovery diff --git a/src/test/testing/testController/resultResolver.unit.test.ts b/src/test/testing/testController/resultResolver.unit.test.ts index c56f53750336..f10dfab803cd 100644 --- a/src/test/testing/testController/resultResolver.unit.test.ts +++ b/src/test/testing/testController/resultResolver.unit.test.ts @@ -155,7 +155,7 @@ suite('Result Resolver tests', () => { resultResolver.resolveDiscovery(payload, cancelationToken); - sinon.assert.calledWithMatch(sendTelemetryStub, EventName.UNITTEST_DISCOVERY_DONE, undefined, { + sinon.assert.calledWithMatch(sendTelemetryStub, EventName.UNITTEST_DISCOVERY_DONE, sinon.match.any, { failed: true, failureCategory: 'unknown', });