fix(pg): Do not add SQLCommenter comments to prepared statements (#2456)

Co-authored-by: Amir Blum <[email protected]>
open-telemetry · Nov 4, 2024 · 8070c7f · 8070c7f
1 parent a5b5614
commit 8070c7f
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 19 deletions.
diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts
@@ -307,14 +307,17 @@ 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.
         if (instrumentationConfig.addSqlCommenterCommentToQueries) {
-          args[0] = firstArgIsString
-            ? addSqlCommenterComment(span, arg0)
-            : firstArgIsQueryObjectWithText
-            ? {
-                ...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
@@ -853,15 +853,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);
         }
@@ -879,15 +873,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);
@@ -952,6 +942,33 @@ 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();
+          assert.ok(span);
+
+          const executedQueries = getExecutedQueries();
+          assert.equal(executedQueries.length, 1);
+          assert.equal(executedQueries[0].text, query);
+        } 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,