fix(node): Improve mysql integration (#8923)

1. Make sure it works consistently when not calling
`connection.connect()` (which is optional) - it used to only register
the mysql config on the second call, as it calls `connect` under the
hood but only after we already tried to fetch it again.
2. Make sure it works without a callback.

Author: mydea

packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/scenario.ts renamed to packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withConnect/scenario.ts

@@ -0,0 +1,35 @@
+import { assertSentryTransaction, TestEnv } from '../../../../../utils';
+
+test('should auto-instrument `mysql` package when using connection.connect()', async () => {
+  const env = await TestEnv.init(__dirname);
+  const envelope = await env.getEnvelopeRequest({ envelopeType: 'transaction' });
+
+  expect(envelope).toHaveLength(3);
+
+  assertSentryTransaction(envelope[2], {
+    transaction: 'Test Transaction',
+    spans: [
+      {
+        description: 'SELECT 1 + 1 AS solution',
+        op: 'db',
+        data: {
+          'db.system': 'mysql',
+          'server.address': 'localhost',
+          'server.port': 3306,
+          'db.user': 'root',
+        },
+      },
+
+      {
+        description: 'SELECT NOW()',
+        op: 'db',
+        data: {
+          'db.system': 'mysql',
+          'server.address': 'localhost',
+          'server.port': 3306,
+          'db.user': 'root',
+        },
+      },
+    ],
+  });
+});

packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withConnect/test.ts

@@ -0,0 +1,31 @@
+import * as Sentry from '@sentry/node';
+import mysql from 'mysql';
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  tracesSampleRate: 1.0,
+  integrations: [...Sentry.autoDiscoverNodePerformanceMonitoringIntegrations()],
+});
+
+const connection = mysql.createConnection({
+  user: 'root',
+  password: 'docker',
+});
+
+const transaction = Sentry.startTransaction({
+  op: 'transaction',
+  name: 'Test Transaction',
+});
+
+Sentry.configureScope(scope => {
+  scope.setSpan(transaction);
+});
+
+connection.query('SELECT 1 + 1 AS solution');
+connection.query('SELECT NOW()', ['1', '2']);
+
+// Wait a bit to ensure the queries completed
+setTimeout(() => {
+  transaction.finish();
+}, 500);

packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback/scenario.ts

@@ -0,0 +1,35 @@
+import { assertSentryTransaction, TestEnv } from '../../../../../utils';
+
+test('should auto-instrument `mysql` package when using query without callback', async () => {
+  const env = await TestEnv.init(__dirname);
+  const envelope = await env.getEnvelopeRequest({ envelopeType: 'transaction' });
+
+  expect(envelope).toHaveLength(3);
+
+  assertSentryTransaction(envelope[2], {
+    transaction: 'Test Transaction',
+    spans: [
+      {
+        description: 'SELECT 1 + 1 AS solution',
+        op: 'db',
+        data: {
+          'db.system': 'mysql',
+          'server.address': 'localhost',
+          'server.port': 3306,
+          'db.user': 'root',
+        },
+      },
+
+      {
+        description: 'SELECT NOW()',
+        op: 'db',
+        data: {
+          'db.system': 'mysql',
+          'server.address': 'localhost',
+          'server.port': 3306,
+          'db.user': 'root',
+        },
+      },
+    ],
+  });
+});

packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutCallback/test.ts

@@ -0,0 +1,30 @@
+import * as Sentry from '@sentry/node';
+import mysql from 'mysql';
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  release: '1.0',
+  tracesSampleRate: 1.0,
+  integrations: [...Sentry.autoDiscoverNodePerformanceMonitoringIntegrations()],
+});
+
+const connection = mysql.createConnection({
+  user: 'root',
+  password: 'docker',
+});
+
+const transaction = Sentry.startTransaction({
+  op: 'transaction',
+  name: 'Test Transaction',
+});
+
+Sentry.configureScope(scope => {
+  scope.setSpan(transaction);
+});
+
+connection.query('SELECT 1 + 1 AS solution', function () {
+  connection.query('SELECT NOW()', ['1', '2'], () => {
+    if (transaction) transaction.finish();
+    connection.end();
+  });
+});

packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutConnect/scenario.ts

@@ -1,6 +1,6 @@
-import { assertSentryTransaction, TestEnv } from '../../../../utils';
+import { assertSentryTransaction, TestEnv } from '../../../../../utils';
 
-test('should auto-instrument `mysql` package.', async () => {
+test('should auto-instrument `mysql` package without connection.connect()', async () => {
   const env = await TestEnv.init(__dirname);
   const envelope = await env.getEnvelopeRequest({ envelopeType: 'transaction' });
 
@@ -14,6 +14,9 @@ test('should auto-instrument `mysql` package.', async () => {
         op: 'db',
         data: {
           'db.system': 'mysql',
+          'server.address': 'localhost',
+          'server.port': 3306,
+          'db.user': 'root',
         },
       },
 
@@ -22,6 +25,9 @@ test('should auto-instrument `mysql` package.', async () => {
         op: 'db',
         data: {
           'db.system': 'mysql',
+          'server.address': 'localhost',
+          'server.port': 3306,
+          'db.user': 'root',
         },
       },
     ],

packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/test.ts renamed to packages/node-integration-tests/suites/tracing-new/auto-instrument/mysql/withoutConnect/test.ts

@@ -1,5 +1,5 @@
 import type { Hub } from '@sentry/core';
-import type { EventProcessor } from '@sentry/types';
+import type { EventProcessor, Span } from '@sentry/types';
 import { fill, loadModule, logger } from '@sentry/utils';
 
 import type { LazyLoadedIntegration } from './lazy';
@@ -83,6 +83,19 @@ export class Mysql implements LazyLoadedIntegration<MysqlConnection> {
       };
     }
 
+    function finishSpan(span: Span | undefined): void {
+      if (!span) {
+        return;
+      }
+
+      const data = spanDataFromConfig();
+      Object.keys(data).forEach(key => {
+        span.setData(key, data[key]);
+      });
+
+      span.finish();
+    }
+
     // The original function will have one of these signatures:
     //    function (callback) => void
     //    function (options, callback) => void
@@ -91,31 +104,33 @@ export class Mysql implements LazyLoadedIntegration<MysqlConnection> {
       return function (this: unknown, options: unknown, values: unknown, callback: unknown) {
         const scope = getCurrentHub().getScope();
         const parentSpan = scope?.getSpan();
+
         const span = parentSpan?.startChild({
           description: typeof options === 'string' ? options : (options as { sql: string }).sql,
           op: 'db',
           origin: 'auto.db.mysql',
           data: {
-            ...spanDataFromConfig(),
             'db.system': 'mysql',
           },
         });
 
         if (typeof callback === 'function') {
           return orig.call(this, options, values, function (err: Error, result: unknown, fields: unknown) {
-            span?.finish();
+            finishSpan(span);
             callback(err, result, fields);
           });
         }
 
         if (typeof values === 'function') {
           return orig.call(this, options, function (err: Error, result: unknown, fields: unknown) {
-            span?.finish();
+            finishSpan(span);
             values(err, result, fields);
           });
         }
 
-        return orig.call(this, options, values, callback);
+        return orig.call(this, options, values, function () {
+          finishSpan(span);
+        });
       };
     });
   }