From cb27d60b3b225387bf2467ce1aba2b9af1e0016b Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Mon, 14 Nov 2022 14:41:20 +0200
Subject: [PATCH 01/17] feat: add utility for affixing a sqlcommenter
 compatible comment to queries

---
 .../src/utils.ts                              | 66 ++++++++++++++
 .../test/utils.test.ts                        | 91 ++++++++++++++++++-
 2 files changed, 156 insertions(+), 1 deletion(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
index 7e5359c4ec..ee3293b56a 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
@@ -24,6 +24,7 @@ import {
   diag,
   INVALID_SPAN_CONTEXT,
   Attributes,
+  isSpanContextValid,
 } from '@opentelemetry/api';
 import { AttributeNames } from './enums/AttributeNames';
 import {
@@ -289,3 +290,68 @@ export function patchClientConnectCallback(
     cb.call(this, err);
   };
 }
+
+// Simplified W3CTraceContext serialization
+const W3C_TRACE_CONTEXT_VERSION = '00';
+const TRACE_PARENT_HEADER = 'traceparent';
+const TRACE_STATE_HEADER = 'tracestate';
+
+function hasValidSqlComment(query: string): boolean {
+  const indexOpeningDashDashComment = query.indexOf('--');
+  if (indexOpeningDashDashComment >= 0) {
+    return true;
+  }
+
+  const indexOpeningSlashComment = query.indexOf('/*');
+  if (indexOpeningSlashComment < 0) {
+    return false;
+  }
+
+  const indexClosingSlashComment = query.indexOf('*/');
+  return indexOpeningDashDashComment < indexClosingSlashComment;
+}
+
+export function addSqlCommenterComment(span: Span, query: string): string {
+  const spanContext = span.spanContext();
+
+  if (!isSpanContextValid(spanContext)) {
+    return query;
+  }
+
+  if (query.length === 0) {
+    return query;
+  }
+
+  // As per sqlcommenter spec we shall not add a comment if there already is a comment
+  // in the query
+  if (hasValidSqlComment(query)) {
+    return query;
+  }
+
+  const headers: { [key: string]: string } = {
+    [TRACE_PARENT_HEADER]: `${W3C_TRACE_CONTEXT_VERSION}-${
+      spanContext.traceId
+    }-${spanContext.spanId}-0${spanContext.traceFlags.toString(16)}`,
+  };
+
+  if (spanContext.traceState !== undefined) {
+    headers[TRACE_STATE_HEADER] = spanContext.traceState.serialize();
+  }
+
+  // NOTE: If additional keys are added to the header, make sure they are sorted correctly
+  // according to the sqlcommenter spec the comment should use lexicographic order
+  const commentString = Object.keys(headers)
+    .map(key => {
+      const uriEncodedKey = encodeURIComponent(key);
+      const uriEncodedValue = encodeURIComponent(headers[key]).replace(
+        /([^\\])(')/g,
+        (_, prefix) => {
+          return `${prefix}\\'`;
+        }
+      );
+      return `${uriEncodedKey}='${uriEncodedValue}'`;
+    })
+    .join(',');
+
+  return `${query} /*${commentString}*/`;
+}
diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts
index 6bb03076bf..3676ee30fa 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts
@@ -14,7 +14,15 @@
  * limitations under the License.
  */
 
-import { context, INVALID_SPAN_CONTEXT, trace } from '@opentelemetry/api';
+import {
+  context,
+  createTraceState,
+  INVALID_SPAN_CONTEXT,
+  SpanContext,
+  trace,
+  TraceFlags,
+} from '@opentelemetry/api';
+import { wrapSpanContext } from '@opentelemetry/api/build/src/trace/spancontext-utils';
 import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
 import { InstrumentationConfig } from '@opentelemetry/instrumentation';
 import {
@@ -204,4 +212,85 @@ describe('utils.ts', () => {
       assert.strictEqual(pgValues, '[0]');
     });
   });
+
+  describe('addSqlCommenterComment', () => {
+    it('adds comment to a simple query', () => {
+      const spanContext: SpanContext = {
+        traceId: 'd4cda95b652f4a1592b449d5929fda1b',
+        spanId: '6e0c63257de34c92',
+        traceFlags: TraceFlags.SAMPLED,
+      };
+
+      const query = 'SELECT * from FOO;';
+      assert.strictEqual(
+        utils.addSqlCommenterComment(wrapSpanContext(spanContext), query),
+        "SELECT * from FOO; /*traceparent='00-d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-01'*/"
+      );
+    });
+
+    it('does not add a comment if query already has a comment', () => {
+      const span = wrapSpanContext({
+        traceId: 'd4cda95b652f4a1592b449d5929fda1b',
+        spanId: '6e0c63257de34c92',
+        traceFlags: TraceFlags.SAMPLED,
+      });
+
+      const blockComment = 'SELECT * from FOO; /* Test comment */';
+      assert.strictEqual(
+        utils.addSqlCommenterComment(span, blockComment),
+        blockComment
+      );
+
+      const dashedComment = 'SELECT * from FOO; -- Test comment';
+      assert.strictEqual(
+        utils.addSqlCommenterComment(span, dashedComment),
+        dashedComment
+      );
+    });
+
+    it('does not add a comment to an empty query', () => {
+      const spanContext: SpanContext = {
+        traceId: 'd4cda95b652f4a1592b449d5929fda1b',
+        spanId: '6e0c63257de34c92',
+        traceFlags: TraceFlags.SAMPLED,
+      };
+
+      assert.strictEqual(utils.addSqlCommenterComment(wrapSpanContext(spanContext), ''), '');
+    });
+
+    it('does not add a comment if span context is invalid', () => {
+      const query = 'SELECT * from FOO;';
+      assert.strictEqual(utils.addSqlCommenterComment(wrapSpanContext(INVALID_SPAN_CONTEXT), query), query);
+    });
+
+    it('correctly also sets trace state', () => {
+      const spanContext: SpanContext = {
+        traceId: 'd4cda95b652f4a1592b449d5929fda1b',
+        spanId: '6e0c63257de34c92',
+        traceFlags: TraceFlags.SAMPLED,
+        traceState: createTraceState('foo=bar,baz=qux'),
+      };
+
+      const query = 'SELECT * from FOO;';
+      assert.strictEqual(
+        utils.addSqlCommenterComment(wrapSpanContext(spanContext), query),
+        "SELECT * from FOO; /*traceparent='00-d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-01',tracestate='foo%3Dbar%2Cbaz%3Dqux'*/"
+      );
+    });
+
+    it('escapes single quotes in values', () => {
+      const spanContext: SpanContext = {
+        traceId: 'd4cda95b652f4a1592b449d5929fda1b',
+        spanId: '6e0c63257de34c92',
+        traceFlags: TraceFlags.SAMPLED,
+        traceState: createTraceState("foo=bar,baz='qux',hack='DROP TABLE"),
+      };
+
+      const query = 'SELECT * from FOO;';
+      assert.strictEqual(
+        utils.addSqlCommenterComment(wrapSpanContext(spanContext), query),
+        "SELECT * from FOO; /*traceparent='00-d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-01',tracestate='foo%3Dbar%2Cbaz%3D\\'qux\\'%2Chack%3D\\'DROP%20TABLE'*/"
+      );
+    });
+  });
 });

