From b88f767adce3e5cd800c93e58e1b2c6561c3b6a8 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Mon, 29 Jul 2024 18:10:04 +0200 Subject: [PATCH 1/2] fix(logging): do not accumulate telemetry events that are never sent Compass currently does not emit `evaluate-started`/`evaluate-finished` events for shell API calls. We should not gather data for the corresponding telemetry events if we never get to send them. --- packages/logging/src/setup-logger-and-telemetry.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/logging/src/setup-logger-and-telemetry.ts b/packages/logging/src/setup-logger-and-telemetry.ts index c2ce8834f..e82987970 100644 --- a/packages/logging/src/setup-logger-and-telemetry.ts +++ b/packages/logging/src/setup-logger-and-telemetry.ts @@ -632,7 +632,10 @@ export function setupLoggerAndTelemetry( const deprecatedApiCalls = new MultiSet>(); const apiCalls = new MultiSet>(); + let apiCallTrackingEnabled = false; bus.on('mongosh:api-call', function (ev: ApiEvent) { + // Only track if we have previously seen a mongosh:evaluate-started call + if (!apiCallTrackingEnabled) return; if (ev.deprecated) { deprecatedApiCalls.add({ class: ev.class, method: ev.method }); } @@ -641,6 +644,7 @@ export function setupLoggerAndTelemetry( } }); bus.on('mongosh:evaluate-started', function () { + apiCallTrackingEnabled = true; // Clear API calls before evaluation starts. This is important because // some API calls are also emitted by mongosh CLI repl internals, // but we only care about those emitted from user code (i.e. during @@ -680,6 +684,7 @@ export function setupLoggerAndTelemetry( } deprecatedApiCalls.clear(); apiCalls.clear(); + apiCallTrackingEnabled = false; }); // Log ids 1_000_000_034 through 1_000_000_042 are reserved for the From 017bc991c93cc551fb1595567f320c932618116d Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 30 Jul 2024 11:07:45 +0200 Subject: [PATCH 2/2] fixup: tests --- .../src/setup-logger-and-telemetry.spec.ts | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/packages/logging/src/setup-logger-and-telemetry.spec.ts b/packages/logging/src/setup-logger-and-telemetry.spec.ts index 76a715cec..1b2ea9e6c 100644 --- a/packages/logging/src/setup-logger-and-telemetry.spec.ts +++ b/packages/logging/src/setup-logger-and-telemetry.spec.ts @@ -1,3 +1,4 @@ +/* eslint-disable mocha/max-top-level-suites */ import { expect } from 'chai'; import { MongoLogWriter } from 'mongodb-log-writer'; import { setupLoggerAndTelemetry } from './'; @@ -561,6 +562,7 @@ describe('setupLoggerAndTelemetry', function () { expect(analyticsOutput).to.be.empty; bus.emit('mongosh:new-user', { userId, anonymousId: userId }); + bus.emit('mongosh:evaluate-started'); logOutput = []; analyticsOutput = []; @@ -704,6 +706,7 @@ describe('setupLoggerAndTelemetry', function () { logOutput = []; analyticsOutput = []; + bus.emit('mongosh:evaluate-started'); bus.emit('mongosh:api-call', { method: 'cloneDatabase', class: 'Database', @@ -724,4 +727,27 @@ describe('setupLoggerAndTelemetry', function () { }); expect(analyticsOutput).to.have.lengthOf(2); }); + + it('does not track database calls outside of evaluate-{started,finished}', function () { + setupLoggerAndTelemetry(bus, logger, analytics, {}, '1.0.0'); + expect(logOutput).to.have.lengthOf(0); + expect(analyticsOutput).to.be.empty; + + bus.emit('mongosh:new-user', { userId, anonymousId: userId }); + + logOutput = []; + analyticsOutput = []; + + bus.emit('mongosh:api-call', { + method: 'cloneDatabase', + class: 'Database', + deprecated: true, + callDepth: 0, + isAsync: true, + }); + bus.emit('mongosh:evaluate-finished'); + + expect(logOutput).to.have.lengthOf(0); + expect(analyticsOutput).to.be.empty; + }); });