From 17ed7b957e93608bc2e1c280a3f8faa067d8b84c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Tue, 24 Nov 2020 15:41:54 +0100 Subject: [PATCH 1/9] feat: MongoDB Tracing Support --- packages/tracing/src/integrations/index.ts | 1 + packages/tracing/src/integrations/mongo.ts | 211 +++++++++++++++++++++ 2 files changed, 212 insertions(+) create mode 100644 packages/tracing/src/integrations/mongo.ts diff --git a/packages/tracing/src/integrations/index.ts b/packages/tracing/src/integrations/index.ts index abe1faf43c27..f4255a0e3937 100644 --- a/packages/tracing/src/integrations/index.ts +++ b/packages/tracing/src/integrations/index.ts @@ -1 +1,2 @@ export { Express } from './express'; +export { Mongo } from './mongo'; diff --git a/packages/tracing/src/integrations/mongo.ts b/packages/tracing/src/integrations/mongo.ts new file mode 100644 index 000000000000..2342e79cd9b5 --- /dev/null +++ b/packages/tracing/src/integrations/mongo.ts @@ -0,0 +1,211 @@ +import { Hub } from '@sentry/hub'; +import { EventProcessor, Integration, SpanContext } from '@sentry/types'; +import { dynamicRequire, fill, logger } from '@sentry/utils'; + +// This allows us to use the same array for both, defaults option and the type itself. +// (note `as const` at the end to make it a concrete union type, and not just string[]) +type Operation = typeof OPERATIONS[number]; +const OPERATIONS = [ + 'aggregate', // aggregate(pipeline, options, callback) + 'bulkWrite', // bulkWrite(operations, options, callback) + 'countDocuments', // countDocuments(query, options, callback) + 'createIndex', // createIndex(fieldOrSpec, options, callback) + 'createIndexes', // createIndexes(indexSpecs, options, callback) + 'deleteMany', // deleteMany(filter, options, callback) + 'deleteOne', // deleteOne(filter, options, callback) + 'distinct', // distinct(key, query, options, callback) + 'drop', // drop(options, callback) + 'dropIndex', // dropIndex(indexName, options, callback) + 'dropIndexes', // dropIndexes(options, callback) + 'estimatedDocumentCount', // estimatedDocumentCount(options, callback) + 'findOne', // findOne(query, options, callback) + 'findOneAndDelete', // findOneAndDelete(filter, options, callback) + 'findOneAndReplace', // findOneAndReplace(filter, replacement, options, callback) + 'findOneAndUpdate', // findOneAndUpdate(filter, update, options, callback) + 'indexes', // indexes(options, callback) + 'indexExists', // indexExists(indexes, options, callback) + 'indexInformation', // indexInformation(options, callback) + 'initializeOrderedBulkOp', // initializeOrderedBulkOp(options, callback) + 'insertMany', // insertMany(docs, options, callback) + 'insertOne', // insertOne(doc, options, callback) + 'isCapped', // isCapped(options, callback) + 'mapReduce', // mapReduce(map, reduce, options, callback) + 'options', // options(options, callback) + 'parallelCollectionScan', // parallelCollectionScan(options, callback) + 'rename', // rename(newName, options, callback) + 'replaceOne', // replaceOne(filter, doc, options, callback) + 'stats', // stats(options, callback) + 'updateMany', // updateMany(filter, update, options, callback) + 'updateOne', // updateOne(filter, update, options, callback) +] as const; + +const OPERATION_SIGNATURES: { + [op in Operation]?: string[]; +} = { + bulkWrite: ['operations'], + countDocuments: ['query'], + createIndex: ['fieldOrSpec'], + createIndexes: ['indexSpecs'], + deleteMany: ['filter'], + deleteOne: ['filter'], + distinct: ['key', 'query'], + dropIndex: ['indexName'], + findOne: ['query'], + findOneAndDelete: ['filter'], + findOneAndReplace: ['filter', 'replacement'], + findOneAndUpdate: ['filter', 'update'], + indexExists: ['indexes'], + insertMany: ['docs'], + insertOne: ['doc'], + mapReduce: ['map', 'reduce'], + rename: ['newName'], + replaceOne: ['filter', 'doc'], + updateMany: ['filter', 'update'], + updateOne: ['filter', 'update'], +}; + +interface MongoCollection { + collectionName: string; + dbName: string; + namespace: string; + prototype: { + [operation in Operation]: (...args: unknown[]) => unknown; + }; +} + +interface MongoOptions { + operations?: Operation[]; + describeOperations?: boolean | Operation[]; +} + +/** Tracing integration for node-postgres package */ +export class Mongo implements Integration { + /** + * @inheritDoc + */ + public static id: string = 'Mongo'; + + /** + * @inheritDoc + */ + public name: string = Mongo.id; + + private _operations: Operation[]; + private _describeOperations?: boolean | Operation[]; + + /** + * @inheritDoc + */ + public constructor(options: MongoOptions = {}) { + this._operations = Array.isArray(options.operations) + ? options.operations + : ((OPERATIONS as unknown) as Operation[]); + this._describeOperations = 'describeOperations' in options ? options.describeOperations : true; + } + + /** + * @inheritDoc + */ + public setupOnce(_: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void { + let collection: MongoCollection; + + try { + const mongodbModule = dynamicRequire(module, 'mongodb') as { Collection: MongoCollection }; + collection = mongodbModule.Collection; + } catch (e) { + logger.error('Mongo Integration was unable to require `mongodb` package.'); + return; + } + + this._instrumentOperations(collection, this._operations, getCurrentHub); + } + + /** + * Patches original collection methods + */ + private _instrumentOperations(collection: MongoCollection, operations: Operation[], getCurrentHub: () => Hub): void { + operations.forEach((operation: Operation) => this._patchOperation(collection, operation, getCurrentHub)); + } + + /** + * Patches original collection to utilize our tracing functionality + */ + private _patchOperation(collection: MongoCollection, operation: Operation, getCurrentHub: () => Hub): void { + if (!(operation in collection.prototype)) return; + + const getSpanContext = this._getSpanContextFromOperationArguments.bind(this); + + fill(collection.prototype, operation, function(orig: () => void | Promise) { + return function(this: unknown, ...args: unknown[]) { + const lastArg = args[args.length - 1]; + const scope = getCurrentHub().getScope(); + const transaction = scope?.getTransaction(); + + // mapReduce is a special edge-case, as it's the only operation that accepts functions + // other than the callback as it's own arguments. Therefore despite lastArg being + // a function, it can be still a promise-based call without a callback. + // mapReduce(map, reduce, options, callback) where `[map|reduce]: function | string` + if (typeof lastArg !== 'function' || (operation === 'mapReduce' && args.length === 2)) { + const span = transaction?.startChild(getSpanContext(this, operation, args)); + return (orig.call(this, ...args) as Promise).then((res: unknown) => { + span?.finish(); + return res; + }); + } + + const span = transaction?.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); + }); + }; + }); + } + + /** + * Form a SpanContext based on the user input to a given operation. + */ + private _getSpanContextFromOperationArguments( + collection: MongoCollection, + operation: Operation, + args: unknown[], + ): SpanContext { + const data: { [key: string]: string } = { + collectionName: collection.collectionName, + dbName: collection.dbName, + namespace: collection.namespace, + }; + const spanContext: SpanContext = { + op: `query.${operation}`, + data, + }; + + // If there was no signature available for us to be used for the extracted data description. + // Or user decided to not describe given operation, just return early. + const signature = OPERATION_SIGNATURES[operation]; + const shouldDescribe = Array.isArray(this._describeOperations) + ? this._describeOperations.includes(operation) + : this._describeOperations; + + if (!signature || !shouldDescribe) { + return spanContext; + } + + try { + // Special case for `mapReduce`, as the only one accepting functions as arguments. + if (operation === 'mapReduce') { + const [map, reduce] = args as { name?: string }[]; + data[signature[0]] = typeof map === 'string' ? map : map.name || ''; + data[signature[1]] = typeof reduce === 'string' ? reduce : reduce.name || ''; + } else { + for (let i = 0; i < signature.length; i++) { + data[signature[i]] = JSON.stringify(args[i]); + } + } + } catch (_oO) { + // no-empty + } + + return spanContext; + } +} From 878456660747a6df2c89da5a54a655c29845b647 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 3 Dec 2020 22:24:51 -0800 Subject: [PATCH 2/9] s/concrete/literal --- packages/tracing/src/integrations/mongo.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/tracing/src/integrations/mongo.ts b/packages/tracing/src/integrations/mongo.ts index 2342e79cd9b5..89038634304e 100644 --- a/packages/tracing/src/integrations/mongo.ts +++ b/packages/tracing/src/integrations/mongo.ts @@ -2,8 +2,9 @@ import { Hub } from '@sentry/hub'; import { EventProcessor, Integration, SpanContext } from '@sentry/types'; import { dynamicRequire, fill, logger } from '@sentry/utils'; -// This allows us to use the same array for both, defaults option and the type itself. -// (note `as const` at the end to make it a concrete union type, and not just string[]) +// This allows us to use the same array for both defaults options and the type itself. +// (note `as const` at the end to make it a union of string literal types (i.e. "a" | "b" | ... ) +// and not just a string[]) type Operation = typeof OPERATIONS[number]; const OPERATIONS = [ 'aggregate', // aggregate(pipeline, options, callback) From ff88e167e45c3422675c19bd6a5f591478bb8dd3 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 3 Dec 2020 22:26:31 -0800 Subject: [PATCH 3/9] add comment explaining operation signature map --- packages/tracing/src/integrations/mongo.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/tracing/src/integrations/mongo.ts b/packages/tracing/src/integrations/mongo.ts index 89038634304e..6758d10f8049 100644 --- a/packages/tracing/src/integrations/mongo.ts +++ b/packages/tracing/src/integrations/mongo.ts @@ -40,6 +40,10 @@ const OPERATIONS = [ 'updateOne', // updateOne(filter, update, options, callback) ] as const; +// All of the operations above take `options` and `callback` as their final parameters, but some of them +// take additional parameters as well. For those operations, this is a map of +// { : [] }, as a way to know what to call the operation's +// positional arguments when we add them to the span's `data` object later const OPERATION_SIGNATURES: { [op in Operation]?: string[]; } = { From 5dedbe785369e65d6c59318bccc601ed74a709fc Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 3 Dec 2020 22:26:52 -0800 Subject: [PATCH 4/9] add missing operation --- packages/tracing/src/integrations/mongo.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/tracing/src/integrations/mongo.ts b/packages/tracing/src/integrations/mongo.ts index 6758d10f8049..46b81d40e929 100644 --- a/packages/tracing/src/integrations/mongo.ts +++ b/packages/tracing/src/integrations/mongo.ts @@ -47,6 +47,7 @@ const OPERATIONS = [ const OPERATION_SIGNATURES: { [op in Operation]?: string[]; } = { + aggregate: ['pipeline'], bulkWrite: ['operations'], countDocuments: ['query'], createIndex: ['fieldOrSpec'], From 4891c8226c77b1a38191510118e6e58466e72614 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 3 Dec 2020 22:27:17 -0800 Subject: [PATCH 5/9] copy pasta --- packages/tracing/src/integrations/mongo.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tracing/src/integrations/mongo.ts b/packages/tracing/src/integrations/mongo.ts index 46b81d40e929..fb2b26f14cbf 100644 --- a/packages/tracing/src/integrations/mongo.ts +++ b/packages/tracing/src/integrations/mongo.ts @@ -84,7 +84,7 @@ interface MongoOptions { describeOperations?: boolean | Operation[]; } -/** Tracing integration for node-postgres package */ +/** Tracing integration for mongo package */ export class Mongo implements Integration { /** * @inheritDoc From 52d082c1d660c36f0e17b6c0c317d6c63ba76320 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 3 Dec 2020 22:28:36 -0800 Subject: [PATCH 6/9] streamline comment --- packages/tracing/src/integrations/mongo.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/tracing/src/integrations/mongo.ts b/packages/tracing/src/integrations/mongo.ts index fb2b26f14cbf..944a5bc9e377 100644 --- a/packages/tracing/src/integrations/mongo.ts +++ b/packages/tracing/src/integrations/mongo.ts @@ -147,10 +147,8 @@ export class Mongo implements Integration { const scope = getCurrentHub().getScope(); const transaction = scope?.getTransaction(); - // mapReduce is a special edge-case, as it's the only operation that accepts functions - // other than the callback as it's own arguments. Therefore despite lastArg being - // a function, it can be still a promise-based call without a callback. - // mapReduce(map, reduce, options, callback) where `[map|reduce]: function | string` + // Check if the operation was passed a callback. (mapReduce requires a different check, as + // its (non-callback) arguments can also be functions.) if (typeof lastArg !== 'function' || (operation === 'mapReduce' && args.length === 2)) { const span = transaction?.startChild(getSpanContext(this, operation, args)); return (orig.call(this, ...args) as Promise).then((res: unknown) => { From 9b8bdd2f3309b336dd7303f7f186833db6f91df0 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 3 Dec 2020 22:29:03 -0800 Subject: [PATCH 7/9] clarify comment --- packages/tracing/src/integrations/mongo.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/tracing/src/integrations/mongo.ts b/packages/tracing/src/integrations/mongo.ts index 944a5bc9e377..5aeac299a381 100644 --- a/packages/tracing/src/integrations/mongo.ts +++ b/packages/tracing/src/integrations/mongo.ts @@ -184,8 +184,8 @@ export class Mongo implements Integration { data, }; - // If there was no signature available for us to be used for the extracted data description. - // Or user decided to not describe given operation, just return early. + // If the operation takes no arguments besides `options` and `callback`, or if argument + // collection is disabled for this operation, just return early. const signature = OPERATION_SIGNATURES[operation]; const shouldDescribe = Array.isArray(this._describeOperations) ? this._describeOperations.includes(operation) From 4096ba1e0c0db91b611c302e1142ddab343e4153 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Fri, 4 Dec 2020 11:55:30 +0100 Subject: [PATCH 8/9] ref: Use getSpan vs. getTransaction --- packages/tracing/src/integrations/mongo.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/tracing/src/integrations/mongo.ts b/packages/tracing/src/integrations/mongo.ts index 5aeac299a381..9a037c4ff815 100644 --- a/packages/tracing/src/integrations/mongo.ts +++ b/packages/tracing/src/integrations/mongo.ts @@ -3,7 +3,7 @@ import { EventProcessor, Integration, SpanContext } from '@sentry/types'; import { dynamicRequire, fill, logger } from '@sentry/utils'; // This allows us to use the same array for both defaults options and the type itself. -// (note `as const` at the end to make it a union of string literal types (i.e. "a" | "b" | ... ) +// (note `as const` at the end to make it a union of string literal types (i.e. "a" | "b" | ... ) // and not just a string[]) type Operation = typeof OPERATIONS[number]; const OPERATIONS = [ @@ -41,7 +41,7 @@ const OPERATIONS = [ ] as const; // All of the operations above take `options` and `callback` as their final parameters, but some of them -// take additional parameters as well. For those operations, this is a map of +// take additional parameters as well. For those operations, this is a map of // { : [] }, as a way to know what to call the operation's // positional arguments when we add them to the span's `data` object later const OPERATION_SIGNATURES: { @@ -145,19 +145,19 @@ export class Mongo implements Integration { return function(this: unknown, ...args: unknown[]) { const lastArg = args[args.length - 1]; const scope = getCurrentHub().getScope(); - const transaction = scope?.getTransaction(); + const parentSpan = scope?.getSpan(); // Check if the operation was passed a callback. (mapReduce requires a different check, as // its (non-callback) arguments can also be functions.) if (typeof lastArg !== 'function' || (operation === 'mapReduce' && args.length === 2)) { - const span = transaction?.startChild(getSpanContext(this, operation, args)); + const span = parentSpan?.startChild(getSpanContext(this, operation, args)); return (orig.call(this, ...args) as Promise).then((res: unknown) => { span?.finish(); return res; }); } - const span = transaction?.startChild(getSpanContext(this, operation, args.slice(0, -1))); + 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 0253cf29c9cd47f2d0f3764a9604dbbb20b591b4 Mon Sep 17 00:00:00 2001 From: Daniel Griesser Date: Fri, 4 Dec 2020 12:34:17 +0100 Subject: [PATCH 9/9] fix: Express name and span op --- packages/node/src/handlers.ts | 4 +++- packages/node/src/integrations/http.ts | 10 +++++----- packages/tracing/src/integrations/mongo.ts | 3 ++- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index cddcc2529f28..c5d819b1b66b 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -108,10 +108,12 @@ function extractRouteInfo(req: ExpressRequest, options: { path?: boolean; method let path; if (req.baseUrl && req.route) { path = `${req.baseUrl}${req.route.path}`; + } else if (req.route) { + path = `${req.route.path}`; } else if (req.originalUrl || req.url) { path = stripUrlQueryAndFragment(req.originalUrl || req.url || ''); } else { - path = req.route?.path || ''; + path = ''; } let info = ''; diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts index eec8553e18c2..e331f69b109f 100644 --- a/packages/node/src/integrations/http.ts +++ b/packages/node/src/integrations/http.ts @@ -1,5 +1,5 @@ import { getCurrentHub } from '@sentry/core'; -import { Integration, Span, Transaction } from '@sentry/types'; +import { Integration, Span } from '@sentry/types'; import { fill, logger, parseSemver } from '@sentry/utils'; import * as http from 'http'; import * as https from 'https'; @@ -104,13 +104,13 @@ function _createWrappedRequestMethodFactory( } let span: Span | undefined; - let transaction: Transaction | undefined; + let parentSpan: Span | undefined; const scope = getCurrentHub().getScope(); if (scope && tracingEnabled) { - transaction = scope.getTransaction(); - if (transaction) { - span = transaction.startChild({ + parentSpan = scope.getSpan(); + if (parentSpan) { + span = parentSpan.startChild({ description: `${requestOptions.method || 'GET'} ${requestUrl}`, op: 'request', }); diff --git a/packages/tracing/src/integrations/mongo.ts b/packages/tracing/src/integrations/mongo.ts index 9a037c4ff815..7870631a2948 100644 --- a/packages/tracing/src/integrations/mongo.ts +++ b/packages/tracing/src/integrations/mongo.ts @@ -180,7 +180,8 @@ export class Mongo implements Integration { namespace: collection.namespace, }; const spanContext: SpanContext = { - op: `query.${operation}`, + op: `db`, + description: operation, data, };