Skip to content

Commit

Permalink
fix(tracing): Gate mongo operation span data behind sendDefaultPii
Browse files Browse the repository at this point in the history
  • Loading branch information
AbhiPrasad committed Jan 17, 2024
1 parent ba9999f commit 08f3e00
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ conditionalTest({ min: 12 })('MongoDB Test', () => {
'db.name': 'admin',
'db.operation': 'insertOne',
'db.mongodb.collection': 'movies',
'db.mongodb.doc': '{"title":"Rick and Morty"}',
},
description: 'insertOne',
op: 'db',
Expand All @@ -45,7 +44,6 @@ conditionalTest({ min: 12 })('MongoDB Test', () => {
'db.name': 'admin',
'db.operation': 'findOne',
'db.mongodb.collection': 'movies',
'db.mongodb.query': '{"title":"Back to the Future"}',
},
description: 'findOne',
op: 'db',
Expand All @@ -56,8 +54,6 @@ conditionalTest({ min: 12 })('MongoDB Test', () => {
'db.name': 'admin',
'db.operation': 'updateOne',
'db.mongodb.collection': 'movies',
'db.mongodb.filter': '{"title":"Back to the Future"}',
'db.mongodb.update': '{"$set":{"title":"South Park"}}',
},
description: 'updateOne',
op: 'db',
Expand All @@ -68,7 +64,6 @@ conditionalTest({ min: 12 })('MongoDB Test', () => {
'db.name': 'admin',
'db.operation': 'findOne',
'db.mongodb.collection': 'movies',
'db.mongodb.query': '{"title":"South Park"}',
},
description: 'findOne',
op: 'db',
Expand All @@ -79,7 +74,6 @@ conditionalTest({ min: 12 })('MongoDB Test', () => {
'db.name': 'admin',
'db.operation': 'find',
'db.mongodb.collection': 'movies',
'db.mongodb.query': '{"title":"South Park"}',
},
description: 'find',
op: 'db',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ conditionalTest({ min: 12 })('MongoDB Test', () => {
'db.name': 'admin',
'db.operation': 'insertOne',
'db.mongodb.collection': 'movies',
'db.mongodb.doc': '{"title":"Rick and Morty"}',
},
description: 'insertOne',
op: 'db',
Expand All @@ -45,7 +44,6 @@ conditionalTest({ min: 12 })('MongoDB Test', () => {
'db.name': 'admin',
'db.operation': 'findOne',
'db.mongodb.collection': 'movies',
'db.mongodb.query': '{"title":"Back to the Future"}',
},
description: 'findOne',
op: 'db',
Expand All @@ -56,8 +54,6 @@ conditionalTest({ min: 12 })('MongoDB Test', () => {
'db.name': 'admin',
'db.operation': 'updateOne',
'db.mongodb.collection': 'movies',
'db.mongodb.filter': '{"title":"Back to the Future"}',
'db.mongodb.update': '{"$set":{"title":"South Park"}}',
},
description: 'updateOne',
op: 'db',
Expand All @@ -68,7 +64,6 @@ conditionalTest({ min: 12 })('MongoDB Test', () => {
'db.name': 'admin',
'db.operation': 'findOne',
'db.mongodb.collection': 'movies',
'db.mongodb.query': '{"title":"South Park"}',
},
description: 'findOne',
op: 'db',
Expand All @@ -79,7 +74,6 @@ conditionalTest({ min: 12 })('MongoDB Test', () => {
'db.name': 'admin',
'db.operation': 'find',
'db.mongodb.collection': 'movies',
'db.mongodb.query': '{"title":"South Park"}',
},
description: 'find',
op: 'db',
Expand Down
15 changes: 11 additions & 4 deletions packages/tracing-internal/src/node/integrations/mongo.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Hub } from '@sentry/core';
import { Hub, getClient } from '@sentry/core';
import type { EventProcessor, SpanContext } from '@sentry/types';
import { fill, isThenable, loadModule, logger } from '@sentry/utils';

Expand Down Expand Up @@ -175,15 +175,21 @@ export class Mongo implements LazyLoadedIntegration<MongoModule> {
return function (this: unknown, ...args: unknown[]) {
const lastArg = args[args.length - 1];
// eslint-disable-next-line deprecation/deprecation
const scope = getCurrentHub().getScope();
const hub = getCurrentHub();
// eslint-disable-next-line deprecation/deprecation
const scope = hub.getScope();
// eslint-disable-next-line deprecation/deprecation
const client = hub.getClient();
// eslint-disable-next-line deprecation/deprecation
const parentSpan = scope.getSpan();

const sendDefaultPii = client?.getOptions().sendDefaultPii;

// 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)) {
// eslint-disable-next-line deprecation/deprecation
const span = parentSpan?.startChild(getSpanContext(this, operation, args));
const span = parentSpan?.startChild(getSpanContext(this, operation, args, sendDefaultPii));
const maybePromiseOrCursor = orig.call(this, ...args);

if (isThenable(maybePromiseOrCursor)) {
Expand Down Expand Up @@ -232,6 +238,7 @@ export class Mongo implements LazyLoadedIntegration<MongoModule> {
collection: MongoCollection,
operation: Operation,
args: unknown[],
sendDefaultPii: boolean | undefined = false,
): SpanContext {
const data: { [key: string]: string } = {
'db.system': 'mongodb',
Expand All @@ -254,7 +261,7 @@ export class Mongo implements LazyLoadedIntegration<MongoModule> {
? this._describeOperations.includes(operation)
: this._describeOperations;

if (!signature || !shouldDescribe) {
if (!signature || !shouldDescribe || !sendDefaultPii) {
return spanContext;
}

Expand Down
25 changes: 22 additions & 3 deletions packages/tracing/test/integrations/node/mongo.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,20 +50,22 @@ describe('patchOperation()', () => {
let scope = new Scope();
let parentSpan: Span;
let childSpan: Span;
let testClient = getTestClient({});

beforeAll(() => {
new Integrations.Mongo({
operations: ['insertOne', 'initializeOrderedBulkOp'],
}).setupOnce(
() => undefined,
() => new Hub(undefined, scope),
() => new Hub(testClient, scope),
);
});

beforeEach(() => {
scope = new Scope();
parentSpan = new Span();
childSpan = parentSpan.startChild();
testClient = getTestClient({});
jest.spyOn(scope, 'getSpan').mockReturnValueOnce(parentSpan);
jest.spyOn(parentSpan, 'startChild').mockReturnValueOnce(childSpan);
jest.spyOn(childSpan, 'end');
Expand All @@ -77,7 +79,6 @@ describe('patchOperation()', () => {
'db.mongodb.collection': 'mockedCollectionName',
'db.name': 'mockedDbName',
'db.operation': 'insertOne',
'db.mongodb.doc': JSON.stringify(doc),
'db.system': 'mongodb',
},
op: 'db',
Expand All @@ -97,7 +98,25 @@ describe('patchOperation()', () => {
'db.mongodb.collection': 'mockedCollectionName',
'db.name': 'mockedDbName',
'db.operation': 'insertOne',
'db.mongodb.doc': JSON.stringify(doc),
'db.system': 'mongodb',
},
op: 'db',
origin: 'auto.db.mongo',
description: 'insertOne',
});
expect(childSpan.end).toBeCalled();
});

it('attaches mongodb operation spans if sendDefaultPii is enabled', async () => {
testClient.getOptions().sendDefaultPii = true;
await collection.insertOne(doc, {});
expect(scope.getSpan).toBeCalled();
expect(parentSpan.startChild).toBeCalledWith({
data: {
'db.mongodb.collection': 'mockedCollectionName',
'db.mongodb.doc': '{"name":"PickleRick","answer":42}',
'db.name': 'mockedDbName',
'db.operation': 'insertOne',
'db.system': 'mongodb',
},
op: 'db',
Expand Down

0 comments on commit 08f3e00

Please sign in to comment.