Skip to content

Commit

Permalink
feat(instrumentation-mongodb): support aggregation commands and suppo…
Browse files Browse the repository at this point in the history 
…rt nested statements (#1728)

* feat(opentelemetry-instrumentation-mongodb): support aggregation commands and support nested statements

Signed-off-by: Oliver Sand <[email protected]>

* docs: fix typos in guidelines

Signed-off-by: Oliver Sand <[email protected]>

* Apply suggestions from code review

update semantic conventions imports

---------

Signed-off-by: Oliver Sand <[email protected]>
Co-authored-by: Marc Pichler <[email protected]>
Co-authored-by: Jamie Danielson <[email protected]>
  • Loading branch information
@Fox32 @pichlermarc @JamieDanielson
3 people authored Jun 7, 2024
1 parent 6721bdd commit 2b1360d
Show file tree
Hide file tree
Showing 6 changed files with 228 additions and 5 deletions.
2 changes: 1 addition & 1 deletion GUIDELINES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
25 changes: 21 additions & 4 deletions plugins/node/opentelemetry-instrumentation-mongodb/src/instrumentation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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),

This comment has been minimized.

Sign in to view

Copy link
@tzjames

tzjames Jun 14, 2024

I am using mongoose which seems to have added a __parentArray, to an object that is part of an array. Hence there is a circular reference and so _scrubStatement then throws:
@opentelemetry/instrumentation-mongodb Error running dbStatementSerializer hook RangeError: Maximum call stack size exceeded at /Users/james/Library/CloudStorage/Dropbox/growthbookzip/growthbook/node_modules/@opentelemetry/instrumentation-mongodb/build/src/instrumentation.js:581:65 at Array.map (<anonymous>) at MongoDBInstrumentation._scrubStatement

My mongoose schema has a property phases that is defined as:

 phases: [
    {
      _id: false,
      dateStarted: Date,
      dateEnded: Date,
      phase: String,
      name: String,
      reason: String,
      coverage: Number,
      condition: String,
      savedGroups: [
        {
          _id: false,
          ids: [String],
          match: String,
        },
      ],
      prerequisites: [
        {
          _id: false,
          id: String,
          condition: String,
        },
      ],
      namespace: {},
      seed: String,
      variationWeights: [Number],
      groups: [String],
    },
  ],

You probably need to add a circular reference check. Or some hard coded check for __parentArray, since I'm assuming mongoose is pretty common.
])
);
}

// 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.
Expand Down
2 changes: 2 additions & 0 deletions plugins/node/opentelemetry-instrumentation-mongodb/src/internal-types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export type MongoInternalCommand = {
findandmodify: boolean;
createIndexes: boolean;
count: boolean;
aggregate: boolean;
ismaster: boolean;
indexes?: unknown[];
query?: Record<string, unknown>;
Expand Down Expand Up @@ -166,6 +167,7 @@ export enum MongodbCommandType {
FIND_AND_MODIFY = 'findAndModify',
IS_MASTER = 'isMaster',
COUNT = 'count',
AGGREGATE = 'aggregate',
UNKNOWN = 'unknown',
}

Expand Down
68 changes: 68 additions & 0 deletions plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v3.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down
68 changes: 68 additions & 0 deletions plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v4.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down
68 changes: 68 additions & 0 deletions plugins/node/opentelemetry-instrumentation-mongodb/test/mongodb-v5-v6.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down

0 comments on commit 2b1360d

Please sign in to comment.