From e352126e6328a39c462ecd9a4784c6fcbc7e3fb4 Mon Sep 17 00:00:00 2001 From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com> Date: Wed, 13 Jan 2021 23:48:55 +0100 Subject: [PATCH] fix: avoid races because of parallel ioredis tests ioredis tests run parallel for serverl node versions using the same redis instance. Add a random part to keys to avoid that one run impacts others. Additionally improve lua test asserts as the script may be already cached by redis resulting in one span less. --- .../test/ioredis.test.ts | 56 +++++++++++-------- 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/plugins/node/opentelemetry-plugin-ioredis/test/ioredis.test.ts b/plugins/node/opentelemetry-plugin-ioredis/test/ioredis.test.ts index 6cedb6a763..f10729a933 100644 --- a/plugins/node/opentelemetry-plugin-ioredis/test/ioredis.test.ts +++ b/plugins/node/opentelemetry-plugin-ioredis/test/ioredis.test.ts @@ -145,6 +145,11 @@ describe('ioredis', () => { }); describe('#send_internal_message()', () => { + // use a random part in key names because redis instance is used for parallel running tests + const randomId = ((Math.random() * 2 ** 32) >>> 0).toString(16); + const testKeyName = `test-${randomId}`; + const hashKeyName = `hash-${randomId}`; + let client: ioredisTypes.Redis; const IOREDIS_CALLBACK_OPERATIONS: Array<{ @@ -156,16 +161,16 @@ describe('ioredis', () => { { description: 'insert', name: 'hset', - args: ['hash', 'testField', 'testValue'], + args: [hashKeyName, 'testField', 'testValue'], method: (cb: ioredisTypes.CallbackFunction) => - client.hset('hash', 'testField', 'testValue', cb), + client.hset(hashKeyName, 'testField', 'testValue', cb), }, { description: 'get', name: 'get', - args: ['test'], + args: [testKeyName], method: (cb: ioredisTypes.CallbackFunction) => - client.get('test', cb), + client.get(testKeyName, cb), }, ]; @@ -178,7 +183,7 @@ describe('ioredis', () => { }); beforeEach(async () => { - await client.set('test', 'data'); + await client.set(testKeyName, 'data'); memoryExporter.reset(); }); @@ -187,7 +192,8 @@ describe('ioredis', () => { }); afterEach(async () => { - client.del('hash'); + await client.del(hashKeyName); + await client.del(testKeyName); memoryExporter.reset(); }); @@ -228,12 +234,12 @@ describe('ioredis', () => { it('should create a child span for hset promise', async () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [DatabaseAttribute.DB_STATEMENT]: 'hset hash random random', + [DatabaseAttribute.DB_STATEMENT]: `hset ${hashKeyName} random random`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); await provider.getTracer('ioredis-test').withSpan(span, async () => { try { - await client.hset('hash', 'random', 'random'); + await client.hset(hashKeyName, 'random', 'random'); assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); span.end(); const endedSpans = memoryExporter.getFinishedSpans(); @@ -350,8 +356,7 @@ describe('ioredis', () => { it('should create a child span for lua', done => { const attributes = { ...DEFAULT_ATTRIBUTES, - [DatabaseAttribute.DB_STATEMENT]: - 'evalsha bfbf458525d6a0b19200bfd6db3af481156b367b 1 test', + [DatabaseAttribute.DB_STATEMENT]: `evalsha bfbf458525d6a0b19200bfd6db3af481156b367b 1 ${testKeyName}`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); @@ -363,16 +368,21 @@ describe('ioredis', () => { }); // Now `echo` can be used just like any other ordinary command, // and ioredis will try to use `EVALSHA` internally when possible for better performance. - client.echo('test', (err, result) => { + client.echo(testKeyName, (err, result) => { assert.ifError(err); - assert.strictEqual(memoryExporter.getFinishedSpans().length, 2); span.end(); const endedSpans = memoryExporter.getFinishedSpans(); - assert.strictEqual(endedSpans.length, 3); - assert.strictEqual(endedSpans[2].name, 'test span'); - assert.strictEqual(endedSpans[1].name, 'eval'); - assert.strictEqual(endedSpans[0].name, 'evalsha'); + // 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'); + } else { + assert.strictEqual(endedSpans.length, 2); + assert.strictEqual(endedSpans[1].name, 'test span'); + assert.strictEqual(endedSpans[0].name, 'evalsha'); + } testUtils.assertSpan( endedSpans[0], SpanKind.CLIENT, @@ -459,12 +469,12 @@ describe('ioredis', () => { it('should create a child span for get promise', async () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [DatabaseAttribute.DB_STATEMENT]: 'get test', + [DatabaseAttribute.DB_STATEMENT]: `get ${testKeyName}`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); await provider.getTracer('ioredis-test').withSpan(span, async () => { try { - const value = await client.get('test'); + const value = await client.get(testKeyName); assert.strictEqual(value, 'data'); assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); span.end(); @@ -488,12 +498,12 @@ describe('ioredis', () => { it('should create a child span for del', async () => { const attributes = { ...DEFAULT_ATTRIBUTES, - [DatabaseAttribute.DB_STATEMENT]: 'del test', + [DatabaseAttribute.DB_STATEMENT]: `del ${testKeyName}`, }; const span = provider.getTracer('ioredis-test').startSpan('test span'); await provider.getTracer('ioredis-test').withSpan(span, async () => { try { - const result = await client.del('test'); + const result = await client.del(testKeyName); assert.strictEqual(result, 1); assert.strictEqual(memoryExporter.getFinishedSpans().length, 1); span.end(); @@ -521,8 +531,8 @@ describe('ioredis', () => { plugin.enable(ioredis, provider, new NoopLogger(), {}); }); it('should not create child span', async () => { - await client.set('test', 'data'); - const result = await client.del('test'); + await client.set(testKeyName, 'data'); + const result = await client.del(testKeyName); assert.strictEqual(result, 1); assert.strictEqual(memoryExporter.getFinishedSpans().length, 0); }); @@ -602,7 +612,7 @@ describe('ioredis', () => { const span = provider.getTracer('ioredis-test').startSpan('test span'); await provider.getTracer('ioredis-test').withSpan(span, async () => { try { - await client.hset('hash', 'random', 'random'); + await client.hset(hashKeyName, 'random', 'random'); assert.strictEqual(memoryExporter.getFinishedSpans().length, 0); span.end(); const endedSpans = memoryExporter.getFinishedSpans();