From afde5531dac594f527041d089f7fa92b4e57bb45 Mon Sep 17 00:00:00 2001 From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com> Date: Fri, 29 Jan 2021 08:54:13 +0100 Subject: [PATCH] chore: merge changes from #239 --- .../src/utils.ts | 16 ++- .../test/ioredis.test.ts | 98 ++++++++++++++++--- 2 files changed, 101 insertions(+), 13 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts b/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts index 933c9266fb..7955c3a55c 100644 --- a/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-ioredis/src/utils.ts @@ -36,6 +36,7 @@ import { const endSpan = (span: Span, err: NodeJS.ErrnoException | null | undefined) => { if (err) { + span.recordException(err); span.setStatus({ code: StatusCode.ERROR, message: err.message, @@ -116,7 +117,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); diff --git a/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts b/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts index 68fa0fc891..80f15e1a8c 100644 --- a/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts +++ b/plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts @@ -27,6 +27,7 @@ import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks'; import * as testUtils from '@opentelemetry/test-utils'; import { InMemorySpanExporter, + ReadableSpan, SimpleSpanProcessor, } from '@opentelemetry/tracing'; import * as assert from 'assert'; @@ -38,6 +39,7 @@ import { } from '../src/types'; import { DatabaseAttribute, + ExceptionAttribute, GeneralAttribute, } from '@opentelemetry/semantic-conventions'; @@ -61,6 +63,20 @@ const unsetStatus: Status = { code: StatusCode.UNSET, }; +const predictableStackTrace = + '-- Stack trace replaced by test to predictable value -- '; +const sanitizeEventForAssertion = (span: ReadableSpan) => { + span.events.forEach(e => { + // stack trace includes data such as /user/{userName}/repos/{projectName} + if (e.attributes?.[ExceptionAttribute.STACKTRACE]) { + e.attributes[ExceptionAttribute.STACKTRACE] = predictableStackTrace; + } + + // since time will change on each test invocation, it is being replaced to predicable value + e.time = [0, 0]; + }); +}; + describe('ioredis', () => { const provider = new NodeTracerProvider(); let ioredis: typeof ioredisTypes; @@ -138,9 +154,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); @@ -270,6 +288,38 @@ describe('ioredis', () => { }); }); + it('should set span with error when redis return reject', async () => { + const span = provider.getTracer('ioredis-test').startSpan('test span'); + await context.with(setSpan(context.active(), span), async () => { + await client.set('non-int-key', 'non-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); + const ioredisSpan = endedSpans[1]; + // redis 'incr' operation failed with exception, so span should indicate it + assert.strictEqual(ioredisSpan.status.code, StatusCode.ERROR); + const exceptionEvent = ioredisSpan.events[0]; + assert.strictEqual(exceptionEvent.name, 'exception'); + assert.strictEqual( + exceptionEvent.attributes?.[ExceptionAttribute.MESSAGE], + ex.message + ); + assert.strictEqual( + exceptionEvent.attributes?.[ExceptionAttribute.STACKTRACE], + ex.stack + ); + assert.strictEqual( + exceptionEvent.attributes?.[ExceptionAttribute.TYPE], + ex.name + ); + } + }); + }); + it('should create a child span for streamify scanning', done => { const attributes = { ...DEFAULT_ATTRIBUTES, @@ -329,10 +379,10 @@ describe('ioredis', () => { const spanNames = [ 'connect', 'connect', - 'subscribe', 'info', 'info', 'subscribe', + 'subscribe', 'publish', 'publish', 'unsubscribe', @@ -384,24 +434,48 @@ describe('ioredis', () => { span.end(); const endedSpans = memoryExporter.getFinishedSpans(); + const evalshaSpan = endedSpans[0]; // the script may be already cached on server therefore we get either 2 or 3 spans 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 + sanitizeEventForAssertion(evalshaSpan); + testUtils.assertSpan( + evalshaSpan, + SpanKind.CLIENT, + attributes, + [ + { + attributes: { + [ExceptionAttribute.MESSAGE]: + 'NOSCRIPT No matching script. Please use EVAL.', + [ExceptionAttribute.STACKTRACE]: predictableStackTrace, + [ExceptionAttribute.TYPE]: 'ReplyError', + }, + name: 'exception', + time: [0, 0], + }, + ], + { + code: StatusCode.ERROR, + } + ); } else { assert.strictEqual(endedSpans.length, 2); assert.strictEqual(endedSpans[1].name, 'test span'); assert.strictEqual(endedSpans[0].name, 'evalsha'); + testUtils.assertSpan( + evalshaSpan, + SpanKind.CLIENT, + attributes, + [], + unsetStatus + ); } - testUtils.assertSpan( - endedSpans[0], - SpanKind.CLIENT, - attributes, - [], - unsetStatus - ); - testUtils.assertPropagation(endedSpans[0], span); + testUtils.assertPropagation(evalshaSpan, span); done(); }); });