Skip to content

Commit

Permalink
fix(instrumentation-redis-4): fix multi.exec() instrumentation for re…
Browse files Browse the repository at this point in the history
…dis >=4.6.12 (open-telemetry#1904)

In redis@4.6.12 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 <dyladan@users.noreply.github.com>
  • Loading branch information
trentm and dyladan authored Jan 22, 2024
1 parent f65f2f1 commit fce7d3b
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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 redis@4.6.12.
// https://github.com/redis/node-redis/blob/redis@4.6.12/packages/client/lib/errors.ts#L69-L84
export interface MultiErrorReply extends Error {
replies: unknown[];
errorIndexes: Array<number>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 redis@4.6.12 `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

Expand Down

0 comments on commit fce7d3b

Please sign in to comment.