fix(instrumentation-mysql2)!: do not include parameterized values to …

…db.statement span attribute (#1758)
open-telemetry · Dec 7, 2024 · ee6f9b6 · ee6f9b6
1 parent 6158af0
commit ee6f9b6
Show file tree

Hide file tree

Showing 3 changed files with 184 additions and 105 deletions.
diff --git a/plugins/node/opentelemetry-instrumentation-mysql2/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-mysql2/src/instrumentation.ts
@@ -39,8 +39,6 @@ import {
 /** @knipignore */
 import { PACKAGE_NAME, PACKAGE_VERSION } from './version';
 
-type formatType = typeof mysqlTypes.format;
-
 export class MySQL2Instrumentation extends InstrumentationBase<MySQL2InstrumentationConfig> {
   static readonly COMMON_ATTRIBUTES = {
     [SEMATTRS_DB_SYSTEM]: DBSYSTEMVALUES_MYSQL,
@@ -64,7 +62,7 @@ export class MySQL2Instrumentation extends InstrumentationBase<MySQL2Instrumenta
           this._wrap(
             ConnectionPrototype,
             'query',
-            this._patchQuery(moduleExports.format, false) as any
+            this._patchQuery(false) as any
           );
 
           if (isWrapped(ConnectionPrototype.execute)) {
@@ -73,7 +71,7 @@ export class MySQL2Instrumentation extends InstrumentationBase<MySQL2Instrumenta
           this._wrap(
             ConnectionPrototype,
             'execute',
-            this._patchQuery(moduleExports.format, true) as any
+            this._patchQuery(true) as any
           );
 
           return moduleExports;
@@ -82,14 +80,18 @@ export class MySQL2Instrumentation extends InstrumentationBase<MySQL2Instrumenta
           if (moduleExports === undefined) return;
           const ConnectionPrototype: mysqlTypes.Connection =
             moduleExports.Connection.prototype;
-          this._unwrap(ConnectionPrototype, 'query');
-          this._unwrap(ConnectionPrototype, 'execute');
+          if (isWrapped(ConnectionPrototype.query)) {
+            this._unwrap(ConnectionPrototype, 'query');
+          }
+          if (isWrapped(ConnectionPrototype.execute)) {
+            this._unwrap(ConnectionPrototype, 'execute');
+          }
         }
       ),
     ];
   }
 
-  private _patchQuery(format: formatType, isPrepared: boolean) {
+  private _patchQuery(isPrepared: boolean) {
     return (originalQuery: Function): Function => {
       const thisPlugin = this;
       return function query(
@@ -110,7 +112,7 @@ export class MySQL2Instrumentation extends InstrumentationBase<MySQL2Instrumenta
           attributes: {
             ...MySQL2Instrumentation.COMMON_ATTRIBUTES,
             ...getConnectionAttributes(this.config),
-            [SEMATTRS_DB_STATEMENT]: getDbStatement(query, format, values),
+            [SEMATTRS_DB_STATEMENT]: getDbStatement(query, values),
           },
         });
 
@@ -150,11 +152,10 @@ export class MySQL2Instrumentation extends InstrumentationBase<MySQL2Instrumenta
               );
             }
           }
-
           span.end();
         });
 
-        if (arguments.length === 1) {
+        if (typeof arguments[arguments.length - 1] !== 'function') {
           if (typeof (query as any).onResult === 'function') {
             thisPlugin._wrap(
               query as any,
@@ -178,22 +179,13 @@ export class MySQL2Instrumentation extends InstrumentationBase<MySQL2Instrumenta
             });
 
           return streamableQuery;
-        }
-
-        if (typeof arguments[1] === 'function') {
+        } else {
           thisPlugin._wrap(
             arguments,
-            1,
-            thisPlugin._patchCallbackQuery(endSpan)
-          );
-        } else if (typeof arguments[2] === 'function') {
-          thisPlugin._wrap(
-            arguments,
-            2,
+            arguments.length - 1,
             thisPlugin._patchCallbackQuery(endSpan)
           );
         }
-
         return originalQuery.apply(this, arguments);
       };
     };

diff --git a/plugins/node/opentelemetry-instrumentation-mysql2/src/utils.ts b/plugins/node/opentelemetry-instrumentation-mysql2/src/utils.ts
@@ -36,17 +36,14 @@ interface QueryOptions {
   values?: any | any[] | { [param: string]: any };
 }
 
-interface Query {
-  sql: string;
-}
-
 interface Config {
   host?: string;
   port?: number;
   database?: string;
   user?: string;
   connectionConfig?: Config;
 }
+
 /**
  * Get an Attributes map from a mysql connection config object
  *
@@ -97,29 +94,28 @@ function getJDBCString(
 }
 
 /**
- * Conjures up the value for the db.statement attribute by formatting a SQL query.
- *
- * @returns the database statement being executed.
+ * Returns a string representing the SQL query that is appropriate to return
+ * in telemetry. If there are no `values` then, to be cautious, it is assumed
+ * the query SQL may include sensitive data and `undefined` is returned, so
+ * that no DB statement is included in telemetry.
  */
 export function getDbStatement(
-  query: string | Query | QueryOptions,
-  format: (
-    sql: string,
-    values: any[],
-    stringifyObjects?: boolean,
-    timeZone?: string
-  ) => 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 {
-    // 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;
+    if (!query.values && !values) {
+      return;
+    }
+    statement = query.sql;
   }
+  return statement;
 }
 
 /**
@@ -128,7 +124,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(' ');