From 762b7b4d77cd1f6ba8e70d7e3787a98df58ed618 Mon Sep 17 00:00:00 2001 From: Josh Pinkney <103940141+jpinkney-aws@users.noreply.github.com> Date: Wed, 2 Oct 2024 12:04:17 -0400 Subject: [PATCH] refactor(core): Remove trackPerformance from telemetry (#5708) ## Problem - withPerformance was an experiment to see if we could correlate cpu/memory with number of files ## Solution - Cpu usage/memory usage in the extension host is way too volitile so we might as well clean up this code --- License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --- .../src/shared/performance/performance.ts | 5 --- packages/core/src/shared/telemetry/spans.ts | 29 +------------- .../src/shared/telemetry/vscodeTelemetry.json | 21 +--------- .../src/shared/utilities/workspaceUtils.ts | 3 -- .../src/test/shared/telemetry/spans.test.ts | 40 ------------------- .../shared/utilities/workspaceUtils.test.ts | 2 - 6 files changed, 2 insertions(+), 98 deletions(-) diff --git a/packages/core/src/shared/performance/performance.ts b/packages/core/src/shared/performance/performance.ts index 96a73f528d0..54d108ccea4 100644 --- a/packages/core/src/shared/performance/performance.ts +++ b/packages/core/src/shared/performance/performance.ts @@ -5,7 +5,6 @@ import assert from 'assert' import { getLogger } from '../logger' -import { isWeb } from '../extensionGlobals' interface PerformanceMetrics { /** @@ -46,10 +45,6 @@ export class PerformanceTracker { constructor(private readonly name: string) {} - static enabled(name: string, trackPerformance: boolean): boolean { - return name === 'function_call' && trackPerformance && !isWeb() - } - start() { this.#startPerformance = { cpuUsage: process.cpuUsage(), diff --git a/packages/core/src/shared/telemetry/spans.ts b/packages/core/src/shared/telemetry/spans.ts index d0d2cc47ae0..1d1cbf7cb2d 100644 --- a/packages/core/src/shared/telemetry/spans.ts +++ b/packages/core/src/shared/telemetry/spans.ts @@ -23,7 +23,6 @@ import { getTelemetryResult, } from '../errors' import { entries, NumericKeys } from '../utilities/tsUtils' -import { PerformanceTracker } from '../performance/performance' import { randomUUID } from '../crypto' const AsyncLocalStorage: typeof AsyncLocalStorageClass = @@ -135,10 +134,7 @@ export type SpanOptions = { */ export class TelemetrySpan { #startTime?: Date - #options: SpanOptions & { - trackPerformance: boolean - } - #performance?: PerformanceTracker + #options: SpanOptions private readonly state: Partial = {} private readonly definition = definitions[this.name] ?? { @@ -163,10 +159,6 @@ export class TelemetrySpan { // do emit by default emit: options?.emit === undefined ? true : options.emit, functionId: options?.functionId, - trackPerformance: PerformanceTracker.enabled( - this.name, - this.definition.trackPerformance && (options?.emit ?? false) // only track the performance if we are also emitting - ), } this.metricId = randomUUID() @@ -209,10 +201,6 @@ export class TelemetrySpan { */ public start(): this { this.#startTime = new globals.clock.Date() - if (this.#options.trackPerformance) { - ;(this.#performance ??= new PerformanceTracker(this.name)).start() - } - return this } @@ -224,21 +212,6 @@ export class TelemetrySpan { public stop(err?: unknown): void { const duration = this.startTime !== undefined ? globals.clock.Date.now() - this.startTime.getTime() : undefined - if (this.#options.trackPerformance) { - // TODO add these to the global metrics, right now it just forces them in the telemetry and ignores the type - // if someone enables this action - const performanceMetrics = this.#performance?.stop() - if (performanceMetrics) { - this.record({ - userCpuUsage: performanceMetrics.userCpuUsage, - systemCpuUsage: performanceMetrics.systemCpuUsage, - heapTotal: performanceMetrics.heapTotal, - functionName: this.#options.functionId?.name ?? this.name, - architecture: process.arch, - } as any) - } - } - if (this.#options.emit) { this.emit({ duration, diff --git a/packages/core/src/shared/telemetry/vscodeTelemetry.json b/packages/core/src/shared/telemetry/vscodeTelemetry.json index 1f7e82ab060..85b42d98daf 100644 --- a/packages/core/src/shared/telemetry/vscodeTelemetry.json +++ b/packages/core/src/shared/telemetry/vscodeTelemetry.json @@ -307,16 +307,6 @@ "type": "string", "description": "The CPU architecture" }, - { - "name": "totalFileSizeInMB", - "type": "int", - "description": "The total size of all files being sent to Amazon Q" - }, - { - "name": "totalFiles", - "type": "int", - "description": "The total number of files being sent to Amazon Q" - }, { "name": "traceId", "type": "string", @@ -1165,14 +1155,6 @@ "type": "systemCpuUsage", "required": false }, - { - "type": "totalFiles", - "required": false - }, - { - "type": "totalFileSizeInMB", - "required": false - }, { "type": "architecture", "required": false @@ -1186,8 +1168,7 @@ "required": true } ], - "passive": true, - "trackPerformance": true + "passive": true }, { "name": "webview_load", diff --git a/packages/core/src/shared/utilities/workspaceUtils.ts b/packages/core/src/shared/utilities/workspaceUtils.ts index 4f28b0644af..c60a79bd885 100644 --- a/packages/core/src/shared/utilities/workspaceUtils.ts +++ b/packages/core/src/shared/utilities/workspaceUtils.ts @@ -344,7 +344,6 @@ export async function collectFiles( } let totalSizeBytes = 0 - let totalFiles = 0 for (const rootPath of sourcePaths) { const allFiles = await vscode.workspace.findFiles( new vscode.RelativePattern(rootPath, '**'), @@ -352,7 +351,6 @@ export async function collectFiles( ) const files = respectGitIgnore ? await filterOutGitignoredFiles(rootPath, allFiles) : allFiles - totalFiles += files.length for (const file of files) { const relativePath = getWorkspaceRelativePath(file.fsPath, { workspaceFolders }) @@ -385,7 +383,6 @@ export async function collectFiles( }) } } - span.record({ totalFiles, totalFileSizeInMB: totalSizeBytes / (1024 * 1024) }) return storage }, { diff --git a/packages/core/src/test/shared/telemetry/spans.test.ts b/packages/core/src/test/shared/telemetry/spans.test.ts index d35d2435960..4a6efe1c95f 100644 --- a/packages/core/src/test/shared/telemetry/spans.test.ts +++ b/packages/core/src/test/shared/telemetry/spans.test.ts @@ -14,7 +14,6 @@ import { sleep } from '../../../shared' import { withTelemetryContext } from '../../../shared/telemetry/util' import { SinonSandbox } from 'sinon' import sinon from 'sinon' -import { stubPerformance } from '../../utilities/performance' import * as crypto from '../../../shared/crypto' describe('TelemetrySpan', function () { @@ -77,23 +76,6 @@ describe('TelemetrySpan', function () { { result: 'Succeeded', duration: 100 }, ]) }) - - it('records performance', function () { - const { expectedUserCpuUsage, expectedSystemCpuUsage, expectedHeapTotal } = stubPerformance(sandbox) - const span = new TelemetrySpan('function_call', { - emit: true, - }) - span.start() - clock.tick(90) - span.stop() - assertTelemetry('function_call', { - userCpuUsage: expectedUserCpuUsage, - systemCpuUsage: expectedSystemCpuUsage, - heapTotal: expectedHeapTotal, - duration: 90, - result: 'Succeeded', - }) - }) }) describe('TelemetryTracer', function () { @@ -269,28 +251,6 @@ describe('TelemetryTracer', function () { assert.match(metric.requestId ?? '', /[a-z0-9-]+/) }) - it('records performance', function () { - clock = installFakeClock() - const { expectedUserCpuUsage, expectedSystemCpuUsage, expectedHeapTotal } = stubPerformance(sandbox) - tracer.run( - 'function_call', - () => { - clock?.tick(90) - }, - { - emit: true, - } - ) - - assertTelemetry('function_call', { - userCpuUsage: expectedUserCpuUsage, - systemCpuUsage: expectedSystemCpuUsage, - heapTotal: expectedHeapTotal, - duration: 90, - result: 'Succeeded', - }) - }) - describe('nested run()', function () { let uuidStub: sinon.SinonStub const nestedName = 'nested_metric' as MetricName diff --git a/packages/core/src/testInteg/shared/utilities/workspaceUtils.test.ts b/packages/core/src/testInteg/shared/utilities/workspaceUtils.test.ts index 1034422a018..93992bc425b 100644 --- a/packages/core/src/testInteg/shared/utilities/workspaceUtils.test.ts +++ b/packages/core/src/testInteg/shared/utilities/workspaceUtils.test.ts @@ -301,8 +301,6 @@ describe('collectFiles', function () { ) assertTelemetry('function_call', { - totalFiles: 8, - totalFileSizeInMB: 0.0002593994140625, functionName: 'collectFiles', }) })