From 97305e1880ecbfb3b87d6c38f0c6521570583510 Mon Sep 17 00:00:00 2001 From: Haddas Bronfman <85441461+haddasbronfman@users.noreply.github.com> Date: Sun, 11 Dec 2022 11:28:55 +0200 Subject: [PATCH] feat(mongodb): add db.operation span attribute (#1321) * chore(mongo): add DB_OPERATION attribute * chore(mongo): replace 'remove' with 'delete' * chore(mongo): add tests * chore(mongo): add condition in case cmd=unknown * chore(mongo): make code more readable --- .../examples/README.md | 4 +- .../src/instrumentation.ts | 30 ++++++--- .../test/mongodb-v3.test.ts | 63 +++++++++++++++---- .../test/mongodb-v4.test.ts | 63 +++++++++++++++---- .../test/utils.ts | 5 ++ 5 files changed, 127 insertions(+), 38 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/examples/README.md b/plugins/node/opentelemetry-instrumentation-mongodb/examples/README.md index d30fb9ed20..120f97fc84 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/examples/README.md +++ b/plugins/node/opentelemetry-instrumentation-mongodb/examples/README.md @@ -37,11 +37,11 @@ Setup [Jaeger Tracing](https://www.jaegertracing.io/docs/latest/getting-started/ npm run zipkin:server ``` -- Run the zipkin:client +- Run the client ```sh # from this directory - npm run client + npm run zipkin:client ``` #### Zipkin UI diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts index f950dd49a8..8361de772c 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts @@ -210,7 +210,8 @@ export class MongoDBInstrumentation extends InstrumentationBase { ns, server, // eslint-disable-next-line @typescript-eslint/no-explicit-any - ops[0] as any + ops[0] as any, + operationName ); const patchedCallback = instrumentation._patchEnd(span, resultHandler); // handle when options is the callback to send the correct number of args @@ -255,7 +256,9 @@ export class MongoDBInstrumentation extends InstrumentationBase { const span = instrumentation.tracer.startSpan(`mongodb.${type}`, { kind: SpanKind.CLIENT, }); - instrumentation._populateV3Attributes(span, ns, server, cmd); + const operation = + commandType === MongodbCommandType.UNKNOWN ? undefined : commandType; + instrumentation._populateV3Attributes(span, ns, server, cmd, operation); const patchedCallback = instrumentation._patchEnd(span, resultHandler); // handle when options is the callback to send the correct number of args if (typeof options === 'function') { @@ -296,7 +299,7 @@ export class MongoDBInstrumentation extends InstrumentationBase { kind: SpanKind.CLIENT, } ); - instrumentation._populateV4Attributes(span, this, ns, cmd); + instrumentation._populateV4Attributes(span, this, ns, cmd, commandType); const patchedCallback = instrumentation._patchEnd(span, resultHandler); return original.call(this, ns, cmd, options, patchedCallback); }; @@ -341,7 +344,7 @@ export class MongoDBInstrumentation extends InstrumentationBase { const span = instrumentation.tracer.startSpan('mongodb.find', { kind: SpanKind.CLIENT, }); - instrumentation._populateV3Attributes(span, ns, server, cmd); + instrumentation._populateV3Attributes(span, ns, server, cmd, 'find'); const patchedCallback = instrumentation._patchEnd(span, resultHandler); // handle when options is the callback to send the correct number of args if (typeof options === 'function') { @@ -413,7 +416,8 @@ export class MongoDBInstrumentation extends InstrumentationBase { span, ns, server, - cursorState.cmd + cursorState.cmd, + 'getMore' ); const patchedCallback = instrumentation._patchEnd(span, resultHandler); // handle when options is the callback to send the correct number of args @@ -472,7 +476,8 @@ export class MongoDBInstrumentation extends InstrumentationBase { span: Span, connectionCtx: any, ns: any, - command?: any + command?: any, + operation?: string ) { let host, port: undefined | string; if (connectionCtx) { @@ -501,7 +506,8 @@ export class MongoDBInstrumentation extends InstrumentationBase { ns.collection, host, port, - commandObj + commandObj, + operation ); } @@ -516,7 +522,8 @@ export class MongoDBInstrumentation extends InstrumentationBase { span: Span, ns: string, topology: MongoInternalTopology, - command?: MongoInternalCommand + command?: MongoInternalCommand, + operation?: string | undefined ) { // add network attributes to determine the remote server let host: undefined | string; @@ -548,7 +555,8 @@ export class MongoDBInstrumentation extends InstrumentationBase { dbCollection, host, port, - commandObj + commandObj, + operation ); } @@ -558,13 +566,15 @@ export class MongoDBInstrumentation extends InstrumentationBase { dbCollection?: string, host?: undefined | string, port?: undefined | string, - commandObj?: any + commandObj?: any, + operation?: string | undefined ) { // add database related attributes span.setAttributes({ [SemanticAttributes.DB_SYSTEM]: DbSystemValues.MONGODB, [SemanticAttributes.DB_NAME]: dbName, [SemanticAttributes.DB_MONGODB_COLLECTION]: dbCollection, + [SemanticAttributes.DB_OPERATION]: operation, }); if (host && port) { 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 3a663957f6..5a0b39e773 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v3.test.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v3.test.ts @@ -112,7 +112,12 @@ describe('MongoDBInstrumentation', () => { .insertMany(insertData) .then(() => { span.end(); - assertSpans(getTestSpans(), 'mongodb.insert', SpanKind.CLIENT); + assertSpans( + getTestSpans(), + 'mongodb.insert', + SpanKind.CLIENT, + 'insert' + ); done(); }) .catch(err => { @@ -128,7 +133,12 @@ describe('MongoDBInstrumentation', () => { .updateOne({ a: 2 }, { $set: { b: 1 } }) .then(() => { span.end(); - assertSpans(getTestSpans(), 'mongodb.update', SpanKind.CLIENT); + assertSpans( + getTestSpans(), + 'mongodb.update', + SpanKind.CLIENT, + 'update' + ); done(); }) .catch(err => { @@ -144,7 +154,12 @@ describe('MongoDBInstrumentation', () => { .deleteOne({ a: 3 }) .then(() => { span.end(); - assertSpans(getTestSpans(), 'mongodb.remove', SpanKind.CLIENT); + assertSpans( + getTestSpans(), + 'mongodb.remove', + SpanKind.CLIENT, + 'remove' + ); done(); }) .catch(err => { @@ -164,7 +179,12 @@ describe('MongoDBInstrumentation', () => { .toArray() .then(() => { span.end(); - assertSpans(getTestSpans(), 'mongodb.find', SpanKind.CLIENT); + assertSpans( + getTestSpans(), + 'mongodb.find', + SpanKind.CLIENT, + 'find' + ); done(); }) .catch(err => { @@ -190,7 +210,8 @@ describe('MongoDBInstrumentation', () => { span => !span.name.includes('mongodb.getMore') ), 'mongodb.find', - SpanKind.CLIENT + SpanKind.CLIENT, + 'find' ); // assert that we correctly got the first as a find assertSpans( @@ -198,7 +219,8 @@ describe('MongoDBInstrumentation', () => { span => !span.name.includes('mongodb.find') ), 'mongodb.getMore', - SpanKind.CLIENT + SpanKind.CLIENT, + 'getMore' ); done(); }) @@ -222,7 +244,8 @@ describe('MongoDBInstrumentation', () => { assertSpans( getTestSpans(), 'mongodb.createIndexes', - SpanKind.CLIENT + SpanKind.CLIENT, + 'createIndexes' ); done(); }) @@ -253,7 +276,14 @@ describe('MongoDBInstrumentation', () => { span.end(); const spans = getTestSpans(); const operationName = 'mongodb.insert'; - assertSpans(spans, operationName, SpanKind.CLIENT, false, false); + assertSpans( + spans, + operationName, + SpanKind.CLIENT, + 'insert', + false, + false + ); const mongoSpan = spans.find(s => s.name === operationName); const dbStatement = JSON.parse( mongoSpan!.attributes[SemanticAttributes.DB_STATEMENT] as string @@ -291,7 +321,14 @@ describe('MongoDBInstrumentation', () => { span.end(); const spans = getTestSpans(); const operationName = 'mongodb.insert'; - assertSpans(spans, operationName, SpanKind.CLIENT, false, true); + assertSpans( + spans, + operationName, + SpanKind.CLIENT, + 'insert', + false, + true + ); const mongoSpan = spans.find(s => s.name === operationName); const dbStatement = JSON.parse( mongoSpan!.attributes[SemanticAttributes.DB_STATEMENT] as string @@ -324,7 +361,7 @@ describe('MongoDBInstrumentation', () => { .then(() => { span.end(); const spans = getTestSpans(); - assertSpans(spans, 'mongodb.insert', SpanKind.CLIENT); + assertSpans(spans, 'mongodb.insert', SpanKind.CLIENT, 'insert'); done(); }) .catch(err => { @@ -421,7 +458,7 @@ describe('MongoDBInstrumentation', () => { .then(() => { span.end(); const spans = getTestSpans(); - assertSpans(spans, 'mongodb.find', SpanKind.CLIENT); + assertSpans(spans, 'mongodb.find', SpanKind.CLIENT, 'find'); done(); }) .catch(err => { @@ -443,7 +480,7 @@ describe('MongoDBInstrumentation', () => { span.end(); const spans = getTestSpans(); const mainSpan = spans[spans.length - 1]; - assertSpans(spans, 'mongodb.insert', SpanKind.CLIENT); + assertSpans(spans, 'mongodb.insert', SpanKind.CLIENT, 'insert'); resetMemoryExporter(); collection @@ -453,7 +490,7 @@ describe('MongoDBInstrumentation', () => { const spans2 = getTestSpans(); spans2.push(mainSpan); - assertSpans(spans2, 'mongodb.find', SpanKind.CLIENT); + assertSpans(spans2, 'mongodb.find', SpanKind.CLIENT, 'find'); assert.strictEqual( mainSpan.spanContext().spanId, spans2[0].parentSpanId 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 f42be4b03b..3101a947d2 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v4.test.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v4.test.ts @@ -111,7 +111,12 @@ describe('MongoDBInstrumentation', () => { .insertMany(insertData) .then(() => { span.end(); - assertSpans(getTestSpans(), 'mongodb.insert', SpanKind.CLIENT); + assertSpans( + getTestSpans(), + 'mongodb.insert', + SpanKind.CLIENT, + 'insert' + ); done(); }) .catch(err => { @@ -127,7 +132,12 @@ describe('MongoDBInstrumentation', () => { .updateOne({ a: 2 }, { $set: { b: 1 } }) .then(() => { span.end(); - assertSpans(getTestSpans(), 'mongodb.update', SpanKind.CLIENT); + assertSpans( + getTestSpans(), + 'mongodb.update', + SpanKind.CLIENT, + 'update' + ); done(); }) .catch(err => { @@ -143,7 +153,12 @@ describe('MongoDBInstrumentation', () => { .deleteOne({ a: 3 }) .then(() => { span.end(); - assertSpans(getTestSpans(), 'mongodb.delete', SpanKind.CLIENT); + assertSpans( + getTestSpans(), + 'mongodb.delete', + SpanKind.CLIENT, + 'delete' + ); done(); }) .catch(err => { @@ -163,7 +178,12 @@ describe('MongoDBInstrumentation', () => { .toArray() .then(() => { span.end(); - assertSpans(getTestSpans(), 'mongodb.find', SpanKind.CLIENT); + assertSpans( + getTestSpans(), + 'mongodb.find', + SpanKind.CLIENT, + 'find' + ); done(); }) .catch(err => { @@ -189,7 +209,8 @@ describe('MongoDBInstrumentation', () => { span => !span.name.includes('mongodb.getMore') ), 'mongodb.find', - SpanKind.CLIENT + SpanKind.CLIENT, + 'find' ); // assert that we correctly got the first as a find assertSpans( @@ -197,7 +218,8 @@ describe('MongoDBInstrumentation', () => { span => !span.name.includes('mongodb.find') ), 'mongodb.getMore', - SpanKind.CLIENT + SpanKind.CLIENT, + 'getMore' ); done(); }) @@ -221,7 +243,8 @@ describe('MongoDBInstrumentation', () => { assertSpans( getTestSpans(), 'mongodb.createIndexes', - SpanKind.CLIENT + SpanKind.CLIENT, + 'createIndexes' ); done(); }) @@ -252,7 +275,14 @@ describe('MongoDBInstrumentation', () => { span.end(); const spans = getTestSpans(); const operationName = 'mongodb.insert'; - assertSpans(spans, operationName, SpanKind.CLIENT, false, false); + assertSpans( + spans, + operationName, + SpanKind.CLIENT, + 'insert', + false, + false + ); const mongoSpan = spans.find(s => s.name === operationName); const dbStatement = JSON.parse( mongoSpan!.attributes[SemanticAttributes.DB_STATEMENT] as string @@ -290,7 +320,14 @@ describe('MongoDBInstrumentation', () => { span.end(); const spans = getTestSpans(); const operationName = 'mongodb.insert'; - assertSpans(spans, operationName, SpanKind.CLIENT, false, true); + assertSpans( + spans, + operationName, + SpanKind.CLIENT, + 'insert', + false, + true + ); const mongoSpan = spans.find(s => s.name === operationName); const dbStatement = JSON.parse( mongoSpan!.attributes[SemanticAttributes.DB_STATEMENT] as string @@ -323,7 +360,7 @@ describe('MongoDBInstrumentation', () => { .then(() => { span.end(); const spans = getTestSpans(); - assertSpans(spans, 'mongodb.insert', SpanKind.CLIENT); + assertSpans(spans, 'mongodb.insert', SpanKind.CLIENT, 'insert'); done(); }) .catch(err => { @@ -417,7 +454,7 @@ describe('MongoDBInstrumentation', () => { .then(() => { span.end(); const spans = getTestSpans(); - assertSpans(spans, 'mongodb.find', SpanKind.CLIENT); + assertSpans(spans, 'mongodb.find', SpanKind.CLIENT, 'find'); done(); }) .catch(err => { @@ -439,7 +476,7 @@ describe('MongoDBInstrumentation', () => { span.end(); const spans = getTestSpans(); const mainSpan = spans[spans.length - 1]; - assertSpans(spans, 'mongodb.insert', SpanKind.CLIENT); + assertSpans(spans, 'mongodb.insert', SpanKind.CLIENT, 'insert'); resetMemoryExporter(); collection @@ -448,7 +485,7 @@ describe('MongoDBInstrumentation', () => { .then(() => { const spans2 = getTestSpans(); spans2.push(mainSpan); - assertSpans(spans2, 'mongodb.find', SpanKind.CLIENT); + assertSpans(spans2, 'mongodb.find', SpanKind.CLIENT, 'find'); assert.strictEqual( mainSpan.spanContext().spanId, spans2[0].parentSpanId diff --git a/plugins/node/opentelemetry-instrumentation-mongodb/test/utils.ts b/plugins/node/opentelemetry-instrumentation-mongodb/test/utils.ts index fde0af56d8..3da0a1833c 100644 --- a/plugins/node/opentelemetry-instrumentation-mongodb/test/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-mongodb/test/utils.ts @@ -65,6 +65,7 @@ export function assertSpans( spans: ReadableSpan[], expectedName: string, expectedKind: SpanKind, + expectedOperation: string, log = false, isEnhancedDatabaseReportingEnabled = false ) { @@ -79,6 +80,10 @@ export function assertSpans( const [mongoSpan] = spans; assert.strictEqual(mongoSpan.name, expectedName); assert.strictEqual(mongoSpan.kind, expectedKind); + assert.strictEqual( + mongoSpan.attributes[SemanticAttributes.DB_OPERATION], + expectedOperation + ); assert.strictEqual( mongoSpan.attributes[SemanticAttributes.DB_SYSTEM], 'mongodb'