From bacaab4630383d6d5f28006ae86e248ec8aadfbf Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Th=C3=A9riault?=
 <raphael.theriault@solarwinds.com>
Date: Fri, 27 Sep 2024 11:19:01 -0400
Subject: [PATCH 1/2] fix(pg): do not add comment to prepared queries

---
 .../src/instrumentation.ts                    |  3 +-
 .../test/pg.test.ts                           | 31 +++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts
index e7c3da7a5f..f0f68c86b1 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts
@@ -297,10 +297,11 @@ export class PgInstrumentation extends InstrumentationBase<PgInstrumentationConf
 
         // Modify query text w/ a tracing comment before invoking original for
         // tracing, but only if args[0] has one of our expected shapes.
+        // Also omit the tracing comment if args[0] is a prepared query object.
         if (instrumentationConfig.addSqlCommenterCommentToQueries) {
           args[0] = firstArgIsString
             ? addSqlCommenterComment(span, arg0)
-            : firstArgIsQueryObjectWithText
+            : firstArgIsQueryObjectWithText && !('name' in arg0)
             ? {
                 ...arg0,
                 text: addSqlCommenterComment(span, arg0.text),
diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts
index b394893399..706cc9b43a 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts
@@ -948,6 +948,37 @@ describe('pg', () => {
       });
     });
 
+    it('should not add sqlcommenter comment when addSqlCommenterCommentToQueries=true is specified with a prepared statement', 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({
+            text: query,
+            name: 'prepared sqlcommenter',
+          });
+          assert.ok(resPromise);
+
+          const [span] = memoryExporter.getFinishedSpans();
+          const commentedQuery = addSqlCommenterComment(
+            trace.wrapSpanContext(span.spanContext()),
+            query
+          );
+
+          const executedQueries = getExecutedQueries();
+          assert.equal(executedQueries.length, 1);
+          assert.equal(executedQueries[0].text, query);
+          assert.notEqual(query, commentedQuery);
+        } catch (e: any) {
+          assert.ok(false, e.message);
+        }
+      });
+    });
+
     it('should not generate traces for client.query() when requireParentSpan=true is specified', done => {
       instrumentation.setConfig({
         requireParentSpan: true,

From df5ec4ac1c261548fc389b4e957d1e6377568462 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rapha=C3=ABl=20Th=C3=A9riault?=
 <raphael.theriault@solarwinds.com>
Date: Fri, 1 Nov 2024 16:04:57 -0400
Subject: [PATCH 2/2] chore(instrumentation-pg): refactor ternary & some tests

---
 .../src/instrumentation.ts                    | 20 ++++++++++---------
 .../test/pg.test.ts                           | 18 ++---------------
 2 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts
index f0f68c86b1..42de6e28b2 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts
@@ -297,16 +297,18 @@ export class PgInstrumentation extends InstrumentationBase<PgInstrumentationConf
 
         // Modify query text w/ a tracing comment before invoking original for
         // tracing, but only if args[0] has one of our expected shapes.
-        // Also omit the tracing comment if args[0] is a prepared query object.
         if (instrumentationConfig.addSqlCommenterCommentToQueries) {
-          args[0] = firstArgIsString
-            ? addSqlCommenterComment(span, arg0)
-            : firstArgIsQueryObjectWithText && !('name' in arg0)
-            ? {
-                ...arg0,
-                text: addSqlCommenterComment(span, arg0.text),
-              }
-            : args[0];
+          if (firstArgIsString) {
+            args[0] = addSqlCommenterComment(span, arg0);
+          } else if (firstArgIsQueryObjectWithText && !('name' in arg0)) {
+            // In the case of a query object, we need to ensure there's no name field
+            // as this indicates a prepared query, where the comment would remain the same
+            // for every invocation and contain an outdated trace context.
+            args[0] = {
+              ...arg0,
+              text: addSqlCommenterComment(span, arg0.text),
+            };
+          }
         }
 
         // Bind callback (if any) to parent span (if any)
diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts
index 706cc9b43a..17a72ecfa2 100644
--- a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts
+++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts
@@ -849,15 +849,9 @@ describe('pg', () => {
           const [span] = memoryExporter.getFinishedSpans();
           assert.ok(span);
 
-          const commentedQuery = addSqlCommenterComment(
-            trace.wrapSpanContext(span.spanContext()),
-            query
-          );
-
           const executedQueries = getExecutedQueries();
           assert.equal(executedQueries.length, 1);
           assert.equal(executedQueries[0].text, query);
-          assert.notEqual(query, commentedQuery);
         } catch (e: any) {
           assert.ok(false, e.message);
         }
@@ -875,15 +869,11 @@ describe('pg', () => {
             assert.ok(res);
 
             const [span] = memoryExporter.getFinishedSpans();
-            const commentedQuery = addSqlCommenterComment(
-              trace.wrapSpanContext(span.spanContext()),
-              query
-            );
+            assert.ok(span);
 
             const executedQueries = getExecutedQueries();
             assert.equal(executedQueries.length, 1);
             assert.equal(executedQueries[0].text, query);
-            assert.notEqual(query, commentedQuery);
             done();
           },
         } as pg.QueryConfig);
@@ -964,15 +954,11 @@ describe('pg', () => {
           assert.ok(resPromise);
 
           const [span] = memoryExporter.getFinishedSpans();
-          const commentedQuery = addSqlCommenterComment(
-            trace.wrapSpanContext(span.spanContext()),
-            query
-          );
+          assert.ok(span);
 
           const executedQueries = getExecutedQueries();
           assert.equal(executedQueries.length, 1);
           assert.equal(executedQueries[0].text, query);
-          assert.notEqual(query, commentedQuery);
         } catch (e: any) {
           assert.ok(false, e.message);
         }