From 5c5b6efba99597ef9636d9b8f04c7a55bd99912e Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 10 Apr 2024 12:37:28 +1000 Subject: [PATCH 1/4] Detect changes instead of querying every time --- .../jupyterCellOutputMimeTypeTracker.ts | 18 ++++++++++++++---- src/platform/telemetry/index.ts | 13 ++++++++++++- src/standalone/import-export/importTracker.ts | 13 +++++++++---- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/notebooks/outputs/jupyterCellOutputMimeTypeTracker.ts b/src/notebooks/outputs/jupyterCellOutputMimeTypeTracker.ts index 2bc49f1efb9..4d0bad5ded6 100644 --- a/src/notebooks/outputs/jupyterCellOutputMimeTypeTracker.ts +++ b/src/notebooks/outputs/jupyterCellOutputMimeTypeTracker.ts @@ -15,7 +15,12 @@ import { JupyterNotebookView } from '../../platform/common/constants'; import { dispose } from '../../platform/common/utils/lifecycle'; import { IDisposable, IDisposableRegistry } from '../../platform/common/types'; import { isJupyterNotebook } from '../../platform/common/utils'; -import { ResourceTypeTelemetryProperty, sendTelemetryEvent, Telemetry } from '../../telemetry'; +import { + onDidChangeTelemetryEnablement, + ResourceTypeTelemetryProperty, + sendTelemetryEvent, + Telemetry +} from '../../telemetry'; import { isTelemetryDisabled } from '../../telemetry'; /** @@ -25,14 +30,13 @@ import { isTelemetryDisabled } from '../../telemetry'; export class CellOutputMimeTypeTracker implements IExtensionSyncActivationService, IDisposable { private sentMimeTypes: Set = new Set(); private readonly disposables: IDisposable[] = []; - private get isTelemetryDisabled() { - return isTelemetryDisabled(); - } + private isTelemetryDisabled: boolean; constructor(@inject(IDisposableRegistry) disposables: IDisposableRegistry) { disposables.push(this); } public activate() { + this.isTelemetryDisabled = isTelemetryDisabled(); workspace.onDidOpenNotebookDocument(this.onDidOpenCloseDocument, this, this.disposables); workspace.onDidCloseNotebookDocument(this.onDidOpenCloseDocument, this, this.disposables); workspace.onDidSaveNotebookDocument(this.onDidOpenCloseDocument, this, this.disposables); @@ -41,6 +45,12 @@ export class CellOutputMimeTypeTracker implements IExtensionSyncActivationServic this, this.disposables ); + this.disposables.push( + onDidChangeTelemetryEnablement((enabled) => { + this.isTelemetryDisabled = enabled; + }), + this + ); } public dispose() { diff --git a/src/platform/telemetry/index.ts b/src/platform/telemetry/index.ts index 7af62df13c8..f4f53656bab 100644 --- a/src/platform/telemetry/index.ts +++ b/src/platform/telemetry/index.ts @@ -8,7 +8,7 @@ import { StopWatch } from '../common/utils/stopWatch'; import { ExcludeType, noop, PickType, UnionToIntersection } from '../common/utils/misc'; import { populateTelemetryWithErrorInfo } from '../errors'; import { TelemetryEventInfo, IEventNamePropertyMapping } from '../../telemetry'; -import { workspace } from 'vscode'; +import { workspace, type Disposable } from 'vscode'; /** * TODO@rebornix @@ -45,6 +45,17 @@ export function isTelemetryDisabled(): boolean { return settings.globalValue === false ? true : false; } +export function onDidChangeTelemetryEnablement(handler: (enabled: boolean) => void): Disposable { + return workspace.onDidChangeConfiguration((e) => { + if (!e.affectsConfiguration('telemetry')) { + return; + } + const settings = workspace.getConfiguration('telemetry').inspect('enableTelemetry')!; + const enabled = settings.globalValue === false ? true : false; + handler(enabled); + }); +} + const sharedProperties: Partial = {}; /** * Set shared properties for all telemetry events. diff --git a/src/standalone/import-export/importTracker.ts b/src/standalone/import-export/importTracker.ts index 594c15a996a..6bb5a13bbb2 100644 --- a/src/standalone/import-export/importTracker.ts +++ b/src/standalone/import-export/importTracker.ts @@ -12,7 +12,7 @@ import { notebooks, workspace } from 'vscode'; -import { ResourceTypeTelemetryProperty, sendTelemetryEvent } from '../../telemetry'; +import { ResourceTypeTelemetryProperty, onDidChangeTelemetryEnablement, sendTelemetryEvent } from '../../telemetry'; import { IExtensionSyncActivationService } from '../../platform/activation/types'; import { isCI, isTestExecution, JupyterNotebookView, PYTHON_LANGUAGE } from '../../platform/common/constants'; import { dispose } from '../../platform/common/utils/lifecycle'; @@ -64,11 +64,10 @@ export class ImportTracker implements IExtensionSyncActivationService, IDisposab private pendingChecks = new ResourceMap(); private disposables: IDisposable[] = []; private sentMatches = new Set(); - private get isTelemetryDisabled() { - return isTelemetryDisabled(); - } + private isTelemetryDisabled: boolean; constructor(@inject(IDisposableRegistry) disposables: IDisposableRegistry) { disposables.push(this); + this.isTelemetryDisabled = isTelemetryDisabled(); workspace.onDidOpenNotebookDocument( (t) => this.onOpenedOrClosedNotebookDocument(t, 'onOpenCloseOrSave'), this.disposables @@ -90,6 +89,12 @@ export class ImportTracker implements IExtensionSyncActivationService, IDisposab this, disposables ); + this.disposables.push( + onDidChangeTelemetryEnablement((enabled) => { + this.isTelemetryDisabled = enabled; + }), + this + ); } public dispose() { From bd3c4f16af6aee4e8337f8130430dd3b0fdb2212 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 10 Apr 2024 13:14:07 +1000 Subject: [PATCH 2/4] Fix tests --- src/platform/common/utils/events.ts | 3 +++ src/standalone/import-export/importTracker.unit.test.ts | 2 ++ 2 files changed, 5 insertions(+) diff --git a/src/platform/common/utils/events.ts b/src/platform/common/utils/events.ts index d34e16f8bb8..d4195c06581 100644 --- a/src/platform/common/utils/events.ts +++ b/src/platform/common/utils/events.ts @@ -3,6 +3,7 @@ import type { Event } from 'vscode'; import { IDisposable } from '../types'; +import { EmptyDisposable } from './lifecycle'; /** * Given an event, returns another event which only fires once. @@ -43,3 +44,5 @@ export function once(event: Event): Event { export function toPromise(event: Event, thisArgs: any = null, disposables?: IDisposable[]): Promise { return new Promise((resolve) => once(event)(resolve, thisArgs, disposables)); } + +export const EmptyEvent: Event = () => EmptyDisposable; diff --git a/src/standalone/import-export/importTracker.unit.test.ts b/src/standalone/import-export/importTracker.unit.test.ts index d226d920f88..bd7aee9bb64 100644 --- a/src/standalone/import-export/importTracker.unit.test.ts +++ b/src/standalone/import-export/importTracker.unit.test.ts @@ -30,6 +30,7 @@ import { ResourceTypeTelemetryProperty, getTelemetryReporter } from '../../telem import { waitForCondition } from '../../test/common'; import { createMockedNotebookDocument } from '../../test/datascience/editor-integration/helpers'; import { mockedVSCodeNamespaces } from '../../test/vscode-mock'; +import { EmptyEvent } from '../../platform/common/utils/events'; [true, false].forEach((useCustomMetadata) => { suite(`Import Tracker (${useCustomMetadata ? 'with custom metadata' : 'without custom metadata'})`, async () => { @@ -140,6 +141,7 @@ import { mockedVSCodeNamespaces } from '../../test/vscode-mock'; onDidChangeNotebookCellExecutionState.event ); when(mockedVSCodeNamespaces.workspace.notebookDocuments).thenReturn([]); + when(mockedVSCodeNamespaces.workspace.onDidChangeConfiguration).thenReturn(EmptyEvent); when(mockedVSCodeNamespaces.workspace.getConfiguration('telemetry')).thenReturn({ inspect: () => { return { From 9cab51761a2bb4d546c1759f4caef38013b8b5bc Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 10 Apr 2024 13:31:50 +1000 Subject: [PATCH 3/4] oops --- src/platform/common/utils/events.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/platform/common/utils/events.ts b/src/platform/common/utils/events.ts index d4195c06581..a84cd13eed6 100644 --- a/src/platform/common/utils/events.ts +++ b/src/platform/common/utils/events.ts @@ -45,4 +45,5 @@ export function toPromise(event: Event, thisArgs: any = null, disposables? return new Promise((resolve) => once(event)(resolve, thisArgs, disposables)); } +// eslint-disable-next-line @typescript-eslint/no-explicit-any export const EmptyEvent: Event = () => EmptyDisposable; From 6baf45bc261610a654928f18c4e1b800eedba919 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 10 Apr 2024 13:35:29 +1000 Subject: [PATCH 4/4] Fix tests --- src/platform/common/utils/lifecycle.ts | 4 ++-- src/standalone/import-export/importTracker.unit.test.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/platform/common/utils/lifecycle.ts b/src/platform/common/utils/lifecycle.ts index 0a5793841bc..9b1f7df2fda 100644 --- a/src/platform/common/utils/lifecycle.ts +++ b/src/platform/common/utils/lifecycle.ts @@ -7,11 +7,11 @@ import { once } from './functional'; import { Iterable } from './iterable'; let disposableTracker: IDisposable[] | undefined = undefined; -export const EmptyDisposable = Object.freeze({ +export const EmptyDisposable = { dispose: () => { /** */ } -}); +}; export function setDisposableTracker(tracker: IDisposable[] | undefined): void { disposableTracker = tracker; diff --git a/src/standalone/import-export/importTracker.unit.test.ts b/src/standalone/import-export/importTracker.unit.test.ts index bd7aee9bb64..4846f2b97ae 100644 --- a/src/standalone/import-export/importTracker.unit.test.ts +++ b/src/standalone/import-export/importTracker.unit.test.ts @@ -150,7 +150,7 @@ import { EmptyEvent } from '../../platform/common/utils/events'; }; } } as any); - importTracker = new ImportTracker(disposables); + importTracker = new ImportTracker([]); }); teardown(() => { sinon.restore();