Skip to content

Commit

Permalink
test: fix ioredis tests (#830)
Browse files Browse the repository at this point in the history 
* test: fix tests for ioredis
* test: narrowed down versions we test
* test: enable checking response param in the hook test
* style: fix types and lint
* style: lint again
  • Loading branch information
@rauno56
rauno56 authored Jan 14, 2022
1 parent 88db0a4 commit fcdc14e
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 42 deletions.
4 changes: 2 additions & 2 deletions plugins/node/opentelemetry-instrumentation-ioredis/.tav.yml
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
ioredis:
# Ignoring v4.19.0. Tests never ends. Caused by https://github.com/luin/ioredis/pull/1219
versions: ">1 < 4.19.0 || > 4.19.0 < 5"
versions: "^2.5.0 || ^3.2.2 || 4.14.0 || 4.14.1 || 4.16.3 || 4.17.3 || 4.18.0 || 4.19.2 || 4.19.4 || 4.22.0 || 4.24.5 || 4.26.0 || 4.27.2 || 4.27.3 || ^4.27.6"
commands: npm run test

# Fix missing `contrib-test-utils` package
pretest: npm run --prefix ../../../ lerna:link
pretest: npm run --prefix ../../../ lerna:link
2 changes: 2 additions & 0 deletions plugins/node/opentelemetry-instrumentation-ioredis/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,15 @@
"@opentelemetry/sdk-trace-node": "1.0.1",
"@types/mocha": "7.0.2",
"@types/node": "14.17.9",
"@types/sinon": "^10.0.6",
"codecov": "3.8.3",
"cross-env": "7.0.3",
"gts": "3.1.0",
"ioredis": "4.27.7",
"mocha": "7.2.0",
"nyc": "15.1.0",
"rimraf": "3.0.2",
"sinon": "^12.0.1",
"test-all-versions": "5.0.1",
"ts-mocha": "8.0.0",
"typescript": "4.3.5"
Expand Down
100 changes: 60 additions & 40 deletions plugins/node/opentelemetry-instrumentation-ioredis/test/ioredis.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
SimpleSpanProcessor,
} from '@opentelemetry/sdk-trace-base';
import * as assert from 'assert';
import * as sinon from 'sinon';
import * as ioredisTypes from 'ioredis';
import { IORedisInstrumentation } from '../src';
import {
Expand Down Expand Up @@ -221,6 +222,7 @@ describe('ioredis', () => {
afterEach(async () => {
await client.del(hashKeyName);
await client.del(testKeyName);
await client.del('response-hook-test');
memoryExporter.reset();
});

Expand Down Expand Up @@ -330,11 +332,14 @@ describe('ioredis', () => {
it('should create a child span for streamify scanning', done => {
const attributes = {
...DEFAULT_ATTRIBUTES,
[SemanticAttributes.DB_STATEMENT]: 'scan 0',
[SemanticAttributes.DB_STATEMENT]: 'scan 0 MATCH test-* COUNT 1000',
};
const span = provider.getTracer('ioredis-test').startSpan('test span');
context.with(trace.setSpan(context.active(), span), () => {
const stream = client.scanStream();
const stream = client.scanStream({
count: 1000,
match: 'test-*',
});
stream
.on('end', () => {
assert.strictEqual(memoryExporter.getFinishedSpans().length, 1);
Expand Down Expand Up @@ -771,26 +776,17 @@ describe('ioredis', () => {
});

it('should call requestHook when set in config', async () => {
const config: IORedisInstrumentationConfig = {
requestHook: (
span: Span,
requestInfo: IORedisRequestHookInformation
) => {
assert.ok(
/\d{1,4}\.\d{1,4}\.\d{1,5}.*/.test(
requestInfo.moduleVersion as string
)
);
assert.strictEqual(requestInfo.cmdName, 'incr');
assert.deepStrictEqual(requestInfo.cmdArgs, ['request-hook-test']);

const requestHook = sinon.spy(
(span: Span, requestInfo: IORedisRequestHookInformation) => {
span.setAttribute(
'attribute key from request hook',
'custom value from request hook'
);
},
};
instrumentation.setConfig(config);
}
);
instrumentation.setConfig(<IORedisInstrumentationConfig>{
requestHook,
});

const span = provider.getTracer('ioredis-test').startSpan('test span');
await context.with(trace.setSpan(context.active(), span), async () => {
Expand All @@ -802,22 +798,31 @@ describe('ioredis', () => {
'custom value from request hook'
);
});

sinon.assert.calledOnce(requestHook);
const [, requestInfo] = requestHook.firstCall.args;
assert.ok(
/\d{1,4}\.\d{1,4}\.\d{1,5}.*/.test(
requestInfo.moduleVersion as string
)
);
assert.strictEqual(requestInfo.cmdName, 'incr');
assert.deepStrictEqual(requestInfo.cmdArgs, ['request-hook-test']);
});

it('should ignore requestHook which throws exception', async () => {
const config: IORedisInstrumentationConfig = {
requestHook: (
span: Span,
_requestInfo: IORedisRequestHookInformation
) => {
const requestHook = sinon.spy(
(span: Span, _requestInfo: IORedisRequestHookInformation) => {
span.setAttribute(
'attribute key BEFORE exception',
'this attribute is added to span BEFORE exception is thrown thus we can expect it'
);
throw Error('error thrown in requestHook');
},
};
instrumentation.setConfig(config);
}
);
instrumentation.setConfig(<IORedisInstrumentationConfig>{
requestHook,
});

const span = provider.getTracer('ioredis-test').startSpan('test span');
await context.with(trace.setSpan(context.active(), span), async () => {
Expand All @@ -829,51 +834,64 @@ describe('ioredis', () => {
'this attribute is added to span BEFORE exception is thrown thus we can expect it'
);
});

sinon.assert.threw(requestHook);
});

it('should call responseHook when set in config', async () => {
const config: IORedisInstrumentationConfig = {
responseHook: (
const responseHook = sinon.spy(
(
span: Span,
cmdName: string,
_cmdArgs: Array<string | Buffer | number>,
response: unknown
) => {
assert.strictEqual(cmdName, 'incr');
// the command is 'incr' on a key which does not exist, thus it increase 0 by 1 and respond 1
assert.strictEqual(response, 1);
span.setAttribute(
'attribute key from hook',
'custom value from hook'
);
},
};
instrumentation.setConfig(config);
}
);
instrumentation.setConfig(<IORedisInstrumentationConfig>{
responseHook,
});

const span = provider.getTracer('ioredis-test').startSpan('test span');
await context.with(trace.setSpan(context.active(), span), async () => {
await client.incr('response-hook-test');
await client.set('response-hook-test', 'test-value');
const endedSpans = memoryExporter.getFinishedSpans();
assert.strictEqual(endedSpans.length, 1);
assert.strictEqual(
endedSpans[0].attributes['attribute key from hook'],
'custom value from hook'
);
});

sinon.assert.calledOnce(responseHook);
const [, cmdName, , response] = responseHook.firstCall.args as [
Span,
string,
unknown,
Buffer
];
assert.strictEqual(cmdName, 'set');
assert.strictEqual(response.toString(), 'OK');
});

it('should ignore responseHook which throws exception', async () => {
const config: IORedisInstrumentationConfig = {
responseHook: (
const responseHook = sinon.spy(
(
_span: Span,
_cmdName: string,
_cmdArgs: Array<string | Buffer | number>,
_response: unknown
) => {
throw Error('error thrown in responseHook');
},
};
instrumentation.setConfig(config);
}
);
instrumentation.setConfig(<IORedisInstrumentationConfig>{
responseHook,
});

const span = provider.getTracer('ioredis-test').startSpan('test span');
await context.with(trace.setSpan(context.active(), span), async () => {
Expand All @@ -883,6 +901,8 @@ describe('ioredis', () => {
// hook throw exception, but span should not be affected
assert.strictEqual(endedSpans.length, 1);
});

sinon.assert.threw(responseHook);
});
});

Expand Down

0 comments on commit fcdc14e

Please sign in to comment.