From 9f099e6c062829e19fca1a4cc99a675780cb8907 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ketil=20=C3=98vreb=C3=B8?= Date: Wed, 8 Jun 2022 16:20:40 +0200 Subject: [PATCH 1/5] feat(ioredis): only serialize first argument in redis command --- .../src/utils.ts | 10 ++++++---- .../test/ioredis.test.ts | 17 ++++++++--------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts b/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts index dc52e6e4a8..59077a3f87 100644 --- a/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts @@ -34,7 +34,9 @@ export const endSpan = ( export const defaultDbStatementSerializer: DbStatementSerializer = ( cmdName, cmdArgs -) => - Array.isArray(cmdArgs) && cmdArgs.length - ? `${cmdName} ${cmdArgs.join(' ')}` - : cmdName; +) => { + if (Array.isArray(cmdArgs) && cmdArgs.length) { + return `${cmdName} ${cmdArgs[0]}`; + } + return cmdName; +}; diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts b/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts index b57adcb76b..f0f5ab6209 100644 --- a/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts +++ b/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts @@ -241,9 +241,7 @@ describe('ioredis', () => { it(`should create a child span for cb style ${command.description}`, done => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: `${ - command.name - } ${command.args.join(' ')}`, + [SemanticAttributes.DB_STATEMENT]: `${command.name} ${command.args[0]}`, }; const span = provider .getTracer('ioredis-test') @@ -273,7 +271,7 @@ describe('ioredis', () => { it('should create a child span for hset promise', async () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: `hset ${hashKeyName} random random`, + [SemanticAttributes.DB_STATEMENT]: `hset ${hashKeyName}`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); await context.with(trace.setSpan(context.active(), span), async () => { @@ -335,7 +333,7 @@ describe('ioredis', () => { it('should create a child span for streamify scanning', done => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: 'scan 0 MATCH test-* COUNT 1000', + [SemanticAttributes.DB_STATEMENT]: 'scan 0', }; const span = provider.getTracer('ioredis-test').startSpan('test span'); context.with(trace.setSpan(context.active(), span), () => { @@ -411,7 +409,7 @@ describe('ioredis', () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: 'subscribe news music', + [SemanticAttributes.DB_STATEMENT]: 'subscribe news', }; testUtils.assertSpan( endedSpans[4], @@ -466,7 +464,7 @@ describe('ioredis', () => { it('should create a child span for pipeline', done => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: 'set foo bar', + [SemanticAttributes.DB_STATEMENT]: 'set foo', }; const span = provider.getTracer('ioredis-test').startSpan('test span'); @@ -563,7 +561,8 @@ describe('ioredis', () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: `evalsha bfbf458525d6a0b19200bfd6db3af481156b367b 1 ${testKeyName}`, + [SemanticAttributes.DB_STATEMENT]: + 'evalsha bfbf458525d6a0b19200bfd6db3af481156b367b', }; const span = provider.getTracer('ioredis-test').startSpan('test span'); @@ -663,7 +662,7 @@ describe('ioredis', () => { SpanKind.CLIENT, { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: `set ${testKeyName} data`, + [SemanticAttributes.DB_STATEMENT]: `set ${testKeyName}`, }, [], unsetStatus From be2b5764da3f84f3aa4b4c54928e8b77929f0d66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ketil=20=C3=98vreb=C3=B8?= Date: Thu, 9 Jun 2022 10:20:24 +0200 Subject: [PATCH 2/5] feat(ioredis): specify serialization subsets per command --- .../README.md | 4 +- .../src/utils.ts | 51 ++++++++++++++++++- .../test/ioredis.test.ts | 20 +++++--- 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/README.md b/plugins/node/opentelemetry-instrumentation-ioredis/README.md index 5a05d80be5..a9610a3c6c 100644 --- a/plugins/node/opentelemetry-instrumentation-ioredis/README.md +++ b/plugins/node/opentelemetry-instrumentation-ioredis/README.md @@ -56,7 +56,9 @@ IORedis instrumentation has few options available to choose from. You can set th #### Custom db.statement Serializer -The instrumentation serializes the whole command into a Span attribute called `db.statement`. The standard serialization format is `{cmdName} {cmdArgs.join(',')}`. +The instrumentation serializes the command into a Span attribute called `db.statement`. The standard serialization format attempts to be as informative +as possible while avoiding the export of potentially sensitive data. The number of serialized arguments depends on the specific command, see the configuration +list in `src/utils.ts`. It is also possible to define a custom serialization function. The function will receive the command name and arguments and must return a string. Here is a simple example to serialize the command name skipping arguments: diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts b/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts index 59077a3f87..02e3e8e511 100644 --- a/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts @@ -17,6 +17,47 @@ import { Span, SpanStatusCode } from '@opentelemetry/api'; import { DbStatementSerializer } from './types'; +/** + * Map of command names to the number of arguments that should be serialized. + * For example, SET should serialize which key it's operating on, but not its value. + * Commands not listed will have all their arguments serialized. + * + * Refer to https://redis.io/commands/ for the full list. + */ +const SerializationSubsets: { [name: string]: number } = { + APPEND: 1, + 'FUNCTION LOAD': 0, + GETSET: 1, + HMSET: 2, + HSET: 2, + HSETNX: 2, + LINSERT: 3, + LPUSH: 1, + LPUSHX: 1, + LSET: 2, + + // MSET and MSETNX have repeating argument lists, so this serialization is likely to be incomplete. + MSET: 1, + MSETNX: 1, + + PDFADD: 1, + PSETEX: 2, + PUBLISH: 1, + RESTORE: 2, + RPUSH: 1, + RPUSHX: 1, + SADD: 1, + 'SCRIPT LOAD': 0, + SET: 1, + SETEX: 2, + SETNX: 1, + SISMEMBER: 1, + SMISMEMBER: 1, + SPUBLISH: 1, + XADD: 3, + ZADD: 2, +}; + export const endSpan = ( span: Span, err: NodeJS.ErrnoException | null | undefined @@ -36,7 +77,15 @@ export const defaultDbStatementSerializer: DbStatementSerializer = ( cmdArgs ) => { if (Array.isArray(cmdArgs) && cmdArgs.length) { - return `${cmdName} ${cmdArgs[0]}`; + const argsSubset = SerializationSubsets[cmdName.toUpperCase()]; + if (argsSubset) { + const args = cmdArgs.slice(0, argsSubset); + const remainder = cmdArgs.slice(argsSubset); + return `${cmdName} ${args.join(' ')} [${ + remainder.length + } other arguments]`; + } + return `${cmdName} ${cmdArgs.join(' ')}`; } return cmdName; }; diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts b/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts index f0f5ab6209..f70aafe000 100644 --- a/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts +++ b/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts @@ -187,12 +187,14 @@ describe('ioredis', () => { description: string; name: string; args: Array; + serializedArgs: Array; method: (cb: ioredisTypes.CallbackFunction) => unknown; }> = [ { description: 'insert', name: 'hset', args: [hashKeyName, 'testField', 'testValue'], + serializedArgs: [hashKeyName, 'testField', '[1 other arguments]'], method: (cb: ioredisTypes.CallbackFunction) => client.hset(hashKeyName, 'testField', 'testValue', cb), }, @@ -200,6 +202,7 @@ describe('ioredis', () => { description: 'get', name: 'get', args: [testKeyName], + serializedArgs: [testKeyName], method: (cb: ioredisTypes.CallbackFunction) => client.get(testKeyName, cb), }, @@ -241,7 +244,9 @@ describe('ioredis', () => { it(`should create a child span for cb style ${command.description}`, done => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: `${command.name} ${command.args[0]}`, + [SemanticAttributes.DB_STATEMENT]: `${ + command.name + } ${command.serializedArgs.join(' ')}`, }; const span = provider .getTracer('ioredis-test') @@ -271,7 +276,7 @@ describe('ioredis', () => { it('should create a child span for hset promise', async () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: `hset ${hashKeyName}`, + [SemanticAttributes.DB_STATEMENT]: `hset ${hashKeyName} random [1 other arguments]`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); await context.with(trace.setSpan(context.active(), span), async () => { @@ -333,7 +338,7 @@ describe('ioredis', () => { it('should create a child span for streamify scanning', done => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: 'scan 0', + [SemanticAttributes.DB_STATEMENT]: 'scan 0 MATCH test-* COUNT 1000', }; const span = provider.getTracer('ioredis-test').startSpan('test span'); context.with(trace.setSpan(context.active(), span), () => { @@ -409,7 +414,7 @@ describe('ioredis', () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: 'subscribe news', + [SemanticAttributes.DB_STATEMENT]: 'subscribe news music', }; testUtils.assertSpan( endedSpans[4], @@ -464,7 +469,7 @@ describe('ioredis', () => { it('should create a child span for pipeline', done => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: 'set foo', + [SemanticAttributes.DB_STATEMENT]: 'set foo [1 other arguments]', }; const span = provider.getTracer('ioredis-test').startSpan('test span'); @@ -561,8 +566,7 @@ describe('ioredis', () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: - 'evalsha bfbf458525d6a0b19200bfd6db3af481156b367b', + [SemanticAttributes.DB_STATEMENT]: `evalsha bfbf458525d6a0b19200bfd6db3af481156b367b 1 ${testKeyName}`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); @@ -662,7 +666,7 @@ describe('ioredis', () => { SpanKind.CLIENT, { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: `set ${testKeyName}`, + [SemanticAttributes.DB_STATEMENT]: `set ${testKeyName} [1 other arguments]`, }, [], unsetStatus From 3601dd0a69f7c378a7a90cbc4ab9842befe4e6df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ketil=20=C3=98vreb=C3=B8?= Date: Fri, 10 Jun 2022 11:03:33 +0200 Subject: [PATCH 3/5] feat(ioredis): update command list, fallback to serialize one argument --- .../src/utils.ts | 76 ++++++++----------- .../test/ioredis.test.ts | 14 ++-- 2 files changed, 39 insertions(+), 51 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts b/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts index 02e3e8e511..48a53fe62c 100644 --- a/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts @@ -18,45 +18,32 @@ import { Span, SpanStatusCode } from '@opentelemetry/api'; import { DbStatementSerializer } from './types'; /** - * Map of command names to the number of arguments that should be serialized. - * For example, SET should serialize which key it's operating on, but not its value. - * Commands not listed will have all their arguments serialized. + * List of regexes and the number of arguments that should be serialized for matching commands. + * For example, HSET should serialize which key and field it's operating on, but not its value. + * Setting the subset to -1 will serialize all arguments. + * Commands without a match will have their first argument serialized. * * Refer to https://redis.io/commands/ for the full list. */ -const SerializationSubsets: { [name: string]: number } = { - APPEND: 1, - 'FUNCTION LOAD': 0, - GETSET: 1, - HMSET: 2, - HSET: 2, - HSETNX: 2, - LINSERT: 3, - LPUSH: 1, - LPUSHX: 1, - LSET: 2, - - // MSET and MSETNX have repeating argument lists, so this serialization is likely to be incomplete. - MSET: 1, - MSETNX: 1, - - PDFADD: 1, - PSETEX: 2, - PUBLISH: 1, - RESTORE: 2, - RPUSH: 1, - RPUSHX: 1, - SADD: 1, - 'SCRIPT LOAD': 0, - SET: 1, - SETEX: 2, - SETNX: 1, - SISMEMBER: 1, - SMISMEMBER: 1, - SPUBLISH: 1, - XADD: 3, - ZADD: 2, -}; +const SerializationSubsets = [ + { + regex: /^(LPUSH|MSET|SET|PFA|PUBLISH|RPUSH|SADD|SET|SPUBLISH|XADD|ZADD)/i, + args: 1, + }, + { + regex: /^(HSET|HMSET|LSET)/i, + args: 2, + }, + { + regex: /^(LINSERT)/i, + args: 3, + }, + { + regex: + /^(ACL|BIT|B[LRZ]|CLIENT|CLUSTER|CONFIG|COMMAND|DECR|DEL|EVAL|EX|FUNCTION|GEO|GET|HINCR|HMGET|HSCAN|INCR|L[TRLM]|MEMORY|P[EFISTU]|RPOP|S[CDIMORSU]|XACK|X[CDGILPRT]|Z[CDILMPRS])/i, + args: -1, + }, +]; export const endSpan = ( span: Span, @@ -77,15 +64,14 @@ export const defaultDbStatementSerializer: DbStatementSerializer = ( cmdArgs ) => { if (Array.isArray(cmdArgs) && cmdArgs.length) { - const argsSubset = SerializationSubsets[cmdName.toUpperCase()]; - if (argsSubset) { - const args = cmdArgs.slice(0, argsSubset); - const remainder = cmdArgs.slice(argsSubset); - return `${cmdName} ${args.join(' ')} [${ - remainder.length - } other arguments]`; - } - return `${cmdName} ${cmdArgs.join(' ')}`; + const argsSubset = + SerializationSubsets.find(({ regex }) => { + return regex.test(cmdName); + })?.args || 1; + const args = argsSubset > 0 ? cmdArgs.slice(0, argsSubset) : cmdArgs; + return `${cmdName} ${args.join(' ')} [${ + args.length !== cmdArgs.length ? cmdArgs.length - argsSubset : 0 + } other arguments]`; } return cmdName; }; diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts b/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts index f70aafe000..cd1dd3b716 100644 --- a/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts +++ b/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts @@ -202,7 +202,7 @@ describe('ioredis', () => { description: 'get', name: 'get', args: [testKeyName], - serializedArgs: [testKeyName], + serializedArgs: [testKeyName, '[0 other arguments]'], method: (cb: ioredisTypes.CallbackFunction) => client.get(testKeyName, cb), }, @@ -338,7 +338,8 @@ describe('ioredis', () => { it('should create a child span for streamify scanning', done => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: 'scan 0 MATCH test-* COUNT 1000', + [SemanticAttributes.DB_STATEMENT]: + 'scan 0 MATCH test-* COUNT 1000 [0 other arguments]', }; const span = provider.getTracer('ioredis-test').startSpan('test span'); context.with(trace.setSpan(context.active(), span), () => { @@ -414,7 +415,8 @@ describe('ioredis', () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: 'subscribe news music', + [SemanticAttributes.DB_STATEMENT]: + 'subscribe news music [0 other arguments]', }; testUtils.assertSpan( endedSpans[4], @@ -503,7 +505,7 @@ describe('ioredis', () => { it('should create a child span for get promise', async () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: `get ${testKeyName}`, + [SemanticAttributes.DB_STATEMENT]: `get ${testKeyName} [0 other arguments]`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); await context.with(trace.setSpan(context.active(), span), async () => { @@ -532,7 +534,7 @@ describe('ioredis', () => { it('should create a child span for del', async () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: `del ${testKeyName}`, + [SemanticAttributes.DB_STATEMENT]: `del ${testKeyName} [0 other arguments]`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); await context.with(trace.setSpan(context.active(), span), async () => { @@ -566,7 +568,7 @@ describe('ioredis', () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: `evalsha bfbf458525d6a0b19200bfd6db3af481156b367b 1 ${testKeyName}`, + [SemanticAttributes.DB_STATEMENT]: `evalsha bfbf458525d6a0b19200bfd6db3af481156b367b 1 ${testKeyName} [0 other arguments]`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); From d32b872222d1d5ee35bfb8c63effe9104050d2e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ketil=20=C3=98vreb=C3=B8?= Date: Sun, 12 Jun 2022 23:02:52 +0200 Subject: [PATCH 4/5] fix(ioredis): improve readability, add test for serializer --- .../src/utils.ts | 30 +++++++----- .../test/ioredis.test.ts | 47 +++++++++++++++---- 2 files changed, 56 insertions(+), 21 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts b/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts index 48a53fe62c..b20b159ff3 100644 --- a/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts @@ -25,19 +25,19 @@ import { DbStatementSerializer } from './types'; * * Refer to https://redis.io/commands/ for the full list. */ -const SerializationSubsets = [ +const serializationSubsets = [ + { + regex: /^ECHO/i, + args: 0, + }, { regex: /^(LPUSH|MSET|SET|PFA|PUBLISH|RPUSH|SADD|SET|SPUBLISH|XADD|ZADD)/i, args: 1, }, { - regex: /^(HSET|HMSET|LSET)/i, + regex: /^(HSET|HMSET|LSET|LINSERT)/i, args: 2, }, - { - regex: /^(LINSERT)/i, - args: 3, - }, { regex: /^(ACL|BIT|B[LRZ]|CLIENT|CLUSTER|CONFIG|COMMAND|DECR|DEL|EVAL|EX|FUNCTION|GEO|GET|HINCR|HMGET|HSCAN|INCR|L[TRLM]|MEMORY|P[EFISTU]|RPOP|S[CDIMORSU]|XACK|X[CDGILPRT]|Z[CDILMPRS])/i, @@ -64,14 +64,18 @@ export const defaultDbStatementSerializer: DbStatementSerializer = ( cmdArgs ) => { if (Array.isArray(cmdArgs) && cmdArgs.length) { - const argsSubset = - SerializationSubsets.find(({ regex }) => { + const nArgsToSerialize = + serializationSubsets.find(({ regex }) => { return regex.test(cmdName); - })?.args || 1; - const args = argsSubset > 0 ? cmdArgs.slice(0, argsSubset) : cmdArgs; - return `${cmdName} ${args.join(' ')} [${ - args.length !== cmdArgs.length ? cmdArgs.length - argsSubset : 0 - } other arguments]`; + })?.args ?? 1; + const argsToSerialize = + nArgsToSerialize >= 0 ? cmdArgs.slice(0, nArgsToSerialize) : cmdArgs; + if (cmdArgs.length > argsToSerialize.length) { + argsToSerialize.push( + `[${cmdArgs.length - nArgsToSerialize} other arguments]` + ); + } + return `${cmdName} ${argsToSerialize.join(' ')}`; } return cmdName; }; diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts b/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts index cd1dd3b716..567bf5ca86 100644 --- a/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts +++ b/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts @@ -43,6 +43,7 @@ import { DbSystemValues, SemanticAttributes, } from '@opentelemetry/semantic-conventions'; +import { defaultDbStatementSerializer } from '../src/utils'; const memoryExporter = new InMemorySpanExporter(); @@ -202,7 +203,7 @@ describe('ioredis', () => { description: 'get', name: 'get', args: [testKeyName], - serializedArgs: [testKeyName, '[0 other arguments]'], + serializedArgs: [testKeyName], method: (cb: ioredisTypes.CallbackFunction) => client.get(testKeyName, cb), }, @@ -338,8 +339,7 @@ describe('ioredis', () => { it('should create a child span for streamify scanning', done => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: - 'scan 0 MATCH test-* COUNT 1000 [0 other arguments]', + [SemanticAttributes.DB_STATEMENT]: 'scan 0 MATCH test-* COUNT 1000', }; const span = provider.getTracer('ioredis-test').startSpan('test span'); context.with(trace.setSpan(context.active(), span), () => { @@ -415,8 +415,7 @@ describe('ioredis', () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: - 'subscribe news music [0 other arguments]', + [SemanticAttributes.DB_STATEMENT]: 'subscribe news music', }; testUtils.assertSpan( endedSpans[4], @@ -505,7 +504,7 @@ describe('ioredis', () => { it('should create a child span for get promise', async () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: `get ${testKeyName} [0 other arguments]`, + [SemanticAttributes.DB_STATEMENT]: `get ${testKeyName}`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); await context.with(trace.setSpan(context.active(), span), async () => { @@ -534,7 +533,7 @@ describe('ioredis', () => { it('should create a child span for del', async () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: `del ${testKeyName} [0 other arguments]`, + [SemanticAttributes.DB_STATEMENT]: `del ${testKeyName}`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); await context.with(trace.setSpan(context.active(), span), async () => { @@ -568,7 +567,7 @@ describe('ioredis', () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [SemanticAttributes.DB_STATEMENT]: `evalsha bfbf458525d6a0b19200bfd6db3af481156b367b 1 ${testKeyName} [0 other arguments]`, + [SemanticAttributes.DB_STATEMENT]: `evalsha bfbf458525d6a0b19200bfd6db3af481156b367b 1 ${testKeyName}`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); @@ -956,4 +955,36 @@ describe('ioredis', () => { }); }); }); + + describe('#defaultDbStatementSerializer()', () => { + [ + { + cmdName: 'ECHO', + cmdArgs: ['echo'], + expected: 'ECHO [1 other arguments]', + }, + { + cmdName: 'LPUSH', + cmdArgs: ['list', 'value'], + expected: 'LPUSH list [1 other arguments]', + }, + { + cmdName: 'HSET', + cmdArgs: ['hash', 'field', 'value'], + expected: 'HSET hash field [1 other arguments]', + }, + { + cmdName: 'INCRBY', + cmdArgs: ['key', 5], + expected: 'INCRBY key 5', + }, + ].forEach(({ cmdName, cmdArgs, expected }) => { + it(`should serialize the correct number of arguments for ${cmdName}`, () => { + assert.strictEqual( + defaultDbStatementSerializer(cmdName, cmdArgs), + expected + ); + }); + }); + }); }); From 5f1793838a983a3e11bef4664cef22646f899260 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ketil=20=C3=98vreb=C3=B8?= Date: Tue, 14 Jun 2022 21:23:10 +0200 Subject: [PATCH 5/5] fix(ioredis): fall back to serializing no arguments --- .../node/opentelemetry-instrumentation-ioredis/src/utils.ts | 4 ++-- .../test/ioredis.test.ts | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts b/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts index b20b159ff3..55c93fee0f 100644 --- a/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts @@ -31,7 +31,7 @@ const serializationSubsets = [ args: 0, }, { - regex: /^(LPUSH|MSET|SET|PFA|PUBLISH|RPUSH|SADD|SET|SPUBLISH|XADD|ZADD)/i, + regex: /^(LPUSH|MSET|PFA|PUBLISH|RPUSH|SADD|SET|SPUBLISH|XADD|ZADD)/i, args: 1, }, { @@ -67,7 +67,7 @@ export const defaultDbStatementSerializer: DbStatementSerializer = ( const nArgsToSerialize = serializationSubsets.find(({ regex }) => { return regex.test(cmdName); - })?.args ?? 1; + })?.args ?? 0; const argsToSerialize = nArgsToSerialize >= 0 ? cmdArgs.slice(0, nArgsToSerialize) : cmdArgs; if (cmdArgs.length > argsToSerialize.length) { diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts b/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts index 567bf5ca86..e8bbe24fdf 100644 --- a/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts +++ b/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts @@ -958,6 +958,11 @@ describe('ioredis', () => { describe('#defaultDbStatementSerializer()', () => { [ + { + cmdName: 'UNKNOWN', + cmdArgs: ['something'], + expected: 'UNKNOWN [1 other arguments]', + }, { cmdName: 'ECHO', cmdArgs: ['echo'],