feat(opentelemetry-plugin-pg): omit pg.values by default (#174)

* refactor(opentelemetry-plugin-pg): fix eslint warnings * refactor(opentelemetry-plugin-pg): do not intersect types all the time * refactor(opentelemetry-plugin-pg): rename interface to reduce confusion * refactor(opentelemetry-plugin-pg): clarify method signatures * feat(opentelemetry-plugin-pg): do not track values by default
open-telemetry · Aug 18, 2020 · 6af1022 · 6af1022
1 parent 711f2ce
commit 6af1022
Show file tree

Hide file tree

Showing 6 changed files with 215 additions and 50 deletions.
diff --git a/plugins/node/opentelemetry-plugin-pg/README.md b/plugins/node/opentelemetry-plugin-pg/README.md
@@ -65,6 +65,14 @@ const provider = new NodeTracerProvider({
 
 See [examples/postgres](https://github.com/open-telemetry/opentelemetry-js/tree/master/examples/postgres) for a short example.
 
+### PostgreSQL Plugin Options
+
+PostgreSQL plugin has few options available to choose from. You can set the following:
+
+| Options | Type | Description |
+| ------- | ---- | ----------- |
+| [`enhancedDatabaseReporting`](https://github.com/open-telemetry/opentelemetry-js/blob/master/packages/opentelemetry-api/src/trace/instrumentation/Plugin.ts#L90) | `boolean` | If true, additional information about query parameters and results will be attached (as `attributes`) to spans representing database operations |
+
 ## Supported Versions
 
 - [pg](https://npmjs.com/package/pg): `7.x`

diff --git a/plugins/node/opentelemetry-plugin-pg/src/pg.ts b/plugins/node/opentelemetry-plugin-pg/src/pg.ts
@@ -20,15 +20,13 @@ import * as pgTypes from 'pg';
 import * as shimmer from 'shimmer';
 import {
   PgClientExtended,
-  PgPluginQueryConfig,
+  NormalizedQueryConfig,
   PostgresCallback,
 } from './types';
 import * as utils from './utils';
 import { VERSION } from './version';
 
 export class PostgresPlugin extends BasePlugin<typeof pgTypes> {
-  protected _config: {};
-
   static readonly COMPONENT = 'pg';
   static readonly DB_TYPE = 'sql';
 
@@ -62,25 +60,32 @@ export class PostgresPlugin extends BasePlugin<typeof pgTypes> {
       plugin._logger.debug(
         `Patching ${PostgresPlugin.COMPONENT}.Client.prototype.query`
       );
-      return function query(
-        this: pgTypes.Client & PgClientExtended,
-        ...args: unknown[]
-      ) {
+      return function query(this: PgClientExtended, ...args: unknown[]) {
         let span: Span;
 
         // Handle different client.query(...) signatures
         if (typeof args[0] === 'string') {
+          const query = args[0];
           if (args.length > 1 && args[1] instanceof Array) {
+            const params = args[1];
             span = utils.handleParameterizedQuery.call(
               this,
               plugin._tracer,
-              ...args
+              plugin._config,
+              query,
+              params
             );
           } else {
-            span = utils.handleTextQuery.call(this, plugin._tracer, ...args);
+            span = utils.handleTextQuery.call(this, plugin._tracer, query);
           }
         } else if (typeof args[0] === 'object') {
-          span = utils.handleConfigQuery.call(this, plugin._tracer, ...args);
+          const queryConfig = args[0] as NormalizedQueryConfig;
+          span = utils.handleConfigQuery.call(
+            this,
+            plugin._tracer,
+            plugin._config,
+            queryConfig
+          );
         } else {
           return utils.handleInvalidQuery.call(
             this,
@@ -106,12 +111,12 @@ export class PostgresPlugin extends BasePlugin<typeof pgTypes> {
               );
             }
           } else if (
-            typeof (args[0] as PgPluginQueryConfig).callback === 'function'
+            typeof (args[0] as NormalizedQueryConfig).callback === 'function'
           ) {
             // Patch ConfigQuery callback
             let callback = utils.patchCallback(
               span,
-              (args[0] as PgPluginQueryConfig).callback!
+              (args[0] as NormalizedQueryConfig).callback!
             );
             // If a parent span existed, bind the callback
             if (parentSpan) {
@@ -132,7 +137,7 @@ export class PostgresPlugin extends BasePlugin<typeof pgTypes> {
           return result
             .then((result: unknown) => {
               // Return a pass-along promise which ends the span and then goes to user's orig resolvers
-              return new Promise((resolve, _) => {
+              return new Promise(resolve => {
                 span.setStatus({ code: CanonicalCode.OK });
                 span.end();
                 resolve(result);

diff --git a/plugins/node/opentelemetry-plugin-pg/src/types.ts b/plugins/node/opentelemetry-plugin-pg/src/types.ts
@@ -27,10 +27,12 @@ export interface PgClientConnectionParams {
   user: string;
 }
 
-export interface PgClientExtended {
+export interface PgClientExtended extends pgTypes.Client {
   connectionParameters: PgClientConnectionParams;
 }
 
-export interface PgPluginQueryConfig extends pgTypes.QueryConfig {
+// Interface name based on original driver implementation
+// https://github.com/brianc/node-postgres/blob/2ef55503738eb2cbb6326744381a92c0bc0439ab/packages/pg/lib/utils.js#L157
+export interface NormalizedQueryConfig extends pgTypes.QueryConfig {
   callback?: PostgresCallback;
 }
diff --git a/plugins/node/opentelemetry-plugin-pg/src/utils.ts b/plugins/node/opentelemetry-plugin-pg/src/utils.ts
@@ -14,11 +14,17 @@
  * limitations under the License.
  */
 
-import { Span, CanonicalCode, Tracer, SpanKind } from '@opentelemetry/api';
+import {
+  Span,
+  CanonicalCode,
+  Tracer,
+  SpanKind,
+  PluginConfig,
+} from '@opentelemetry/api';
 import { AttributeNames } from './enums';
 import {
   PgClientExtended,
-  PgPluginQueryConfig,
+  NormalizedQueryConfig,
   PostgresCallback,
   PgClientConnectionParams,
 } from './types';
@@ -44,11 +50,7 @@ function getJDBCString(params: PgClientConnectionParams) {
 }
 
 // Private helper function to start a span
-function pgStartSpan(
-  tracer: Tracer,
-  client: pgTypes.Client & PgClientExtended,
-  name: string
-) {
+function pgStartSpan(tracer: Tracer, client: PgClientExtended, name: string) {
   const jdbcString = getJDBCString(client.connectionParameters);
   return tracer.startSpan(name, {
     kind: SpanKind.CLIENT,
@@ -66,69 +68,72 @@ function pgStartSpan(
 
 // Queries where args[0] is a QueryConfig
 export function handleConfigQuery(
-  this: pgTypes.Client & PgClientExtended,
+  this: PgClientExtended,
   tracer: Tracer,
-  ...args: unknown[]
+  pluginConfig: PluginConfig,
+  queryConfig: NormalizedQueryConfig
 ) {
-  const argsConfig = args[0] as PgPluginQueryConfig;
-
   // Set child span name
-  const queryCommand = getCommandFromText(argsConfig.name || argsConfig.text);
+  const queryCommand = getCommandFromText(queryConfig.name || queryConfig.text);
   const name = PostgresPlugin.BASE_SPAN_NAME + ':' + queryCommand;
   const span = pgStartSpan(tracer, this, name);
 
   // Set attributes
-  if (argsConfig.text) {
-    span.setAttribute(AttributeNames.DB_STATEMENT, argsConfig.text);
+  if (queryConfig.text) {
+    span.setAttribute(AttributeNames.DB_STATEMENT, queryConfig.text);
   }
-
-  if (argsConfig.values instanceof Array) {
+  if (
+    pluginConfig.enhancedDatabaseReporting &&
+    queryConfig.values instanceof Array
+  ) {
     span.setAttribute(
       AttributeNames.PG_VALUES,
-      arrayStringifyHelper(argsConfig.values)
+      arrayStringifyHelper(queryConfig.values)
     );
   }
   // Set plan name attribute, if present
-  if (argsConfig.name) {
-    span.setAttribute(AttributeNames.PG_PLAN, argsConfig.name);
+  if (queryConfig.name) {
+    span.setAttribute(AttributeNames.PG_PLAN, queryConfig.name);
   }
 
   return span;
 }
 
 // Queries where args[1] is a 'values' array
 export function handleParameterizedQuery(
-  this: pgTypes.Client & PgClientExtended,
+  this: PgClientExtended,
   tracer: Tracer,
-  ...args: unknown[]
+  pluginConfig: PluginConfig,
+  query: string,
+  values: unknown[]
 ) {
   // Set child span name
-  const queryCommand = getCommandFromText(args[0] as string);
+  const queryCommand = getCommandFromText(query);
   const name = PostgresPlugin.BASE_SPAN_NAME + ':' + queryCommand;
   const span = pgStartSpan(tracer, this, name);
 
   // Set attributes
-  span.setAttribute(AttributeNames.DB_STATEMENT, args[0]);
-  if (args[1] instanceof Array) {
-    span.setAttribute(AttributeNames.PG_VALUES, arrayStringifyHelper(args[1]));
+  span.setAttribute(AttributeNames.DB_STATEMENT, query);
+  if (pluginConfig.enhancedDatabaseReporting) {
+    span.setAttribute(AttributeNames.PG_VALUES, arrayStringifyHelper(values));
   }
 
   return span;
 }
 
 // Queries where args[0] is a text query and 'values' was not specified
 export function handleTextQuery(
-  this: pgTypes.Client & PgClientExtended,
+  this: PgClientExtended,
   tracer: Tracer,
-  ...args: unknown[]
+  query: string
 ) {
   // Set child span name
-  const queryCommand = getCommandFromText(args[0] as string);
+  const queryCommand = getCommandFromText(query);
   const name = PostgresPlugin.BASE_SPAN_NAME + ':' + queryCommand;
   const span = pgStartSpan(tracer, this, name);
 
   // Set attributes
-  span.setAttribute(AttributeNames.DB_STATEMENT, args[0]);
+  span.setAttribute(AttributeNames.DB_STATEMENT, query);
 
   return span;
 }
@@ -138,7 +143,7 @@ export function handleTextQuery(
  * Create and immediately end a new span
  */
 export function handleInvalidQuery(
-  this: pgTypes.Client & PgClientExtended,
+  this: PgClientExtended,
   tracer: Tracer,
   originalQuery: typeof pgTypes.Client.prototype.query,
   ...args: unknown[]
@@ -162,7 +167,7 @@ export function patchCallback(
   cb: PostgresCallback
 ): PostgresCallback {
   return function patchedCallback(
-    this: pgTypes.Client & PgClientExtended,
+    this: PgClientExtended,
     err: Error,
     res: object
   ) {

diff --git a/plugins/node/opentelemetry-plugin-pg/test/pg.test.ts b/plugins/node/opentelemetry-plugin-pg/test/pg.test.ts
@@ -211,7 +211,6 @@ describe('[email protected]', () => {
       const attributes = {
         ...DEFAULT_ATTRIBUTES,
         [AttributeNames.DB_STATEMENT]: query,
-        [AttributeNames.PG_VALUES]: '[0]',
       };
       const events: TimedEvent[] = [];
       const span = tracer.startSpan('test span');
@@ -273,7 +272,6 @@ describe('[email protected]', () => {
       const attributes = {
         ...DEFAULT_ATTRIBUTES,
         [AttributeNames.DB_STATEMENT]: query,
-        [AttributeNames.PG_VALUES]: '[0]',
       };
       const events: TimedEvent[] = [];
       const span = tracer.startSpan('test span');
@@ -294,7 +292,6 @@ describe('[email protected]', () => {
       const attributes = {
         ...DEFAULT_ATTRIBUTES,
         [AttributeNames.DB_STATEMENT]: query,
-        [AttributeNames.PG_VALUES]: '[0]',
       };
       const events: TimedEvent[] = [];
       const span = tracer.startSpan('test span');
@@ -320,7 +317,6 @@ describe('[email protected]', () => {
         ...DEFAULT_ATTRIBUTES,
         [AttributeNames.PG_PLAN]: name,
         [AttributeNames.DB_STATEMENT]: query,
-        [AttributeNames.PG_VALUES]: '[0]',
       };
       const events: TimedEvent[] = [];
       const span = tracer.startSpan('test span');