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
…10227)

As was raised in
#10219, we set some
extra operation based fields on span data for mongodb.

This is unfortunately just serialized output, which means it can contain
PII (it needs to be paramaterized for PII to be removed).

Given this data will change a lot when we switch to OTEL, gate setting
this specific span data behind `sendDefaultPii`.
  • Loading branch information
AbhiPrasad committed Jan 18, 2024
1 parent b6e84dc commit 97c4e3b
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 18 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
13 changes: 10 additions & 3 deletions packages/tracing-internal/src/node/integrations/mongo.ts
Original file line number Diff line number Diff line change
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 97c4e3b

Please sign in to comment.