diff --git a/plugins/node/opentelemetry-instrumentation-mysql2/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-mysql2/src/instrumentation.ts index 7557cf7a01..4ba2c31172 100644 --- a/plugins/node/opentelemetry-instrumentation-mysql2/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-mysql2/src/instrumentation.ts @@ -38,8 +38,6 @@ import { /** @knipignore */ import { PACKAGE_NAME, PACKAGE_VERSION } from './version'; -type formatType = typeof mysqlTypes.format; - export class MySQL2Instrumentation extends InstrumentationBase { static readonly COMMON_ATTRIBUTES = { [SEMATTRS_DB_SYSTEM]: DBSYSTEMVALUES_MYSQL, @@ -63,7 +61,7 @@ export class MySQL2Instrumentation extends InstrumentationBase { const thisPlugin = this; return function query( @@ -109,7 +107,7 @@ export class MySQL2Instrumentation extends InstrumentationBase string, + query: string | QueryOptions, values?: any[] -): string { +): string | undefined { + let statement = ''; if (typeof query === 'string') { - return values ? format(query, values) : query; + if (!values) { + return; + } + statement = query; } else { + if (!query.values && !values) { + return; + } // According to https://github.com/mysqljs/mysql#performing-queries // The values argument will override the values in the option object. - return values || (query as QueryOptions).values - ? format(query.sql, values || (query as QueryOptions).values) - : query.sql; + statement = query.sql; } + return statement; } /** @@ -128,7 +125,7 @@ export function getDbStatement( * * @returns SQL statement without variable arguments or SQL verb */ -export function getSpanName(query: string | Query | QueryOptions): string { +export function getSpanName(query: string | QueryOptions): string { const rawQuery = typeof query === 'object' ? query.sql : query; // Extract the SQL verb const firstSpace = rawQuery?.indexOf(' '); diff --git a/plugins/node/opentelemetry-instrumentation-mysql2/test/mysql.test.ts b/plugins/node/opentelemetry-instrumentation-mysql2/test/mysql.test.ts index a11480418c..659ea31915 100644 --- a/plugins/node/opentelemetry-instrumentation-mysql2/test/mysql.test.ts +++ b/plugins/node/opentelemetry-instrumentation-mysql2/test/mysql.test.ts @@ -67,7 +67,7 @@ describe('mysql2', () => { const memoryExporter = new InMemorySpanExporter(); const getLastQueries = (count: number) => - new Promise(res => { + new Promise((res, reject) => { const queries: string[] = []; const query = rootConnection.query({ sql: "SELECT * FROM mysql.general_log WHERE command_type = 'Query' ORDER BY event_time DESC LIMIT ? OFFSET 1", @@ -176,13 +176,17 @@ describe('mysql2', () => { it('should name the span accordingly ', done => { const span = provider.getTracer('default').startSpan('test span'); context.with(trace.setSpan(context.active(), span), () => { - const sql = 'SELECT 1+1 as solution'; - const query = connection.query(sql); + const sql = 'SELECT 1+? as solution'; + const query = connection.query(sql, [1]); query.on('end', () => { const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); assert.strictEqual(spans[0].name, 'SELECT'); - assert.strictEqual(spans[0].attributes[SEMATTRS_DB_STATEMENT], sql); + assert.strictEqual( + spans[0].attributes[SEMATTRS_DB_STATEMENT], + 'SELECT 1+? as solution' + ); done(); }); }); @@ -195,13 +199,20 @@ describe('mysql2', () => { context.with(trace.setSpan(context.active(), span), () => { const sql = 'SELECT 1+? as solution'; const query = connection.query({ sql, values: [1] }); + let rows = 0; + + query.on('result', (row: mysqlTypes.RowDataPacket) => { + assert.strictEqual(row.solution, 2); + rows += 1; + }); query.on('end', () => { + assert.strictEqual(rows, 1); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans[0].name, 'SELECT'); assert.strictEqual( spans[0].attributes[SEMATTRS_DB_STATEMENT], - query.sql + 'SELECT 1+? as solution' ); done(); }); @@ -213,8 +224,8 @@ describe('mysql2', () => { it('should intercept connection.query(text: string)', done => { const span = provider.getTracer('default').startSpan('test span'); context.with(trace.setSpan(context.active(), span), () => { - const sql = 'SELECT 1+1 as solution'; - const query = connection.query(sql); + const sql = 'SELECT 1+? as solution'; + const query = connection.query(sql, [1]); let rows = 0; query.on('result', (row: mysqlTypes.RowDataPacket) => { @@ -226,7 +237,7 @@ describe('mysql2', () => { assert.strictEqual(rows, 1); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql); + assertSpan(spans[0], sql, true); done(); }); }); @@ -242,7 +253,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 2); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql); + assert.ok(!(SEMATTRS_DB_STATEMENT in spans[0].attributes)); done(); }); }); @@ -260,7 +271,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 2); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); } ); @@ -280,7 +291,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 2); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); } ); @@ -297,7 +308,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 1); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); }); }); @@ -313,7 +324,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 1); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); }); }); @@ -322,12 +333,14 @@ describe('mysql2', () => { it('should attach error messages to spans', done => { const span = provider.getTracer('default').startSpan('test span'); context.with(trace.setSpan(context.active(), span), () => { - const sql = 'SELECT ? as solution'; - connection.query(sql, (err, res) => { + const sql = `SELECT c1 + from t1_${Date.now()} + where c2 = ?`; + connection.query(sql, ['test'], (err, res) => { assert.ok(err); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, undefined, err!.message); + assertSpan(spans[0], sql, true, err!.message); done(); }); }); @@ -339,10 +352,16 @@ describe('mysql2', () => { connection.query('SELECT 1+1 as solution', () => { const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - getLastQueries(1).then(([query]) => { - assert.doesNotMatch(query, /.*traceparent.*/); - done(); - }); + getLastQueries(1) + .then(([query]) => { + console.log('here', query); + assert.doesNotMatch(query, /.*traceparent.*/); + done(); + }) + .catch(e => { + console.error(e); + done(e); + }); }); }); }); @@ -450,7 +469,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 2); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); } ); @@ -470,7 +489,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 2); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); } ); @@ -487,7 +506,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 2); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); }); }); @@ -503,7 +522,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 2); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); }); }); @@ -617,7 +636,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 2); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); } ); @@ -634,7 +653,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 2); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); }); }); @@ -650,7 +669,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 1); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); }); }); @@ -666,7 +685,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 1); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); }); }); @@ -762,7 +781,7 @@ describe('mysql2', () => { assert.strictEqual(row[0].solution, 2); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql); + assertSpan(spans[0], sql, true); done(); }); }); @@ -773,6 +792,9 @@ describe('mysql2', () => { context.with(trace.setSpan(context.active(), span), () => { const sql = 'SELECT 1+1 as solution'; pool.getConnection((err, conn) => { + if (err) { + return done(err); + } const query = conn.execute(sql); let rows = 0; @@ -782,11 +804,15 @@ describe('mysql2', () => { }); query.on('end', () => { - assert.strictEqual(rows, 1); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql); - done(); + try { + assert.strictEqual(rows, 1); + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); + assertSpan(spans[0], sql); + done(); + } catch (e) { + done(e); + } }); }); }); @@ -797,13 +823,17 @@ describe('mysql2', () => { context.with(trace.setSpan(context.active(), span), () => { const sql = 'SELECT 1+1 as solution'; pool.execute(sql, (err, res: mysqlTypes.RowDataPacket[]) => { - assert.ifError(err); - assert.ok(res); - assert.strictEqual(res[0].solution, 2); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql); - done(); + try { + assert.ifError(err); + assert.ok(res); + assert.strictEqual(res[0].solution, 2); + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); + assertSpan(spans[0], sql, true); + done(); + } catch (e) { + done(e); + } }); }); }); @@ -838,7 +868,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 2); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql); + assertSpan(spans[0], sql, true); done(); } ); @@ -855,7 +885,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 2); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); }); }); @@ -871,7 +901,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 2); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); }); }); @@ -887,7 +917,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 2); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); }); }); @@ -898,11 +928,15 @@ describe('mysql2', () => { context.with(trace.setSpan(context.active(), span), () => { const sql = 'SELECT ? as solution'; pool.execute(sql, (err, res) => { - assert.ok(err); - const spans = memoryExporter.getFinishedSpans(); - assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, undefined, err!.message); - done(); + try { + assert.ok(err); + const spans = memoryExporter.getFinishedSpans(); + assert.strictEqual(spans.length, 1); + assertSpan(spans[0], sql, true, err!.message); + done(); + } catch (e) { + done(e); + } }); }); }); @@ -970,7 +1004,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 2); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); } ); @@ -993,7 +1027,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 2); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); } ); @@ -1016,7 +1050,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 1); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); } ); @@ -1039,7 +1073,7 @@ describe('mysql2', () => { assert.strictEqual(res[0].solution, 1); const spans = memoryExporter.getFinishedSpans(); assert.strictEqual(spans.length, 1); - assertSpan(spans[0], sql, [1]); + assertSpan(spans[0], sql, true); done(); } ); @@ -1214,7 +1248,7 @@ describe('mysql2', () => { function assertSpan( span: ReadableSpan, sql: string, - values?: any, + hasAttrDbStatement?: boolean, errorMessage?: string ) { assert.strictEqual(span.attributes[SEMATTRS_DB_SYSTEM], DBSYSTEMVALUES_MYSQL); @@ -1222,10 +1256,12 @@ function assertSpan( assert.strictEqual(span.attributes[SEMATTRS_NET_PEER_PORT], port); assert.strictEqual(span.attributes[SEMATTRS_NET_PEER_NAME], host); assert.strictEqual(span.attributes[SEMATTRS_DB_USER], user); - assert.strictEqual( - span.attributes[SEMATTRS_DB_STATEMENT], - mysqlTypes.format(sql, values) - ); + if (hasAttrDbStatement) { + assert.strictEqual(span.attributes[SEMATTRS_DB_STATEMENT], sql); + } else { + assert.equal(span.attributes[SEMATTRS_DB_STATEMENT], null); + } + if (errorMessage) { assert.strictEqual(span.status.message, errorMessage); assert.strictEqual(span.status.code, SpanStatusCode.ERROR);