From 33b03320e79769fb591564119d61b34f9f0327da Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Tue, 28 Nov 2023 17:14:26 +0100 Subject: [PATCH 1/4] chore: disable initial showCommands from telemetry MONGOSH-1652 (#1755) * chore: use _untrackedShow for startup banners * chore: test that _untrackedShow passes telemetry option * chore: test disabling telemetry on mongo.show * chore: remove .only * chore: fix linting issues * chore: fix sent errors * chore: remove offending imports --- packages/cli-repl/src/mongosh-repl.ts | 6 +++--- packages/shell-api/src/mongo.spec.ts | 18 ++++++++++++++++++ packages/shell-api/src/mongo.ts | 14 ++++++++++---- packages/shell-api/src/shell-api.spec.ts | 12 ++++++++++++ packages/shell-api/src/shell-api.ts | 7 +++++++ 5 files changed, 50 insertions(+), 7 deletions(-) diff --git a/packages/cli-repl/src/mongosh-repl.ts b/packages/cli-repl/src/mongosh-repl.ts index b8ca4688e..0bb99e49c 100644 --- a/packages/cli-repl/src/mongosh-repl.ts +++ b/packages/cli-repl/src/mongosh-repl.ts @@ -431,9 +431,9 @@ class MongoshNodeRepl implements EvaluationListener { // https://github.com/mongodb/mongo/blob/a6df396047a77b90bf1ce9463eecffbee16fb864/src/mongo/shell/mongo_main.cpp#L1003-L1026 const { shellApi } = instanceState; const banners = await Promise.all([ - (async () => await shellApi.show('startupWarnings'))(), - (async () => await shellApi.show('automationNotices'))(), - (async () => await shellApi.show('nonGenuineMongoDBCheck'))(), + (async () => await shellApi._untrackedShow('startupWarnings'))(), + (async () => await shellApi._untrackedShow('automationNotices'))(), + (async () => await shellApi._untrackedShow('nonGenuineMongoDBCheck'))(), ]); for (const banner of banners) { if (banner.value) { diff --git a/packages/shell-api/src/mongo.spec.ts b/packages/shell-api/src/mongo.spec.ts index 704373be3..6302840a4 100644 --- a/packages/shell-api/src/mongo.spec.ts +++ b/packages/shell-api/src/mongo.spec.ts @@ -120,6 +120,24 @@ describe('Mongo', function () { instanceState.currentDb = database; }); describe('show', function () { + it('should send telemetry by default', async function () { + serviceProvider.listDatabases.resolves({ ok: 1, databases: [] }); + await mongo.show('dbs'); + + expect(bus.emit).to.have.been.calledWith('mongosh:show', { + method: 'show dbs', + }); + }); + + it('should not send telemetry when disabled', async function () { + serviceProvider.listDatabases.resolves({ ok: 1, databases: [] }); + await mongo.show('dbs', undefined, false); + + expect(bus.emit).to.not.have.been.calledWith('mongosh:show', { + method: 'show dbs', + }); + }); + ['databases', 'dbs'].forEach((t) => { describe(t, function () { it('calls serviceProvider.listDatabases on the admin database', async function () { diff --git a/packages/shell-api/src/mongo.ts b/packages/shell-api/src/mongo.ts index a5d4d7539..49e003423 100644 --- a/packages/shell-api/src/mongo.ts +++ b/packages/shell-api/src/mongo.ts @@ -363,13 +363,19 @@ export default class Mongo extends ShellApiClass { @returnsPromise @apiVersions([1]) - async show(cmd: string, arg?: string): Promise { + async show( + cmd: string, + arg?: string, + tracked = true + ): Promise { const db = this._instanceState.currentDb; // legacy shell: // https://github.com/mongodb/mongo/blob/a6df396047a77b90bf1ce9463eecffbee16fb864/src/mongo/shell/utils.js#L900-L1226 - this._instanceState.messageBus.emit('mongosh:show', { - method: `show ${cmd}`, - }); + + tracked && + this._instanceState.messageBus.emit('mongosh:show', { + method: `show ${cmd}`, + }); switch (cmd) { case 'databases': diff --git a/packages/shell-api/src/shell-api.spec.ts b/packages/shell-api/src/shell-api.spec.ts index 4f4020730..f9c1ded2a 100644 --- a/packages/shell-api/src/shell-api.spec.ts +++ b/packages/shell-api/src/shell-api.spec.ts @@ -226,6 +226,18 @@ describe('ShellApi', function () { expect(mongo.show).to.have.been.calledWith('databases'); }); }); + describe('_untrackedShow', function () { + beforeEach(async function () { + await instanceState.shellApi._untrackedShow('databases'); + }); + it('calls show with arg and without telemetry', function () { + expect(mongo.show).to.have.been.calledWith( + 'databases', + undefined, + false + ); + }); + }); describe('it', function () { it('returns empty result if no current cursor', async function () { instanceState.currentCursor = null; diff --git a/packages/shell-api/src/shell-api.ts b/packages/shell-api/src/shell-api.ts index 27bf229fe..4eaaf1b64 100644 --- a/packages/shell-api/src/shell-api.ts +++ b/packages/shell-api/src/shell-api.ts @@ -210,6 +210,13 @@ export default class ShellApi extends ShellApiClass { return await this._instanceState.currentDb._mongo.show(cmd, arg); } + @directShellCommand + @returnsPromise + @shellCommandCompleter(showCompleter) + async _untrackedShow(cmd: string, arg?: string): Promise { + return await this._instanceState.currentDb._mongo.show(cmd, arg, false); + } + @directShellCommand @returnsPromise @platforms(['CLI']) From cfe3d981130048c16c155abd4bc233548bc434cf Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 28 Nov 2023 16:17:55 +0000 Subject: [PATCH 2/4] chore: update auto-generated files (#1758) * chore: update THIRD_PARTY_NOTICES * chore: update error documentation --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- THIRD_PARTY_NOTICES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/THIRD_PARTY_NOTICES.md b/THIRD_PARTY_NOTICES.md index aa7db170e..ba8b47cca 100644 --- a/THIRD_PARTY_NOTICES.md +++ b/THIRD_PARTY_NOTICES.md @@ -1,5 +1,5 @@ The following third-party software is used by and included in **mongosh**. -This document was automatically generated on Mon Nov 27 2023. +This document was automatically generated on Tue Nov 28 2023. ## List of dependencies From 29af686e350ef23a15af9f0639386fb541fd2f4d Mon Sep 17 00:00:00 2001 From: Kevin Mas Ruiz Date: Tue, 28 Nov 2023 19:30:14 +0100 Subject: [PATCH 3/4] feat: sample sessions sent to telemetry MONGOSH-1651 (#1754) * chore: analytics sampler * chore: connect to the cli-repl * chore: fix docs * chore: optimise imports * chore: refactoring * chore: refactor so tests are moved to cli-repl * chore: reuse interface for tests * chore: forgot to force the analytics on the test * chore: always flush * chore: only sample 1% of sessions --- packages/cli-repl/src/cli-repl.spec.ts | 36 +++++++++++++++ packages/cli-repl/src/cli-repl.ts | 17 ++++--- .../logging/src/analytics-helpers.spec.ts | 44 ++++++++++++++++++- packages/logging/src/analytics-helpers.ts | 31 +++++++++++++ packages/logging/src/index.ts | 1 + 5 files changed, 122 insertions(+), 7 deletions(-) diff --git a/packages/cli-repl/src/cli-repl.spec.ts b/packages/cli-repl/src/cli-repl.spec.ts index b479ad351..f8eb21669 100644 --- a/packages/cli-repl/src/cli-repl.spec.ts +++ b/packages/cli-repl/src/cli-repl.spec.ts @@ -1208,6 +1208,7 @@ describe('CliRepl', function () { }; beforeEach(async function () { + process.env.MONGOSH_ANALYTICS_SAMPLE = 'true'; requests = []; srv = http .createServer((req, res) => { @@ -1236,6 +1237,7 @@ describe('CliRepl', function () { }); afterEach(async function () { + delete process.env.MONGOSH_ANALYTICS_SAMPLE; srv.close(); await once(srv, 'close'); setTelemetryDelay(0); @@ -1271,6 +1273,40 @@ describe('CliRepl', function () { expect(requests[0].body).to.include(process.platform); }); + it('posts analytics if the environment variable MONGOSH_ANALYTICS_SAMPLE is provided', async function () { + process.env.MONGOSH_ANALYTICS_SAMPLE = 'true'; + await cliRepl.start(await testServer.connectionString(), {}); + input.write('use somedb;\n'); + await waitEval(cliRepl.bus); + // There are warnings generated by the driver if exit is used to close + // the REPL too early. That might be worth investigating at some point. + await delay(100); + input.write('exit\n'); + await waitBus(cliRepl.bus, 'mongosh:closed'); + const useEvents = requests.flatMap((req) => + JSON.parse(req.body).batch.filter((entry) => entry.event === 'Use') + ); + expect(useEvents).to.have.lengthOf(1); + }); + + it('does not post analytics if the environment variable MONGOSH_ANALYTICS_SAMPLE is true but user disabled telemetry', async function () { + process.env.MONGOSH_ANALYTICS_SAMPLE = 'true'; + await cliRepl.start(await testServer.connectionString(), {}); + input.write('disableTelemetry()\n'); + await waitEval(cliRepl.bus); + input.write('use somedb;\n'); + await waitEval(cliRepl.bus); + // There are warnings generated by the driver if exit is used to close + // the REPL too early. That might be worth investigating at some point. + await delay(100); + input.write('exit\n'); + await waitBus(cliRepl.bus, 'mongosh:closed'); + const useEvents = requests.flatMap((req) => + JSON.parse(req.body).batch.filter((entry) => entry.event === 'Use') + ); + expect(useEvents).to.have.lengthOf(0); + }); + it('stops posting analytics data after disableTelemetry()', async function () { await cliRepl.start(await testServer.connectionString(), {}); input.write('use somedb;\n'); diff --git a/packages/cli-repl/src/cli-repl.ts b/packages/cli-repl/src/cli-repl.ts index 07d7e4394..03483b86e 100644 --- a/packages/cli-repl/src/cli-repl.ts +++ b/packages/cli-repl/src/cli-repl.ts @@ -34,6 +34,7 @@ import { setupLoggerAndTelemetry, ToggleableAnalytics, ThrottledAnalytics, + SampledAnalytics, } from '@mongosh/logging'; import type { MongoshBus } from '@mongosh/types'; import { CliUserConfig, CliUserConfigValidator } from '@mongosh/types'; @@ -500,12 +501,16 @@ export class CliRepl implements MongoshIOProvider { } as any /* axiosConfig and axiosRetryConfig are existing options, but don't have type definitions */ ); this.toggleableAnalytics = new ToggleableAnalytics( - new ThrottledAnalytics({ - target: this.segmentAnalytics, - throttle: { - rate: 30, - metadataPath: this.shellHomeDirectory.paths.shellLocalDataPath, - }, + new SampledAnalytics({ + target: new ThrottledAnalytics({ + target: this.segmentAnalytics, + throttle: { + rate: 30, + metadataPath: this.shellHomeDirectory.paths.shellLocalDataPath, + }, + }), + sampling: () => + !!process.env.MONGOSH_ANALYTICS_SAMPLE || Math.random() <= 0.01, }) ); } diff --git a/packages/logging/src/analytics-helpers.spec.ts b/packages/logging/src/analytics-helpers.spec.ts index 04454db14..9efda4e1e 100644 --- a/packages/logging/src/analytics-helpers.spec.ts +++ b/packages/logging/src/analytics-helpers.spec.ts @@ -4,7 +4,11 @@ import fs from 'fs'; import { promisify } from 'util'; import { expect } from 'chai'; import type { MongoshAnalytics } from './analytics-helpers'; -import { ToggleableAnalytics, ThrottledAnalytics } from './analytics-helpers'; +import { + ToggleableAnalytics, + ThrottledAnalytics, + SampledAnalytics, +} from './analytics-helpers'; const wait = promisify(setTimeout); @@ -245,4 +249,42 @@ describe('analytics helpers', function () { ).to.match(/^(hi,hi,hi|bye,bye,bye)$/); }); }); + + describe('SampledAnalytics', function () { + const userId = `u-${Date.now()}`; + const iEvt = { userId, traits: { platform: 'what', session_id: 'abc' } }; + const tEvt = { + userId, + event: 'hi', + properties: { mongosh_version: '1.2.3', session_id: 'abc' }, + }; + + it('should send the event forward when sampled', function () { + const analytics = new SampledAnalytics({ + target, + sampling: () => true, + }); + + expect(analytics.enabled).to.be.true; + + analytics.identify(iEvt); + analytics.track(tEvt); + + expect(events.length).to.equal(2); + }); + + it('should not send the event forward when not sampled', function () { + const analytics = new SampledAnalytics({ + target, + sampling: () => false, + }); + + expect(analytics.enabled).to.be.false; + + analytics.identify(iEvt); + analytics.track(tEvt); + + expect(events.length).to.equal(0); + }); + }); }); diff --git a/packages/logging/src/analytics-helpers.ts b/packages/logging/src/analytics-helpers.ts index 546f3b85c..dc0b162d8 100644 --- a/packages/logging/src/analytics-helpers.ts +++ b/packages/logging/src/analytics-helpers.ts @@ -378,3 +378,34 @@ export class ThrottledAnalytics implements MongoshAnalytics { }); } } + +type SampledAnalyticsOptions = { + target?: MongoshAnalytics; + sampling: () => boolean; +}; + +export class SampledAnalytics implements MongoshAnalytics { + private isEnabled: boolean; + private target: MongoshAnalytics; + + constructor(configuration: SampledAnalyticsOptions) { + this.isEnabled = configuration.sampling(); + this.target = configuration.target || new NoopAnalytics(); + } + + get enabled(): boolean { + return this.isEnabled; + } + + identify(message: AnalyticsIdentifyMessage): void { + this.isEnabled && this.target.identify(message); + } + + track(message: AnalyticsTrackMessage): void { + this.isEnabled && this.target.track(message); + } + + flush(callback: (err?: Error | undefined) => void): void { + this.target.flush(callback); + } +} diff --git a/packages/logging/src/index.ts b/packages/logging/src/index.ts index 672187842..263682d08 100644 --- a/packages/logging/src/index.ts +++ b/packages/logging/src/index.ts @@ -2,6 +2,7 @@ export { setupLoggerAndTelemetry } from './setup-logger-and-telemetry'; export { MongoshAnalytics, ToggleableAnalytics, + SampledAnalytics, NoopAnalytics, ThrottledAnalytics, } from './analytics-helpers'; From 85488c7c6dab68c1a9a787c782fb0321321fce6b Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 28 Nov 2023 18:33:50 +0000 Subject: [PATCH 4/4] chore: update auto-generated files (#1759) * chore: update AUTHORS * chore: update error documentation --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- packages/logging/AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/logging/AUTHORS b/packages/logging/AUTHORS index d6dfa8bba..e8d1b33bd 100644 --- a/packages/logging/AUTHORS +++ b/packages/logging/AUTHORS @@ -5,3 +5,4 @@ Sergey Petushkov Leonardo Rossi Le Roux Bodenstein Martin Rodriguez Reboredo +Kevin Mas Ruiz