feat(ioredis): only serialize non sensitive arguments in db statement…

… attribute (open-telemetry#1052)

plugins/node/opentelemetry-instrumentation-ioredis/README.md

@@ -55,7 +55,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:

plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts

@@ -17,6 +17,34 @@
 import { Span, SpanStatusCode } from '@opentelemetry/api';
 import { DbStatementSerializer } from './types';
 
+/**
+ * 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 = [
+  {
+    regex: /^ECHO/i,
+    args: 0,
+  },
+  {
+    regex: /^(LPUSH|MSET|PFA|PUBLISH|RPUSH|SADD|SET|SPUBLISH|XADD|ZADD)/i,
+    args: 1,
+  },
+  {
+    regex: /^(HSET|HMSET|LSET|LINSERT)/i,
+    args: 2,
+  },
+  {
+    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,
   err: NodeJS.ErrnoException | null | undefined
@@ -34,7 +62,20 @@ export const endSpan = (
 export const defaultDbStatementSerializer: DbStatementSerializer = (
   cmdName,
   cmdArgs
-) =>
-  Array.isArray(cmdArgs) && cmdArgs.length
-    ? `${cmdName} ${cmdArgs.join(' ')}`
-    : cmdName;
+) => {
+  if (Array.isArray(cmdArgs) && cmdArgs.length) {
+    const nArgsToSerialize =
+      serializationSubsets.find(({ regex }) => {
+        return regex.test(cmdName);
+      })?.args ?? 0;
+    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;
+};

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();
 
@@ -187,19 +188,22 @@ describe('ioredis', () => {
       description: string;
       name: string;
       args: Array<string>;
+      serializedArgs: Array<string>;
       method: (cb: ioredisTypes.CallbackFunction<unknown>) => unknown;
     }> = [
       {
         description: 'insert',
         name: 'hset',
         args: [hashKeyName, 'testField', 'testValue'],
+        serializedArgs: [hashKeyName, 'testField', '[1 other arguments]'],
         method: (cb: ioredisTypes.CallbackFunction<number>) =>
           client.hset(hashKeyName, 'testField', 'testValue', cb),
       },
       {
         description: 'get',
         name: 'get',
         args: [testKeyName],
+        serializedArgs: [testKeyName],
         method: (cb: ioredisTypes.CallbackFunction<string | null>) =>
           client.get(testKeyName, cb),
       },
@@ -243,7 +247,7 @@ describe('ioredis', () => {
             ...DEFAULT_ATTRIBUTES,
             [SemanticAttributes.DB_STATEMENT]: `${
               command.name
-            } ${command.args.join(' ')}`,
+            } ${command.serializedArgs.join(' ')}`,
           };
           const span = provider
             .getTracer('ioredis-test')
@@ -273,7 +277,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} random [1 other arguments]`,
         };
         const span = provider.getTracer('ioredis-test').startSpan('test span');
         await context.with(trace.setSpan(context.active(), span), async () => {
@@ -466,7 +470,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 [1 other arguments]',
         };
 
         const span = provider.getTracer('ioredis-test').startSpan('test span');
@@ -663,7 +667,7 @@ describe('ioredis', () => {
           SpanKind.CLIENT,
           {
             ...DEFAULT_ATTRIBUTES,
-            [SemanticAttributes.DB_STATEMENT]: `set ${testKeyName} data`,
+            [SemanticAttributes.DB_STATEMENT]: `set ${testKeyName} [1 other arguments]`,
           },
           [],
           unsetStatus
@@ -951,4 +955,41 @@ describe('ioredis', () => {
       });
     });
   });
+
+  describe('#defaultDbStatementSerializer()', () => {
+    [
+      {
+        cmdName: 'UNKNOWN',
+        cmdArgs: ['something'],
+        expected: 'UNKNOWN [1 other arguments]',
+      },
+      {
+        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
+        );
+      });
+    });
+  });
 });