From a01803e162b71d526b72dc6930da8abbb317bbb9 Mon Sep 17 00:00:00 2001 From: Eugene Orlovsky Date: Thu, 5 Dec 2024 11:22:16 +0100 Subject: [PATCH 1/8] feat: mongodg reduced spans test --- src/bootstrap.ts | 4 +- src/samplers/combinedSampler.test.ts | 81 ++++++++++ src/samplers/combinedSampler.ts | 53 ++++++ src/samplers/mongodbSampler.test.ts | 151 ++++++++++++++++++ src/samplers/mongodbSampler.ts | 76 +++++++++ .../mongodb/app/mongodb_app.js | 42 +++++ test/instrumentations/mongodb/mongodb.test.ts | 24 ++- 7 files changed, 428 insertions(+), 3 deletions(-) create mode 100644 src/samplers/combinedSampler.test.ts create mode 100644 src/samplers/combinedSampler.ts create mode 100644 src/samplers/mongodbSampler.test.ts create mode 100644 src/samplers/mongodbSampler.ts diff --git a/src/bootstrap.ts b/src/bootstrap.ts index 9ab18c0e..a5e944f7 100644 --- a/src/bootstrap.ts +++ b/src/bootstrap.ts @@ -81,7 +81,7 @@ import { dirname, join } from 'path'; import { logger } from './logging'; import { ProcessEnvironmentDetector } from './resources/detectors/ProcessEnvironmentDetector'; import { LumigoSpanProcessor } from './resources/spanProcessor'; -import { getLumigoSampler } from './samplers/lumigoSampler'; +import { getCombinedSampler } from './samplers/combinedSampler'; import { LumigoLogRecordProcessor } from './processors/LumigoLogRecordProcessor'; const lumigoTraceEndpoint = process.env.LUMIGO_ENDPOINT || DEFAULT_LUMIGO_TRACES_ENDPOINT; @@ -209,7 +209,7 @@ export const init = async (): Promise => { .merge(await new ProcessEnvironmentDetector().detect()); const tracerProvider = new NodeTracerProvider({ - sampler: getLumigoSampler(), + sampler: getCombinedSampler(), resource, spanLimits: { attributeValueLengthLimit: getSpanAttributeMaxLength(), diff --git a/src/samplers/combinedSampler.test.ts b/src/samplers/combinedSampler.test.ts new file mode 100644 index 00000000..7116e3b7 --- /dev/null +++ b/src/samplers/combinedSampler.test.ts @@ -0,0 +1,81 @@ +import { CombinedSampler, getCombinedSampler } from './combinedSampler'; +import { LumigoSampler } from './lumigoSampler'; +import { MongodbSampler } from './mongodbSampler'; +import { Context, SpanKind } from '@opentelemetry/api'; +import { SamplingDecision } from '@opentelemetry/sdk-trace-base'; + +describe('CombinedSampler', () => { + let lumigoSampler: LumigoSampler; + let mongodbSampler: MongodbSampler; + let combinedSampler: CombinedSampler; + + beforeEach(() => { + lumigoSampler = new LumigoSampler(); + mongodbSampler = new MongodbSampler(); + combinedSampler = new CombinedSampler(lumigoSampler, mongodbSampler); + }); + + it('should return NOT_RECORD if any sampler returns NOT_RECORD', () => { + jest + .spyOn(lumigoSampler, 'shouldSample') + .mockReturnValue({ decision: SamplingDecision.NOT_RECORD }); + jest + .spyOn(mongodbSampler, 'shouldSample') + .mockReturnValue({ decision: SamplingDecision.RECORD_AND_SAMPLED }); + + const result = combinedSampler.shouldSample( + {} as Context, + 'traceId', + 'spanName', + SpanKind.CLIENT, + {}, + [] + ); + expect(result.decision).toBe(SamplingDecision.NOT_RECORD); + }); + + it('should return RECORD_AND_SAMPLED if all samplers return RECORD_AND_SAMPLED', () => { + jest + .spyOn(lumigoSampler, 'shouldSample') + .mockReturnValue({ decision: SamplingDecision.RECORD_AND_SAMPLED }); + jest + .spyOn(mongodbSampler, 'shouldSample') + .mockReturnValue({ decision: SamplingDecision.RECORD_AND_SAMPLED }); + + const result = combinedSampler.shouldSample( + {} as Context, + 'traceId', + 'spanName', + SpanKind.CLIENT, + {}, + [] + ); + expect(result.decision).toBe(SamplingDecision.RECORD_AND_SAMPLED); + }); + + it('should return RECORD_AND_SAMPLED if no samplers return NOT_RECORD', () => { + jest + .spyOn(lumigoSampler, 'shouldSample') + .mockReturnValue({ decision: SamplingDecision.RECORD_AND_SAMPLED }); + jest + .spyOn(mongodbSampler, 'shouldSample') + .mockReturnValue({ decision: SamplingDecision.RECORD }); + + const result = combinedSampler.shouldSample( + {} as Context, + 'traceId', + 'spanName', + SpanKind.CLIENT, + {}, + [] + ); + expect(result.decision).toBe(SamplingDecision.RECORD_AND_SAMPLED); + }); +}); + +describe('getCombinedSampler', () => { + it('should return a ParentBasedSampler with CombinedSampler as root', () => { + const sampler = getCombinedSampler(); + expect(sampler.root).toBeInstanceOf(CombinedSampler); + }); +}); diff --git a/src/samplers/combinedSampler.ts b/src/samplers/combinedSampler.ts new file mode 100644 index 00000000..1c0d32d2 --- /dev/null +++ b/src/samplers/combinedSampler.ts @@ -0,0 +1,53 @@ +import { + Sampler, + SamplingResult, + SamplingDecision, + ParentBasedSampler, +} from '@opentelemetry/sdk-trace-base'; +import type { Context, Link, Attributes, SpanKind } from '@opentelemetry/api'; + +import { LumigoSampler } from './lumigoSampler'; +import { MongodbSampler } from './mongodbSampler'; + +export class CombinedSampler implements Sampler { + private samplers: Sampler[]; + + constructor(...samplers: Sampler[]) { + this.samplers = samplers; + } + /* eslint-disable @typescript-eslint/no-unused-vars */ + shouldSample( + context: Context, + traceId: string, + spanName: string, + spanKind: SpanKind, + attributes: Attributes, + links: Link[] + ): SamplingResult { + // Iterate through each sampler + for (const sampler of this.samplers) { + console.log(`spanName: ${spanName}, attributes: ${JSON.stringify(attributes)}`); + const result = sampler.shouldSample(context, traceId, spanName, spanKind, attributes, links); + + // If any sampler decides NOT_RECORD, we respect that decision + if (result.decision === SamplingDecision.NOT_RECORD) { + return result; + } + } + + // If none decided to NOT_RECORD, we default to RECORD_AND_SAMPLED + return { decision: SamplingDecision.RECORD_AND_SAMPLED }; + } +} + +export const getCombinedSampler = () => { + const lumigoSampler = new LumigoSampler(); + const mongodbSampler = new MongodbSampler(); + const combinedSampler = new CombinedSampler(lumigoSampler, mongodbSampler); + + return new ParentBasedSampler({ + root: combinedSampler, + remoteParentSampled: combinedSampler, + localParentSampled: combinedSampler, + }); +}; diff --git a/src/samplers/mongodbSampler.test.ts b/src/samplers/mongodbSampler.test.ts new file mode 100644 index 00000000..309d5dec --- /dev/null +++ b/src/samplers/mongodbSampler.test.ts @@ -0,0 +1,151 @@ +import { MongodbSampler, extractClientAttribute, matchMongoIsMaster } from './mongodbSampler'; +import { Context, SpanKind, Attributes, Link } from '@opentelemetry/api'; +import { SamplingDecision } from '@opentelemetry/sdk-trace-base'; + +describe('MongodbSampler', () => { + let sampler: MongodbSampler; + + beforeEach(() => { + sampler = new MongodbSampler(); + delete process.env.LUMIGO_REDUCED_MONGO_INSTRUMENTATION; + }); + + it('should return RECORD_AND_SAMPLED when dbSystem and dbOperation are not provided', () => { + const result = sampler.shouldSample( + {} as Context, + 'traceId', + 'spanName', + SpanKind.CLIENT, + {}, + [] + ); + expect(result.decision).toBe(SamplingDecision.RECORD_AND_SAMPLED); + }); + + it('should return NOT_RECORD when dbSystem is mongodb and dbOperation is isMaster and LUMIGO_REDUCED_MONGO_INSTRUMENTATION is true', () => { + process.env.LUMIGO_REDUCED_MONGO_INSTRUMENTATION = 'true'; + const attributes: Attributes = { 'db.system': 'mongodb', 'db.operation': 'isMaster' }; + const result = sampler.shouldSample( + {} as Context, + 'traceId', + 'spanName', + SpanKind.CLIENT, + attributes, + [] + ); + expect(result.decision).toBe(SamplingDecision.NOT_RECORD); + }); + + it('should return NOT_RECORD when dbSystem is mongodb and dbOperation is isMaster', () => { + const attributes: Attributes = { 'db.system': 'mongodb', 'db.operation': 'isMaster' }; + const result = sampler.shouldSample( + {} as Context, + 'traceId', + 'spanName', + SpanKind.CLIENT, + attributes, + [] + ); + expect(result.decision).toBe(SamplingDecision.NOT_RECORD); + }); + + it('should return NOT_RECORD when spanName is mongodb.isMaster', () => { + const attributes: Attributes = {}; + const result = sampler.shouldSample( + {} as Context, + 'traceId', + 'mongodb.isMaster', + SpanKind.CLIENT, + attributes, + [] + ); + expect(result.decision).toBe(SamplingDecision.NOT_RECORD); + }); + + it('should return RECORD_AND_SAMPLED when dbSystem is mongodb and dbOperation is not isMaster', () => { + process.env.LUMIGO_REDUCED_MONGO_INSTRUMENTATION = 'true'; + const attributes: Attributes = { 'db.system': 'mongodb', 'db.operation': 'find' }; + const result = sampler.shouldSample( + {} as Context, + 'traceId', + 'spanName', + SpanKind.CLIENT, + attributes, + [] + ); + expect(result.decision).toBe(SamplingDecision.RECORD_AND_SAMPLED); + }); + + it('should return RECORD_AND_SAMPLED when LUMIGO_REDUCED_MONGO_INSTRUMENTATION is false', () => { + process.env.LUMIGO_REDUCED_MONGO_INSTRUMENTATION = 'false'; + const attributes: Attributes = { 'db.system': 'mongodb', 'db.operation': 'isMaster' }; + const result = sampler.shouldSample( + {} as Context, + 'traceId', + 'mongodb.isMaster', + SpanKind.CLIENT, + attributes, + [] + ); + expect(result.decision).toBe(SamplingDecision.RECORD_AND_SAMPLED); + }); +}); + +describe('extractClientAttribute', () => { + it('should return the attribute value as string when attributeName is present and spanKind is CLIENT', () => { + const attributes: Attributes = { 'db.system': 'mongodb' }; + const result = extractClientAttribute(attributes, 'db.system', SpanKind.CLIENT); + expect(result).toBe('mongodb'); + }); + + it('should return null when attributeName is not present', () => { + const attributes: Attributes = {}; + const result = extractClientAttribute(attributes, 'db.system', SpanKind.CLIENT); + expect(result).toBeNull(); + }); + + it('should return null when spanKind is not CLIENT', () => { + const attributes: Attributes = { 'db.system': 'mongodb' }; + const result = extractClientAttribute(attributes, 'db.system', SpanKind.SERVER); + expect(result).toBeNull(); + }); +}); + +describe('doesMatchClientSpanFiltering', () => { + beforeEach(() => { + delete process.env.LUMIGO_REDUCED_MONGO_INSTRUMENTATION; + }); + it('should return true when dbSystem is mongodb, dbOperation is isMaster and LUMIGO_REDUCED_MONGO_INSTRUMENTATION is true', () => { + process.env.LUMIGO_REDUCED_MONGO_INSTRUMENTATION = 'true'; + const result = matchMongoIsMaster('any', 'mongodb', 'isMaster'); + expect(result).toBe(true); + }); + + it('should return true when dbSystem is mongodb, dbOperation is isMaster', () => { + const result = matchMongoIsMaster('any', 'mongodb', 'isMaster'); + expect(result).toBe(true); + }); + + it('should return true when spanName is mongodb.isMaster', () => { + const result = matchMongoIsMaster('mongodb.isMaster', 'any', 'any'); + expect(result).toBe(true); + }); + + it('should return false when dbSystem is not mongodb', () => { + process.env.LUMIGO_REDUCED_MONGO_INSTRUMENTATION = 'true'; + const result = matchMongoIsMaster('any', 'mysql', 'isMaster'); + expect(result).toBe(false); + }); + + it('should return false when dbOperation is not isMaster', () => { + process.env.LUMIGO_REDUCED_MONGO_INSTRUMENTATION = 'true'; + const result = matchMongoIsMaster('any', 'mongodb', 'find'); + expect(result).toBe(false); + }); + + it('should return false when LUMIGO_REDUCED_MONGO_INSTRUMENTATION is false', () => { + process.env.LUMIGO_REDUCED_MONGO_INSTRUMENTATION = 'false'; + const result = matchMongoIsMaster('any', 'mongodb', 'isMaster'); + expect(result).toBe(false); + }); +}); diff --git a/src/samplers/mongodbSampler.ts b/src/samplers/mongodbSampler.ts new file mode 100644 index 00000000..2892f549 --- /dev/null +++ b/src/samplers/mongodbSampler.ts @@ -0,0 +1,76 @@ +import { + Sampler, + ParentBasedSampler, + SamplingResult, + SamplingDecision, +} from '@opentelemetry/sdk-trace-base'; +import { Context, Link, Attributes, SpanKind } from '@opentelemetry/api'; +import { logger } from '../logging'; + +export class MongodbSampler implements Sampler { + /* eslint-disable @typescript-eslint/no-unused-vars */ + shouldSample( + context: Context, + traceId: string, + spanName: string, + spanKind: SpanKind, + attributes: Attributes, + links: Link[] + ): SamplingResult { + let decision = SamplingDecision.RECORD_AND_SAMPLED; + const dbSystem = extractClientAttribute(attributes, 'db.system', spanKind); + const dbOperation = extractClientAttribute(attributes, 'db.operation', spanKind); + + if (spanKind === SpanKind.CLIENT && matchMongoIsMaster(spanName, dbSystem, dbOperation)) { + logger.debug( + `Drop span ${spanName} with db.system: ${dbSystem} and db.operation: ${dbOperation}, because LUMIGO_REDUCED_MONGO_INSTRUMENTATION is enabled` + ); + decision = SamplingDecision.NOT_RECORD; + } + + return { decision: decision }; + } +} + +export const extractClientAttribute = ( + attributes: Attributes, + attributeName: string, + spanKind: SpanKind +): string | null => { + if (attributeName && spanKind === SpanKind.CLIENT) { + const attributeValue = attributes[attributeName]; + return attributeValue ? attributeValue.toString() : null; + } + + return null; +}; + +export const matchMongoIsMaster = ( + spanName: string, + dbSystem: string, + dbOperation: string +): boolean => { + const reduceMongoInstrumentation = process.env.LUMIGO_REDUCED_MONGO_INSTRUMENTATION; + let isReducedMongoInstrumentationEnabled: boolean; + + if (reduceMongoInstrumentation == null || reduceMongoInstrumentation === '') { + isReducedMongoInstrumentationEnabled = true; // Default to true + } else if (reduceMongoInstrumentation.toLowerCase() === 'true') { + isReducedMongoInstrumentationEnabled = true; + } else + isReducedMongoInstrumentationEnabled = reduceMongoInstrumentation.toLowerCase() !== 'false'; + + return ( + isReducedMongoInstrumentationEnabled && + (spanName == 'mongodb.isMaster' || (dbSystem === 'mongodb' && dbOperation === 'isMaster')) + ); +}; + +export const getMongoDBSampler = () => { + const mongodbSampler = new MongodbSampler(); + return new ParentBasedSampler({ + root: mongodbSampler, + remoteParentSampled: mongodbSampler, + localParentSampled: mongodbSampler, + }); +}; diff --git a/test/instrumentations/mongodb/app/mongodb_app.js b/test/instrumentations/mongodb/app/mongodb_app.js index b7717aae..ef348382 100644 --- a/test/instrumentations/mongodb/app/mongodb_app.js +++ b/test/instrumentations/mongodb/app/mongodb_app.js @@ -5,6 +5,40 @@ const DB = require('./dbUtils'); let db; let httpServer; +async function sendIsMasterRequest(res) { + try { + // Perform isMaster command + const isMasterResult = await db.command({ isMaster: 1 }); + console.log('isMaster Result:', isMasterResult); + + res.setHeader('Content-Type', 'application/json'); + res.setHeader('access-control-allow-origin', '*'); + res.writeHead(200); + res.end(JSON.stringify(isMasterResult)); + } catch (e) { + console.error(e); + res.writeHead(500); + res.end(JSON.stringify({ error: 'Failed to execute isMaster command' })); + } +} + +async function sendHelloRequest(res) { + try { + // Perform hello command + const helloResult = await db.command({ hello: 1 }); + console.log('Hello Result:', helloResult); + + res.setHeader('Content-Type', 'application/json'); + res.setHeader('access-control-allow-origin', '*'); + res.writeHead(200); + res.end(JSON.stringify(helloResult)); + } catch (e) { + console.error(e); + res.writeHead(500); + res.end(JSON.stringify({ error: 'Failed to execute hello command' })); + } +} + async function sendMongoDbRequest(res) { try { let collection = db.collection('insertOne'); @@ -38,6 +72,14 @@ const requestListener = async (req, res) => { await sendMongoDbRequest(res); break; + case '/mongodb-isMaster': + await sendIsMasterRequest(res); + break; + + case '/mongodb-hello': + await sendHelloRequest(res); + break; + case '/quit': console.error('Received quit command'); res.writeHead(200); diff --git a/test/instrumentations/mongodb/mongodb.test.ts b/test/instrumentations/mongodb/mongodb.test.ts index 1d759eba..3b960cc0 100644 --- a/test/instrumentations/mongodb/mongodb.test.ts +++ b/test/instrumentations/mongodb/mongodb.test.ts @@ -5,7 +5,7 @@ import { join } from 'path'; import { MongoDBContainer, StartedMongoDBContainer } from 'testcontainers'; import { itTest } from '../../integration/setup'; -import { getSpanByName } from '../../utils/spans'; +import { getSpanByName, getSpansByAttribute } from '../../utils/spans'; import { TestApp } from '../../utils/test-apps'; import { installPackage, reinstallPackages, uninstallPackage } from '../../utils/test-setup'; import { versionsToTest } from '../../utils/versions'; @@ -205,5 +205,27 @@ describe.each(versionsToTest(INSTRUMENTATION_NAME, INSTRUMENTATION_NAME))( ); } ); + + itTest( + { + testName: `filter isMaster request: ${versionToTest}`, + packageName: INSTRUMENTATION_NAME, + version: versionToTest, + timeout: TEST_TIMEOUT, + }, + async function () { + if (versionToTest.startsWith('3')) { + + await testApp.invokeGetPath(`/mongodb-isMaster`); + // older versions of mongodb driver add extra spans + let spans = await testApp.getFinalSpans(2); + expect(getSpansByAttribute(spans, 'db.system', 'mongodb')).toHaveLength(1); + } else { + await testApp.invokeGetPath(`/mongodb-isMaster`); + let spans = await testApp.getFinalSpans(1); + expect(getSpansByAttribute(spans, 'db.system', 'mongodb')).toHaveLength(0); + } + } + ); } ); From 4dd4095ea3173a9707807bc454088ce1799187da Mon Sep 17 00:00:00 2001 From: Eugene Orlovsky Date: Thu, 5 Dec 2024 12:20:06 +0100 Subject: [PATCH 2/8] feat: mongodg reduced spans test --- src/samplers/combinedSampler.test.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/samplers/combinedSampler.test.ts b/src/samplers/combinedSampler.test.ts index 7116e3b7..aad4dc0c 100644 --- a/src/samplers/combinedSampler.test.ts +++ b/src/samplers/combinedSampler.test.ts @@ -72,10 +72,3 @@ describe('CombinedSampler', () => { expect(result.decision).toBe(SamplingDecision.RECORD_AND_SAMPLED); }); }); - -describe('getCombinedSampler', () => { - it('should return a ParentBasedSampler with CombinedSampler as root', () => { - const sampler = getCombinedSampler(); - expect(sampler.root).toBeInstanceOf(CombinedSampler); - }); -}); From e07614184e0ad6297d6c6ffdb15055c76fa91435 Mon Sep 17 00:00:00 2001 From: Eugene Orlovsky Date: Fri, 6 Dec 2024 17:50:21 +0100 Subject: [PATCH 3/8] feat: redisSampler --- src/samplers/combinedSampler.ts | 4 +- src/samplers/redisSampler.test.ts | 173 ++++++++++++++++++++++++++++++ src/samplers/redisSampler.ts | 98 +++++++++++++++++ 3 files changed, 274 insertions(+), 1 deletion(-) create mode 100644 src/samplers/redisSampler.test.ts create mode 100644 src/samplers/redisSampler.ts diff --git a/src/samplers/combinedSampler.ts b/src/samplers/combinedSampler.ts index 1c0d32d2..2986d8e5 100644 --- a/src/samplers/combinedSampler.ts +++ b/src/samplers/combinedSampler.ts @@ -8,6 +8,7 @@ import type { Context, Link, Attributes, SpanKind } from '@opentelemetry/api'; import { LumigoSampler } from './lumigoSampler'; import { MongodbSampler } from './mongodbSampler'; +import { RedisSampler } from './redisSampler'; export class CombinedSampler implements Sampler { private samplers: Sampler[]; @@ -43,7 +44,8 @@ export class CombinedSampler implements Sampler { export const getCombinedSampler = () => { const lumigoSampler = new LumigoSampler(); const mongodbSampler = new MongodbSampler(); - const combinedSampler = new CombinedSampler(lumigoSampler, mongodbSampler); + const redisSampler = new RedisSampler(); + const combinedSampler = new CombinedSampler(lumigoSampler, mongodbSampler, redisSampler); return new ParentBasedSampler({ root: combinedSampler, diff --git a/src/samplers/redisSampler.test.ts b/src/samplers/redisSampler.test.ts new file mode 100644 index 00000000..81430ec0 --- /dev/null +++ b/src/samplers/redisSampler.test.ts @@ -0,0 +1,173 @@ +import { + RedisSampler, + extractClientAttribute, + matchRedisInfoStatement, + getRedisDBSampler, +} from './redisSampler'; +import { Context, SpanKind, Attributes, Link } from '@opentelemetry/api'; +import { SamplingDecision } from '@opentelemetry/sdk-trace-base'; +import { logger } from '../logging'; + +describe('RedisSampler', () => { + let sampler: RedisSampler; + + beforeEach(() => { + sampler = new RedisSampler(); + delete process.env.LUMIGO_REDUCED_REDIS_INSTRUMENTATION; + }); + + it('should return RECORD_AND_SAMPLED when dbSystem and dbStatement are not provided', () => { + const result = sampler.shouldSample( + {} as Context, + 'traceId', + 'spanName', + SpanKind.CLIENT, + {}, + [] + ); + expect(result.decision).toBe(SamplingDecision.RECORD_AND_SAMPLED); + }); + + it('should return NOT_RECORD when dbSystem is redis and dbStatement is INFO and LUMIGO_REDUCED_REDIS_INSTRUMENTATION is true', () => { + process.env.LUMIGO_REDUCED_REDIS_INSTRUMENTATION = 'true'; + const attributes: Attributes = { 'db.system': 'redis', 'db.statement': 'INFO' }; + const result = sampler.shouldSample( + {} as Context, + 'traceId', + 'spanName', + SpanKind.CLIENT, + attributes, + [] + ); + expect(result.decision).toBe(SamplingDecision.NOT_RECORD); + }); + + it('should return RECORD_AND_SAMPLED when dbSystem is redis and dbStatement is not INFO', () => { + process.env.LUMIGO_REDUCED_REDIS_INSTRUMENTATION = 'true'; + const attributes: Attributes = { 'db.system': 'redis', 'db.statement': 'SET key value' }; + const result = sampler.shouldSample( + {} as Context, + 'traceId', + 'spanName', + SpanKind.CLIENT, + attributes, + [] + ); + expect(result.decision).toBe(SamplingDecision.RECORD_AND_SAMPLED); + }); + + it('should return RECORD_AND_SAMPLED when LUMIGO_REDUCED_REDIS_INSTRUMENTATION is false', () => { + process.env.LUMIGO_REDUCED_REDIS_INSTRUMENTATION = 'false'; + const attributes: Attributes = { 'db.system': 'redis', 'db.statement': 'INFO' }; + const result = sampler.shouldSample( + {} as Context, + 'traceId', + 'spanName', + SpanKind.CLIENT, + attributes, + [] + ); + expect(result.decision).toBe(SamplingDecision.RECORD_AND_SAMPLED); + }); + + it('should return RECORD_AND_SAMPLED when dbSystem and dbStatement are null', () => { + const attributes: Attributes = { 'db.system': null, 'db.statement': null }; + const result = sampler.shouldSample( + {} as Context, + 'traceId', + 'spanName', + SpanKind.CLIENT, + attributes, + [] + ); + expect(result.decision).toBe(SamplingDecision.RECORD_AND_SAMPLED); + }); + + it('should return RECORD_AND_SAMPLED when dbSystem is null and dbStatement is provided', () => { + const attributes: Attributes = { 'db.system': null, 'db.statement': 'SET key value' }; + const result = sampler.shouldSample( + {} as Context, + 'traceId', + 'spanName', + SpanKind.CLIENT, + attributes, + [] + ); + expect(result.decision).toBe(SamplingDecision.RECORD_AND_SAMPLED); + }); + + it('should return RECORD_AND_SAMPLED when dbSystem is provided and dbStatement is null', () => { + const attributes: Attributes = { 'db.system': 'redis', 'db.statement': null }; + const result = sampler.shouldSample( + {} as Context, + 'traceId', + 'spanName', + SpanKind.CLIENT, + attributes, + [] + ); + expect(result.decision).toBe(SamplingDecision.RECORD_AND_SAMPLED); + }); + + it('should return NOT_RECORD when dbSystem is redis and dbStatement is INFO with surrounding quotes', () => { + process.env.LUMIGO_REDUCED_REDIS_INSTRUMENTATION = 'true'; + const attributes: Attributes = { 'db.system': 'redis', 'db.statement': '"INFO"' }; + const result = sampler.shouldSample( + {} as Context, + 'traceId', + 'spanName', + SpanKind.CLIENT, + attributes, + [] + ); + expect(result.decision).toBe(SamplingDecision.NOT_RECORD); + }); + + it('should return NOT_RECORD when dbSystem is redis and dbStatement is INFO SERVER', () => { + process.env.LUMIGO_REDUCED_REDIS_INSTRUMENTATION = 'true'; + const attributes: Attributes = { 'db.system': 'redis', 'db.statement': 'INFO SERVER' }; + const result = sampler.shouldSample( + {} as Context, + 'traceId', + 'spanName', + SpanKind.CLIENT, + attributes, + [] + ); + expect(result.decision).toBe(SamplingDecision.NOT_RECORD); + }); +}); + +describe('extractClientAttribute', () => { + it('should return the attribute value as string when attributeName is present and spanKind is CLIENT', () => { + const attributes: Attributes = { 'db.system': 'redis' }; + const result = extractClientAttribute(attributes, 'db.system', SpanKind.CLIENT); + expect(result).toBe('redis'); + }); + + it('should return null when attributeName is not present', () => { + const attributes: Attributes = {}; + const result = extractClientAttribute(attributes, 'db.system', SpanKind.CLIENT); + expect(result).toBeNull(); + }); + + it('should return null when spanKind is not CLIENT', () => { + const attributes: Attributes = { 'db.system': 'redis' }; + const result = extractClientAttribute(attributes, 'db.system', SpanKind.SERVER); + expect(result).toBeNull(); + }); +}); + +describe('matchRedisInfoStatement', () => { + it('should return true when dbSystem is redis, dbStatement is INFO and LUMIGO_REDUCED_REDIS_INSTRUMENTATION is true', () => { + process.env.LUMIGO_REDUCED_REDIS_INSTRUMENTATION = 'true'; + const result = matchRedisInfoStatement('redis.Info', 'redis', 'INFO'); + expect(result).toBe(true); + }); + + it('should return false when LUMIGO_REDUCED_REDIS_INSTRUMENTATION is false', () => { + process.env.LUMIGO_REDUCED_REDIS_INSTRUMENTATION = 'false'; + const result = matchRedisInfoStatement('redis.Info', 'redis', 'INFO'); + expect(result).toBe(false); + }); +}); diff --git a/src/samplers/redisSampler.ts b/src/samplers/redisSampler.ts new file mode 100644 index 00000000..4d117a2e --- /dev/null +++ b/src/samplers/redisSampler.ts @@ -0,0 +1,98 @@ +import { + Sampler, + ParentBasedSampler, + SamplingResult, + SamplingDecision, +} from '@opentelemetry/sdk-trace-base'; +import { Context, Link, Attributes, SpanKind } from '@opentelemetry/api'; +import { logger } from '../logging'; + +export class RedisSampler implements Sampler { + /* eslint-disable @typescript-eslint/no-unused-vars */ + shouldSample( + context: Context, + traceId: string, + spanName: string, + spanKind: SpanKind, + attributes: Attributes, + links: Link[] + ): SamplingResult { + console.log(`spanName: ${spanName}, attributes: ${JSON.stringify(attributes)}`); + + let decision = SamplingDecision.RECORD_AND_SAMPLED; + const dbSystem = extractClientAttribute(attributes, 'db.system', spanKind); + const dbStatement = extractClientAttribute(attributes, 'db.statement', spanKind); + + if (spanKind === SpanKind.CLIENT && matchRedisInfoStatement(spanName, dbSystem, dbStatement)) { + logger.debug( + `Dropping span ${spanName} with db.system: ${dbSystem} and db.statement: ${dbStatement}, because LUMIGO_REDUCED_REDIS_INSTRUMENTATION is enabled` + ); + decision = SamplingDecision.NOT_RECORD; + } + + return { decision: decision }; + } +} + +export const extractClientAttribute = ( + attributes: Attributes, + attributeName: string, + spanKind: SpanKind +): string | null => { + if (attributeName && spanKind === SpanKind.CLIENT) { + const attributeValue = attributes[attributeName]; + return attributeValue ? attributeValue.toString() : null; + } + + return null; +}; + +export const matchRedisInfoStatement = ( + spanName: string, + dbSystem: string, + dbStatement: string | null | undefined +): boolean => { + const reduceRedisInstrumentation = process.env.LUMIGO_REDUCED_REDIS_INSTRUMENTATION; + let isReducedRedisInstrumentationEnabled: boolean; + + if (reduceRedisInstrumentation == null || reduceRedisInstrumentation === '') { + isReducedRedisInstrumentationEnabled = true; // Default to true + } else if (reduceRedisInstrumentation.toLowerCase() === 'true') { + isReducedRedisInstrumentationEnabled = true; + } else { + isReducedRedisInstrumentationEnabled = reduceRedisInstrumentation.toLowerCase() !== 'false'; + } + + // Safely handle null or undefined dbStatement by defaulting to empty string + const safeDbStatement = dbStatement ?? ''; + + // Normalize dbStatement: + // 1. Remove surrounding double quotes if present. + // 2. Convert to uppercase for case-insensitive comparison. + // 3. Trim whitespace. + const normalizedDbStatement = safeDbStatement + .replace(/^"(.*)"$/, '$1') + .toUpperCase() + .trim(); + + // Matches either: + // - "INFO" alone + // - "INFO SERVER" (with one or more spaces in between) + // + // Does NOT match just "SERVER". + const infoRegex = /^INFO(\s+SERVER)?$/i; + + return ( + isReducedRedisInstrumentationEnabled && + (spanName === 'redis.Info' || (dbSystem === 'redis' && infoRegex.test(normalizedDbStatement))) + ); +}; + +export const getRedisDBSampler = () => { + const redisSampler = new RedisSampler(); + return new ParentBasedSampler({ + root: redisSampler, + remoteParentSampled: redisSampler, + localParentSampled: redisSampler, + }); +}; From b7b0fd38f3fc11fcdb4b84bd4de242b36fae36cc Mon Sep 17 00:00:00 2001 From: Eugene Orlovsky Date: Fri, 6 Dec 2024 19:14:16 +0100 Subject: [PATCH 4/8] feat: redisSampler and tests --- src/samplers/combinedSampler.ts | 1 - src/samplers/redisSampler.test.ts | 10 +- src/samplers/redisSampler.ts | 4 +- test/instrumentations/mongodb/mongodb.test.ts | 104 +++++++++++++----- test/instrumentations/redis/app/redis_app.js | 12 ++ test/instrumentations/redis/redis.test.ts | 55 +++++++++ 6 files changed, 155 insertions(+), 31 deletions(-) diff --git a/src/samplers/combinedSampler.ts b/src/samplers/combinedSampler.ts index 2986d8e5..458ae466 100644 --- a/src/samplers/combinedSampler.ts +++ b/src/samplers/combinedSampler.ts @@ -27,7 +27,6 @@ export class CombinedSampler implements Sampler { ): SamplingResult { // Iterate through each sampler for (const sampler of this.samplers) { - console.log(`spanName: ${spanName}, attributes: ${JSON.stringify(attributes)}`); const result = sampler.shouldSample(context, traceId, spanName, spanKind, attributes, links); // If any sampler decides NOT_RECORD, we respect that decision diff --git a/src/samplers/redisSampler.test.ts b/src/samplers/redisSampler.test.ts index 81430ec0..cea0e559 100644 --- a/src/samplers/redisSampler.test.ts +++ b/src/samplers/redisSampler.test.ts @@ -161,13 +161,19 @@ describe('extractClientAttribute', () => { describe('matchRedisInfoStatement', () => { it('should return true when dbSystem is redis, dbStatement is INFO and LUMIGO_REDUCED_REDIS_INSTRUMENTATION is true', () => { process.env.LUMIGO_REDUCED_REDIS_INSTRUMENTATION = 'true'; - const result = matchRedisInfoStatement('redis.Info', 'redis', 'INFO'); + const result = matchRedisInfoStatement('any', 'redis', 'INFO'); + expect(result).toBe(true); + }); + + it('should return true when spanName is redis-INFO and LUMIGO_REDUCED_REDIS_INSTRUMENTATION is true', () => { + process.env.LUMIGO_REDUCED_REDIS_INSTRUMENTATION = 'true'; + const result = matchRedisInfoStatement('redis-INFO', 'any', 'any'); expect(result).toBe(true); }); it('should return false when LUMIGO_REDUCED_REDIS_INSTRUMENTATION is false', () => { process.env.LUMIGO_REDUCED_REDIS_INSTRUMENTATION = 'false'; - const result = matchRedisInfoStatement('redis.Info', 'redis', 'INFO'); + const result = matchRedisInfoStatement('any', 'redis', 'INFO'); expect(result).toBe(false); }); }); diff --git a/src/samplers/redisSampler.ts b/src/samplers/redisSampler.ts index 4d117a2e..3bcbaddc 100644 --- a/src/samplers/redisSampler.ts +++ b/src/samplers/redisSampler.ts @@ -17,8 +17,6 @@ export class RedisSampler implements Sampler { attributes: Attributes, links: Link[] ): SamplingResult { - console.log(`spanName: ${spanName}, attributes: ${JSON.stringify(attributes)}`); - let decision = SamplingDecision.RECORD_AND_SAMPLED; const dbSystem = extractClientAttribute(attributes, 'db.system', spanKind); const dbStatement = extractClientAttribute(attributes, 'db.statement', spanKind); @@ -84,7 +82,7 @@ export const matchRedisInfoStatement = ( return ( isReducedRedisInstrumentationEnabled && - (spanName === 'redis.Info' || (dbSystem === 'redis' && infoRegex.test(normalizedDbStatement))) + (spanName === 'redis-INFO' || (dbSystem === 'redis' && infoRegex.test(normalizedDbStatement))) ); }; diff --git a/test/instrumentations/mongodb/mongodb.test.ts b/test/instrumentations/mongodb/mongodb.test.ts index 3b960cc0..a1ee8cca 100644 --- a/test/instrumentations/mongodb/mongodb.test.ts +++ b/test/instrumentations/mongodb/mongodb.test.ts @@ -63,6 +63,23 @@ const warmupContainer = async () => { } }; +function getMongoContainerConnectionUrl(mongoContainer: StartedMongoDBContainer, versionToTest: string): URL { + let mongoConnectionUrl = new URL(mongoContainer.getConnectionString()); + // On Node.js 18 there are pesky issues with IPv6; ensure we use IPv4 + mongoConnectionUrl.hostname = '127.0.0.1'; + + if (!versionToTest.startsWith('3.')) { + /* + * Prevent `MongoServerSelectionError: getaddrinfo EAI_AGAIN` errors + * by disabling MongoDB topology. + */ + mongoConnectionUrl.searchParams.set('directConnection', 'true'); + } + + console.info(`Mongo container started, URL: ${mongoConnectionUrl}`); + return mongoConnectionUrl; +} + describe.each(versionsToTest(INSTRUMENTATION_NAME, INSTRUMENTATION_NAME))( 'Instrumentation tests for the mongodb package', function (versionToTest) { @@ -84,31 +101,7 @@ describe.each(versionsToTest(INSTRUMENTATION_NAME, INSTRUMENTATION_NAME))( }); mongoContainer = await startMongoDbContainer(); - - let mongoConnectionUrl = new URL(mongoContainer.getConnectionString()); - // On Node.js 18 there are pesky issues with IPv6; ensure we use IPv4 - mongoConnectionUrl.hostname = '127.0.0.1'; - - if (!versionToTest.startsWith('3.')) { - /* - * Prevent `MongoServerSelectionError: getaddrinfo EAI_AGAIN` errors - * by disabling MongoDB topology. - */ - mongoConnectionUrl.searchParams.set('directConnection', 'true'); - } - - console.info(`Mongo container started, URL: ${mongoConnectionUrl}`); - - testApp = new TestApp( - TEST_APP_DIR, - INSTRUMENTATION_NAME, - { - spanDumpPath: `${SPANS_DIR}/basic-@${versionToTest}.json`, - env: { - MONGODB_URL: mongoConnectionUrl.toString(), - OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT: '4096', - } - }); + getMongoContainerConnectionUrl(mongoContainer, versionToTest); }, DOCKER_WARMUP_TIMEOUT); afterEach(async function () { @@ -140,6 +133,18 @@ describe.each(versionsToTest(INSTRUMENTATION_NAME, INSTRUMENTATION_NAME))( timeout: TEST_TIMEOUT, }, async function () { + const exporterFile = `${SPANS_DIR}/basic-@${versionToTest}.json` + let mongoConnectionUrl = getMongoContainerConnectionUrl(mongoContainer, versionToTest); + testApp = new TestApp( + TEST_APP_DIR, + INSTRUMENTATION_NAME, + { + spanDumpPath: exporterFile, + env: { + MONGODB_URL: mongoConnectionUrl.toString(), + OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT: '4096', + } + }); await testApp.invokeGetPath(`/test-mongodb`); const spans = await testApp.getFinalSpans(5); @@ -214,6 +219,19 @@ describe.each(versionsToTest(INSTRUMENTATION_NAME, INSTRUMENTATION_NAME))( timeout: TEST_TIMEOUT, }, async function () { + const exporterFile = `${SPANS_DIR}/filter-isMaster@${versionToTest}.json` + let mongoConnectionUrl = getMongoContainerConnectionUrl(mongoContainer, versionToTest); + testApp = new TestApp( + TEST_APP_DIR, + INSTRUMENTATION_NAME, + { + spanDumpPath: exporterFile, + env: { + MONGODB_URL: mongoConnectionUrl.toString(), + OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT: '4096', + } + }); + if (versionToTest.startsWith('3')) { await testApp.invokeGetPath(`/mongodb-isMaster`); @@ -227,5 +245,41 @@ describe.each(versionsToTest(INSTRUMENTATION_NAME, INSTRUMENTATION_NAME))( } } ); + + itTest( + { + testName: `filter isMaster disabled: ${versionToTest}`, + packageName: INSTRUMENTATION_NAME, + version: versionToTest, + timeout: TEST_TIMEOUT, + }, + async function () { + const exporterFile = `${SPANS_DIR}/filter-isMaster-disabled@${versionToTest}.json` + let mongoConnectionUrl = getMongoContainerConnectionUrl(mongoContainer, versionToTest); + testApp = new TestApp( + TEST_APP_DIR, + INSTRUMENTATION_NAME, + { + spanDumpPath: exporterFile, + env: { + MONGODB_URL: mongoConnectionUrl.toString(), + OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT: '4096', + LUMIGO_REDUCED_MONGO_INSTRUMENTATION: 'false' + } + }); + + if (versionToTest.startsWith('3')) { + + await testApp.invokeGetPath(`/mongodb-isMaster`); + // older versions of mongodb driver add extra spans + let spans = await testApp.getFinalSpans(3); + expect(getSpansByAttribute(spans, 'db.operation', 'isMaster')).toHaveLength(1); + } else { + await testApp.invokeGetPath(`/mongodb-isMaster`); + let spans = await testApp.getFinalSpans(2); + expect(getSpansByAttribute(spans, 'db.operation', 'isMaster')).toHaveLength(1); + } + } + ); } ); diff --git a/test/instrumentations/redis/app/redis_app.js b/test/instrumentations/redis/app/redis_app.js index d25506c6..d84ad199 100644 --- a/test/instrumentations/redis/app/redis_app.js +++ b/test/instrumentations/redis/app/redis_app.js @@ -166,6 +166,18 @@ const requestListener = async function (req, res) { } break; + case '/info': + try { + client = await openRedisConnection(host, port); + const infoData = await client.info(); + // infoData is a multi-line string. You may wish to parse or just return as-is. + respond(res, 200, { info: infoData }); + } catch (err) { + console.error(`Error retrieving info`, err); + respond(res, 500, { error: err }); + } + break; + case '/quit': console.error('Received quit command'); respond(res, 200, {}); diff --git a/test/instrumentations/redis/redis.test.ts b/test/instrumentations/redis/redis.test.ts index 299e3586..0b03eec0 100644 --- a/test/instrumentations/redis/redis.test.ts +++ b/test/instrumentations/redis/redis.test.ts @@ -343,5 +343,60 @@ describe.each(versionsToTest(INSTRUMENTATION_NAME, INSTRUMENTATION_NAME))( expect(getSpans).toHaveLength(3); } ); + + itTest( + { + testName: `${INSTRUMENTATION_NAME} filter INFO works: ${versionToTest}`, + packageName: INSTRUMENTATION_NAME, + version: versionToTest, + timeout: TEST_TIMEOUT, + }, + async function () { + const exporterFile = `${SPANS_DIR}/${INSTRUMENTATION_NAME}.filter-info-works@${versionToTest}.json`; + + testApp = new TestApp(TEST_APP_DIR, INSTRUMENTATION_NAME, { spanDumpPath: exporterFile, env: { + OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT: '4096', + }}); + + const host = redisContainer.getHost(); + const port = redisContainer.getMappedPort(DEFAULT_REDIS_PORT); + await testApp.invokeGetPath(`/info?&host=${host}&port=${port}`); + + const spans = await testApp.getFinalSpans(2); + + const redisSpans = filterRedisSpans(spans); + // redis connection span only expected + expect(redisSpans).toHaveLength(1); + } + ); + + itTest( + { + testName: `${INSTRUMENTATION_NAME} filter INFO disabled works: ${versionToTest}`, + packageName: INSTRUMENTATION_NAME, + version: versionToTest, + timeout: TEST_TIMEOUT, + }, + async function () { + const exporterFile = `${SPANS_DIR}/${INSTRUMENTATION_NAME}.filter-info-disabled-works@${versionToTest}.json`; + + testApp = new TestApp(TEST_APP_DIR, INSTRUMENTATION_NAME, { spanDumpPath: exporterFile, env: { + OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT: '4096', + LUMIGO_REDUCED_REDIS_INSTRUMENTATION: 'false', + }}); + + const host = redisContainer.getHost(); + const port = redisContainer.getMappedPort(DEFAULT_REDIS_PORT); + await testApp.invokeGetPath(`/info?&host=${host}&port=${port}`); + + const spans = await testApp.getFinalSpans(3); + + const redisSpans = filterRedisSpans(spans); + // redis connection span + redis INFO span expected + expect(redisSpans).toHaveLength(2); + expect(getSpanByName(redisSpans, 'redis-INFO')).toBeDefined(); + } + ); + } // describe function ); From 67c666fc9e94955bd8696b02506fae7967082576 Mon Sep 17 00:00:00 2001 From: Eugene Orlovsky Date: Sat, 7 Dec 2024 18:57:32 +0100 Subject: [PATCH 5/8] feat: readme updated --- README.md | 2 ++ src/samplers/mongodbSampler.ts | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/README.md b/README.md index 9a3e454e..bb8ea8d3 100644 --- a/README.md +++ b/README.md @@ -122,6 +122,8 @@ This setting is independent from `LUMIGO_DEBUG`, that is, `LUMIGO_DEBUG` does no * `LUMIGO_FILTER_HTTP_ENDPOINTS_REGEX='["regex1", "regex2"]'`: This option enables the filtering of client and server endpoints through regular expression searches. Fine-tune your settings via the following environment variables, which work in conjunction with `LUMIGO_FILTER_HTTP_ENDPOINTS_REGEX` for a specific span type: * `LUMIGO_FILTER_HTTP_ENDPOINTS_REGEX_SERVER` applies the regular expression search exclusively to server spans. Searching is performed against the following attributes on a span: `url.path` and `http.target`. * `LUMIGO_FILTER_HTTP_ENDPOINTS_REGEX_CLIENT` applies the regular expression search exclusively to client spans. Searching is performed against the following attributes on a span: `url.full` and `http.url`. +* `LUMIGO_REDUCED_MONGO_INSTRUMENTATION=true`: Reduces the amount of data collected by the MongoDB [instrumentation](https://www.npmjs.com/package/@opentelemetry/instrumentation-mongodb), such as not collecting the `db.operation` attribute `isMaster`. By default, the MongoDB instrumentation reduces the amount of data collected. +* `LUMIGO_REDUCED_REDIS_INSTRUMENTATION=true`: Reduces the amount of data collected by the Redis [instrumentation](https://www.npmjs.com/package/@opentelemetry/instrumentation-redis-4), such as not collecting the `db.statement` attribute `INFO`. By default, the Redis instrumentation reduces the amount of data collected. For more information check out [Filtering http endpoints](#filtering-http-endpoints). diff --git a/src/samplers/mongodbSampler.ts b/src/samplers/mongodbSampler.ts index 2892f549..82307036 100644 --- a/src/samplers/mongodbSampler.ts +++ b/src/samplers/mongodbSampler.ts @@ -17,6 +17,10 @@ export class MongodbSampler implements Sampler { attributes: Attributes, links: Link[] ): SamplingResult { + // Note, there is probably a bug in opentelemetry api, making mongoSampler always receives attributes array empty. + // This makes it impossible to filter based on db.system and db.operation attributes. Filter based on spanName only. + // Opentemetry version upgrade might fix this issue. + // https://lumigo.atlassian.net/browse/RD-14250 let decision = SamplingDecision.RECORD_AND_SAMPLED; const dbSystem = extractClientAttribute(attributes, 'db.system', spanKind); const dbOperation = extractClientAttribute(attributes, 'db.operation', spanKind); From 75ccf807ca148b85806b43c6c6263120ab88c5d8 Mon Sep 17 00:00:00 2001 From: Eugene Orlovsky Date: Mon, 9 Dec 2024 11:39:53 +0100 Subject: [PATCH 6/8] Update README.md Co-authored-by: Harel Moshe --- README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index bb8ea8d3..9a78b599 100644 --- a/README.md +++ b/README.md @@ -122,8 +122,10 @@ This setting is independent from `LUMIGO_DEBUG`, that is, `LUMIGO_DEBUG` does no * `LUMIGO_FILTER_HTTP_ENDPOINTS_REGEX='["regex1", "regex2"]'`: This option enables the filtering of client and server endpoints through regular expression searches. Fine-tune your settings via the following environment variables, which work in conjunction with `LUMIGO_FILTER_HTTP_ENDPOINTS_REGEX` for a specific span type: * `LUMIGO_FILTER_HTTP_ENDPOINTS_REGEX_SERVER` applies the regular expression search exclusively to server spans. Searching is performed against the following attributes on a span: `url.path` and `http.target`. * `LUMIGO_FILTER_HTTP_ENDPOINTS_REGEX_CLIENT` applies the regular expression search exclusively to client spans. Searching is performed against the following attributes on a span: `url.full` and `http.url`. -* `LUMIGO_REDUCED_MONGO_INSTRUMENTATION=true`: Reduces the amount of data collected by the MongoDB [instrumentation](https://www.npmjs.com/package/@opentelemetry/instrumentation-mongodb), such as not collecting the `db.operation` attribute `isMaster`. By default, the MongoDB instrumentation reduces the amount of data collected. -* `LUMIGO_REDUCED_REDIS_INSTRUMENTATION=true`: Reduces the amount of data collected by the Redis [instrumentation](https://www.npmjs.com/package/@opentelemetry/instrumentation-redis-4), such as not collecting the `db.statement` attribute `INFO`. By default, the Redis instrumentation reduces the amount of data collected. +* `LUMIGO_REDUCED_MONGO_INSTRUMENTATION=true`: Reduces the amount of data collected by the MongoDB [instrumentation](https://www.npmjs.com/package/@opentelemetry/instrumentation-mongodb), such as not collecting the `db.operation` attribute `isMaster`. +Defaults to `true`, meaning the MongoDB instrumentation reduces the amount of data collected. +* `LUMIGO_REDUCED_REDIS_INSTRUMENTATION=true`: Reduces the amount of data collected by the Redis [instrumentation](https://www.npmjs.com/package/@opentelemetry/instrumentation-redis-4), such as not collecting the `db.statement` attribute `INFO`. +Defaults to `true`, meaning the Redis instrumentation reduces the amount of data collected. For more information check out [Filtering http endpoints](#filtering-http-endpoints). From e626662d2891528b7e8ab82003815740cc8b9760 Mon Sep 17 00:00:00 2001 From: Eugene Orlovsky Date: Mon, 9 Dec 2024 11:41:48 +0100 Subject: [PATCH 7/8] Update src/samplers/mongodbSampler.ts Co-authored-by: Harel Moshe --- src/samplers/mongodbSampler.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/samplers/mongodbSampler.ts b/src/samplers/mongodbSampler.ts index 82307036..6f149714 100644 --- a/src/samplers/mongodbSampler.ts +++ b/src/samplers/mongodbSampler.ts @@ -54,15 +54,9 @@ export const matchMongoIsMaster = ( dbSystem: string, dbOperation: string ): boolean => { - const reduceMongoInstrumentation = process.env.LUMIGO_REDUCED_MONGO_INSTRUMENTATION; - let isReducedMongoInstrumentationEnabled: boolean; - - if (reduceMongoInstrumentation == null || reduceMongoInstrumentation === '') { - isReducedMongoInstrumentationEnabled = true; // Default to true - } else if (reduceMongoInstrumentation.toLowerCase() === 'true') { - isReducedMongoInstrumentationEnabled = true; - } else - isReducedMongoInstrumentationEnabled = reduceMongoInstrumentation.toLowerCase() !== 'false'; +const reduceMongoInstrumentation = process.env.LUMIGO_REDUCED_MONGO_INSTRUMENTATION; +const isReducedMongoInstrumentationEnabled = + reduceMongoInstrumentation == null || reduceMongoInstrumentation === '' || reduceMongoInstrumentation.toLowerCase() !== 'false'; return ( isReducedMongoInstrumentationEnabled && From f8f0c985e602f9e6d87e9727e17ff3dfd8eba2db Mon Sep 17 00:00:00 2001 From: Eugene Orlovsky Date: Mon, 9 Dec 2024 11:55:36 +0100 Subject: [PATCH 8/8] feat: review updates, --- src/samplers/mongodbSampler.ts | 13 ++++---- src/samplers/redisSampler.ts | 18 ++++------ test/instrumentations/mongodb/mongodb.test.ts | 33 ++++++++----------- 3 files changed, 26 insertions(+), 38 deletions(-) diff --git a/src/samplers/mongodbSampler.ts b/src/samplers/mongodbSampler.ts index 6f149714..c550ec6c 100644 --- a/src/samplers/mongodbSampler.ts +++ b/src/samplers/mongodbSampler.ts @@ -21,7 +21,6 @@ export class MongodbSampler implements Sampler { // This makes it impossible to filter based on db.system and db.operation attributes. Filter based on spanName only. // Opentemetry version upgrade might fix this issue. // https://lumigo.atlassian.net/browse/RD-14250 - let decision = SamplingDecision.RECORD_AND_SAMPLED; const dbSystem = extractClientAttribute(attributes, 'db.system', spanKind); const dbOperation = extractClientAttribute(attributes, 'db.operation', spanKind); @@ -29,10 +28,10 @@ export class MongodbSampler implements Sampler { logger.debug( `Drop span ${spanName} with db.system: ${dbSystem} and db.operation: ${dbOperation}, because LUMIGO_REDUCED_MONGO_INSTRUMENTATION is enabled` ); - decision = SamplingDecision.NOT_RECORD; + return { decision: SamplingDecision.NOT_RECORD }; } - return { decision: decision }; + return { decision: SamplingDecision.RECORD_AND_SAMPLED }; } } @@ -54,9 +53,11 @@ export const matchMongoIsMaster = ( dbSystem: string, dbOperation: string ): boolean => { -const reduceMongoInstrumentation = process.env.LUMIGO_REDUCED_MONGO_INSTRUMENTATION; -const isReducedMongoInstrumentationEnabled = - reduceMongoInstrumentation == null || reduceMongoInstrumentation === '' || reduceMongoInstrumentation.toLowerCase() !== 'false'; + const reduceMongoInstrumentation = process.env.LUMIGO_REDUCED_MONGO_INSTRUMENTATION; + const isReducedMongoInstrumentationEnabled = + reduceMongoInstrumentation == null || + reduceMongoInstrumentation === '' || + reduceMongoInstrumentation.toLowerCase() !== 'false'; return ( isReducedMongoInstrumentationEnabled && diff --git a/src/samplers/redisSampler.ts b/src/samplers/redisSampler.ts index 3bcbaddc..558051af 100644 --- a/src/samplers/redisSampler.ts +++ b/src/samplers/redisSampler.ts @@ -17,7 +17,6 @@ export class RedisSampler implements Sampler { attributes: Attributes, links: Link[] ): SamplingResult { - let decision = SamplingDecision.RECORD_AND_SAMPLED; const dbSystem = extractClientAttribute(attributes, 'db.system', spanKind); const dbStatement = extractClientAttribute(attributes, 'db.statement', spanKind); @@ -25,10 +24,10 @@ export class RedisSampler implements Sampler { logger.debug( `Dropping span ${spanName} with db.system: ${dbSystem} and db.statement: ${dbStatement}, because LUMIGO_REDUCED_REDIS_INSTRUMENTATION is enabled` ); - decision = SamplingDecision.NOT_RECORD; + return { decision: SamplingDecision.NOT_RECORD }; } - return { decision: decision }; + return { decision: SamplingDecision.RECORD_AND_SAMPLED }; } } @@ -51,15 +50,10 @@ export const matchRedisInfoStatement = ( dbStatement: string | null | undefined ): boolean => { const reduceRedisInstrumentation = process.env.LUMIGO_REDUCED_REDIS_INSTRUMENTATION; - let isReducedRedisInstrumentationEnabled: boolean; - - if (reduceRedisInstrumentation == null || reduceRedisInstrumentation === '') { - isReducedRedisInstrumentationEnabled = true; // Default to true - } else if (reduceRedisInstrumentation.toLowerCase() === 'true') { - isReducedRedisInstrumentationEnabled = true; - } else { - isReducedRedisInstrumentationEnabled = reduceRedisInstrumentation.toLowerCase() !== 'false'; - } + const isReducedRedisInstrumentationEnabled = + reduceRedisInstrumentation == null || + reduceRedisInstrumentation === '' || + reduceRedisInstrumentation.toLowerCase() !== 'false'; // Safely handle null or undefined dbStatement by defaulting to empty string const safeDbStatement = dbStatement ?? ''; diff --git a/test/instrumentations/mongodb/mongodb.test.ts b/test/instrumentations/mongodb/mongodb.test.ts index a1ee8cca..81229fd1 100644 --- a/test/instrumentations/mongodb/mongodb.test.ts +++ b/test/instrumentations/mongodb/mongodb.test.ts @@ -232,17 +232,15 @@ describe.each(versionsToTest(INSTRUMENTATION_NAME, INSTRUMENTATION_NAME))( } }); - if (versionToTest.startsWith('3')) { + // older versions of mongodb driver add extra spans + const expectedSpanCount = versionToTest.startsWith('3') ? 2 : 1; + const expectedDbSystemAttributeSpans = versionToTest.startsWith('3') ? 1 : 0; + + await testApp.invokeGetPath(`/mongodb-isMaster`); + + let spans = await testApp.getFinalSpans(expectedSpanCount); + expect(getSpansByAttribute(spans, 'db.system', 'mongodb')).toHaveLength(expectedDbSystemAttributeSpans); - await testApp.invokeGetPath(`/mongodb-isMaster`); - // older versions of mongodb driver add extra spans - let spans = await testApp.getFinalSpans(2); - expect(getSpansByAttribute(spans, 'db.system', 'mongodb')).toHaveLength(1); - } else { - await testApp.invokeGetPath(`/mongodb-isMaster`); - let spans = await testApp.getFinalSpans(1); - expect(getSpansByAttribute(spans, 'db.system', 'mongodb')).toHaveLength(0); - } } ); @@ -268,17 +266,12 @@ describe.each(versionsToTest(INSTRUMENTATION_NAME, INSTRUMENTATION_NAME))( } }); - if (versionToTest.startsWith('3')) { + // older versions of mongodb driver add extra spans + const expectedSpanCount = versionToTest.startsWith('3') ? 3 : 2; + await testApp.invokeGetPath(`/mongodb-isMaster`); + let spans = await testApp.getFinalSpans(expectedSpanCount); + expect(getSpansByAttribute(spans, 'db.operation', 'isMaster')).toHaveLength(1); - await testApp.invokeGetPath(`/mongodb-isMaster`); - // older versions of mongodb driver add extra spans - let spans = await testApp.getFinalSpans(3); - expect(getSpansByAttribute(spans, 'db.operation', 'isMaster')).toHaveLength(1); - } else { - await testApp.invokeGetPath(`/mongodb-isMaster`); - let spans = await testApp.getFinalSpans(2); - expect(getSpansByAttribute(spans, 'db.operation', 'isMaster')).toHaveLength(1); - } } ); }