refactor(cli-repl): use a object-oriented structure, hook loggers at initialization

I think this refactor provides much better readability and provides an easier interface for us to test and debug the functionality of this. This also moves the hookLoggers call to the point of initialization as

Author: gagik

packages/cli-repl/src/cli-repl.spec.ts

@@ -1326,7 +1326,7 @@ describe('CliRepl', function () {
 
         expect(cliRepl.getConfig('disableLogging')).is.false;
 
-        expect(emitSpy).calledWith('mongosh:log-initialized');
+        expect(emitSpy).calledWith('mongosh:logger-initialized');
         expect(cliRepl.logWriter).is.instanceOf(MongoLogWriter);
       });
 
@@ -1339,7 +1339,7 @@ describe('CliRepl', function () {
         expect(cliRepl.getConfig('disableLogging')).is.true;
 
         expect(emitSpy).called;
-        expect(emitSpy).not.calledWith('mongosh:log-initialized');
+        expect(emitSpy).not.calledWith('mongosh:logger-initialized');
         expect(cliRepl.logWriter).is.undefined;
       });
     });

packages/cli-repl/src/cli-repl.ts

@@ -31,10 +31,10 @@ import { MongoLogManager, mongoLogId } from 'mongodb-log-writer';
 import type { MongoshNodeReplOptions, MongoshIOProvider } from './mongosh-repl';
 import MongoshNodeRepl from './mongosh-repl';
 import {
-  setupLoggerAndTelemetry,
   ToggleableAnalytics,
   ThrottledAnalytics,
   SampledAnalytics,
+  MongoshLoggingAndTelemetry,
 } from '@mongosh/logging';
 import type { MongoshBus } from '@mongosh/types';
 import {
@@ -53,7 +53,6 @@ import type {
   DevtoolsProxyOptions,
 } from '@mongodb-js/devtools-proxy-support';
 import { useOrCreateAgent } from '@mongodb-js/devtools-proxy-support';
-import { setupMongoLogWriter } from '@mongosh/logging/lib/setup-logger-and-telemetry';
 
 /**
  * Connecting text key.
@@ -258,7 +257,7 @@ export class CliRepl implements MongoshIOProvider {
   }
 
   /** Setup log writer and start logging. */
-  private async startLogging() {
+  private async startLogging(loggingAndTelemetry: MongoshLoggingAndTelemetry) {
     await this.logManager.cleanupOldLogFiles();
     markTime(TimingCategories.Logging, 'cleaned up log files');
     const logger = await this.logManager.createLogWriter();
@@ -268,13 +267,10 @@ export class CliRepl implements MongoshIOProvider {
     }
 
     this.logWriter = logger;
-    this.logWriter = logger;
-    setupMongoLogWriter(logger);
-    this.logWriter = logger;
-    setupMongoLogWriter(logger);
+    loggingAndTelemetry.setupLogger(logger);
+
     markTime(TimingCategories.Logging, 'instantiated log writer');
-    setupMongoLogWriter(logger);
-    this.bus.emit('mongosh:log-initialized');
+    this.bus.emit('mongosh:logger-initialized');
     logger.info('MONGOSH', mongoLogId(1_000_000_000), 'log', 'Starting log', {
       execPath: process.execPath,
       envInfo: redactSensitiveData(this.getLoggedEnvironmentVariables()),
@@ -346,7 +342,7 @@ export class CliRepl implements MongoshIOProvider {
 
     markTime(TimingCategories.Telemetry, 'created analytics instance');
 
-    setupLoggerAndTelemetry(
+    const loggingAndTelemetry = new MongoshLoggingAndTelemetry(
       this.bus,
       this.toggleableAnalytics,
       {
@@ -357,6 +353,8 @@ export class CliRepl implements MongoshIOProvider {
       },
       version
     );
+    loggingAndTelemetry.setup();
+
     markTime(TimingCategories.Telemetry, 'completed telemetry setup');
 
     if (analyticsSetupError) {
@@ -377,7 +375,7 @@ export class CliRepl implements MongoshIOProvider {
 
     const disableLogging = this.getConfig('disableLogging');
     if (disableLogging !== true) {
-      await this.startLogging();
+      await this.startLogging(loggingAndTelemetry);
     }
 
     // Needs to happen after loading the mongosh config file(s)

packages/logging/src/helpers.spec.ts

@@ -0,0 +1,26 @@
+import { expect } from 'chai';
+import { toSnakeCase } from './helpers';
+
+describe('logging helpers', function () {
+  describe('toSnakeCase', function () {
+    const useCases = [
+      { input: 'MongoDB REPL', output: 'mongo_db_repl' },
+      {
+        input: 'Node.js REPL Instantiation',
+        output: 'node_js_repl_instantiation',
+      },
+      { input: 'A', output: 'a' },
+      {
+        input: 'OneLongThingInPascalCase',
+        output: 'one_long_thing_in_pascal_case',
+      },
+      { input: 'Removes .Dots in Node.js', output: 'removes_dots_in_node_js' },
+    ];
+
+    for (const { input, output } of useCases) {
+      it(`should convert ${input} to ${output}`, function () {
+        expect(toSnakeCase(input)).to.equal(output);
+      });
+    }
+  });
+});

packages/logging/src/multi-set.ts renamed to packages/logging/src/helpers.ts

@@ -1,4 +1,4 @@
-export { setupLoggerAndTelemetry } from './setup-logger-and-telemetry';
+export { MongoshLoggingAndTelemetry } from './logging-and-telemetry';
 export {
   MongoshAnalytics,
   ToggleableAnalytics,

packages/logging/src/index.ts

@@ -1,58 +1,30 @@
 /* eslint-disable mocha/max-top-level-suites */
 import { expect } from 'chai';
 import { MongoLogWriter } from 'mongodb-log-writer';
-import { setupLoggerAndTelemetry } from './';
 import { EventEmitter } from 'events';
 import { MongoshInvalidInputError } from '@mongosh/errors';
 import type { MongoshBus } from '@mongosh/types';
-import { setupMongoLogWriter, toSnakeCase } from './setup-logger-and-telemetry';
 import type { Writable } from 'stream';
+import { MongoshLoggingAndTelemetry } from './logging-and-telemetry';
 
-describe('toSnakeCase', function () {
-  const useCases = [
-    { input: 'MongoDB REPL', output: 'mongo_db_repl' },
-    {
-      input: 'Node.js REPL Instantiation',
-      output: 'node_js_repl_instantiation',
-    },
-    { input: 'A', output: 'a' },
-    {
-      input: 'OneLongThingInPascalCase',
-      output: 'one_long_thing_in_pascal_case',
-    },
-    { input: 'Removes .Dots in Node.js', output: 'removes_dots_in_node_js' },
-  ];
-
-  for (const { input, output } of useCases) {
-    it(`should convert ${input} to ${output}`, function () {
-      expect(toSnakeCase(input)).to.equal(output);
-    });
-  }
-});
-
-describe('setupLoggerAndTelemetry', function () {
+describe('MongoshLoggingAndTelemetry', function () {
   let logOutput: any[];
   let analyticsOutput: ['identify' | 'track' | 'log', any][];
   let bus: MongoshBus;
 
   const userId = '53defe995fa47e6c13102d9d';
   const logId = '5fb3c20ee1507e894e5340f3';
 
-  setupMongoLogWriter(
-    new MongoLogWriter({
-      logId,
-      logFilePath: `/tmp/${logId}_log`,
-      target: {
-        write(chunk: string, cb: () => void) {
-          logOutput.push(JSON.parse(chunk));
-          cb();
-        },
-        end(cb: () => void) {
-          cb();
-        },
-      } as Writable,
-    })
-  );
+  const logger = new MongoLogWriter(logId, `/tmp/${logId}_log`, {
+    write(chunk: string, cb: () => void) {
+      logOutput.push(JSON.parse(chunk));
+      cb();
+    },
+    end(cb: () => void) {
+      cb();
+    },
+  } as Writable);
+
   const analytics = {
     identify(info: any) {
       analyticsOutput.push(['identify', info]);
@@ -71,87 +43,41 @@ describe('setupLoggerAndTelemetry', function () {
     bus = new EventEmitter();
   });
 
-  it('accepts user ID at setup', function () {
-    setupLoggerAndTelemetry(
+  it('throws when trying to setup writer prematurely', function () {
+    const loggingAndTelemetry = new MongoshLoggingAndTelemetry(
       bus,
       analytics,
-      { userId },
       {
         platform: process.platform,
         arch: process.arch,
       },
       '1.0.0'
     );
-    expect(logOutput).to.have.lengthOf(0);
-    expect(analyticsOutput).to.be.empty;
-
-    bus.emit('mongosh:connect', {
-      uri: 'mongodb://localhost/',
-      is_localhost: true,
-      is_atlas: false,
-      resolved_hostname: 'localhost',
-      node_version: 'v12.19.0',
-    });
-
-    expect(analyticsOutput).to.deep.equal([
-      [
-        'track',
-        {
-          anonymousId: userId,
-          event: 'New Connection',
-          properties: {
-            mongosh_version: '1.0.0',
-            session_id: logId,
-            is_localhost: true,
-            is_atlas: false,
-            atlas_hostname: null,
-            node_version: 'v12.19.0',
-          },
-        },
-      ],
-    ]);
-  });
 
-  it('throws if an event occurs before the user is set', function () {
-    setupLoggerAndTelemetry(
-      bus,
-      analytics,
-      {},
-      {
-        platform: process.platform,
-        arch: process.arch,
-      },
-      '1.0.0'
+    expect(() => loggingAndTelemetry.setupLogger(logger)).throws(
+      'Run setup() before setting up the log writer.'
     );
-    expect(logOutput).to.have.lengthOf(0);
-    expect(analyticsOutput).to.be.empty;
-
-    expect(() =>
-      bus.emit('mongosh:connect', {
-        uri: 'mongodb://localhost/',
-        is_localhost: true,
-        is_atlas: false,
-        resolved_hostname: 'localhost',
-        node_version: 'v12.19.0',
-      })
-    ).throws('No telemetry identity found');
   });
 
   it('tracks new local connection events', function () {
-    setupLoggerAndTelemetry(
+    const loggingAndTelemetry = new MongoshLoggingAndTelemetry(
       bus,
       analytics,
-      {},
       {
         platform: process.platform,
         arch: process.arch,
       },
       '1.0.0'
     );
+
+    loggingAndTelemetry.setup();
+    loggingAndTelemetry.setupLogger(logger);
+
     expect(logOutput).to.have.lengthOf(0);
     expect(analyticsOutput).to.be.empty;
 
     bus.emit('mongosh:new-user', { userId, anonymousId: userId });
+    bus.emit('mongosh:logger-initialized');
 
     bus.emit('mongosh:connect', {
       uri: 'mongodb://localhost/',
@@ -199,20 +125,24 @@ describe('setupLoggerAndTelemetry', function () {
   });
 
   it('tracks new atlas connection events', function () {
-    setupLoggerAndTelemetry(
+    const loggingAndTelemetry = new MongoshLoggingAndTelemetry(
       bus,
       analytics,
-      {},
       {
         platform: process.platform,
         arch: process.arch,
       },
       '1.0.0'
     );
+
+    loggingAndTelemetry.setup();
+    loggingAndTelemetry.setupLogger(logger);
+
     expect(logOutput).to.have.lengthOf(0);
     expect(analyticsOutput).to.be.empty;
 
     bus.emit('mongosh:new-user', { userId, anonymousId: userId });
+    bus.emit('mongosh:logger-initialized');
 
     bus.emit('mongosh:connect', {
       uri: 'mongodb://test-data-sets-a011bb.mongodb.net/',
@@ -264,20 +194,25 @@ describe('setupLoggerAndTelemetry', function () {
   });
 
   it('tracks a sequence of events', function () {
-    setupLoggerAndTelemetry(
+    const loggingAndTelemetry = new MongoshLoggingAndTelemetry(
       bus,
       analytics,
-      {},
       {
         platform: process.platform,
         arch: process.arch,
       },
       '1.0.0'
     );
+
+    loggingAndTelemetry.setup();
+    loggingAndTelemetry.setupLogger(logger);
+
     expect(logOutput).to.have.lengthOf(0);
     expect(analyticsOutput).to.be.empty;
 
     bus.emit('mongosh:new-user', { userId, anonymousId: userId });
+    bus.emit('mongosh:logger-initialized');
+
     bus.emit('mongosh:update-user', { userId, anonymousId: userId });
     bus.emit('mongosh:start-session', {
       isInteractive: true,
@@ -728,11 +663,25 @@ describe('setupLoggerAndTelemetry', function () {
   });
 
   it('buffers deprecated API calls', function () {
-    setupLoggerAndTelemetry(bus, analytics, {}, {}, '1.0.0');
+    const loggingAndTelemetry = new MongoshLoggingAndTelemetry(
+      bus,
+      analytics,
+      {
+        platform: process.platform,
+        arch: process.arch,
+      },
+      '1.0.0'
+    );
+
+    loggingAndTelemetry.setup();
+    loggingAndTelemetry.setupLogger(logger);
+
     expect(logOutput).to.have.lengthOf(0);
     expect(analyticsOutput).to.be.empty;
 
     bus.emit('mongosh:new-user', { userId, anonymousId: userId });
+    bus.emit('mongosh:logger-initialized');
+
     bus.emit('mongosh:evaluate-started');
 
     logOutput = [];
@@ -900,7 +849,19 @@ describe('setupLoggerAndTelemetry', function () {
   });
 
   it('does not track database calls outside of evaluate-{started,finished}', function () {
-    setupLoggerAndTelemetry(bus, analytics, {}, {}, '1.0.0');
+    const loggingAndTelemetry = new MongoshLoggingAndTelemetry(
+      bus,
+      analytics,
+      {
+        platform: process.platform,
+        arch: process.arch,
+      },
+      '1.0.0'
+    );
+
+    loggingAndTelemetry.setup();
+    loggingAndTelemetry.setupLogger(logger);
+
     expect(logOutput).to.have.lengthOf(0);
     expect(analyticsOutput).to.be.empty;