From 5c3ceb93b2d417afbebab33981edda2962754f36 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Thu, 22 May 2025 15:53:21 -0400 Subject: [PATCH 1/6] chore(data-service): add a log whenever connecting throws and include connection id --- packages/data-service/src/data-service.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/data-service/src/data-service.ts b/packages/data-service/src/data-service.ts index 2d2b3a12a0e..ef404e344cd 100644 --- a/packages/data-service/src/data-service.ts +++ b/packages/data-service/src/data-service.ts @@ -1580,6 +1580,7 @@ class DataServiceImpl extends WithLogContext implements DataService { this._isConnecting = true; this._logger.info(mongoLogId(1_001_000_014), 'Connecting', { + connectionId: this._id, url: redactConnectionString(this._connectionOptions.connectionString), csfle: this._csfleLogInformation(this._connectionOptions.fleOptions), }); @@ -1599,11 +1600,16 @@ class DataServiceImpl extends WithLogContext implements DataService { }); const attr = { + connectionId: this._id, isWritable: this.isWritable(), isMongos: this.isMongos(), }; - this._logger.info(mongoLogId(1_001_000_015), 'Connected', attr); + this._logger.info( + mongoLogId(1_001_000_015), + 'Connecting Succeeded', + attr + ); debug('connected!', attr); state.oidcPlugin.logger.on('mongodb-oidc-plugin:state-updated', () => { @@ -1626,6 +1632,15 @@ class DataServiceImpl extends WithLogContext implements DataService { this, this._crudClient ); + } catch (error) { + this._logger.info(mongoLogId(1_001_000_359), 'Connecting Error', { + connectionId: this._id, + error: + error && typeof error === 'object' && 'message' in error + ? error?.message + : 'unknown error', + }); + throw error; } finally { this._isConnecting = false; } From b5ef25deb378d7ca86bcf268460fc23797a2901e Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Thu, 22 May 2025 16:45:12 -0400 Subject: [PATCH 2/6] chore: update test --- packages/compass-e2e-tests/tests/logging.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/compass-e2e-tests/tests/logging.test.ts b/packages/compass-e2e-tests/tests/logging.test.ts index 7506a0e7655..3ba2544e5dd 100644 --- a/packages/compass-e2e-tests/tests/logging.test.ts +++ b/packages/compass-e2e-tests/tests/logging.test.ts @@ -284,7 +284,7 @@ describe('Logging and Telemetry integration', function () { c: 'COMPASS-DATA-SERVICE', id: 1_001_000_015, ctx: 'Connection 0', - msg: 'Connected', + msg: 'Connecting Succeeded', attr: { isMongos: false, isWritable: true, From 2e765805d9bccaf74a0ca56f570f699aaff64de3 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Thu, 22 May 2025 16:50:22 -0400 Subject: [PATCH 3/6] chore: messages --- packages/compass-e2e-tests/tests/logging.test.ts | 2 +- packages/data-service/src/data-service.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/compass-e2e-tests/tests/logging.test.ts b/packages/compass-e2e-tests/tests/logging.test.ts index 3ba2544e5dd..8d7e016a420 100644 --- a/packages/compass-e2e-tests/tests/logging.test.ts +++ b/packages/compass-e2e-tests/tests/logging.test.ts @@ -206,7 +206,7 @@ describe('Logging and Telemetry integration', function () { c: 'COMPASS-DATA-SERVICE', id: 1_001_000_014, ctx: 'Connection 0', - msg: 'Connecting', + msg: 'Connecting Started', attr: (actual: any) => { expect(actual.url).to.match(/^mongodb:\/\/127.0.0.1:27091/); expect(actual.csfle).to.equal(null); diff --git a/packages/data-service/src/data-service.ts b/packages/data-service/src/data-service.ts index ef404e344cd..fdf55005f69 100644 --- a/packages/data-service/src/data-service.ts +++ b/packages/data-service/src/data-service.ts @@ -1579,7 +1579,7 @@ class DataServiceImpl extends WithLogContext implements DataService { debug('connecting...'); this._isConnecting = true; - this._logger.info(mongoLogId(1_001_000_014), 'Connecting', { + this._logger.info(mongoLogId(1_001_000_014), 'Connecting Started', { connectionId: this._id, url: redactConnectionString(this._connectionOptions.connectionString), csfle: this._csfleLogInformation(this._connectionOptions.fleOptions), From 77d12088aeaa7641912058688d94895b269b8d18 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 23 May 2025 09:45:12 -0400 Subject: [PATCH 4/6] chore: fix test --- packages/compass-e2e-tests/tests/logging.test.ts | 2 ++ packages/data-service/src/data-service.ts | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/compass-e2e-tests/tests/logging.test.ts b/packages/compass-e2e-tests/tests/logging.test.ts index 8d7e016a420..b17ee0ebc42 100644 --- a/packages/compass-e2e-tests/tests/logging.test.ts +++ b/packages/compass-e2e-tests/tests/logging.test.ts @@ -210,6 +210,7 @@ describe('Logging and Telemetry integration', function () { attr: (actual: any) => { expect(actual.url).to.match(/^mongodb:\/\/127.0.0.1:27091/); expect(actual.csfle).to.equal(null); + expect(actual).to.have.property('connectionId', 0); }, }, { @@ -286,6 +287,7 @@ describe('Logging and Telemetry integration', function () { ctx: 'Connection 0', msg: 'Connecting Succeeded', attr: { + connectionId: 0, isMongos: false, isWritable: true, }, diff --git a/packages/data-service/src/data-service.ts b/packages/data-service/src/data-service.ts index fdf55005f69..1e7739c583d 100644 --- a/packages/data-service/src/data-service.ts +++ b/packages/data-service/src/data-service.ts @@ -1633,7 +1633,7 @@ class DataServiceImpl extends WithLogContext implements DataService { this._crudClient ); } catch (error) { - this._logger.info(mongoLogId(1_001_000_359), 'Connecting Error', { + this._logger.info(mongoLogId(1_001_000_359), 'Connecting Failed', { connectionId: this._id, error: error && typeof error === 'object' && 'message' in error From c946f795c2a2ac5a94eb3393081299a2f39a8bc1 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 23 May 2025 11:35:54 -0400 Subject: [PATCH 5/6] test: ids --- .../data-service/src/data-service.spec.ts | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/packages/data-service/src/data-service.spec.ts b/packages/data-service/src/data-service.spec.ts index bdde5c9cd60..3d09bc36115 100644 --- a/packages/data-service/src/data-service.spec.ts +++ b/packages/data-service/src/data-service.spec.ts @@ -23,6 +23,7 @@ import { mochaTestServer } from '@mongodb-js/compass-test-server'; import type { SearchIndex } from './search-index-detail-helper'; import { range } from 'lodash'; import ConnectionString from 'mongodb-connection-string-url'; +import { DataServiceImplLogger, MongoLogId } from './logger'; const { expect } = chai; chai.use(chaiAsPromised); @@ -93,6 +94,112 @@ describe('DataService', function () { .drop(); }); + describe('#connect', function () { + const connectionStartedId = 1_001_000_014; + const connectionSucceededId = 1_001_000_015; + const connectionFailedId = 1_001_000_359; + const logs: { + info: [ + component: string, + id: MongoLogId, + context: string, + message: string, + attr?: unknown + ]; + }[] = []; + const logCollector: DataServiceImplLogger = { + error: () => {}, + info: (...args) => logs.push({ info: args }), + debug: () => {}, + warn: () => {}, + fatal: () => {}, + }; + + let dataServiceLogTest; + + beforeEach(function () { + logs.length = 0; + }); + + afterEach(async function () { + await dataServiceLogTest?.disconnect(); + dataServiceLogTest = undefined; + }); + + // The log and connection ids in this test are used by CompassWeb to measure connect time metrics. + // The intention of these tests is not to restrict any changes to the logs or ids. + // If a change to the ids is necessary please inform the appropriate team and they can adjust before pulling in the change. + it('when connecting succeeds there is a start and success log with a connectionId', async function () { + dataServiceLogTest = new DataServiceImpl( + connectionOptions, + logCollector + ); + await dataServiceLogTest.connect(); + + expect(logs.map(({ info: [, id] }) => id.__value)).to.include( + connectionStartedId + ); + expect(logs.map(({ info: [, id] }) => id.__value)).to.include( + connectionSucceededId + ); + + const startedLog = logs.find( + ({ info: [, id] }) => id.__value === connectionStartedId + ); + const succeededLog = logs.find( + ({ info: [, id] }) => id.__value === connectionSucceededId + ); + + const { info: [, , , , startedAttr] = [] } = startedLog ?? {}; + expect(startedAttr).to.have.property( + 'connectionId', + dataServiceLogTest._id + ); + const { info: [, , , , succeededAttr] = [] } = succeededLog ?? {}; + expect(succeededAttr).to.have.property( + 'connectionId', + dataServiceLogTest._id + ); + }); + + it('when connecting fails there is a start and failure log with a connectionId', async function () { + dataServiceLogTest = new DataServiceImpl( + { connectionString: 'mongodb://iLoveJavascript' }, + logCollector + ); + + const result = await dataServiceLogTest + .connect() + .catch((error) => error); + expect(result).to.be.instanceOf(Error); + + expect(logs.map(({ info: [, id] }) => id.__value)).to.include( + connectionStartedId + ); + expect(logs.map(({ info: [, id] }) => id.__value)).to.include( + connectionFailedId + ); + + const startedLog = logs.find( + ({ info: [, id] }) => id.__value === connectionStartedId + ); + const failedLog = logs.find( + ({ info: [, id] }) => id.__value === connectionFailedId + ); + + const { info: [, , , , startedAttr] = [] } = startedLog ?? {}; + expect(startedAttr).to.have.property( + 'connectionId', + dataServiceLogTest._id + ); + const { info: [, , , , succeededAttr] = [] } = failedLog ?? {}; + expect(succeededAttr).to.have.property( + 'connectionId', + dataServiceLogTest._id + ); + }); + }); + describe('#isConnected', function () { let dataServiceIsConnected: DataService; From d9a3d243199ac11bfcab676e13066410c61db07d Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Fri, 23 May 2025 12:11:43 -0400 Subject: [PATCH 6/6] test: fix lint and timeout --- packages/data-service/src/data-service.spec.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/data-service/src/data-service.spec.ts b/packages/data-service/src/data-service.spec.ts index 3d09bc36115..c18b9903795 100644 --- a/packages/data-service/src/data-service.spec.ts +++ b/packages/data-service/src/data-service.spec.ts @@ -23,7 +23,7 @@ import { mochaTestServer } from '@mongodb-js/compass-test-server'; import type { SearchIndex } from './search-index-detail-helper'; import { range } from 'lodash'; import ConnectionString from 'mongodb-connection-string-url'; -import { DataServiceImplLogger, MongoLogId } from './logger'; +import type { DataServiceImplLogger, MongoLogId } from './logger'; const { expect } = chai; chai.use(chaiAsPromised); @@ -164,7 +164,13 @@ describe('DataService', function () { it('when connecting fails there is a start and failure log with a connectionId', async function () { dataServiceLogTest = new DataServiceImpl( - { connectionString: 'mongodb://iLoveJavascript' }, + { + connectionString: + 'mongodb://iLoveJavascript?serverSelectionTimeoutMS=5', + lookup: () => { + throw new Error('test error'); + }, + }, logCollector );