Merge pull request #42 from vmarchaud/improv/io-redis-require-parent

open-telemetry · Jun 23, 2020 · fd1d39b · fd1d39b
2 parents bb67b42 + 56c4845
commit fd1d39b
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 30 deletions.
diff --git a/plugins/node/opentelemetry-plugin-ioredis/package.json b/plugins/node/opentelemetry-plugin-ioredis/package.json
@@ -48,7 +48,7 @@
     "@opentelemetry/node": "0.9.0",
     "@opentelemetry/test-utils": "^0.8.0",
     "@opentelemetry/tracing": "0.9.0",
-    "@types/ioredis": "4.16.6",
+    "@types/ioredis": "4.16.7",
     "@types/mocha": "7.0.2",
     "@types/node": "13.13.12",
     "@types/shimmer": "1.0.1",

diff --git a/plugins/node/opentelemetry-plugin-ioredis/src/utils.ts b/plugins/node/opentelemetry-plugin-ioredis/src/utils.ts
@@ -84,35 +84,41 @@ export const traceSendCommand = (
     this: ioredisTypes.Redis & IORedisPluginClientTypes,
     cmd?: IORedisCommand
   ) {
-    if (arguments.length >= 1 && typeof cmd === 'object') {
-      const span = tracer.startSpan(cmd.name, {
-        kind: SpanKind.CLIENT,
-        attributes: {
-          [AttributeNames.COMPONENT]: IORedisPlugin.COMPONENT,
-          [AttributeNames.DB_TYPE]: IORedisPlugin.DB_TYPE,
-          [AttributeNames.DB_STATEMENT]: dbStatementSerializer(
-            cmd.name,
-            cmd.args
-          ),
-        },
-      });
+    if (arguments.length < 1 || typeof cmd !== 'object') {
+      return original.apply(this, arguments);
+    }
+    // Do not trace if there is not parent span
+    if (tracer.getCurrentSpan() === undefined) {
+      return original.apply(this, arguments);
+    }
 
-      const { host, port } = this.options;
+    const span = tracer.startSpan(cmd.name, {
+      kind: SpanKind.CLIENT,
+      attributes: {
+        [AttributeNames.COMPONENT]: IORedisPlugin.COMPONENT,
+        [AttributeNames.DB_TYPE]: IORedisPlugin.DB_TYPE,
+        [AttributeNames.DB_STATEMENT]: dbStatementSerializer(
+          cmd.name,
+          cmd.args
+        ),
+      },
+    });
 
-      span.setAttributes({
-        [AttributeNames.PEER_HOSTNAME]: host,
-        [AttributeNames.PEER_PORT]: port,
-        [AttributeNames.PEER_ADDRESS]: `redis://${host}:${port}`,
-      });
+    const { host, port } = this.options;
 
-      try {
-        const result = original.apply(this, arguments);
-        endSpan(span, null);
-        return result;
-      } catch (error) {
-        endSpan(span, error);
-        throw error;
-      }
-    } else return original.apply(this, arguments);
+    span.setAttributes({
+      [AttributeNames.PEER_HOSTNAME]: host,
+      [AttributeNames.PEER_PORT]: port,
+      [AttributeNames.PEER_ADDRESS]: `redis://${host}:${port}`,
+    });
+
+    try {
+      const result = original.apply(this, arguments);
+      endSpan(span, null);
+      return result;
+    } catch (error) {
+      endSpan(span, error);
+      throw error;
+    }
   };
 };
diff --git a/plugins/node/opentelemetry-plugin-ioredis/test/ioredis.test.ts b/plugins/node/opentelemetry-plugin-ioredis/test/ioredis.test.ts
@@ -348,7 +348,8 @@ describe('ioredis', () => {
       it(`should create a child span for lua`, done => {
         const attributes = {
           ...DEFAULT_ATTRIBUTES,
-          [AttributeNames.DB_STATEMENT]: 'eval return {KEYS[1],ARGV[1]} 1 test',
+          [AttributeNames.DB_STATEMENT]:
+            'evalsha bfbf458525d6a0b19200bfd6db3af481156b367b 1 test',
         };
 
         const span = provider.getTracer('ioredis-test').startSpan('test span');
@@ -371,7 +372,7 @@ describe('ioredis', () => {
             assert.strictEqual(endedSpans[1].name, 'eval');
             assert.strictEqual(endedSpans[0].name, 'evalsha');
             testUtils.assertSpan(
-              endedSpans[1],
+              endedSpans[0],
               SpanKind.CLIENT,
               attributes,
               [],
@@ -512,6 +513,19 @@ describe('ioredis', () => {
       });
     });
 
+    describe('Instrumenting without parent span', () => {
+      before(() => {
+        plugin.disable();
+        plugin.enable(ioredis, provider, new NoopLogger(), {});
+      });
+      it('should not create child span', async () => {
+        await client.set('test', 'data');
+        const result = await client.del('test');
+        assert.strictEqual(result, 1);
+        assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
+      });
+    });
+
     describe('Instrumenting with a custom db.statement serializer', () => {
       const dbStatementSerializer: DbStatementSerializer = (cmdName, cmdArgs) =>
         `FOOBAR_${cmdName}: ${cmdArgs[0]}`;