Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(instrumentation-mongoose): Fix instrumentation for Mongoose Model methods, save() and remove(), by passing options argument to original method calls #2009

Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
6b7cf21
Fixing Mongoose instrumentation on Model methods with callback + tests
ferrelucas Mar 8, 2024
2283303
Fixing and adding more tests
ferrelucas Mar 10, 2024
fac1251
Removing skip on existing failing test
ferrelucas Mar 10, 2024
f205174
Renaming callback index variable
ferrelucas Mar 10, 2024
8aa008e
Merge branch 'open-telemetry:main' into fix_mongoose_model_method_ses…
ferrelucas Mar 15, 2024
ea60ff1
Merge branch 'open-telemetry:main' into fix_mongoose_model_method_ses…
ferrelucas Mar 21, 2024
763a1a2
Merge branch 'open-telemetry:main' into fix_mongoose_model_method_ses…
ferrelucas Mar 25, 2024
adde5d2
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas Mar 27, 2024
1381823
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas Mar 31, 2024
d3294fe
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas Apr 3, 2024
57a9e9f
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas Apr 4, 2024
4a6a12b
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas Apr 5, 2024
d89bc0f
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas Apr 12, 2024
1361a24
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas Apr 16, 2024
95c1042
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas Apr 19, 2024
4d99a63
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas Apr 22, 2024
453f3a7
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas Apr 22, 2024
019d49d
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas Apr 24, 2024
0a2ffb3
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas Apr 25, 2024
d609e1f
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas Apr 26, 2024
a8219a1
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas Apr 29, 2024
784e99e
use exported strings for attributes
ferrelucas May 22, 2024
7d3b460
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas May 22, 2024
908b8af
Defining args as IArguments and updating its values directly
ferrelucas May 22, 2024
7f12860
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
blumamir May 22, 2024
1764acf
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
blumamir May 22, 2024
8e9630f
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas May 23, 2024
cbce832
Fix lint issues
ferrelucas May 23, 2024
b6b04f7
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas May 23, 2024
b414324
Replacing tests with session option to use wtimeout to avoid the need…
ferrelucas May 24, 2024
a4336d3
Merge branch 'main' into fix_mongoose_model_method_session_instrument…
ferrelucas May 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions plugins/node/instrumentation-mongoose/src/mongoose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ export class MongooseInstrumentation extends InstrumentationBase {
exec,
originalThis,
span,
args,
self._config.responseHook,
moduleVersion
)
Expand Down
25 changes: 16 additions & 9 deletions plugins/node/instrumentation-mongoose/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,23 @@ export function handleCallbackResponse(
exec: Function,
originalThis: any,
span: Span,
args: IArguments,
responseHook?: MongooseResponseCustomAttributesFunction,
moduleVersion: string | undefined = undefined
) {
return exec.apply(originalThis, [
(err: Error, response: any) => {
err
? setErrorStatus(span, err)
: applyResponseHook(span, response, responseHook, moduleVersion);
span.end();
return callback!(err, response);
},
]);
let callbackArgumentIndex = 0;
if (args.length === 2) {
callbackArgumentIndex = 1;
}

args[callbackArgumentIndex] = (err: Error, response: any): any => {
err
? setErrorStatus(span, err)
: applyResponseHook(span, response, responseHook, moduleVersion);

span.end();
return callback!(err, response);
};

return exec.apply(originalThis, args);
}
161 changes: 150 additions & 11 deletions plugins/node/instrumentation-mongoose/test/mongoose.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,11 @@ describe('mongoose instrumentation', () => {
beforeEach(async () => {
instrumentation.disable();
instrumentation.setConfig({
dbStatementSerializer: (_operation: string, payload) =>
JSON.stringify(payload),
dbStatementSerializer: (_operation: string, payload) => {
return JSON.stringify(payload, (key, value) => {
return key === 'session' ? '[Session]' : value;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Note this replacer function was only introduced here to avoid an error due to the Session object having cyclical reference

});
},
});
instrumentation.enable();
await loadUsers();
Expand Down Expand Up @@ -97,23 +100,159 @@ describe('mongoose instrumentation', () => {
expect(statement.document).toEqual(expect.objectContaining(document));
});

it('instrumenting save operation with callback', done => {
const document = {
firstName: 'Test first name',
lastName: 'Test last name',
email: 'test@example.com',
};
const user: IUser = new User(document);
describe('when save call does not have callback', async () => {
it('instrumenting save operation with promise and committed session', async () => {
const session = await User.startSession();
await session.startTransaction();
const document = {
firstName: 'Test first name',
lastName: 'Test last name',
email: 'test@example.com',
};
const user: IUser = new User(document);

await user.save({ session });
await session.commitTransaction();
session.endSession();

user.save(() => {
const spans = getTestSpans();
expect(spans.length).toBe(1);
assertSpan(spans[0] as ReadableSpan);
expect(spans[0].attributes[SEMATTRS_DB_OPERATION]).toBe('save');
const statement = getStatement(spans[0] as ReadableSpan);
expect(statement.document).toEqual(expect.objectContaining(document));

const createdUser = await User.findById(user._id).lean();
expect(createdUser?._id.toString()).toEqual(user._id.toString());
});

it('instrumenting save operation with promise and session with aborted transaction ', async () => {
const session = await User.startSession();
await session.startTransaction();
const document = {
firstName: 'Test first name',
lastName: 'Test last name',
email: 'test@example.com',
};
const user: IUser = new User(document);

await user.save({ session });
await session.abortTransaction();
session.endSession();

const spans = getTestSpans();
expect(spans.length).toBe(1);
assertSpan(spans[0] as ReadableSpan);
expect(spans[0].attributes[SEMATTRS_DB_OPERATION]).toBe('save');
const statement = getStatement(spans[0] as ReadableSpan);
expect(statement.document).toEqual(expect.objectContaining(document));
done();

const createdUser = await User.findById(user._id).lean();
expect(createdUser).toEqual(null);
});
});

describe('when save call has callback', async () => {
it('instrumenting save operation with promise, session and committed transaction', done => {
User.startSession().then(async session => {
await session.startTransaction();
const document = {
firstName: 'Test first name',
lastName: 'Test last name',
email: 'test@example.com',
};
const user: IUser = new User(document);
await user.save({ session }, async () => {
const spans = getTestSpans();
expect(spans.length).toBe(1);
assertSpan(spans[0] as ReadableSpan);
expect(spans[0].attributes[SEMATTRS_DB_OPERATION]).toBe(
'save'
);
const statement = getStatement(spans[0] as ReadableSpan);
expect(statement.document).toEqual(expect.objectContaining(document));

const createdUser = await User.findById(user._id).lean();
expect(createdUser?._id.toString()).toEqual(user._id.toString());
done();
});
await session.commitTransaction();
session.endSession();
});
});

it('instrumenting save operation with promise, session and aborted transaction', done => {
User.startSession().then(async session => {
await session.startTransaction();
const document = {
firstName: 'Test first name',
lastName: 'Test last name',
email: 'test@example.com',
};
const user: IUser = new User(document);

await user.save({ session }, async () => {
await session.abortTransaction();
session.endSession();

const spans = getTestSpans();
expect(spans.length).toBe(1);
assertSpan(spans[0] as ReadableSpan);
expect(spans[0].attributes[SEMATTRS_DB_OPERATION]).toBe(
'save'
);
const statement = getStatement(spans[0] as ReadableSpan);
expect(statement.document).toEqual(expect.objectContaining(document));

const createdUser = await User.findById(user._id).lean();
expect(createdUser).toEqual(null);
done();
});
});
});

it('instrumenting save operation with generic options and callback', done => {
const document = {
firstName: 'Test first name',
lastName: 'Test last name',
email: 'test@example.com',
};
const user: IUser = new User(document);

user.save({}, () => {
const spans = getTestSpans();

expect(spans.length).toBe(1);
assertSpan(spans[0] as ReadableSpan);
expect(spans[0].attributes[SEMATTRS_DB_OPERATION]).toBe(
'save'
);
const statement = getStatement(spans[0] as ReadableSpan);
expect(statement.document).toEqual(expect.objectContaining(document));
done();
});
});

it('instrumenting save operation with only callback', done => {
const document = {
firstName: 'Test first name',
lastName: 'Test last name',
email: 'test@example.com',
};
const user: IUser = new User(document);

user.save(() => {
const spans = getTestSpans();

expect(spans.length).toBe(1);
assertSpan(spans[0] as ReadableSpan);
expect(spans[0].attributes[SEMATTRS_DB_OPERATION]).toBe(
'save'
);
const statement = getStatement(spans[0] as ReadableSpan);
expect(statement.document).toEqual(expect.objectContaining(document));
done();
});
});
});

Expand Down