Skip to content

fix(tracing): Gate mongo operation span data behind sendDefaultPii #10227

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 18, 2024
Merged

fix(tracing): Gate mongo operation span data behind sendDefaultPii #10227

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e70abfd
fix(tracing): Gate mongo operation span data behind sendDefaultPii
AbhiPrasad Jan 17, 2024
3592dbf
lint fix
AbhiPrasad Jan 18, 2024
097261a
Merge branch 'develop' into abhi-mongodb-pii
AbhiPrasad Jan 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions dev-packages/node-integration-tests/suites/tracing-new/auto-instrument/mongodb/test.ts
View file
Open in desktop
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
6 changes: 0 additions & 6 deletions dev-packages/node-integration-tests/suites/tracing/auto-instrument/mongodb/test.ts
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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