fix(instrumentation-redis-4): fix multi.exec() instrumentation for redis >=4.6.12 (open-telemetry#1904)

In [email protected] the behaviour of multi.exec() changed to *throw* a
MultiErrorReply if any of the commands errored out. The individual
command replies are available at 'err.replies', instead of as the
promise result. This adjusts the instrumentation to generate
spans as before: only setting SpanStatusCode.ERROR and calling
span.recordException for the individual commands that failed.

Fixes: open-telemetry#1874
Co-authored-by: Daniel Dyla <[email protected]>

Author: trentm

plugins/node/opentelemetry-instrumentation-redis-4/src/instrumentation.ts

@@ -33,6 +33,7 @@ import { defaultDbStatementSerializer } from '@opentelemetry/redis-common';
 import { RedisInstrumentationConfig } from './types';
 import { VERSION } from './version';
 import { SemanticAttributes } from '@opentelemetry/semantic-conventions';
+import type { MultiErrorReply } from './internal-types';
 
 const OTEL_OPEN_SPANS = Symbol(
   'opentelemetry.instruemntation.redis.open_spans'
@@ -289,55 +290,21 @@ export class RedisInstrumentation extends InstrumentationBase<any> {
         return execRes
           .then((redisRes: unknown[]) => {
             const openSpans = this[OTEL_OPEN_SPANS];
-            if (!openSpans) {
-              return plugin._diag.error(
-                'cannot find open spans to end for redis multi command'
-              );
-            }
-            if (redisRes.length !== openSpans.length) {
-              return plugin._diag.error(
-                'number of multi command spans does not match response from redis'
-              );
-            }
-            for (let i = 0; i < openSpans.length; i++) {
-              const { span, commandName, commandArgs } = openSpans[i];
-              const currCommandRes = redisRes[i];
-              if (currCommandRes instanceof Error) {
-                plugin._endSpanWithResponse(
-                  span,
-                  commandName,
-                  commandArgs,
-                  null,
-                  currCommandRes
-                );
-              } else {
-                plugin._endSpanWithResponse(
-                  span,
-                  commandName,
-                  commandArgs,
-                  currCommandRes,
-                  undefined
-                );
-              }
-            }
+            plugin._endSpansWithRedisReplies(openSpans, redisRes);
             return redisRes;
           })
           .catch((err: Error) => {
             const openSpans = this[OTEL_OPEN_SPANS];
             if (!openSpans) {
-              return plugin._diag.error(
+              plugin._diag.error(
                 'cannot find open spans to end for redis multi command'
               );
-            }
-            for (let i = 0; i < openSpans.length; i++) {
-              const { span, commandName, commandArgs } = openSpans[i];
-              plugin._endSpanWithResponse(
-                span,
-                commandName,
-                commandArgs,
-                null,
-                err
-              );
+            } else {
+              const replies =
+                err.constructor.name === 'MultiErrorReply'
+                  ? (err as MultiErrorReply).replies
+                  : new Array(openSpans.length).fill(err);
+              plugin._endSpansWithRedisReplies(openSpans, replies);
             }
             return Promise.reject(err);
           });
@@ -487,6 +454,31 @@ export class RedisInstrumentation extends InstrumentationBase<any> {
     return res;
   }
 
+  private _endSpansWithRedisReplies(
+    openSpans: Array<MutliCommandInfo>,
+    replies: unknown[]
+  ) {
+    if (!openSpans) {
+      return this._diag.error(
+        'cannot find open spans to end for redis multi command'
+      );
+    }
+    if (replies.length !== openSpans.length) {
+      return this._diag.error(
+        'number of multi command spans does not match response from redis'
+      );
+    }
+    for (let i = 0; i < openSpans.length; i++) {
+      const { span, commandName, commandArgs } = openSpans[i];
+      const currCommandRes = replies[i];
+      const [res, err] =
+        currCommandRes instanceof Error
+          ? [null, currCommandRes]
+          : [currCommandRes, undefined];
+      this._endSpanWithResponse(span, commandName, commandArgs, res, err);
+    }
+  }
+
   private _endSpanWithResponse(
     span: Span,
     commandName: string,

plugins/node/opentelemetry-instrumentation-redis-4/src/internal-types.ts

@@ -0,0 +1,22 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+// Error class introduced in [email protected].
+// https://github.com/redis/node-redis/blob/[email protected]/packages/client/lib/errors.ts#L69-L84
+export interface MultiErrorReply extends Error {
+  replies: unknown[];
+  errorIndexes: Array<number>;
+}

plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts

@@ -18,6 +18,7 @@ import {
   registerInstrumentationTesting,
 } from '@opentelemetry/contrib-test-utils';
 import { RedisInstrumentation } from '../src';
+import type { MultiErrorReply } from '../src/internal-types';
 import * as assert from 'assert';
 
 import {
@@ -377,11 +378,16 @@ describe('redis@^4.0.0', () => {
     });
 
     it('multi command with error', async () => {
-      const [setReply, incrReply] = await client
-        .multi()
-        .set('key', 'value')
-        .incr('key')
-        .exec(); // ['OK', 'ReplyError']
+      let replies;
+      try {
+        replies = await client.multi().set('key', 'value').incr('key').exec();
+      } catch (err) {
+        // Starting in [email protected] `multi().exec()` will *throw* a
+        // MultiErrorReply, with `err.replies`, if any of the commands error.
+        replies = (err as MultiErrorReply).replies;
+      }
+      const [setReply, incrReply] = replies;
+
       assert.strictEqual(setReply, 'OK'); // verify we did not screw up the normal functionality
       assert.ok(incrReply instanceof Error); // verify we did not screw up the normal functionality