fix(plugin-ioredis): end span on response from the server and set span status according to response

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

@@ -109,7 +109,20 @@ export const traceSendCommand = (
 
     try {
       const result = original.apply(this, arguments);
-      endSpan(span, null);
+
+      const origResolve = cmd.resolve;
+      /* eslint-disable @typescript-eslint/no-explicit-any */
+      cmd.resolve = function (result: any) {
+        endSpan(span, null);
+        origResolve(result);
+      };
+
+      const origReject = cmd.reject;
+      cmd.reject = function (err: Error) {
+        endSpan(span, err);
+        origReject(err);
+      };
+
       return result;
     } catch (error) {
       endSpan(span, error);

plugins/node/opentelemetry-plugin-ioredis/test/ioredis.test.ts

@@ -127,9 +127,11 @@ describe('ioredis', () => {
         assert.strictEqual(endedSpans.length, 3);
         assert.strictEqual(endedSpans[2].name, 'test span');
 
-        client.quit(done);
-        assert.strictEqual(endedSpans.length, 4);
-        assert.strictEqual(endedSpans[3].name, 'quit');
+        client.quit(() => {
+          assert.strictEqual(endedSpans.length, 4);
+          assert.strictEqual(endedSpans[3].name, 'quit');
+          done();
+        });
       };
       const errorHandler = (err: Error) => {
         assert.ifError(err);
@@ -259,6 +261,23 @@ describe('ioredis', () => {
         });
       });
 
+      it('should set span with error when redis return reject', async () => {
+        const span = provider.getTracer('ioredis-test').startSpan('test span');
+        await provider.getTracer('ioredis-test').withSpan(span, async () => {
+          await client.set('non-int-key', 'no-int-value');
+          try {
+            // should throw 'ReplyError: ERR value is not an integer or out of range'
+            // because the value im the key is not numeric and we try to increment it
+            await client.incr('non-int-key');
+          } catch (ex) {
+            const endedSpans = memoryExporter.getFinishedSpans();
+            assert.strictEqual(endedSpans.length, 2);
+            // redis 'incr' operation failed with exception, so span should indicate it
+            assert.notStrictEqual(endedSpans[1].status.code, StatusCode.UNSET);
+          }
+        });
+      });
+
       it('should create a child span for streamify scanning', done => {
         const attributes = {
           ...DEFAULT_ATTRIBUTES,
@@ -318,10 +337,10 @@ describe('ioredis', () => {
             const spanNames = [
               'connect',
               'connect',
-              'subscribe',
               'info',
               'info',
               'subscribe',
+              'subscribe',
               'publish',
               'publish',
               'unsubscribe',
@@ -374,21 +393,28 @@ describe('ioredis', () => {
             span.end();
             const endedSpans = memoryExporter.getFinishedSpans();
             // the script may be already cached on server therefore we get either 2 or 3 spans
+            let expectedEvalshaStatus;
             if (endedSpans.length === 3) {
               assert.strictEqual(endedSpans[2].name, 'test span');
               assert.strictEqual(endedSpans[1].name, 'eval');
               assert.strictEqual(endedSpans[0].name, 'evalsha');
+              // in this case, server returns NOSCRIPT error for evalsha,
+              // telling the client to use EVAL instead
+              expectedEvalshaStatus = {
+                code: StatusCode.ERROR,
+              };
             } else {
               assert.strictEqual(endedSpans.length, 2);
               assert.strictEqual(endedSpans[1].name, 'test span');
               assert.strictEqual(endedSpans[0].name, 'evalsha');
+              expectedEvalshaStatus = unsetStatus;
             }
             testUtils.assertSpan(
               endedSpans[0],
               SpanKind.CLIENT,
               attributes,
               [],
-              unsetStatus
+              expectedEvalshaStatus
             );
             testUtils.assertPropagation(endedSpans[0], span);
             done();