fix(ioredis): requireParentSpan not applied to connect spans (open-te…

…lemetry#1151) * Add tests to validate that requireParentSpan applies to connect * Copy finishedSpans * Implement requireParentSpan for connect Co-authored-by: Amir Blum <amir@aspecto.io>
dynatrace-oss-contrib · Sep 7, 2022 · d3cb60d · d3cb60d
1 parent d8767a9
commit d3cb60d
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 4 deletions.
diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-ioredis/src/instrumentation.ts
@@ -184,6 +184,13 @@ export class IORedisInstrumentation extends InstrumentationBase<
   private traceConnection = (original: Function) => {
     const instrumentation = this;
     return function (this: ioredisTypes.Redis) {
+      const config =
+        instrumentation.getConfig() as IORedisInstrumentationConfig;
+      const hasNoParentSpan = trace.getSpan(context.active()) === undefined;
+      if (config?.requireParentSpan === true && hasNoParentSpan) {
+        return original.apply(this, arguments);
+      }
+
       const span = instrumentation.tracer.startSpan('connect', {
         kind: SpanKind.CLIENT,
         attributes: {

diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts b/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts
@@ -640,16 +640,24 @@ describe('ioredis', () => {
         };
         instrumentation.setConfig(config);
       });
-      it('should not create child span', async () => {
+      it('should not create child span for query', async () => {
         await client.set(testKeyName, 'data');
         const result = await client.del(testKeyName);
         assert.strictEqual(result, 1);
         assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
       });
+
+      it('should not create child span for connect', async () => {
+        const lazyClient = new ioredis(URL, { lazyConnect: true });
+        await lazyClient.connect();
+        const spans = memoryExporter.getFinishedSpans();
+        await lazyClient.quit();
+        assert.strictEqual(spans.length, 0);
+      });
     });
 
     describe('Instrumentation with requireParentSpan', () => {
-      it('should instrument with requireParentSpan equal false', async () => {
+      it('should instrument queries with requireParentSpan equal false', async () => {
         const config: IORedisInstrumentationConfig = {
           requireParentSpan: false,
         };
@@ -658,23 +666,48 @@ describe('ioredis', () => {
         await client.set(testKeyName, 'data');
         const result = await client.del(testKeyName);
         assert.strictEqual(result, 1);
+        const endedSpans = memoryExporter.getFinishedSpans();
 
+        assert.strictEqual(endedSpans.length, 2);
+        testUtils.assertSpan(
+          endedSpans[0],
+          SpanKind.CLIENT,
+          {
+            ...DEFAULT_ATTRIBUTES,
+            [SemanticAttributes.DB_STATEMENT]: `set ${testKeyName} [1 other arguments]`,
+          },
+          [],
+          unsetStatus
+        );
+      });
+
+      it('should instrument connect with requireParentSpan equal false', async () => {
+        const config: IORedisInstrumentationConfig = {
+          requireParentSpan: false,
+        };
+        instrumentation.setConfig(config);
+
+        const lazyClient = new ioredis(URL, { lazyConnect: true });
+        await lazyClient.connect();
         const endedSpans = memoryExporter.getFinishedSpans();
         assert.strictEqual(endedSpans.length, 2);
+        assert.strictEqual(endedSpans[0].name, 'connect');
+        assert.strictEqual(endedSpans[1].name, 'info');
 
+        await lazyClient.quit();
         testUtils.assertSpan(
           endedSpans[0],
           SpanKind.CLIENT,
           {
             ...DEFAULT_ATTRIBUTES,
-            [SemanticAttributes.DB_STATEMENT]: `set ${testKeyName} [1 other arguments]`,
+            [SemanticAttributes.DB_STATEMENT]: 'connect',
           },
           [],
           unsetStatus
         );
       });
 
-      it('should not instrument with requireParentSpan equal true', async () => {
+      it('should not instrument queries with requireParentSpan equal true', async () => {
         const config: IORedisInstrumentationConfig = {
           requireParentSpan: true,
         };
@@ -686,6 +719,20 @@ describe('ioredis', () => {
 
         assert.strictEqual(memoryExporter.getFinishedSpans().length, 0);
       });
+
+      it('should not instrument connect with requireParentSpan equal true', async () => {
+        const config: IORedisInstrumentationConfig = {
+          requireParentSpan: true,
+        };
+        instrumentation.setConfig(config);
+
+        const lazyClient = new ioredis(URL, { lazyConnect: true });
+        await lazyClient.connect();
+        const endedSpans = memoryExporter.getFinishedSpans();
+        assert.strictEqual(endedSpans.length, 0);
+
+        await lazyClient.quit();
+      });
     });
 
     describe('Instrumenting with a custom db.statement serializer', () => {