Skip to content

Commit

Permalink
fix(instr-mongodb): fix function patch missing one argument introduce…
Browse files Browse the repository at this point in the history
…d in v6.8.0 (#2314)

Co-authored-by: Marc Pichler <[email protected]>
  • Loading branch information
david-luna and pichlermarc authored Jul 3, 2024
1 parent 0c46dfe commit 9dc55da
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 57 deletions.
54 changes: 16 additions & 38 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,9 @@
"@opentelemetry/sdk-trace-node": "^1.8.0",
"@types/bson": "4.0.5",
"@types/mocha": "7.0.2",
"@types/mongodb": "3.6.20",
"@types/node": "18.6.5",
"mocha": "7.2.0",
"mongodb": "6.5.0",
"mongodb": "6.8.0",
"nyc": "15.1.0",
"rimraf": "5.0.5",
"test-all-versions": "6.1.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import {
ServerSession,
MongodbCommandType,
MongoInternalCommand,
MongodbNamespace,
MongoInternalTopology,
WireProtocolInternal,
V4Connection,
Expand Down Expand Up @@ -529,7 +530,7 @@ export class MongoDBInstrumentation extends InstrumentationBase {
return (original: V4Connection['commandCallback']) => {
return function patchedV4ServerCommand(
this: any,
ns: any,
ns: MongodbNamespace,
cmd: any,
options: undefined | unknown,
callback: any
Expand Down Expand Up @@ -577,16 +578,15 @@ export class MongoDBInstrumentation extends InstrumentationBase {
return (original: V4Connection['commandPromise']) => {
return function patchedV4ServerCommand(
this: any,
ns: any,
cmd: any,
options: undefined | unknown
...args: Parameters<V4Connection['commandPromise']>
) {
const [ns, cmd] = args;
const currentSpan = trace.getSpan(context.active());
const commandType = Object.keys(cmd)[0];
const resultHandler = () => undefined;

if (typeof cmd !== 'object' || cmd.ismaster || cmd.hello) {
return original.call(this, ns, cmd, options);
return original.apply(this, args);
}

let span = undefined;
Expand All @@ -610,7 +610,7 @@ export class MongoDBInstrumentation extends InstrumentationBase {
commandType
);

const result = original.call(this, ns, cmd, options);
const result = original.apply(this, args);
result.then(
(res: any) => patchedCallback(null, res),
(err: any) => patchedCallback(err)
Expand Down Expand Up @@ -792,7 +792,7 @@ export class MongoDBInstrumentation extends InstrumentationBase {
private _populateV4Attributes(
span: Span,
connectionCtx: any,
ns: any,
ns: MongodbNamespace,
command?: any,
operation?: string
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,19 +176,28 @@ export type Document = {
[key: string]: any;
};

// https://github.com/mongodb/node-mongodb-native/blob/v6.4.0/src/utils.ts#L281
export interface MongodbNamespace {
db: string;
collection?: string;
}

export type V4Connection = {
command: Function;
// From version 6.4.0 the method does not expect a callback and returns a promise
// https://github.com/mongodb/node-mongodb-native/blob/v6.4.2/src/cmap/connection.ts
commandPromise(
ns: any,
ns: MongodbNamespace,
cmd: Document,
options: undefined | unknown
options: undefined | unknown,
// From v6.6.0 we have this new param which is a constructor function
// https://github.com/mongodb/node-mongodb-native/blob/v6.6.0/src/cmap/connection.ts#L588
responseType: undefined | unknown
): Promise<any>;
// Earlier versions expect a callback param and return void
// https://github.com/mongodb/node-mongodb-native/blob/v4.2.2/src/cmap/connection.ts
commandCallback(
ns: any,
ns: MongodbNamespace,
cmd: Document,
options: undefined | unknown,
callback: any
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -499,12 +499,23 @@ describe('MongoDBInstrumentation-Tracing-v5', () => {
});

describe('when specifying a responseHook configuration', () => {
const dataAttributeName = 'mongodb_data';
describe('with a valid function', () => {
beforeEach(() => {
create({
responseHook: (span: Span, result: MongoResponseHookInformation) => {
span.setAttribute(dataAttributeName, JSON.stringify(result.data));
responseHook: (span: Span, result: any) => {
const { data } = result;
if (data.n) {
span.setAttribute('mongodb_insert_count', result.data.n);
}
// from v6.8.0 the cursor property is not an object but an instance of
// `CursorResponse`. We need to use the `toObject` method to be able to inspect the data
const cursorObj = data.cursor.firstBatch
? data.cursor
: data.cursor.toObject();
span.setAttribute(
'mongodb_first_result',
JSON.stringify(cursorObj.firstBatch[0])
);
},
});
});
Expand All @@ -520,8 +531,7 @@ describe('MongoDBInstrumentation-Tracing-v5', () => {
const spans = getTestSpans();
const insertSpan = spans[0];
assert.deepStrictEqual(
JSON.parse(insertSpan.attributes[dataAttributeName] as string)
.n,
insertSpan.attributes['mongodb_insert_count'],
results?.insertedCount
);

Expand All @@ -544,12 +554,12 @@ describe('MongoDBInstrumentation-Tracing-v5', () => {
const spans = getTestSpans();
const findSpan = spans[0];
const hookAttributeValue = JSON.parse(
findSpan.attributes[dataAttributeName] as string
findSpan.attributes['mongodb_first_result'] as string
);

if (results) {
assert.strictEqual(
hookAttributeValue?.cursor?.firstBatch[0]._id,
hookAttributeValue?._id,
results[0]._id.toString()
);
} else {
Expand Down

0 comments on commit 9dc55da

Please sign in to comment.