diff --git a/src/extension/extension.ts b/src/extension/extension.ts index 812eb813..60047c6a 100644 --- a/src/extension/extension.ts +++ b/src/extension/extension.ts @@ -6,11 +6,16 @@ import { createOutputChannel, registerCommand } from './common/vscodeapi'; import { Commands } from './common/constants'; import { registerLogger, traceError } from './common/log/logging'; import { sendTelemetryEvent } from './telemetry'; +import { clearTelemetryReporter } from './telemetry/reporter'; import { EventName } from './telemetry/constants'; import { IExtensionApi } from './apiTypes'; import { commands } from 'vscode'; import { getDebugpyPackagePath } from './debugger/adapter/remoteLaunchers'; +export async function deactivate(): Promise { + await clearTelemetryReporter(); +} + export async function activate(context: IExtensionContext): Promise { const outputChannel = createOutputChannel('Python Debugger'); context.subscriptions.push(outputChannel, registerLogger(outputChannel)); diff --git a/src/extension/telemetry/index.ts b/src/extension/telemetry/index.ts index c2854727..13a0a1ed 100644 --- a/src/extension/telemetry/index.ts +++ b/src/extension/telemetry/index.ts @@ -2,8 +2,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import type { TelemetryReporter } from '@vscode/extension-telemetry'; - import { isTestExecution } from '../common/constants'; import { StopWatch } from '../common/utils/stopWatch'; import { ConsoleType, TriggerType } from '../types'; @@ -32,12 +30,6 @@ function isTelemetrySupported(): boolean { const sharedProperties: Record = {}; -let telemetryReporter: TelemetryReporter | undefined; - -export function clearTelemetryReporter(): void { - telemetryReporter = undefined; -} - export function sendTelemetryEvent

( eventName: E, measuresOrDurationMs?: Record | number, @@ -47,7 +39,7 @@ export function sendTelemetryEvent

{ + const reporter = telemetryReporter; + telemetryReporter = undefined; + await reporter?.dispose(); +} diff --git a/src/test/unittest/adapter/factory.unit.test.ts b/src/test/unittest/adapter/factory.unit.test.ts index b68f7c25..75487cce 100644 --- a/src/test/unittest/adapter/factory.unit.test.ts +++ b/src/test/unittest/adapter/factory.unit.test.ts @@ -27,7 +27,6 @@ import { PersistentState, PersistentStateFactory } from '../../../extension/comm import { EXTENSION_ROOT_DIR } from '../../../extension/common/constants'; import { Architecture } from '../../../extension/common/platform'; import * as pythonApi from '../../../extension/common/python'; -import * as telemetry from '../../../extension/telemetry'; import * as telemetryReporter from '../../../extension/telemetry/reporter'; import * as vscodeApi from '../../../extension/common/vscodeapi'; import { DebugConfigStrings } from '../../../extension/common/utils/localize'; @@ -79,6 +78,7 @@ suite('Debugging - Adapter Factory', () => { public static eventNames: string[] = []; public static properties: Record[] = []; public static measures: {}[] = []; + public dispose = sinon.stub().resolves(); public sendTelemetryEvent(eventName: string, properties?: {}, measures?: {}) { Reporter.eventNames.push(eventName); Reporter.properties.push(properties!); @@ -105,13 +105,13 @@ suite('Debugging - Adapter Factory', () => { factory = new DebugAdapterDescriptorFactory(instance(stateFactory)); }); - teardown(() => { + teardown(async () => { process.env.VSC_PYTHON_UNIT_TEST = oldValueOfVSC_PYTHON_UNIT_TEST; process.env.VSC_PYTHON_CI_TEST = oldValueOfVSC_PYTHON_CI_TEST; Reporter.properties = []; Reporter.eventNames = []; Reporter.measures = []; - telemetry.clearTelemetryReporter(); + await telemetryReporter.clearTelemetryReporter(); sinon.restore(); }); diff --git a/src/test/unittest/adapter/outdatedDebuggerPrompt.unit.test.ts b/src/test/unittest/adapter/outdatedDebuggerPrompt.unit.test.ts index 64091382..7b0532a3 100644 --- a/src/test/unittest/adapter/outdatedDebuggerPrompt.unit.test.ts +++ b/src/test/unittest/adapter/outdatedDebuggerPrompt.unit.test.ts @@ -11,7 +11,7 @@ import { DebugProtocol } from '@vscode/debugprotocol'; import { createDeferred } from '../../../extension/common/utils/async'; import { Common } from '../../../extension/common/utils/localize'; import { OutdatedDebuggerPromptFactory } from '../../../extension/debugger/adapter/outdatedDebuggerPrompt'; -import { clearTelemetryReporter } from '../../../extension/telemetry'; +import { clearTelemetryReporter } from '../../../extension/telemetry/reporter'; import * as vscodeapi from '../../../extension/common/vscodeapi'; import { sleep } from '../../core'; @@ -40,9 +40,9 @@ suite('Debugging - Outdated Debugger Prompt tests.', () => { promptFactory = new OutdatedDebuggerPromptFactory(); }); - teardown(() => { + teardown(async () => { + await clearTelemetryReporter(); sinon.restore(); - clearTelemetryReporter(); }); function createSession(workspaceFolder?: WorkspaceFolder): DebugSession { diff --git a/src/test/unittest/telemetry.unit.test.ts b/src/test/unittest/telemetry.unit.test.ts new file mode 100644 index 00000000..dd3d4b6c --- /dev/null +++ b/src/test/unittest/telemetry.unit.test.ts @@ -0,0 +1,97 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as telemetry from '@vscode/extension-telemetry'; +import { expect } from 'chai'; +import * as sinon from 'sinon'; +import * as constants from '../../extension/common/constants'; +import { createDeferred } from '../../extension/common/utils/async'; +import { sendTelemetryEvent } from '../../extension/telemetry'; +import { EventName } from '../../extension/telemetry/constants'; +import { clearTelemetryReporter, getTelemetryReporter } from '../../extension/telemetry/reporter'; + +const TelemetryReporter = telemetry.TelemetryReporter; + +suite('Telemetry reporter lifetime', () => { + let reporter: sinon.SinonStubbedInstance; + let constructor: sinon.SinonStub; + + setup(async () => { + await clearTelemetryReporter(); + sinon.stub(constants, 'isTestExecution').returns(false); + reporter = sinon.createStubInstance(TelemetryReporter); + reporter.dispose.resolves(); + constructor = sinon.stub(telemetry, 'TelemetryReporter').returns(reporter); + }); + + teardown(async () => { + await clearTelemetryReporter(); + sinon.restore(); + }); + + test('Usage and error events reuse the same reporter', () => { + sendTelemetryEvent(EventName.DEBUG_SUCCESS_ACTIVATION); + sendTelemetryEvent(EventName.DEBUG_SESSION_ERROR, undefined, undefined, new Error('debug failure')); + + expect(getTelemetryReporter()).to.equal(reporter); + expect(getTelemetryReporter()).to.equal(reporter); + sinon.assert.calledOnce(constructor); + sinon.assert.calledOnce(reporter.sendTelemetryEvent); + sinon.assert.calledOnce(reporter.sendTelemetryErrorEvent); + sinon.assert.notCalled(reporter.dispose); + }); + + test('Clearing disposes the reporter once and resets the next session', async () => { + sendTelemetryEvent(EventName.DEBUG_SUCCESS_ACTIVATION); + await clearTelemetryReporter(); + await clearTelemetryReporter(); + sinon.assert.calledOnce(reporter.dispose); + + const nextReporter = sinon.createStubInstance(TelemetryReporter); + nextReporter.dispose.resolves(); + constructor.returns(nextReporter); + sendTelemetryEvent(EventName.DEBUG_SUCCESS_ACTIVATION); + + expect(getTelemetryReporter()).to.equal(nextReporter); + sinon.assert.calledTwice(constructor); + sinon.assert.calledOnce(nextReporter.sendTelemetryEvent); + await clearTelemetryReporter(); + sinon.assert.calledOnce(nextReporter.dispose); + sinon.assert.calledOnce(reporter.dispose); + }); + + test('Clearing an unused reporter does not create it', async () => { + await clearTelemetryReporter(); + + sinon.assert.notCalled(constructor); + sinon.assert.notCalled(reporter.dispose); + }); + + test('Clearing resets the cache before asynchronous disposal completes', async () => { + const disposal = createDeferred(); + reporter.dispose.returns(disposal.promise); + expect(getTelemetryReporter()).to.equal(reporter); + + let completed = false; + const clearing = clearTelemetryReporter().then(() => { + completed = true; + }); + try { + await Promise.resolve(); + expect(completed).to.equal(false); + sinon.assert.calledOnce(reporter.dispose); + + const nextReporter = sinon.createStubInstance(TelemetryReporter); + nextReporter.dispose.resolves(); + constructor.returns(nextReporter); + + expect(getTelemetryReporter()).to.equal(nextReporter); + sinon.assert.calledTwice(constructor); + sinon.assert.notCalled(nextReporter.dispose); + } finally { + disposal.resolve(); + await clearing; + } + expect(completed).to.equal(true); + }); +});