From 25b93b1585ef7d1540814f379fb4d1296a4f60a0 Mon Sep 17 00:00:00 2001 From: Simon Stone Date: Mon, 5 Jul 2021 16:25:56 +0100 Subject: [PATCH 1/3] fix(mysql): bind get connection callback to active context Signed-off-by: Simon Stone --- .../src/instrumentation.ts | 13 ++++- .../test/index.test.ts | 48 +++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts index 82ea9770d4..5bcd9b07db 100644 --- a/plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts @@ -14,7 +14,13 @@ * limitations under the License. */ -import { diag, Span, SpanKind, SpanStatusCode } from '@opentelemetry/api'; +import { + context, + diag, + Span, + SpanKind, + SpanStatusCode, +} from '@opentelemetry/api'; import { InstrumentationBase, InstrumentationNodeModuleDefinition, @@ -203,6 +209,7 @@ export class MySQLInstrumentation extends InstrumentationBase< private _getConnectionCallbackPatchFn(cb: Function, format: formatType) { const thisPlugin = this; + const activeContext = context.active(); return function () { if (arguments[1]) { // this is the callback passed into a query @@ -216,7 +223,9 @@ export class MySQLInstrumentation extends InstrumentationBase< } } if (typeof cb === 'function') { - cb(...arguments); + context.with(activeContext, () => { + cb(...arguments); + }); } }; } diff --git a/plugins/node/opentelemetry-instrumentation-mysql/test/index.test.ts b/plugins/node/opentelemetry-instrumentation-mysql/test/index.test.ts index fbf726281b..afd468f17c 100644 --- a/plugins/node/opentelemetry-instrumentation-mysql/test/index.test.ts +++ b/plugins/node/opentelemetry-instrumentation-mysql/test/index.test.ts @@ -426,6 +426,30 @@ describe('mysql@2.x', () => { }); }); }); + + it('should propagate active context to callback', done => { + const parentSpan = provider.getTracer('default').startSpan('test span'); + context.with(trace.setSpan(context.active(), parentSpan), () => { + pool.getConnection((err, connection) => { + assert.ifError(err); + assert.ok(connection); + const sql = 'SELECT ? as solution'; + connection.query(sql, 1, (err, res) => { + assert.ifError(err); + assert.ok(res); + assert.strictEqual(res[0].solution, 1); + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); + const querySpan = spans[0]; + assert.strictEqual( + querySpan.parentSpanId, + parentSpan.spanContext().spanId + ); + done(); + }); + }); + }); + }); }); describe('#PoolCluster', () => { @@ -600,6 +624,30 @@ describe('mysql@2.x', () => { } ); }); + + it('should propagate active context to callback', done => { + const parentSpan = provider.getTracer('default').startSpan('test span'); + context.with(trace.setSpan(context.active(), parentSpan), () => { + poolCluster.getConnection((err, connection) => { + assert.ifError(err); + assert.ok(connection); + const sql = 'SELECT ? as solution'; + connection.query(sql, 1, (err, res) => { + assert.ifError(err); + assert.ok(res); + assert.strictEqual(res[0].solution, 1); + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); + const querySpan = spans[0]; + assert.strictEqual( + querySpan.parentSpanId, + parentSpan.spanContext().spanId + ); + done(); + }); + }); + }); + }); }); }); From 5f192b8e5f0d03e96ad8814612137e6adcd5bb3e Mon Sep 17 00:00:00 2001 From: Simon Stone Date: Wed, 11 Aug 2021 09:21:08 +0100 Subject: [PATCH 2/3] fix(mysql): address review comments from @flarna, @blumamir Signed-off-by: Simon Stone --- .../src/instrumentation.ts | 24 +++++++++---------- .../test/index.test.ts | 18 ++++---------- 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts index 5bcd9b07db..0e8dc4f946 100644 --- a/plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts @@ -34,6 +34,8 @@ import { VERSION } from './version'; type formatType = typeof mysqlTypes.format; +type getConnectionCallbackType = (err: mysqlTypes.MysqlError, connection: mysqlTypes.PoolConnection) => void; + export class MySQLInstrumentation extends InstrumentationBase< typeof mysqlTypes > { @@ -182,21 +184,21 @@ export class MySQLInstrumentation extends InstrumentationBase< if (arguments.length === 1 && typeof arg1 === 'function') { const patchFn = thisPlugin._getConnectionCallbackPatchFn( - arg1, + arg1 as getConnectionCallbackType, format ); return originalGetConnection.call(pool, patchFn); } if (arguments.length === 2 && typeof arg2 === 'function') { const patchFn = thisPlugin._getConnectionCallbackPatchFn( - arg2, + arg2 as getConnectionCallbackType, format ); return originalGetConnection.call(pool, arg1, patchFn); } if (arguments.length === 3 && typeof arg3 === 'function') { const patchFn = thisPlugin._getConnectionCallbackPatchFn( - arg3, + arg3 as getConnectionCallbackType, format ); return originalGetConnection.call(pool, arg1, arg2, patchFn); @@ -207,25 +209,23 @@ export class MySQLInstrumentation extends InstrumentationBase< }; } - private _getConnectionCallbackPatchFn(cb: Function, format: formatType) { + private _getConnectionCallbackPatchFn(cb: getConnectionCallbackType, format: formatType) { const thisPlugin = this; const activeContext = context.active(); - return function () { - if (arguments[1]) { + return function (this: any, err: mysqlTypes.MysqlError, connection: mysqlTypes.PoolConnection) { + if (connection) { // this is the callback passed into a query // no need to unwrap - if (!isWrapped(arguments[1].query)) { + if (!isWrapped(connection.query)) { thisPlugin._wrap( - arguments[1], + connection, 'query', - thisPlugin._patchQuery(arguments[1], format) + thisPlugin._patchQuery(connection, format) ); } } if (typeof cb === 'function') { - context.with(activeContext, () => { - cb(...arguments); - }); + context.with(activeContext, cb, this, err, connection); } }; } diff --git a/plugins/node/opentelemetry-instrumentation-mysql/test/index.test.ts b/plugins/node/opentelemetry-instrumentation-mysql/test/index.test.ts index afd468f17c..060b56e0cf 100644 --- a/plugins/node/opentelemetry-instrumentation-mysql/test/index.test.ts +++ b/plugins/node/opentelemetry-instrumentation-mysql/test/index.test.ts @@ -438,13 +438,8 @@ describe('mysql@2.x', () => { assert.ifError(err); assert.ok(res); assert.strictEqual(res[0].solution, 1); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(spans.length, 1); - const querySpan = spans[0]; - assert.strictEqual( - querySpan.parentSpanId, - parentSpan.spanContext().spanId - ); + const actualSpan = trace.getSpan(context.active()); + assert.strictEqual(actualSpan, parentSpan); done(); }); }); @@ -636,13 +631,8 @@ describe('mysql@2.x', () => { assert.ifError(err); assert.ok(res); assert.strictEqual(res[0].solution, 1); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(spans.length, 1); - const querySpan = spans[0]; - assert.strictEqual( - querySpan.parentSpanId, - parentSpan.spanContext().spanId - ); + const actualSpan = trace.getSpan(context.active()); + assert.strictEqual(actualSpan, parentSpan); done(); }); }); From 0c7338153494a666ac703efb00460b52b2914eda Mon Sep 17 00:00:00 2001 From: Simon Stone Date: Wed, 11 Aug 2021 10:09:53 +0100 Subject: [PATCH 3/3] fix(mysql): fix linting errors Signed-off-by: Simon Stone --- .../src/instrumentation.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts index 0e8dc4f946..8a0652d503 100644 --- a/plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts @@ -34,7 +34,10 @@ import { VERSION } from './version'; type formatType = typeof mysqlTypes.format; -type getConnectionCallbackType = (err: mysqlTypes.MysqlError, connection: mysqlTypes.PoolConnection) => void; +type getConnectionCallbackType = ( + err: mysqlTypes.MysqlError, + connection: mysqlTypes.PoolConnection +) => void; export class MySQLInstrumentation extends InstrumentationBase< typeof mysqlTypes @@ -209,10 +212,17 @@ export class MySQLInstrumentation extends InstrumentationBase< }; } - private _getConnectionCallbackPatchFn(cb: getConnectionCallbackType, format: formatType) { + private _getConnectionCallbackPatchFn( + cb: getConnectionCallbackType, + format: formatType + ) { const thisPlugin = this; const activeContext = context.active(); - return function (this: any, err: mysqlTypes.MysqlError, connection: mysqlTypes.PoolConnection) { + return function ( + this: any, + err: mysqlTypes.MysqlError, + connection: mysqlTypes.PoolConnection + ) { if (connection) { // this is the callback passed into a query // no need to unwrap