From 2b1360daea5254d15c7dba71c30748630d92c17f Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Fri, 7 Jun 2024 14:54:05 +0200 Subject: [PATCH] feat(instrumentation-mongodb): support aggregation commands and support nested statements (#1728) * feat(opentelemetry-instrumentation-mongodb): support aggregation commands and support nested statements Signed-off-by: Oliver Sand * docs: fix typos in guidelines Signed-off-by: Oliver Sand * Apply suggestions from code review update semantic conventions imports --------- Signed-off-by: Oliver Sand Co-authored-by: Marc Pichler Co-authored-by: Jamie Danielson --- GUIDELINES.md | 2 +- .../src/instrumentation.ts | 25 +++++-- .../src/internal-types.ts | 2 + .../test/mongodb-v3.test.ts | 68 +++++++++++++++++++ .../test/mongodb-v4.test.ts | 68 +++++++++++++++++++ .../test/mongodb-v5-v6.test.ts | 68 +++++++++++++++++++ 6 files changed, 228 insertions(+), 5 deletions(-) diff --git a/GUIDELINES.md b/GUIDELINES.md index 69c3fe283a..fbee8623a8 100644 --- a/GUIDELINES.md +++ b/GUIDELINES.md @@ -115,7 +115,7 @@ To support this use case, you can choose one of the following options: ... ``` - If possible, this is the prefered option, as it uses types from a maintained package. + If possible, this is the preferred option, as it uses types from a maintained package. Notice that types may introduce breaking changes in major semver releases, and instrumentation should choose a `@types/` package that is compatible with the version range it supports. diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts index a1417a9487..54d9dfa26c 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts @@ -775,6 +775,8 @@ export class MongoDBInstrumentation extends InstrumentationBase { return MongodbCommandType.IS_MASTER; } else if (command.count !== undefined) { return MongodbCommandType.COUNT; + } else if (command.aggregate !== undefined) { + return MongodbCommandType.AGGREGATE; } else { return MongodbCommandType.UNKNOWN; } @@ -924,13 +926,28 @@ export class MongoDBInstrumentation extends InstrumentationBase { const enhancedDbReporting = !!this._config?.enhancedDatabaseReporting; const resultObj = enhancedDbReporting ? commandObj - : Object.keys(commandObj).reduce((obj, key) => { - obj[key] = '?'; - return obj; - }, {} as { [key: string]: unknown }); + : this._scrubStatement(commandObj); return JSON.stringify(resultObj); } + private _scrubStatement(value: unknown): unknown { + if (Array.isArray(value)) { + return value.map(element => this._scrubStatement(element)); + } + + if (typeof value === 'object' && value !== null) { + return Object.fromEntries( + Object.entries(value).map(([key, element]) => [ + key, + this._scrubStatement(element), + ]) + ); + } + + // A value like string or number, possible contains PII, scrub it + return '?'; + } + /** * Triggers the response hook in case it is defined. * @param span The span to add the results to. diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/src/internal-types.ts b/plugins/node/opentelemetry-instrumentation-mongodb/src/internal-types.ts index 38b357077b..b660ae879c 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/src/internal-types.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/src/internal-types.ts @@ -56,6 +56,7 @@ export type MongoInternalCommand = { findandmodify: boolean; createIndexes: boolean; count: boolean; + aggregate: boolean; ismaster: boolean; indexes?: unknown[]; query?: Record; @@ -166,6 +167,7 @@ export enum MongodbCommandType { FIND_AND_MODIFY = 'findAndModify', IS_MASTER = 'isMaster', COUNT = 'count', + AGGREGATE = 'aggregate', UNKNOWN = 'unknown', } diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v3.test.ts b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v3.test.ts index dba836dc3a..4a3bd9a6e0 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v3.test.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v3.test.ts @@ -266,6 +266,32 @@ describe('MongoDBInstrumentation-Tracing-v3', () => { }); }); }); + + it('should create a child span for aggregation', done => { + const span = trace.getTracer('default').startSpan('indexRootSpan'); + context.with(trace.setSpan(context.active(), span), () => { + collection + .aggregate([ + { $match: { key: 'value' } }, + { $group: { _id: '$a', count: { $sum: 1 } } }, + ]) + .toArray() + .then(() => { + span.end(); + assertSpans( + getTestSpans(), + 'mongodb.aggregate', + SpanKind.CLIENT, + 'aggregate', + undefined + ); + done(); + }) + .catch(err => { + done(err); + }); + }); + }); }); describe('when using enhanced database reporting without db statementSerializer', () => { @@ -309,6 +335,48 @@ describe('MongoDBInstrumentation-Tracing-v3', () => { }); }); }); + + it('should properly collect nested db statement (hide attribute values)', done => { + const span = trace.getTracer('default').startSpan('insertRootSpan'); + context.with(trace.setSpan(context.active(), span), () => { + collection + .aggregate([ + { $match: { key: 'value' } }, + { $group: { _id: '$a', count: { $sum: 1 } } }, + ]) + .toArray() + .then(() => { + span.end(); + const spans = getTestSpans(); + const operationName = 'mongodb.aggregate'; + assertSpans( + spans, + operationName, + SpanKind.CLIENT, + 'aggregate', + undefined, + false, + false + ); + const mongoSpan = spans.find(s => s.name === operationName); + const dbStatement = JSON.parse( + mongoSpan!.attributes[SEMATTRS_DB_STATEMENT] as string + ); + assert.deepEqual(dbStatement, { + aggregate: '?', + pipeline: [ + { $match: { key: '?' } }, + { $group: { _id: '?', count: { $sum: '?' } } }, + ], + cursor: {}, + }); + done(); + }) + .catch(err => { + done(err); + }); + }); + }); }); describe('when specifying a dbStatementSerializer configuration', () => { diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v4.test.ts b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v4.test.ts index df2c48be5f..febd901776 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v4.test.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v4.test.ts @@ -301,6 +301,32 @@ describe('MongoDBInstrumentation-Tracing-v4', () => { }); }); }); + + it('should create a child span for aggregation', done => { + const span = trace.getTracer('default').startSpan('indexRootSpan'); + context.with(trace.setSpan(context.active(), span), () => { + collection + .aggregate([ + { $match: { key: 'value' } }, + { $group: { _id: '$a', count: { $sum: 1 } } }, + ]) + .toArray() + .then(() => { + span.end(); + assertSpans( + getTestSpans(), + 'mongodb.aggregate', + SpanKind.CLIENT, + 'aggregate', + undefined + ); + done(); + }) + .catch(err => { + done(err); + }); + }); + }); }); describe('when using enhanced database reporting without db statementSerializer', () => { @@ -344,6 +370,48 @@ describe('MongoDBInstrumentation-Tracing-v4', () => { }); }); }); + + it('should properly collect nested db statement (hide attribute values)', done => { + const span = trace.getTracer('default').startSpan('insertRootSpan'); + context.with(trace.setSpan(context.active(), span), () => { + collection + .aggregate([ + { $match: { key: 'value' } }, + { $group: { _id: '$a', count: { $sum: 1 } } }, + ]) + .toArray() + .then(() => { + span.end(); + const spans = getTestSpans(); + const operationName = 'mongodb.aggregate'; + assertSpans( + spans, + operationName, + SpanKind.CLIENT, + 'aggregate', + undefined, + false, + false + ); + const mongoSpan = spans.find(s => s.name === operationName); + const dbStatement = JSON.parse( + mongoSpan!.attributes[SEMATTRS_DB_STATEMENT] as string + ); + assert.deepEqual(dbStatement, { + aggregate: '?', + pipeline: [ + { $match: { key: '?' } }, + { $group: { _id: '?', count: { $sum: '?' } } }, + ], + cursor: {}, + }); + done(); + }) + .catch(err => { + done(err); + }); + }); + }); }); describe('when specifying a dbStatementSerializer configuration', () => { diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v5-v6.test.ts b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v5-v6.test.ts index 546aba2e31..bac9b6d17e 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v5-v6.test.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v5-v6.test.ts @@ -304,6 +304,32 @@ describe('MongoDBInstrumentation-Tracing-v5', () => { }); }); }); + + it('should create a child span for aggregation', done => { + const span = trace.getTracer('default').startSpan('indexRootSpan'); + context.with(trace.setSpan(context.active(), span), () => { + collection + .aggregate([ + { $match: { key: 'value' } }, + { $group: { _id: '$a', count: { $sum: 1 } } }, + ]) + .toArray() + .then(() => { + span.end(); + assertSpans( + getTestSpans(), + 'mongodb.aggregate', + SpanKind.CLIENT, + 'aggregate', + undefined + ); + done(); + }) + .catch(err => { + done(err); + }); + }); + }); }); describe('when using enhanced database reporting without db statementSerializer', () => { @@ -347,6 +373,48 @@ describe('MongoDBInstrumentation-Tracing-v5', () => { }); }); }); + + it('should properly collect nested db statement (hide attribute values)', done => { + const span = trace.getTracer('default').startSpan('insertRootSpan'); + context.with(trace.setSpan(context.active(), span), () => { + collection + .aggregate([ + { $match: { key: 'value' } }, + { $group: { _id: '$a', count: { $sum: 1 } } }, + ]) + .toArray() + .then(() => { + span.end(); + const spans = getTestSpans(); + const operationName = 'mongodb.aggregate'; + assertSpans( + spans, + operationName, + SpanKind.CLIENT, + 'aggregate', + undefined, + false, + false + ); + const mongoSpan = spans.find(s => s.name === operationName); + const dbStatement = JSON.parse( + mongoSpan!.attributes[SEMATTRS_DB_STATEMENT] as string + ); + assert.deepEqual(dbStatement, { + aggregate: '?', + pipeline: [ + { $match: { key: '?' } }, + { $group: { _id: '?', count: { $sum: '?' } } }, + ], + cursor: {}, + }); + done(); + }) + .catch(err => { + done(err); + }); + }); + }); }); describe('when specifying a dbStatementSerializer configuration', () => {