From 2425c571bf90ce5f9b0de164ee3978254172553a Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Mon, 14 Nov 2022 17:30:35 +0200
Subject: [PATCH 02/17] feat: add support for adding sqlcommenter comment

---
 .../opentelemetry-instrumentation-pg/README.md   |  2 ++
 .../src/instrumentation.ts                       | 16 ++++++++++++++++
 .../src/types.ts                                 |  6 ++++++
 3 files changed, 24 insertions(+)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/README.md b/plugins/node/opentelemetry-instrumentation-pg/README.md
index 578b2cc173..5233975b78 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/README.md
+++ b/plugins/node/opentelemetry-instrumentation-pg/README.md
@@ -48,6 +48,8 @@ PostgreSQL instrumentation has few options available to choose from. You can set
 | ------- | ---- | ----------- |
 | [`enhancedDatabaseReporting`](./src/types.ts#L30) | `boolean` | If true, additional information about query parameters and results will be attached (as `attributes`) to spans representing database operations |
 | `responseHook` | `PgInstrumentationExecutionResponseHook` (function) | Function for adding custom attributes from db response |
+| `requireParentSpan` | `boolean` | If true, requires a parent span to create new spans (default false) |
+| `addSqlCommenterCommentToQueries` | `boolean` | If true, adds [sqlcommenter](https://github.com/open-telemetry/opentelemetry-sqlcommenter) specification compliant comment to queries with tracing context |
 
 ## Useful links
 
diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts
index 7c1b6d77f2..f309555951 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts
@@ -178,6 +178,7 @@ export class PgInstrumentation extends InstrumentationBase {
         // 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(
@@ -195,14 +196,29 @@ export class PgInstrumentation extends InstrumentationBase {
               query
             );
           }
+
+          if (plugin.getConfig().addSqlCommenterCommentToQueries) {
+            // Modify the query with a tracing comment
+            args[0] = utils.addSqlCommenterComment(span, args[0]);
+          }
         } else if (typeof args[0] === 'object') {
           const queryConfig = args[0] as NormalizedQueryConfig;
+
           span = utils.handleConfigQuery.call(
             this,
             plugin.tracer,
             plugin.getConfig(),
             queryConfig
           );
+
+          if (plugin.getConfig().addSqlCommenterCommentToQueries) {
+            // Copy the query config instead of writing to args[0].text so that we don't modify user's
+            // original query config reference
+            args[0] = {
+              ...queryConfig,
+              text: utils.addSqlCommenterComment(span, queryConfig.text),
+            };
+          }
         } else {
           return utils.handleInvalidQuery.call(
             this,
diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/types.ts b/plugins/node/opentelemetry-instrumentation-pg/src/types.ts
index d37ab0323f..3e0d280cad 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/src/types.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/src/types.ts
@@ -46,4 +46,10 @@ export interface PgInstrumentationConfig extends InstrumentationConfig {
    * @default false
    */
   requireParentSpan?: boolean;
+
+  /**
+   * If true, queries are modified to also include a comment with
+   * the tracing context, following the {@link https://github.com/open-telemetry/opentelemetry-sqlcommenter sqlcommenter} format
+   */
+  addSqlCommenterCommentToQueries?: boolean;
 }

From 41e49f18630d4bd462c7a5ec7660df4537e91147 Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Tue, 15 Nov 2022 07:13:28 +0200
Subject: [PATCH 03/17] fix: linter errors

---
 .../test/utils.test.ts                              | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts
index 3676ee30fa..6ca2e084e2 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts
@@ -255,12 +255,21 @@ describe('utils.ts', () => {
         traceFlags: TraceFlags.SAMPLED,
       };
 
-      assert.strictEqual(utils.addSqlCommenterComment(wrapSpanContext(spanContext), ''), '');
+      assert.strictEqual(
+        utils.addSqlCommenterComment(wrapSpanContext(spanContext), ''),
+        ''
+      );
     });
 
     it('does not add a comment if span context is invalid', () => {
       const query = 'SELECT * from FOO;';
-      assert.strictEqual(utils.addSqlCommenterComment(wrapSpanContext(INVALID_SPAN_CONTEXT), query), query);
+      assert.strictEqual(
+        utils.addSqlCommenterComment(
+          wrapSpanContext(INVALID_SPAN_CONTEXT),
+          query
+        ),
+        query
+      );
     });
 
     it('correctly also sets trace state', () => {

From 08a9e0ce36ea2cc2cfdf9b120f35794aba69aba8 Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Fri, 18 Nov 2022 16:18:12 +0200
Subject: [PATCH 04/17] refactor: use W3CTraceContextPropagator for comment
 contents

---
 .../package.json                              |  1 +
 .../src/utils.ts                              | 38 ++++++++-----------
 2 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/package.json b/plugins/node/opentelemetry-instrumentation-pg/package.json
index 917139df6e..2801898c28 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/package.json
+++ b/plugins/node/opentelemetry-instrumentation-pg/package.json
@@ -72,6 +72,7 @@
     "typescript": "4.3.5"
   },
   "dependencies": {
+    "@opentelemetry/core": "^1.8.0",
     "@opentelemetry/instrumentation": "^0.34.0",
     "@opentelemetry/semantic-conventions": "^1.0.0",
     "@types/pg": "8.6.1",
diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
index ee3293b56a..4a08e13263 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
@@ -24,8 +24,10 @@ import {
   diag,
   INVALID_SPAN_CONTEXT,
   Attributes,
-  isSpanContextValid,
+  defaultTextMapSetter,
+  ROOT_CONTEXT,
 } from '@opentelemetry/api';
+import { W3CTraceContextPropagator } from '@opentelemetry/core';
 import { AttributeNames } from './enums/AttributeNames';
 import {
   SemanticAttributes,
@@ -291,11 +293,6 @@ export function patchClientConnectCallback(
   };
 }
 
-// Simplified W3CTraceContext serialization
-const W3C_TRACE_CONTEXT_VERSION = '00';
-const TRACE_PARENT_HEADER = 'traceparent';
-const TRACE_STATE_HEADER = 'tracestate';
-
 function hasValidSqlComment(query: string): boolean {
   const indexOpeningDashDashComment = query.indexOf('--');
   if (indexOpeningDashDashComment >= 0) {
@@ -312,12 +309,6 @@ function hasValidSqlComment(query: string): boolean {
 }
 
 export function addSqlCommenterComment(span: Span, query: string): string {
-  const spanContext = span.spanContext();
-
-  if (!isSpanContextValid(spanContext)) {
-    return query;
-  }
-
   if (query.length === 0) {
     return query;
   }
@@ -328,19 +319,22 @@ export function addSqlCommenterComment(span: Span, query: string): string {
     return query;
   }
 
-  const headers: { [key: string]: string } = {
-    [TRACE_PARENT_HEADER]: `${W3C_TRACE_CONTEXT_VERSION}-${
-      spanContext.traceId
-    }-${spanContext.spanId}-0${spanContext.traceFlags.toString(16)}`,
-  };
+  const propagator = new W3CTraceContextPropagator();
+  const headers: { [key: string]: string } = {};
+  propagator.inject(
+    trace.setSpan(ROOT_CONTEXT, span),
+    headers,
+    defaultTextMapSetter
+  );
 
-  if (spanContext.traceState !== undefined) {
-    headers[TRACE_STATE_HEADER] = spanContext.traceState.serialize();
+  // sqlcommenter spec requires keys in the comment to be sorted lexicographically
+  const sortedKeys = Object.keys(headers).sort();
+
+  if (sortedKeys.length === 0) {
+    return query;
   }
 
-  // NOTE: If additional keys are added to the header, make sure they are sorted correctly
-  // according to the sqlcommenter spec the comment should use lexicographic order
-  const commentString = Object.keys(headers)
+  const commentString = sortedKeys
     .map(key => {
       const uriEncodedKey = encodeURIComponent(key);
       const uriEncodedValue = encodeURIComponent(headers[key]).replace(

From d53b3555644fb9dc6792bf6e8a1d5da4a9a9552a Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Fri, 18 Nov 2022 16:16:05 +0200
Subject: [PATCH 05/17] test: cover that the sqlcommenter comment is added when
 the flag is true

---
 .../test/pg.test.ts                           | 118 ++++++++++++++++++
 1 file changed, 118 insertions(+)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts
index d43eb7537f..a8404e7071 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts
@@ -44,6 +44,7 @@ import {
   DbSystemValues,
 } from '@opentelemetry/semantic-conventions';
 import { isSupported } from './utils';
+import { addSqlCommenterComment } from '../src/utils';
 
 const pgVersion = require('pg/package.json').version;
 const nodeVersion = process.versions.node;
@@ -99,6 +100,7 @@ describe('pg', () => {
     instrumentation.enable();
   }
 
+  let executedQueries: (pg.Query & { text?: string })[];
   let postgres: typeof pg;
   let client: pg.Client;
   let instrumentation: PgInstrumentation;
@@ -141,8 +143,21 @@ describe('pg', () => {
     context.setGlobalContextManager(contextManager);
     instrumentation.setTracerProvider(provider);
 
+    executedQueries = [];
     postgres = require('pg');
     client = new postgres.Client(CONFIG);
+
+    // We need to track the actual queries that get executed to assert
+    // that the sqlcommenter logic works
+    const privateClient: {
+      queryQueue: (pg.Query & { text?: string })[];
+    } = client as any;
+    const originalPush = privateClient.queryQueue.push;
+    privateClient.queryQueue.push = (...items) => {
+      executedQueries.push(...items);
+      return originalPush.apply(privateClient.queryQueue, items);
+    };
+
     await client.connect();
   });
 
@@ -161,6 +176,7 @@ describe('pg', () => {
   afterEach(() => {
     memoryExporter.reset();
     context.disable();
+    executedQueries.splice(0, executedQueries.length);
   });
 
   it('should return an instrumentation', () => {
@@ -649,6 +665,108 @@ describe('pg', () => {
       });
     });
 
+    it('should not add sqlcommenter comment when flag is not specified', async () => {
+      const span = tracer.startSpan('test span');
+      await context.with(trace.setSpan(context.active(), span), async () => {
+        try {
+          const query = 'SELECT NOW()';
+          const resPromise = await client.query(query);
+          assert.ok(resPromise);
+
+          const [span] = memoryExporter.getFinishedSpans();
+          assert.ok(span);
+
+          const commentedQuery = addSqlCommenterComment(
+            trace.wrapSpanContext(span.spanContext()),
+            query
+          );
+          assert.equal(executedQueries.length, 1);
+          assert.equal(executedQueries[0].text, query);
+          assert.notEqual(query, commentedQuery);
+        } catch (e) {
+          assert.ok(false, e.message);
+        }
+      });
+    });
+
+    it('should not add sqlcommenter comment with client.query({text, callback}) when flag is not specified', done => {
+      const span = tracer.startSpan('test span');
+      context.with(trace.setSpan(context.active(), span), () => {
+        const query = 'SELECT NOW()';
+        client.query({
+          text: query,
+          callback: (err: Error, res: pg.QueryResult) => {
+            assert.strictEqual(err, null);
+            assert.ok(res);
+
+            const [span] = memoryExporter.getFinishedSpans();
+            const commentedQuery = addSqlCommenterComment(
+              trace.wrapSpanContext(span.spanContext()),
+              query
+            );
+            assert.equal(executedQueries.length, 1);
+            assert.equal(executedQueries[0].text, query);
+            assert.notEqual(query, commentedQuery);
+            done();
+          },
+        } as pg.QueryConfig);
+      });
+    });
+
+    it('should add sqlcommenter comment when addSqlCommenterCommentToQueries=true is specified', async () => {
+      instrumentation.setConfig({
+        addSqlCommenterCommentToQueries: true,
+      });
+
+      const span = tracer.startSpan('test span');
+      await context.with(trace.setSpan(context.active(), span), async () => {
+        try {
+          const query = 'SELECT NOW()';
+          const resPromise = await client.query(query);
+          assert.ok(resPromise);
+
+          const [span] = memoryExporter.getFinishedSpans();
+          const commentedQuery = addSqlCommenterComment(
+            trace.wrapSpanContext(span.spanContext()),
+            query
+          );
+          assert.equal(executedQueries.length, 1);
+          assert.equal(executedQueries[0].text, commentedQuery);
+          assert.notEqual(query, commentedQuery);
+        } catch (e) {
+          assert.ok(false, e.message);
+        }
+      });
+    });
+
+    it('should add sqlcommenter comment when addSqlCommenterCommentToQueries=true is specified with client.query({text, callback})', done => {
+      instrumentation.setConfig({
+        addSqlCommenterCommentToQueries: true,
+      });
+
+      const span = tracer.startSpan('test span');
+      context.with(trace.setSpan(context.active(), span), () => {
+        const query = 'SELECT NOW()';
+        client.query({
+          text: query,
+          callback: (err: Error, res: pg.QueryResult) => {
+            assert.strictEqual(err, null);
+            assert.ok(res);
+
+            const [span] = memoryExporter.getFinishedSpans();
+            const commentedQuery = addSqlCommenterComment(
+              trace.wrapSpanContext(span.spanContext()),
+              query
+            );
+            assert.equal(executedQueries.length, 1);
+            assert.equal(executedQueries[0].text, commentedQuery);
+            assert.notEqual(query, commentedQuery);
+            done();
+          },
+        } as pg.QueryConfig);
+      });
+    });
+
     it('should not generate traces for client.query() when requireParentSpan=true is specified', done => {
       instrumentation.setConfig({
         requireParentSpan: true,

From 6e3302c0d55f6e43462a05eac3f015e60a9a80ff Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Tue, 22 Nov 2022 15:32:51 +0200
Subject: [PATCH 06/17] fix: guard against non-strings

---
 plugins/node/opentelemetry-instrumentation-pg/src/utils.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
index 4a08e13263..f0f383dec9 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
@@ -309,7 +309,7 @@ function hasValidSqlComment(query: string): boolean {
 }
 
 export function addSqlCommenterComment(span: Span, query: string): string {
-  if (query.length === 0) {
+  if (query.length === 0 || typeof query !== 'string') {
     return query;
   }
 

From f296325e224dd9295b8c66eb03fbcd6e4dd713bd Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Tue, 22 Nov 2022 15:35:23 +0200
Subject: [PATCH 07/17] refactor: avoid encoding header values

---
 .../src/utils.ts                                 | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
index f0f383dec9..444520fe9e 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
@@ -335,17 +335,13 @@ export function addSqlCommenterComment(span: Span, query: string): string {
   }
 
   const commentString = sortedKeys
-    .map(key => {
-      const uriEncodedKey = encodeURIComponent(key);
-      const uriEncodedValue = encodeURIComponent(headers[key]).replace(
-        /([^\\])(')/g,
-        (_, prefix) => {
-          return `${prefix}\\'`;
-        }
-      );
-      return `${uriEncodedKey}='${uriEncodedValue}'`;
+    .map((key) => {
+      const escapedValue = headers[key].replace(/([^\\])(')/g, (_, prefix) => {
+        return `${prefix}\\'`;
+      });
+      return `${key}='${escapedValue}'`;
     })
-    .join(',');
+    .join(",");
 
   return `${query} /*${commentString}*/`;
 }

From b79ab31335f3832acb4d02444ffd29a5352bc5ba Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Wed, 23 Nov 2022 08:46:55 +0200
Subject: [PATCH 08/17] fix: order of if condition
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Gerhard Stöbich <deb2001-github@yahoo.de>
---
 plugins/node/opentelemetry-instrumentation-pg/src/utils.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
index 444520fe9e..371f3ced6e 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
@@ -309,7 +309,7 @@ function hasValidSqlComment(query: string): boolean {
 }
 
 export function addSqlCommenterComment(span: Span, query: string): string {
-  if (query.length === 0 || typeof query !== 'string') {
+  if (typeof query !== 'string' || query.length === 0) {
     return query;
   }
 

From a7e76e46d38fda125e7304da52095dbcb8a3ef4a Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Wed, 23 Nov 2022 08:48:56 +0200
Subject: [PATCH 09/17] fix: uri encode values

---
 .../node/opentelemetry-instrumentation-pg/src/utils.ts   | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
index 371f3ced6e..c22a732b6f 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
@@ -336,9 +336,12 @@ export function addSqlCommenterComment(span: Span, query: string): string {
 
   const commentString = sortedKeys
     .map((key) => {
-      const escapedValue = headers[key].replace(/([^\\])(')/g, (_, prefix) => {
-        return `${prefix}\\'`;
-      });
+      const escapedValue = encodeURIComponent(headers[key]).replace(
+        /([^\\])(')/g,
+        (_, prefix) => {
+          return `${prefix}\\'`;
+        }
+      );
       return `${key}='${escapedValue}'`;
     })
     .join(",");

From dfec60506837c77952eb6bcdd49543536e0f255e Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Wed, 23 Nov 2022 09:02:12 +0200
Subject: [PATCH 10/17] refactor: address code review comments

---
 .../package.json                              |  2 ++
 .../src/utils.ts                              | 18 ++++++----
 .../test/pg.test.ts                           | 35 +++++++++++--------
 3 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/package.json b/plugins/node/opentelemetry-instrumentation-pg/package.json
index 2801898c28..fdf69fe8be 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/package.json
+++ b/plugins/node/opentelemetry-instrumentation-pg/package.json
@@ -60,6 +60,7 @@
     "@opentelemetry/sdk-trace-node": "^1.8.0",
     "@types/mocha": "7.0.2",
     "@types/node": "18.11.7",
+    "@types/sinon": "10.0.2",
     "cross-env": "7.0.3",
     "gts": "3.1.0",
     "mocha": "7.2.0",
@@ -67,6 +68,7 @@
     "pg": "8.7.1",
     "pg-pool": "3.4.1",
     "rimraf": "3.0.2",
+    "sinon": "14.0.0",
     "test-all-versions": "5.0.1",
     "ts-mocha": "10.0.0",
     "typescript": "4.3.5"
diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
index c22a732b6f..5269b90776 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
@@ -308,6 +308,13 @@ function hasValidSqlComment(query: string): boolean {
   return indexOpeningDashDashComment < indexClosingSlashComment;
 }
 
+function escapeMetaCharacters(value: string): string {
+  // Single quotes must be escaped by a slash, unless already escaped
+  return value.replace(/([^\\])(')/g, (_, prefix) => {
+    return `${prefix}\\'`;
+  });
+}
+
 export function addSqlCommenterComment(span: Span, query: string): string {
   if (typeof query !== 'string' || query.length === 0) {
     return query;
@@ -335,16 +342,13 @@ export function addSqlCommenterComment(span: Span, query: string): string {
   }
 
   const commentString = sortedKeys
-    .map((key) => {
-      const escapedValue = encodeURIComponent(headers[key]).replace(
-        /([^\\])(')/g,
-        (_, prefix) => {
-          return `${prefix}\\'`;
-        }
+    .map(key => {
+      const escapedValue = escapeMetaCharacters(
+        encodeURIComponent(headers[key])
       );
       return `${key}='${escapedValue}'`;
     })
-    .join(",");
+    .join(',');
 
   return `${query} /*${commentString}*/`;
 }
diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts
index a8404e7071..e8d95b56ac 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts
@@ -32,6 +32,7 @@ import {
 } from '@opentelemetry/sdk-trace-base';
 import * as assert from 'assert';
 import type * as pg from 'pg';
+import * as Sinon from 'sinon';
 import {
   PgInstrumentation,
   PgInstrumentationConfig,
@@ -100,7 +101,6 @@ describe('pg', () => {
     instrumentation.enable();
   }
 
-  let executedQueries: (pg.Query & { text?: string })[];
   let postgres: typeof pg;
   let client: pg.Client;
   let instrumentation: PgInstrumentation;
@@ -112,6 +112,12 @@ describe('pg', () => {
   const testPostgresLocally = process.env.RUN_POSTGRES_TESTS_LOCAL; // For local: spins up local postgres db via docker
   const shouldTest = testPostgres || testPostgresLocally; // Skips these tests if false (default)
 
+  function getExecutedQueries() {
+    return (client as any).queryQueue.push.args.flat() as (pg.Query & {
+      text?: string;
+    })[];
+  }
+
   before(async function () {
     const skipForUnsupported =
       process.env.IN_TAV && !isSupported(nodeVersion, pgVersion);
@@ -143,21 +149,8 @@ describe('pg', () => {
     context.setGlobalContextManager(contextManager);
     instrumentation.setTracerProvider(provider);
 
-    executedQueries = [];
     postgres = require('pg');
     client = new postgres.Client(CONFIG);
-
-    // We need to track the actual queries that get executed to assert
-    // that the sqlcommenter logic works
-    const privateClient: {
-      queryQueue: (pg.Query & { text?: string })[];
-    } = client as any;
-    const originalPush = privateClient.queryQueue.push;
-    privateClient.queryQueue.push = (...items) => {
-      executedQueries.push(...items);
-      return originalPush.apply(privateClient.queryQueue, items);
-    };
-
     await client.connect();
   });
 
@@ -171,12 +164,16 @@ describe('pg', () => {
   beforeEach(() => {
     contextManager = new AsyncHooksContextManager().enable();
     context.setGlobalContextManager(contextManager);
+
+    // Add a spy on the underlying client's internal query queue so that
+    // we could assert on what the final queries are that are executed
+    Sinon.spy((client as any).queryQueue, 'push');
   });
 
   afterEach(() => {
     memoryExporter.reset();
     context.disable();
-    executedQueries.splice(0, executedQueries.length);
+    Sinon.restore();
   });
 
   it('should return an instrumentation', () => {
@@ -680,6 +677,8 @@ describe('pg', () => {
             trace.wrapSpanContext(span.spanContext()),
             query
           );
+
+          const executedQueries = getExecutedQueries();
           assert.equal(executedQueries.length, 1);
           assert.equal(executedQueries[0].text, query);
           assert.notEqual(query, commentedQuery);
@@ -704,6 +703,8 @@ describe('pg', () => {
               trace.wrapSpanContext(span.spanContext()),
               query
             );
+
+            const executedQueries = getExecutedQueries();
             assert.equal(executedQueries.length, 1);
             assert.equal(executedQueries[0].text, query);
             assert.notEqual(query, commentedQuery);
@@ -730,6 +731,8 @@ describe('pg', () => {
             trace.wrapSpanContext(span.spanContext()),
             query
           );
+
+          const executedQueries = getExecutedQueries();
           assert.equal(executedQueries.length, 1);
           assert.equal(executedQueries[0].text, commentedQuery);
           assert.notEqual(query, commentedQuery);
@@ -758,6 +761,8 @@ describe('pg', () => {
               trace.wrapSpanContext(span.spanContext()),
               query
             );
+
+            const executedQueries = getExecutedQueries();
             assert.equal(executedQueries.length, 1);
             assert.equal(executedQueries[0].text, commentedQuery);
             assert.notEqual(query, commentedQuery);

From 1f0ac72395ae0e57efe25f8fc3510a921bd0a376 Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Wed, 23 Nov 2022 09:38:54 +0200
Subject: [PATCH 11/17] fix: encoding of values in the comment

---
 .../src/utils.ts                              | 20 ++++++++++---------
 .../test/utils.test.ts                        |  6 +++---
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
index 5269b90776..02487dc2b7 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
@@ -308,11 +308,15 @@ function hasValidSqlComment(query: string): boolean {
   return indexOpeningDashDashComment < indexClosingSlashComment;
 }
 
-function escapeMetaCharacters(value: string): string {
-  // Single quotes must be escaped by a slash, unless already escaped
-  return value.replace(/([^\\])(')/g, (_, prefix) => {
-    return `${prefix}\\'`;
-  });
+// sqlcommenter specification expects us to URL encode all reserved
+// characters, but encodeURIComponent does not handle ! ' ( ) *,
+// which means we need special handling for this
+// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent
+function fixedEncodeURIComponent(str: string) {
+  return encodeURIComponent(str).replace(
+    /[!'()*]/g,
+    (c) => `%${c.charCodeAt(0).toString(16).toUpperCase()}`
+  );
 }
 
 export function addSqlCommenterComment(span: Span, query: string): string {
@@ -343,10 +347,8 @@ export function addSqlCommenterComment(span: Span, query: string): string {
 
   const commentString = sortedKeys
     .map(key => {
-      const escapedValue = escapeMetaCharacters(
-        encodeURIComponent(headers[key])
-      );
-      return `${key}='${escapedValue}'`;
+      const encodedValue = fixedEncodeURIComponent(headers[key]);
+      return `${key}='${encodedValue}'`;
     })
     .join(',');
 
diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts
index 6ca2e084e2..1899b74729 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts
@@ -287,18 +287,18 @@ describe('utils.ts', () => {
       );
     });
 
-    it('escapes single quotes in values', () => {
+    it('escapes special characters in values', () => {
       const spanContext: SpanContext = {
         traceId: 'd4cda95b652f4a1592b449d5929fda1b',
         spanId: '6e0c63257de34c92',
         traceFlags: TraceFlags.SAMPLED,
-        traceState: createTraceState("foo=bar,baz='qux',hack='DROP TABLE"),
+        traceState: createTraceState("foo='bar,baz='qux',hack='DROP TABLE"),
       };
 
       const query = 'SELECT * from FOO;';
       assert.strictEqual(
         utils.addSqlCommenterComment(wrapSpanContext(spanContext), query),
-        "SELECT * from FOO; /*traceparent='00-d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-01',tracestate='foo%3Dbar%2Cbaz%3D\\'qux\\'%2Chack%3D\\'DROP%20TABLE'*/"
+        "SELECT * from FOO; /*traceparent='00-d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-01',tracestate='foo%3D%27bar%2Cbaz%3D%27qux%27%2Chack%3D%27DROP%20TABLE'*/"
       );
     });
   });

From 16a45fa9ea1929686cea8fdd1aa8381fb33669a3 Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Wed, 23 Nov 2022 09:40:24 +0200
Subject: [PATCH 12/17] fix: linter error

---
 plugins/node/opentelemetry-instrumentation-pg/src/utils.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
index 02487dc2b7..04c501bbcc 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
@@ -315,7 +315,7 @@ function hasValidSqlComment(query: string): boolean {
 function fixedEncodeURIComponent(str: string) {
   return encodeURIComponent(str).replace(
     /[!'()*]/g,
-    (c) => `%${c.charCodeAt(0).toString(16).toUpperCase()}`
+    c => `%${c.charCodeAt(0).toString(16).toUpperCase()}`
   );
 }
 

From 18c6dbe01fa1e8aca8716036fed58154f8aa849c Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Thu, 24 Nov 2022 08:25:00 +0200
Subject: [PATCH 13/17] docs: add reasoning for url encoding

---
 plugins/node/opentelemetry-instrumentation-pg/src/utils.ts | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
index 04c501bbcc..9a19bd6331 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
@@ -308,8 +308,9 @@ function hasValidSqlComment(query: string): boolean {
   return indexOpeningDashDashComment < indexClosingSlashComment;
 }
 
-// sqlcommenter specification expects us to URL encode all reserved
-// characters, but encodeURIComponent does not handle ! ' ( ) *,
+// sqlcommenter specification (https://google.github.io/sqlcommenter/spec/#value-serialization)
+// expects us to URL encode based on the RFC 3986 spec (https://en.wikipedia.org/wiki/Percent-encoding),
+// but encodeURIComponent does not handle some characters correctly (! ' ( ) *),
 // which means we need special handling for this
 // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent
 function fixedEncodeURIComponent(str: string) {

From 486b9037615fb9054165a6e16118dfc90089fffc Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Thu, 24 Nov 2022 08:28:22 +0200
Subject: [PATCH 14/17] refactor: use wrapSpanContext from trace

---
 .../test/utils.test.ts                              | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts
index 1899b74729..65f785ca48 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts
@@ -22,7 +22,6 @@ import {
   trace,
   TraceFlags,
 } from '@opentelemetry/api';
-import { wrapSpanContext } from '@opentelemetry/api/build/src/trace/spancontext-utils';
 import { AsyncHooksContextManager } from '@opentelemetry/context-async-hooks';
 import { InstrumentationConfig } from '@opentelemetry/instrumentation';
 import {
@@ -223,13 +222,13 @@ describe('utils.ts', () => {
 
       const query = 'SELECT * from FOO;';
       assert.strictEqual(
-        utils.addSqlCommenterComment(wrapSpanContext(spanContext), query),
+        utils.addSqlCommenterComment(trace.wrapSpanContext(spanContext), query),
         "SELECT * from FOO; /*traceparent='00-d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-01'*/"
       );
     });
 
     it('does not add a comment if query already has a comment', () => {
-      const span = wrapSpanContext({
+      const span = trace.wrapSpanContext({
         traceId: 'd4cda95b652f4a1592b449d5929fda1b',
         spanId: '6e0c63257de34c92',
         traceFlags: TraceFlags.SAMPLED,
@@ -256,7 +255,7 @@ describe('utils.ts', () => {
       };
 
       assert.strictEqual(
-        utils.addSqlCommenterComment(wrapSpanContext(spanContext), ''),
+        utils.addSqlCommenterComment(trace.wrapSpanContext(spanContext), ''),
         ''
       );
     });
@@ -265,7 +264,7 @@ describe('utils.ts', () => {
       const query = 'SELECT * from FOO;';
       assert.strictEqual(
         utils.addSqlCommenterComment(
-          wrapSpanContext(INVALID_SPAN_CONTEXT),
+          trace.wrapSpanContext(INVALID_SPAN_CONTEXT),
           query
         ),
         query
@@ -282,7 +281,7 @@ describe('utils.ts', () => {
 
       const query = 'SELECT * from FOO;';
       assert.strictEqual(
-        utils.addSqlCommenterComment(wrapSpanContext(spanContext), query),
+        utils.addSqlCommenterComment(trace.wrapSpanContext(spanContext), query),
         "SELECT * from FOO; /*traceparent='00-d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-01',tracestate='foo%3Dbar%2Cbaz%3Dqux'*/"
       );
     });
@@ -297,7 +296,7 @@ describe('utils.ts', () => {
 
       const query = 'SELECT * from FOO;';
       assert.strictEqual(
-        utils.addSqlCommenterComment(wrapSpanContext(spanContext), query),
+        utils.addSqlCommenterComment(trace.wrapSpanContext(spanContext), query),
         "SELECT * from FOO; /*traceparent='00-d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-01',tracestate='foo%3D%27bar%2Cbaz%3D%27qux%27%2Chack%3D%27DROP%20TABLE'*/"
       );
     });

From d37e50c9a3ef86fe31b44e7d0165836869e83b2d Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Thu, 24 Nov 2022 08:29:08 +0200
Subject: [PATCH 15/17] refactor: use lowercase sinon

---
 .../node/opentelemetry-instrumentation-pg/test/pg.test.ts   | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts
index e8d95b56ac..e91b0034fb 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts
@@ -32,7 +32,7 @@ import {
 } from '@opentelemetry/sdk-trace-base';
 import * as assert from 'assert';
 import type * as pg from 'pg';
-import * as Sinon from 'sinon';
+import * as sinon from 'sinon';
 import {
   PgInstrumentation,
   PgInstrumentationConfig,
@@ -167,13 +167,13 @@ describe('pg', () => {
 
     // Add a spy on the underlying client's internal query queue so that
     // we could assert on what the final queries are that are executed
-    Sinon.spy((client as any).queryQueue, 'push');
+    sinon.spy((client as any).queryQueue, 'push');
   });
 
   afterEach(() => {
     memoryExporter.reset();
     context.disable();
-    Sinon.restore();
+    sinon.restore();
   });
 
   it('should return an instrumentation', () => {

From 9a117127b9768d1857257d8434e99e8b3e46b0f7 Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Thu, 24 Nov 2022 08:30:29 +0200
Subject: [PATCH 16/17] test: cover all special characters

---
 .../node/opentelemetry-instrumentation-pg/test/utils.test.ts  | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts
index 65f785ca48..4b43e4da95 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts
@@ -291,13 +291,13 @@ describe('utils.ts', () => {
         traceId: 'd4cda95b652f4a1592b449d5929fda1b',
         spanId: '6e0c63257de34c92',
         traceFlags: TraceFlags.SAMPLED,
-        traceState: createTraceState("foo='bar,baz='qux',hack='DROP TABLE"),
+        traceState: createTraceState("foo='bar,baz='qux!()*',hack='DROP TABLE"),
       };
 
       const query = 'SELECT * from FOO;';
       assert.strictEqual(
         utils.addSqlCommenterComment(trace.wrapSpanContext(spanContext), query),
-        "SELECT * from FOO; /*traceparent='00-d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-01',tracestate='foo%3D%27bar%2Cbaz%3D%27qux%27%2Chack%3D%27DROP%20TABLE'*/"
+        "SELECT * from FOO; /*traceparent='00-d4cda95b652f4a1592b449d5929fda1b-6e0c63257de34c92-01',tracestate='foo%3D%27bar%2Cbaz%3D%27qux%21%28%29%2A%27%2Chack%3D%27DROP%20TABLE'*/"
       );
     });
   });

From 70ac1cfc0eb39813abd91bd408b1195671a8bd61 Mon Sep 17 00:00:00 2001
From: Henri Normak <henri.normak@gmail.com>
Date: Thu, 24 Nov 2022 17:05:52 +0200
Subject: [PATCH 17/17] docs: better document the edge-case in query comment
 detection

---
 plugins/node/opentelemetry-instrumentation-pg/README.md    | 2 +-
 plugins/node/opentelemetry-instrumentation-pg/src/utils.ts | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/README.md b/plugins/node/opentelemetry-instrumentation-pg/README.md
index 5233975b78..b14e036343 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/README.md
+++ b/plugins/node/opentelemetry-instrumentation-pg/README.md
@@ -49,7 +49,7 @@ PostgreSQL instrumentation has few options available to choose from. You can set
 | [`enhancedDatabaseReporting`](./src/types.ts#L30) | `boolean` | If true, additional information about query parameters and results will be attached (as `attributes`) to spans representing database operations |
 | `responseHook` | `PgInstrumentationExecutionResponseHook` (function) | Function for adding custom attributes from db response |
 | `requireParentSpan` | `boolean` | If true, requires a parent span to create new spans (default false) |
-| `addSqlCommenterCommentToQueries` | `boolean` | If true, adds [sqlcommenter](https://github.com/open-telemetry/opentelemetry-sqlcommenter) specification compliant comment to queries with tracing context |
+| `addSqlCommenterCommentToQueries` | `boolean` | If true, adds [sqlcommenter](https://github.com/open-telemetry/opentelemetry-sqlcommenter) specification compliant comment to queries with tracing context (default false). _NOTE: A comment will not be added to queries that already contain `--` or `/* ... */` in them, even if these are not actually part of comments_ |
 
 ## Useful links
 
diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
index 9a19bd6331..048c0d7ad0 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
@@ -293,6 +293,9 @@ export function patchClientConnectCallback(
   };
 }
 
+// NOTE: This function currently is returning false-positives
+// in cases where comment characters appear in string literals
+// ("SELECT '-- not a comment';" would return true, although has no comment)
 function hasValidSqlComment(query: string): boolean {
   const indexOpeningDashDashComment = query.indexOf('--');
   if (indexOpeningDashDashComment >= 0) {