From 402bd0edfc7e52f6dc76839c0d77c38a9df52712 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Wed, 23 Oct 2024 19:38:12 -0400 Subject: [PATCH 1/4] fix(mongo): rewrite Buffer as ? during serialization --- .../node/src/integrations/tracing/mongo.ts | 47 +++++++++++++ .../test/integrations/tracing/mongo.test.ts | 68 +++++++++++++++++++ 2 files changed, 115 insertions(+) create mode 100644 packages/node/test/integrations/tracing/mongo.test.ts diff --git a/packages/node/src/integrations/tracing/mongo.ts b/packages/node/src/integrations/tracing/mongo.ts index 143c7bf99a6d..1d0dc788e82e 100644 --- a/packages/node/src/integrations/tracing/mongo.ts +++ b/packages/node/src/integrations/tracing/mongo.ts @@ -11,12 +11,59 @@ export const instrumentMongo = generateInstrumentOnce( INTEGRATION_NAME, () => new MongoDBInstrumentation({ + dbStatementSerializer: _defaultDbStatementSerializer, responseHook(span) { addOriginToSpan(span, 'auto.db.otel.mongo'); }, }), ); +/** + * Replaces values in document with '?', hiding PII and helping grouping. + */ +export function _defaultDbStatementSerializer(commandObj: Record): string { + const resultObj = _scrubStatement(commandObj); + return JSON.stringify(resultObj); +} + +function _scrubStatement(value: unknown): unknown { + if (Array.isArray(value)) { + return value.map(element => _scrubStatement(element)); + } + + if (isCommandObj(value)) { + const initial: Record = {}; + return Object.entries(value).map(([key, element]) => [ + key, + _scrubStatement(element), + ]).reduce((prev, current) => { + if (isCommandEntry(current)) { + prev[current[0]] = current[1]; + } + return prev; + }, initial); + } + + // A value like string or number, possible contains PII, scrub it + return '?'; +} + +function isCommandObj(value: Record | unknown): value is Record { + return typeof value === 'object' && value !== null && !isBuffer(value); +} + +function isBuffer(value: unknown): boolean { + let isBuffer = false; + if (typeof Buffer !== 'undefined') { + isBuffer = Buffer.isBuffer(value); + } + return isBuffer; +} + +function isCommandEntry(value: [string, unknown] | unknown): value is [string, unknown] { + return Array.isArray(value); +} + const _mongoIntegration = (() => { return { name: INTEGRATION_NAME, diff --git a/packages/node/test/integrations/tracing/mongo.test.ts b/packages/node/test/integrations/tracing/mongo.test.ts new file mode 100644 index 000000000000..dcb9641e728a --- /dev/null +++ b/packages/node/test/integrations/tracing/mongo.test.ts @@ -0,0 +1,68 @@ +import { MongoDBInstrumentation } from '@opentelemetry/instrumentation-mongodb'; + +import { mongoIntegration, instrumentMongo, _defaultDbStatementSerializer } from '../../../src/integrations/tracing/mongo'; +import { INSTRUMENTED } from '../../../src/otel/instrument'; + +jest.mock('@opentelemetry/instrumentation-mongodb'); + +describe('Mongo', () => { + beforeEach(() => { + jest.clearAllMocks(); + delete INSTRUMENTED.Mongo; + + (MongoDBInstrumentation as unknown as jest.SpyInstance).mockImplementation(() => { + return { + setTracerProvider: () => undefined, + setMeterProvider: () => undefined, + getConfig: () => ({}), + setConfig: () => ({}), + enable: () => undefined, + }; + }); + }); + + it('defaults are correct for instrumentMongo', () => { + instrumentMongo(); + + expect(MongoDBInstrumentation).toHaveBeenCalledTimes(1); + expect(MongoDBInstrumentation).toHaveBeenCalledWith({ + dbStatementSerializer: expect.any(Function), + responseHook: expect.any(Function), + }); + }); + + it('defaults are correct for mongoIntegration', () => { + mongoIntegration().setupOnce!(); + + expect(MongoDBInstrumentation).toHaveBeenCalledTimes(1); + expect(MongoDBInstrumentation).toHaveBeenCalledWith({ + responseHook: expect.any(Function), + dbStatementSerializer: expect.any(Function) + }); + }); + + describe('_defaultDbStatementSerializer', () => { + it('rewrites strings as ?', () => { + const serialized = _defaultDbStatementSerializer({ + find: 'foo' + }); + expect(JSON.parse(serialized).find).toBe('?'); + }); + + it('rewrites nested strings as ?', () => { + const serialized = _defaultDbStatementSerializer({ + find: { + inner: 'foo' + } + }); + expect(JSON.parse(serialized).find.inner).toBe('?'); + }); + + it('rewrites Buffer as ?', () => { + const serialized = _defaultDbStatementSerializer({ + find: Buffer.from('foo', 'utf8') + }); + expect(JSON.parse(serialized).find).toBe('?'); + }); + }); +}); From f2f387c382065f200be41d7f77c25634fbdedac8 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Wed, 23 Oct 2024 19:56:03 -0400 Subject: [PATCH 2/4] chore: fix formatting --- .../node/src/integrations/tracing/mongo.ts | 19 +++++++++---------- .../test/integrations/tracing/mongo.test.ts | 16 ++++++++++------ 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/packages/node/src/integrations/tracing/mongo.ts b/packages/node/src/integrations/tracing/mongo.ts index 1d0dc788e82e..3f7e2f0c1478 100644 --- a/packages/node/src/integrations/tracing/mongo.ts +++ b/packages/node/src/integrations/tracing/mongo.ts @@ -33,15 +33,14 @@ function _scrubStatement(value: unknown): unknown { if (isCommandObj(value)) { const initial: Record = {}; - return Object.entries(value).map(([key, element]) => [ - key, - _scrubStatement(element), - ]).reduce((prev, current) => { - if (isCommandEntry(current)) { - prev[current[0]] = current[1]; - } - return prev; - }, initial); + return Object.entries(value) + .map(([key, element]) => [key, _scrubStatement(element)]) + .reduce((prev, current) => { + if (isCommandEntry(current)) { + prev[current[0]] = current[1]; + } + return prev; + }, initial); } // A value like string or number, possible contains PII, scrub it @@ -60,7 +59,7 @@ function isBuffer(value: unknown): boolean { return isBuffer; } -function isCommandEntry(value: [string, unknown] | unknown): value is [string, unknown] { +function isCommandEntry(value: [string, unknown] | unknown): value is [string, unknown] { return Array.isArray(value); } diff --git a/packages/node/test/integrations/tracing/mongo.test.ts b/packages/node/test/integrations/tracing/mongo.test.ts index dcb9641e728a..29571c07babe 100644 --- a/packages/node/test/integrations/tracing/mongo.test.ts +++ b/packages/node/test/integrations/tracing/mongo.test.ts @@ -1,6 +1,10 @@ import { MongoDBInstrumentation } from '@opentelemetry/instrumentation-mongodb'; -import { mongoIntegration, instrumentMongo, _defaultDbStatementSerializer } from '../../../src/integrations/tracing/mongo'; +import { + _defaultDbStatementSerializer, + instrumentMongo, + mongoIntegration, +} from '../../../src/integrations/tracing/mongo'; import { INSTRUMENTED } from '../../../src/otel/instrument'; jest.mock('@opentelemetry/instrumentation-mongodb'); @@ -37,14 +41,14 @@ describe('Mongo', () => { expect(MongoDBInstrumentation).toHaveBeenCalledTimes(1); expect(MongoDBInstrumentation).toHaveBeenCalledWith({ responseHook: expect.any(Function), - dbStatementSerializer: expect.any(Function) + dbStatementSerializer: expect.any(Function), }); }); describe('_defaultDbStatementSerializer', () => { it('rewrites strings as ?', () => { const serialized = _defaultDbStatementSerializer({ - find: 'foo' + find: 'foo', }); expect(JSON.parse(serialized).find).toBe('?'); }); @@ -52,15 +56,15 @@ describe('Mongo', () => { it('rewrites nested strings as ?', () => { const serialized = _defaultDbStatementSerializer({ find: { - inner: 'foo' - } + inner: 'foo', + }, }); expect(JSON.parse(serialized).find.inner).toBe('?'); }); it('rewrites Buffer as ?', () => { const serialized = _defaultDbStatementSerializer({ - find: Buffer.from('foo', 'utf8') + find: Buffer.from('foo', 'utf8'), }); expect(JSON.parse(serialized).find).toBe('?'); }); From 989826c96ed0746f61f7081279c8bdb4da536e6a Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Wed, 23 Oct 2024 20:27:05 -0400 Subject: [PATCH 3/4] chore: update integration tests --- .../node-integration-tests/suites/tracing/mongodb/test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/mongodb/test.ts b/dev-packages/node-integration-tests/suites/tracing/mongodb/test.ts index 59c50d32ebdc..399845a2e854 100644 --- a/dev-packages/node-integration-tests/suites/tracing/mongodb/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/mongodb/test.ts @@ -72,11 +72,11 @@ describe('MongoDB experimental Test', () => { 'net.peer.name': expect.any(String), 'net.peer.port': expect.any(Number), 'db.statement': - '{"title":"?","_id":{"_bsontype":"?","id":{"0":"?","1":"?","2":"?","3":"?","4":"?","5":"?","6":"?","7":"?","8":"?","9":"?","10":"?","11":"?"}}}', + '{"title":"?","_id":{"_bsontype":"?","id":"?"}}', 'otel.kind': 'CLIENT', }, description: - '{"title":"?","_id":{"_bsontype":"?","id":{"0":"?","1":"?","2":"?","3":"?","4":"?","5":"?","6":"?","7":"?","8":"?","9":"?","10":"?","11":"?"}}}', + '{"title":"?","_id":{"_bsontype":"?","id":"?"}}', op: 'db', origin: 'auto.db.otel.mongo', }), @@ -163,11 +163,11 @@ describe('MongoDB experimental Test', () => { 'net.peer.name': expect.any(String), 'net.peer.port': expect.any(Number), 'db.statement': - '{"endSessions":[{"id":{"_bsontype":"?","sub_type":"?","position":"?","buffer":{"0":"?","1":"?","2":"?","3":"?","4":"?","5":"?","6":"?","7":"?","8":"?","9":"?","10":"?","11":"?","12":"?","13":"?","14":"?","15":"?"}}}]}', + '{"endSessions":[{"id":{"_bsontype":"?","sub_type":"?","position":"?","buffer":"?"}}]}', 'otel.kind': 'CLIENT', }, description: - '{"endSessions":[{"id":{"_bsontype":"?","sub_type":"?","position":"?","buffer":{"0":"?","1":"?","2":"?","3":"?","4":"?","5":"?","6":"?","7":"?","8":"?","9":"?","10":"?","11":"?","12":"?","13":"?","14":"?","15":"?"}}}]}', + '{"endSessions":[{"id":{"_bsontype":"?","sub_type":"?","position":"?","buffer":"?"}}]}', op: 'db', origin: 'auto.db.otel.mongo', }), From 998bd686d4a249aae0c5ecc2b34d9762970cd85e Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Wed, 23 Oct 2024 20:27:54 -0400 Subject: [PATCH 4/4] chore: fix formatting --- .../suites/tracing/mongodb/test.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/mongodb/test.ts b/dev-packages/node-integration-tests/suites/tracing/mongodb/test.ts index 399845a2e854..92fc857ed4e8 100644 --- a/dev-packages/node-integration-tests/suites/tracing/mongodb/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/mongodb/test.ts @@ -71,12 +71,10 @@ describe('MongoDB experimental Test', () => { 'db.connection_string': expect.any(String), 'net.peer.name': expect.any(String), 'net.peer.port': expect.any(Number), - 'db.statement': - '{"title":"?","_id":{"_bsontype":"?","id":"?"}}', + 'db.statement': '{"title":"?","_id":{"_bsontype":"?","id":"?"}}', 'otel.kind': 'CLIENT', }, - description: - '{"title":"?","_id":{"_bsontype":"?","id":"?"}}', + description: '{"title":"?","_id":{"_bsontype":"?","id":"?"}}', op: 'db', origin: 'auto.db.otel.mongo', }), @@ -162,12 +160,10 @@ describe('MongoDB experimental Test', () => { 'db.connection_string': expect.any(String), 'net.peer.name': expect.any(String), 'net.peer.port': expect.any(Number), - 'db.statement': - '{"endSessions":[{"id":{"_bsontype":"?","sub_type":"?","position":"?","buffer":"?"}}]}', + 'db.statement': '{"endSessions":[{"id":{"_bsontype":"?","sub_type":"?","position":"?","buffer":"?"}}]}', 'otel.kind': 'CLIENT', }, - description: - '{"endSessions":[{"id":{"_bsontype":"?","sub_type":"?","position":"?","buffer":"?"}}]}', + description: '{"endSessions":[{"id":{"_bsontype":"?","sub_type":"?","position":"?","buffer":"?"}}]}', op: 'db', origin: 'auto.db.otel.mongo', }),