From a89e597d4f0f853ea86a2f9cb1cae9d24a571c79 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 29 Nov 2022 18:07:31 +0000 Subject: [PATCH 1/2] fix(tracing): Handle cursors returned from Mongodb operations. --- .../auto-instrument/mongodb/scenario.ts | 2 ++ .../tracing/auto-instrument/mongodb/test.ts | 10 ------- .../tracing/src/integrations/node/mongo.ts | 29 +++++++++++++++---- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/scenario.ts b/packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/scenario.ts index 870553f8b0b4..896a91181846 100644 --- a/packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/scenario.ts +++ b/packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/scenario.ts @@ -36,6 +36,8 @@ async function run(): Promise { await collection.findOne({ title: 'Back to the Future' }); await collection.updateOne({ title: 'Back to the Future' }, { $set: { title: 'South Park' } }); await collection.findOne({ title: 'South Park' }); + + await collection.find({ title: 'South Park' }).toArray(); } finally { if (transaction) transaction.finish(); await client.close(); diff --git a/packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/test.ts b/packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/test.ts index fda3695c1eb3..5664aac9422b 100644 --- a/packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/test.ts +++ b/packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/test.ts @@ -48,16 +48,6 @@ conditionalTest({ min: 12 })('MongoDB Test', () => { description: 'findOne', op: 'db', }, - { - data: { - collectionName: 'movies', - dbName: 'admin', - namespace: 'admin.movies', - query: '{"title":"Back to the Future"}', - }, - description: 'find', - op: 'db', - }, { data: { collectionName: 'movies', diff --git a/packages/tracing/src/integrations/node/mongo.ts b/packages/tracing/src/integrations/node/mongo.ts index e19711d12460..d117c9879bec 100644 --- a/packages/tracing/src/integrations/node/mongo.ts +++ b/packages/tracing/src/integrations/node/mongo.ts @@ -1,6 +1,7 @@ import { Hub } from '@sentry/core'; import { EventProcessor, Integration, SpanContext } from '@sentry/types'; -import { fill, isThenable, loadModule, logger } from '@sentry/utils'; +import { fill, isInstanceOf, isThenable, loadModule, logger } from '@sentry/utils'; +import { EventEmitter } from 'events'; import { shouldDisableAutoInstrumentation } from './utils/node-utils'; @@ -160,20 +161,38 @@ export class Mongo implements Integration { // its (non-callback) arguments can also be functions.) if (typeof lastArg !== 'function' || (operation === 'mapReduce' && args.length === 2)) { const span = parentSpan?.startChild(getSpanContext(this, operation, args)); - const maybePromise = orig.call(this, ...args) as Promise; + const maybePromiseOrCursor = orig.call(this, ...args); - if (isThenable(maybePromise)) { - return maybePromise.then((res: unknown) => { + if (isThenable(maybePromiseOrCursor)) { + return maybePromiseOrCursor.then((res: unknown) => { span?.finish(); return res; }); + } + // If the operation returns a cursor (which is an EventEmitter), + // we need to attach a listener to it to finish the span when the cursor is closed. + else if (isInstanceOf(maybePromiseOrCursor, EventEmitter)) { + const cursor = maybePromiseOrCursor as EventEmitter; + + try { + cursor.once('close', () => { + span?.finish(); + }); + } catch (e) { + // If the cursor is already closed, `once` will throw an error. In that case, we can + // finish the span immediately. + span?.finish(); + } + + return cursor; } else { span?.finish(); - return maybePromise; + return maybePromiseOrCursor; } } const span = parentSpan?.startChild(getSpanContext(this, operation, args.slice(0, -1))); + return orig.call(this, ...args.slice(0, -1), function (err: Error, result: unknown) { span?.finish(); lastArg(err, result); From 357de02e6750d2a1578a44d241790407e3a4f693 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Tue, 6 Dec 2022 14:45:01 +0000 Subject: [PATCH 2/2] Check the existence of once hook while matching Cursors. --- packages/tracing/src/integrations/node/mongo.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/tracing/src/integrations/node/mongo.ts b/packages/tracing/src/integrations/node/mongo.ts index d117c9879bec..c66b93ff1651 100644 --- a/packages/tracing/src/integrations/node/mongo.ts +++ b/packages/tracing/src/integrations/node/mongo.ts @@ -1,7 +1,6 @@ import { Hub } from '@sentry/core'; import { EventProcessor, Integration, SpanContext } from '@sentry/types'; -import { fill, isInstanceOf, isThenable, loadModule, logger } from '@sentry/utils'; -import { EventEmitter } from 'events'; +import { fill, isThenable, loadModule, logger } from '@sentry/utils'; import { shouldDisableAutoInstrumentation } from './utils/node-utils'; @@ -91,6 +90,14 @@ interface MongoOptions { useMongoose?: boolean; } +interface MongoCursor { + once(event: 'close', listener: () => void): void; +} + +function isCursor(maybeCursor: MongoCursor): maybeCursor is MongoCursor { + return maybeCursor && typeof maybeCursor === 'object' && maybeCursor.once && typeof maybeCursor.once === 'function'; +} + /** Tracing integration for mongo package */ export class Mongo implements Integration { /** @@ -169,10 +176,10 @@ export class Mongo implements Integration { return res; }); } - // If the operation returns a cursor (which is an EventEmitter), + // If the operation returns a Cursor // we need to attach a listener to it to finish the span when the cursor is closed. - else if (isInstanceOf(maybePromiseOrCursor, EventEmitter)) { - const cursor = maybePromiseOrCursor as EventEmitter; + else if (isCursor(maybePromiseOrCursor)) { + const cursor = maybePromiseOrCursor as MongoCursor; try { cursor.once('close', () => {