From a777b785664627d8ec75cf5ae6a1423052e6be28 Mon Sep 17 00:00:00 2001 From: Ran Nozik Date: Fri, 13 Aug 2021 11:25:02 +0300 Subject: [PATCH 1/7] feat: collect mongodb insert statements --- .../src/instrumentation.ts | 5 ++- .../src/types.ts | 7 ++++ .../test/mongodb.test.ts | 32 +++++++++++++++++++ .../test/utils.ts | 6 ++-- 4 files changed, 46 insertions(+), 4 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts index 6872e40ec4..4eee07535c 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts @@ -153,12 +153,15 @@ export class MongoDBInstrumentation extends InstrumentationBase< kind: SpanKind.CLIENT, } ); + const collectCommandPayload = + operationName !== 'insert' || + !!instrumentation._config.collectInsertPayload; instrumentation._populateAttributes( span, ns, server, // eslint-disable-next-line @typescript-eslint/no-explicit-any - operationName !== 'insert' ? (ops[0] as any) : undefined + collectCommandPayload ? (ops[0] as any) : undefined ); const patchedCallback = instrumentation._patchEnd(span, resultHandler); // handle when options is the callback to send the correct number of args diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/src/types.ts b/plugins/node/opentelemetry-instrumentation-mongodb/src/types.ts index 87fe10412b..2584acd9ba 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/src/types.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/src/types.ts @@ -36,6 +36,13 @@ export interface MongoDBInstrumentationConfig extends InstrumentationConfig { * @default undefined */ responseHook?: MongoDBInstrumentationExecutionResponseHook; + + /** + * If true, the payloads that are inserted to the DB will be collected. + * + * @default false + */ + collectInsertPayload?: boolean; } export type Func = (...args: unknown[]) => T; diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts index 7827442a8f..6cf88c6cbf 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts @@ -33,6 +33,7 @@ instrumentation.disable(); import * as mongodb from 'mongodb'; import { assertSpans, accessCollection } from './utils'; +import { SemanticAttributes } from '@opentelemetry/semantic-conventions'; describe('MongoDBInstrumentation', () => { function create(config: MongoDBInstrumentationConfig = {}) { @@ -249,6 +250,37 @@ describe('MongoDBInstrumentation', () => { }); }); + describe('when collecting insert data', () => { + beforeEach(() => { + memoryExporter.reset(); + create({ + collectInsertPayload: true, + enhancedDatabaseReporting: true, + }); + }); + + it('should collect insert data when configured to do so', done => { + const key = 'key'; + const value = 'value'; + const object = { [key]: value }; + const span = provider.getTracer('default').startSpan('insertRootSpan'); + context.with(trace.setSpan(context.active(), span), () => { + collection.insertOne(object).then(() => { + span.end(); + const spans = memoryExporter.getFinishedSpans(); + const operationName = 'mongodb.insert'; + assertSpans(spans, operationName, SpanKind.CLIENT, false, true); + const mongoSpan = spans.find(s => s.name === operationName); + const dbStatement = JSON.parse( + mongoSpan!.attributes[SemanticAttributes.DB_STATEMENT] as string + ); + assert.strictEqual(dbStatement[key], value); + done(); + }); + }); + }); + }); + describe('when specifying a responseHook configuration', () => { const dataAttributeName = 'mongodb_data'; beforeEach(() => { diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/test/utils.ts b/plugins/node/opentelemetry-instrumentation-mongodb/test/utils.ts index bc0336cd65..14f861d0ee 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/test/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/test/utils.ts @@ -86,9 +86,9 @@ export function assertSpans( assert.strictEqual(mongoSpan.status.code, SpanStatusCode.UNSET); if (isEnhancedDatabaseReportingEnabled) { - const dbStatement = mongoSpan.attributes[ - SemanticAttributes.DB_STATEMENT - ] as any; + const dbStatement = JSON.parse( + mongoSpan.attributes[SemanticAttributes.DB_STATEMENT] as string + ); for (const key in dbStatement) { assert.notStrictEqual(dbStatement[key], '?'); } From 870779775b4a1d462be3abd61727f6667703d342 Mon Sep 17 00:00:00 2001 From: Ran Nozik Date: Wed, 18 Aug 2021 12:43:03 +0300 Subject: [PATCH 2/7] pr comments --- .../src/instrumentation.ts | 38 +++++++++++++------ .../src/types.ts | 17 +++++++-- .../test/mongodb.test.ts | 1 - 3 files changed, 39 insertions(+), 17 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts index 4eee07535c..ecc4619050 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts @@ -41,6 +41,7 @@ import { CommandResult, } from './types'; import { VERSION } from './version'; +import { DbStatementSerializer } from './types'; const supportedVersions = ['>=3.3 <4']; @@ -153,15 +154,13 @@ export class MongoDBInstrumentation extends InstrumentationBase< kind: SpanKind.CLIENT, } ); - const collectCommandPayload = - operationName !== 'insert' || - !!instrumentation._config.collectInsertPayload; + instrumentation._populateAttributes( span, ns, server, // eslint-disable-next-line @typescript-eslint/no-explicit-any - collectCommandPayload ? (ops[0] as any) : undefined + (ops[0] as any) ); const patchedCallback = instrumentation._patchEnd(span, resultHandler); // handle when options is the callback to send the correct number of args @@ -411,15 +410,30 @@ export class MongoDBInstrumentation extends InstrumentationBase< // capture parameters within the query as well if enhancedDatabaseReporting is enabled. const commandObj = command.query ?? command.q ?? command; - const query = - this._config?.enhancedDatabaseReporting === true - ? commandObj - : Object.keys(commandObj).reduce((obj, key) => { - obj[key] = '?'; - return obj; - }, {} as { [key: string]: unknown }); + const dbStatementSerializer: DbStatementSerializer = + this._config.dbStatementSerializer || + this.defaultDbStatementSerializer.bind(this); + + safeExecuteInTheMiddle(() => { + const query = dbStatementSerializer(commandObj); + span.setAttribute(SemanticAttributes.DB_STATEMENT, JSON.stringify(query)); + }, err => { + if (err) { + diag.error('Error running dbStatementSerializer hook', err); + } + }, + true) + } - span.setAttribute(SemanticAttributes.DB_STATEMENT, JSON.stringify(query)); + private defaultDbStatementSerializer (commandObj: Record) { + const enhancedDbReporting = !!this._config?.enhancedDatabaseReporting; + const resultObj = enhancedDbReporting + ? commandObj + : Object.keys(commandObj).reduce((obj, key) => { + obj[key] = '?'; + return obj; + }, {} as { [key: string]: unknown }); + return JSON.stringify(resultObj); } /** diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/src/types.ts b/plugins/node/opentelemetry-instrumentation-mongodb/src/types.ts index 2584acd9ba..b074407d44 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/src/types.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/src/types.ts @@ -21,6 +21,17 @@ export interface MongoDBInstrumentationExecutionResponseHook { (span: Span, responseInfo: MongoResponseHookInformation): void; } +/** + * Function that can be used to serialize db.statement tag + * @param cmdName - The name of the command (eg. set, get, mset) + * @param cmdArgs - Array of arguments passed to the command + * + * @returns serialized string that will be used as the db.statement attribute. + */ + export type DbStatementSerializer = ( + cmd: Record +) => string; + export interface MongoDBInstrumentationConfig extends InstrumentationConfig { /** * If true, additional information about query parameters and @@ -38,11 +49,9 @@ export interface MongoDBInstrumentationConfig extends InstrumentationConfig { responseHook?: MongoDBInstrumentationExecutionResponseHook; /** - * If true, the payloads that are inserted to the DB will be collected. - * - * @default false + * Custom serializer function for the db.statement tag */ - collectInsertPayload?: boolean; + dbStatementSerializer?: DbStatementSerializer; } export type Func = (...args: unknown[]) => T; diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts index 6cf88c6cbf..2b5068b089 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts @@ -254,7 +254,6 @@ describe('MongoDBInstrumentation', () => { beforeEach(() => { memoryExporter.reset(); create({ - collectInsertPayload: true, enhancedDatabaseReporting: true, }); }); From 8362ac7a1f9f25cecb41a149e18c2dd0be51a1c8 Mon Sep 17 00:00:00 2001 From: Ran Nozik Date: Wed, 18 Aug 2021 12:51:53 +0300 Subject: [PATCH 3/7] minor fix --- .../src/instrumentation.ts | 29 +++++++++++-------- .../src/types.ts | 7 ++--- .../test/mongodb.test.ts | 3 ++ 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts index ecc4619050..ad1780d996 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts @@ -160,7 +160,7 @@ export class MongoDBInstrumentation extends InstrumentationBase< ns, server, // eslint-disable-next-line @typescript-eslint/no-explicit-any - (ops[0] as any) + ops[0] as any ); const patchedCallback = instrumentation._patchEnd(span, resultHandler); // handle when options is the callback to send the correct number of args @@ -412,20 +412,25 @@ export class MongoDBInstrumentation extends InstrumentationBase< const commandObj = command.query ?? command.q ?? command; const dbStatementSerializer: DbStatementSerializer = this._config.dbStatementSerializer || - this.defaultDbStatementSerializer.bind(this); + this._defaultDbStatementSerializer.bind(this); - safeExecuteInTheMiddle(() => { - const query = dbStatementSerializer(commandObj); - span.setAttribute(SemanticAttributes.DB_STATEMENT, JSON.stringify(query)); - }, err => { - if (err) { - diag.error('Error running dbStatementSerializer hook', err); - } - }, - true) + if (typeof dbStatementSerializer === 'function') { + safeExecuteInTheMiddle( + () => { + const query = dbStatementSerializer(commandObj); + span.setAttribute(SemanticAttributes.DB_STATEMENT, query); + }, + err => { + if (err) { + this._diag.error('Error running dbStatementSerializer hook', err); + } + }, + true + ); + } } - private defaultDbStatementSerializer (commandObj: Record) { + private _defaultDbStatementSerializer(commandObj: Record) { const enhancedDbReporting = !!this._config?.enhancedDatabaseReporting; const resultObj = enhancedDbReporting ? commandObj diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/src/types.ts b/plugins/node/opentelemetry-instrumentation-mongodb/src/types.ts index b074407d44..fd54361449 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/src/types.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/src/types.ts @@ -23,14 +23,11 @@ export interface MongoDBInstrumentationExecutionResponseHook { /** * Function that can be used to serialize db.statement tag - * @param cmdName - The name of the command (eg. set, get, mset) - * @param cmdArgs - Array of arguments passed to the command + * @param cmd - MongoDB command object * * @returns serialized string that will be used as the db.statement attribute. */ - export type DbStatementSerializer = ( - cmd: Record -) => string; +export type DbStatementSerializer = (cmd: Record) => string; export interface MongoDBInstrumentationConfig extends InstrumentationConfig { /** diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts index 2b5068b089..fe1bd7dc9b 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts @@ -255,6 +255,9 @@ describe('MongoDBInstrumentation', () => { memoryExporter.reset(); create({ enhancedDatabaseReporting: true, + dbStatementSerializer: (commandObj: Record) => { + return JSON.stringify(commandObj); + }, }); }); From 2b6eda736fa008ac7cebb4d2883a22da8046268f Mon Sep 17 00:00:00 2001 From: Ran Nozik Date: Wed, 18 Aug 2021 19:06:24 +0300 Subject: [PATCH 4/7] add another test case --- .../test/mongodb.test.ts | 77 +++++++++++++------ 1 file changed, 52 insertions(+), 25 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts index fe1bd7dc9b..b574f6afc1 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts @@ -250,34 +250,61 @@ describe('MongoDBInstrumentation', () => { }); }); - describe('when collecting insert data', () => { - beforeEach(() => { - memoryExporter.reset(); - create({ - enhancedDatabaseReporting: true, - dbStatementSerializer: (commandObj: Record) => { - return JSON.stringify(commandObj); - }, + describe('when specifying a dbStatementSerializer configuration', () => { + const key = 'key'; + const value = 'value'; + const object = { [key]: value }; + + describe('with a valid function', () => { + beforeEach(() => { + memoryExporter.reset(); + create({ + enhancedDatabaseReporting: true, + dbStatementSerializer: (commandObj: Record) => { + return JSON.stringify(commandObj); + }, + }); + }); + + it('should properly collect db statement', done => { + const span = provider.getTracer('default').startSpan('insertRootSpan'); + context.with(trace.setSpan(context.active(), span), () => { + collection.insertOne(object).then(() => { + span.end(); + const spans = memoryExporter.getFinishedSpans(); + const operationName = 'mongodb.insert'; + assertSpans(spans, operationName, SpanKind.CLIENT, false, true); + const mongoSpan = spans.find(s => s.name === operationName); + const dbStatement = JSON.parse( + mongoSpan!.attributes[SemanticAttributes.DB_STATEMENT] as string + ); + assert.strictEqual(dbStatement[key], value); + done(); + }); + }); }); }); - it('should collect insert data when configured to do so', done => { - const key = 'key'; - const value = 'value'; - const object = { [key]: value }; - const span = provider.getTracer('default').startSpan('insertRootSpan'); - context.with(trace.setSpan(context.active(), span), () => { - collection.insertOne(object).then(() => { - span.end(); - const spans = memoryExporter.getFinishedSpans(); - const operationName = 'mongodb.insert'; - assertSpans(spans, operationName, SpanKind.CLIENT, false, true); - const mongoSpan = spans.find(s => s.name === operationName); - const dbStatement = JSON.parse( - mongoSpan!.attributes[SemanticAttributes.DB_STATEMENT] as string - ); - assert.strictEqual(dbStatement[key], value); - done(); + describe('with an invalid function', () => { + beforeEach(() => { + memoryExporter.reset(); + create({ + enhancedDatabaseReporting: true, + dbStatementSerializer: (_commandObj: Record) => { + throw new Error('something went wrong!'); + }, + }); + }); + + it('should not do any harm when throwing an exception', done => { + const span = provider.getTracer('default').startSpan('insertRootSpan'); + context.with(trace.setSpan(context.active(), span), () => { + collection.insertOne(object).then(() => { + span.end(); + const spans = memoryExporter.getFinishedSpans(); + assertSpans(spans, 'mongodb.insert', SpanKind.CLIENT); + done(); + }); }); }); }); From fd1e4992ee655c79870ad3117991aad6809f59c7 Mon Sep 17 00:00:00 2001 From: Ran Nozik Date: Thu, 19 Aug 2021 12:34:11 +0300 Subject: [PATCH 5/7] pr comments --- plugins/node/opentelemetry-instrumentation-mongodb/README.md | 1 + .../opentelemetry-instrumentation-mongodb/test/mongodb.test.ts | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/README.md b/plugins/node/opentelemetry-instrumentation-mongodb/README.md index f31c88feca..8ddc2d634b 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/README.md +++ b/plugins/node/opentelemetry-instrumentation-mongodb/README.md @@ -51,6 +51,7 @@ Mongodb instrumentation has few options available to choose from. You can set th | ------- | ---- | ----------- | | [`enhancedDatabaseReporting`](https://github.com/open-telemetry/opentelemetry-js/blob/main/packages/opentelemetry-api/src/trace/instrumentation/instrumentation.ts#L91) | `string` | If true, additional information about query parameters and results will be attached (as `attributes`) to spans representing database operations | | `responseHook` | `MongoDBInstrumentationExecutionResponseHook` (function) | Function for adding custom attributes from db response | +| `dbStatementSerializer` | `DbStatementSerializer` (function) | Custom serializer function for the db.statement tag | ## Useful links diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts index b574f6afc1..489b45d98a 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts @@ -259,7 +259,6 @@ describe('MongoDBInstrumentation', () => { beforeEach(() => { memoryExporter.reset(); create({ - enhancedDatabaseReporting: true, dbStatementSerializer: (commandObj: Record) => { return JSON.stringify(commandObj); }, From 906ce3b532908139ec3e0eaff0098e6825bb44c9 Mon Sep 17 00:00:00 2001 From: Ran Nozik Date: Thu, 19 Aug 2021 13:55:00 +0300 Subject: [PATCH 6/7] add another test --- .../test/mongodb.test.ts | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts index 489b45d98a..16562ce67d 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb.test.ts @@ -250,6 +250,37 @@ describe('MongoDBInstrumentation', () => { }); }); + describe('when using enhanced database reporting without db statementSerializer', () => { + const key = 'key'; + const value = 'value'; + const object = { [key]: value }; + + beforeEach(() => { + memoryExporter.reset(); + create({ + enhancedDatabaseReporting: false, + }); + }); + + it('should properly collect db statement (hide attribute values)', done => { + const span = provider.getTracer('default').startSpan('insertRootSpan'); + context.with(trace.setSpan(context.active(), span), () => { + collection.insertOne(object).then(() => { + span.end(); + const spans = memoryExporter.getFinishedSpans(); + const operationName = 'mongodb.insert'; + assertSpans(spans, operationName, SpanKind.CLIENT, false, false); + const mongoSpan = spans.find(s => s.name === operationName); + const dbStatement = JSON.parse( + mongoSpan!.attributes[SemanticAttributes.DB_STATEMENT] as string + ); + assert.strictEqual(dbStatement[key], '?'); + done(); + }); + }); + }); + }); + describe('when specifying a dbStatementSerializer configuration', () => { const key = 'key'; const value = 'value'; From 251bdb2191d4b9faf3ef70e1a292cb0fa52e2809 Mon Sep 17 00:00:00 2001 From: Ran Nozik Date: Mon, 30 Aug 2021 21:13:08 +0300 Subject: [PATCH 7/7] cr comment --- .../src/instrumentation.ts | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts index ad1780d996..d7444c7645 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts @@ -41,7 +41,6 @@ import { CommandResult, } from './types'; import { VERSION } from './version'; -import { DbStatementSerializer } from './types'; const supportedVersions = ['>=3.3 <4']; @@ -410,24 +409,23 @@ export class MongoDBInstrumentation extends InstrumentationBase< // capture parameters within the query as well if enhancedDatabaseReporting is enabled. const commandObj = command.query ?? command.q ?? command; - const dbStatementSerializer: DbStatementSerializer = - this._config.dbStatementSerializer || - this._defaultDbStatementSerializer.bind(this); + const dbStatementSerializer = + typeof this._config.dbStatementSerializer === 'function' + ? this._config.dbStatementSerializer + : this._defaultDbStatementSerializer.bind(this); - if (typeof dbStatementSerializer === 'function') { - safeExecuteInTheMiddle( - () => { - const query = dbStatementSerializer(commandObj); - span.setAttribute(SemanticAttributes.DB_STATEMENT, query); - }, - err => { - if (err) { - this._diag.error('Error running dbStatementSerializer hook', err); - } - }, - true - ); - } + safeExecuteInTheMiddle( + () => { + const query = dbStatementSerializer(commandObj); + span.setAttribute(SemanticAttributes.DB_STATEMENT, query); + }, + err => { + if (err) { + this._diag.error('Error running dbStatementSerializer hook', err); + } + }, + true + ); } private _defaultDbStatementSerializer(commandObj: Record) {