fix(mysql): add enhancedDatabaseReporting to mysql (open-telemetry#1337)

* fix(mysql): add DbStatementSerializer tp mysql * fix(mysql): lint * fix(mysql): lint 2 * fix(mysql db.statement): revert to regular function behaviour * fix(mysql db.statement): improve code * fix(mysql db.statement): use enhancedDatabaseReporting and not dbStatementSerializer * fix(mysql db.statement): lint * fix(mysql db.statement): remove format * fix(mysql db.statement): add enhancedDatabaseReporting to README * fix(mysql db.statement): use use template literals * fix(mysql db.statement): add default to enhancedDatabaseReporting * fix(mysql db.statement): remove redundant argument * fix(mysql db.statement): remove redundant argument * fix(mysql db.statement): lint * fix(mysql db.statement): remove format * fix(mysql db.statement): lint * fix(mysql db.statement): lint README * fix(mysql db.statement): fixes * fix(mysql db.statement): lint Co-authored-by: Amir Blum <amirgiraffe@gmail.com>
dynatrace-oss-contrib · Jan 16, 2023 · 04d583b · 04d583b
1 parent 1ee197e
commit 04d583b
Show file tree

Hide file tree

Showing 6 changed files with 176 additions and 62 deletions.
diff --git a/plugins/node/opentelemetry-instrumentation-mysql/README.md b/plugins/node/opentelemetry-instrumentation-mysql/README.md
@@ -42,6 +42,13 @@ registerInstrumentations({
 
 See [examples](https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/plugins/node/opentelemetry-instrumentation-mysql/examples) for a short example.
 
+### MySQL instrumentation Options
+
+| Options | Type | Default | Description |
+| ------- | ---- | --------| ----------- |
+| [`enhancedDatabaseReporting`](./src/types.ts#L24) | `boolean` | `false` | If true, an attribute containing the query's parameters will be attached the spans generated to represent the query |
+|
+
 ## Useful links
 
 - For more information on OpenTelemetry, visit: <https://opentelemetry.io/>

diff --git a/plugins/node/opentelemetry-instrumentation-mysql/src/AttributeNames.ts b/plugins/node/opentelemetry-instrumentation-mysql/src/AttributeNames.ts
@@ -0,0 +1,19 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+// Mysql specific attributes not covered by semantic conventions
+export enum AttributeNames {
+  MYSQL_VALUES = 'db.mysql.values',
+}
diff --git a/plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-mysql/src/instrumentation.ts
@@ -32,19 +32,19 @@ import {
   DbSystemValues,
   SemanticAttributes,
 } from '@opentelemetry/semantic-conventions';
-import * as mysqlTypes from 'mysql';
+import type * as mysqlTypes from 'mysql';
+import { AttributeNames } from './AttributeNames';
 import { MySQLInstrumentationConfig } from './types';
 import {
   getConnectionAttributes,
   getDbStatement,
+  getDbValues,
   getSpanName,
   getPoolName,
 } from './utils';
 import { VERSION } from './version';
 import { UpDownCounter, MeterProvider } from '@opentelemetry/api';
 
-type formatType = typeof mysqlTypes.format;
-
 type getConnectionCallbackType = (
   err: mysqlTypes.MysqlError,
   connection: mysqlTypes.PoolConnection
@@ -94,7 +94,7 @@ export class MySQLInstrumentation extends InstrumentationBase<
           this._wrap(
             moduleExports,
             'createConnection',
-            this._patchCreateConnection(moduleExports.format) as any
+            this._patchCreateConnection() as any
           );
 
           diag.debug('Patching mysql.createPool');
@@ -104,7 +104,7 @@ export class MySQLInstrumentation extends InstrumentationBase<
           this._wrap(
             moduleExports,
             'createPool',
-            this._patchCreatePool(moduleExports.format) as any
+            this._patchCreatePool() as any
           );
 
           diag.debug('Patching mysql.createPoolCluster');
@@ -114,7 +114,7 @@ export class MySQLInstrumentation extends InstrumentationBase<
           this._wrap(
             moduleExports,
             'createPoolCluster',
-            this._patchCreatePoolCluster(moduleExports.format) as any
+            this._patchCreatePoolCluster() as any
           );
 
           return moduleExports;
@@ -130,7 +130,7 @@ export class MySQLInstrumentation extends InstrumentationBase<
   }
 
   // global export function
-  private _patchCreateConnection(format: formatType) {
+  private _patchCreateConnection() {
     return (originalCreateConnection: Function) => {
       const thisPlugin = this;
       diag.debug('MySQLInstrumentation#patch: patched mysql createConnection');
@@ -144,7 +144,7 @@ export class MySQLInstrumentation extends InstrumentationBase<
         thisPlugin._wrap(
           originalResult,
           'query',
-          thisPlugin._patchQuery(originalResult, format) as any
+          thisPlugin._patchQuery(originalResult) as any
         );
 
         return originalResult;
@@ -153,18 +153,18 @@ export class MySQLInstrumentation extends InstrumentationBase<
   }
 
   // global export function
-  private _patchCreatePool(format: formatType) {
+  private _patchCreatePool() {
     return (originalCreatePool: Function) => {
       const thisPlugin = this;
       diag.debug('MySQLInstrumentation#patch: patched mysql createPool');
       return function createPool(_config: string | mysqlTypes.PoolConfig) {
         const pool = originalCreatePool(...arguments);
 
-        thisPlugin._wrap(pool, 'query', thisPlugin._patchQuery(pool, format));
+        thisPlugin._wrap(pool, 'query', thisPlugin._patchQuery(pool));
         thisPlugin._wrap(
           pool,
           'getConnection',
-          thisPlugin._patchGetConnection(pool, format)
+          thisPlugin._patchGetConnection(pool)
         );
         thisPlugin._wrap(pool, 'end', thisPlugin._patchPoolEnd(pool));
         thisPlugin._setPoolcallbacks(pool, thisPlugin, '');
@@ -196,7 +196,7 @@ export class MySQLInstrumentation extends InstrumentationBase<
   }
 
   // global export function
-  private _patchCreatePoolCluster(format: formatType) {
+  private _patchCreatePoolCluster() {
     return (originalCreatePoolCluster: Function) => {
       const thisPlugin = this;
       diag.debug('MySQLInstrumentation#patch: patched mysql createPoolCluster');
@@ -207,15 +207,15 @@ export class MySQLInstrumentation extends InstrumentationBase<
         thisPlugin._wrap(
           cluster,
           'getConnection',
-          thisPlugin._patchGetConnection(cluster, format)
+          thisPlugin._patchGetConnection(cluster)
         );
-        thisPlugin._wrap(cluster, 'add', thisPlugin._patchAdd(cluster, format));
+        thisPlugin._wrap(cluster, 'add', thisPlugin._patchAdd(cluster));
 
         return cluster;
       };
     };
   }
-  private _patchAdd(cluster: mysqlTypes.PoolCluster, format: formatType) {
+  private _patchAdd(cluster: mysqlTypes.PoolCluster) {
     return (originalAdd: Function) => {
       const thisPlugin = this;
       diag.debug('MySQLInstrumentation#patch: patched mysql pool cluster add');
@@ -241,10 +241,7 @@ export class MySQLInstrumentation extends InstrumentationBase<
   }
 
   // method on cluster or pool
-  private _patchGetConnection(
-    pool: mysqlTypes.Pool | mysqlTypes.PoolCluster,
-    format: formatType
-  ) {
+  private _patchGetConnection(pool: mysqlTypes.Pool | mysqlTypes.PoolCluster) {
     return (originalGetConnection: Function) => {
       const thisPlugin = this;
       diag.debug(
@@ -264,22 +261,19 @@ export class MySQLInstrumentation extends InstrumentationBase<
 
         if (arguments.length === 1 && typeof arg1 === 'function') {
           const patchFn = thisPlugin._getConnectionCallbackPatchFn(
-            arg1 as getConnectionCallbackType,
-            format
+            arg1 as getConnectionCallbackType
           );
           return originalGetConnection.call(pool, patchFn);
         }
         if (arguments.length === 2 && typeof arg2 === 'function') {
           const patchFn = thisPlugin._getConnectionCallbackPatchFn(
-            arg2 as getConnectionCallbackType,
-            format
+            arg2 as getConnectionCallbackType
           );
           return originalGetConnection.call(pool, arg1, patchFn);
         }
         if (arguments.length === 3 && typeof arg3 === 'function') {
           const patchFn = thisPlugin._getConnectionCallbackPatchFn(
-            arg3 as getConnectionCallbackType,
-            format
+            arg3 as getConnectionCallbackType
           );
           return originalGetConnection.call(pool, arg1, arg2, patchFn);
         }
@@ -289,10 +283,7 @@ export class MySQLInstrumentation extends InstrumentationBase<
     };
   }
 
-  private _getConnectionCallbackPatchFn(
-    cb: getConnectionCallbackType,
-    format: formatType
-  ) {
+  private _getConnectionCallbackPatchFn(cb: getConnectionCallbackType) {
     const thisPlugin = this;
     const activeContext = context.active();
     return function (
@@ -307,7 +298,7 @@ export class MySQLInstrumentation extends InstrumentationBase<
           thisPlugin._wrap(
             connection,
             'query',
-            thisPlugin._patchQuery(connection, format)
+            thisPlugin._patchQuery(connection)
           );
         }
       }
@@ -317,10 +308,7 @@ export class MySQLInstrumentation extends InstrumentationBase<
     };
   }
 
-  private _patchQuery(
-    connection: mysqlTypes.Connection | mysqlTypes.Pool,
-    format: formatType
-  ) {
+  private _patchQuery(connection: mysqlTypes.Connection | mysqlTypes.Pool) {
     return (originalQuery: Function): mysqlTypes.QueryFunction => {
       const thisPlugin = this;
       diag.debug('MySQLInstrumentation: patched mysql query');
@@ -343,19 +331,29 @@ export class MySQLInstrumentation extends InstrumentationBase<
           },
         });
 
-        let values;
-
-        if (Array.isArray(_valuesOrCallback)) {
-          values = _valuesOrCallback;
-        } else if (arguments[2]) {
-          values = [_valuesOrCallback];
-        }
-
         span.setAttribute(
           SemanticAttributes.DB_STATEMENT,
-          getDbStatement(query, format, values)
+          getDbStatement(query)
         );
 
+        const instrumentationConfig: MySQLInstrumentationConfig =
+          thisPlugin.getConfig();
+
+        if (instrumentationConfig.enhancedDatabaseReporting) {
+          let values;
+
+          if (Array.isArray(_valuesOrCallback)) {
+            values = _valuesOrCallback;
+          } else if (arguments[2]) {
+            values = [_valuesOrCallback];
+          }
+
+          span.setAttribute(
+            AttributeNames.MYSQL_VALUES,
+            getDbValues(query, values)
+          );
+        }
+
         const cbIndex = Array.from(arguments).findIndex(
           arg => typeof arg === 'function'
         );

diff --git a/plugins/node/opentelemetry-instrumentation-mysql/src/types.ts b/plugins/node/opentelemetry-instrumentation-mysql/src/types.ts
@@ -16,4 +16,10 @@
 
 import { InstrumentationConfig } from '@opentelemetry/instrumentation';
 
-export type MySQLInstrumentationConfig = InstrumentationConfig;
+export interface MySQLInstrumentationConfig extends InstrumentationConfig {
+  /**
+   * If true, an attribute containing the query's parameters will be attached
+   * the spans generated to represent the query.
+   */
+  enhancedDatabaseReporting?: boolean;
+}
diff --git a/plugins/node/opentelemetry-instrumentation-mysql/src/utils.ts b/plugins/node/opentelemetry-instrumentation-mysql/src/utils.ts
@@ -72,28 +72,26 @@ function getJDBCString(
 }
 
 /**
- * Conjures up the value for the db.statement attribute by formatting a SQL query.
- *
  * @returns the database statement being executed.
  */
-export function getDbStatement(
+export function getDbStatement(query: string | Query | QueryOptions): string {
+  if (typeof query === 'string') {
+    return query;
+  } else {
+    return query.sql;
+  }
+}
+
+export function getDbValues(
   query: string | Query | QueryOptions,
-  format: (
-    sql: string,
-    values: any[],
-    stringifyObjects?: boolean,
-    timeZone?: string
-  ) => string,
   values?: any[]
 ): string {
   if (typeof query === 'string') {
-    return values ? format(query, values) : query;
+    return arrayStringifyHelper(values);
   } else {
     // According to https://github.com/mysqljs/mysql#performing-queries
     // The values argument will override the values in the option object.
-    return values || query.values
-      ? format(query.sql, values || query.values)
-      : query.sql;
+    return arrayStringifyHelper(values || query.values);
   }
 }
 
@@ -110,6 +108,11 @@ export function getSpanName(query: string | Query | QueryOptions): string {
   return query.split(' ')[0];
 }
 
+export function arrayStringifyHelper(arr: Array<unknown> | undefined): string {
+  if (arr) return `[${arr.toString()}]`;
+  return '';
+}
+
 export function getPoolName(pool: mysqlTypes.Pool): string {
   const c = pool.config.connectionConfig;
   let poolName = '';