fix(instrumentation-pg): connection string parsing (open-telemetry#2715)

david-luna · Feb 28, 2025 · b520d04 · b520d04
1 parent 353dbb0
commit b520d04
Show file tree

Hide file tree

Showing 4 changed files with 110 additions and 5 deletions.
diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/internal-types.ts b/plugins/node/opentelemetry-instrumentation-pg/src/internal-types.ts
@@ -53,6 +53,7 @@ export interface PgPoolOptionsParams {
   user: string;
   idleTimeoutMillis: number; // the minimum amount of time that an object may sit idle in the pool before it is eligible for eviction due to idle time
   maxClient: number; // maximum size of the pool
+  connectionString?: string; // connection string if provided directly
 }
 
 export const EVENT_LISTENERS_SET = Symbol(

diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts
@@ -106,7 +106,28 @@ export function parseNormalizedOperationName(queryText: string) {
   return sqlCommand.endsWith(';') ? sqlCommand.slice(0, -1) : sqlCommand;
 }
 
-export function getConnectionString(params: PgParsedConnectionParams) {
+export function parseAndMaskConnectionString(connectionString: string): string {
+  try {
+    // Parse the connection string
+    const url = new URL(connectionString);
+
+    // Remove all auth information (username and password)
+    url.username = '';
+    url.password = '';
+
+    return url.toString();
+  } catch (e) {
+    // If parsing fails, return a generic connection string
+    return 'postgresql://localhost:5432/';
+  }
+}
+
+export function getConnectionString(
+  params: PgParsedConnectionParams | PgPoolOptionsParams
+) {
+  if ('connectionString' in params && params.connectionString) {
+    return parseAndMaskConnectionString(params.connectionString);
+  }
   const host = params.host || 'localhost';
   const port = params.port || 5432;
   const database = params.database || '';
@@ -138,13 +159,22 @@ export function getSemanticAttributesFromConnection(
 }
 
 export function getSemanticAttributesFromPool(params: PgPoolOptionsParams) {
+  let url: URL | undefined;
+  try {
+    url = params.connectionString
+      ? new URL(params.connectionString)
+      : undefined;
+  } catch (e) {
+    url = undefined;
+  }
+
   return {
     [SEMATTRS_DB_SYSTEM]: DBSYSTEMVALUES_POSTGRESQL,
-    [SEMATTRS_DB_NAME]: params.database, // required
+    [SEMATTRS_DB_NAME]: url?.pathname.slice(1) ?? params.database, // required
     [SEMATTRS_DB_CONNECTION_STRING]: getConnectionString(params), // required
-    [SEMATTRS_NET_PEER_NAME]: params.host, // required
-    [SEMATTRS_NET_PEER_PORT]: getPort(params.port),
-    [SEMATTRS_DB_USER]: params.user,
+    [SEMATTRS_NET_PEER_NAME]: url?.hostname ?? params.host, // required
+    [SEMATTRS_NET_PEER_PORT]: Number(url?.port) || getPort(params.port),
+    [SEMATTRS_DB_USER]: url?.username ?? params.user,
     [AttributeNames.IDLE_TIMEOUT_MILLIS]: params.idleTimeoutMillis,
     [AttributeNames.MAX_CLIENT]: params.maxClient,
   };

diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts
@@ -221,6 +221,36 @@ describe('pg-pool', () => {
       });
     });
 
+    // Test connection string support
+    it('should handle connection string in pool options', async () => {
+      const connectionString = `postgresql://${CONFIG.user}:${CONFIG.password}@${CONFIG.host}:${CONFIG.port}/${CONFIG.database}`;
+      const poolWithConnString = new pgPool({
+        connectionString,
+        idleTimeoutMillis: CONFIG.idleTimeoutMillis,
+      });
+
+      const expectedAttributes = {
+        [SEMATTRS_DB_SYSTEM]: DBSYSTEMVALUES_POSTGRESQL,
+        [SEMATTRS_DB_NAME]: CONFIG.database,
+        [SEMATTRS_NET_PEER_NAME]: CONFIG.host,
+        [SEMATTRS_DB_CONNECTION_STRING]: `postgresql://${CONFIG.host}:${CONFIG.port}/${CONFIG.database}`,
+        [SEMATTRS_NET_PEER_PORT]: CONFIG.port,
+        [SEMATTRS_DB_USER]: CONFIG.user,
+        [AttributeNames.IDLE_TIMEOUT_MILLIS]: CONFIG.idleTimeoutMillis,
+      };
+
+      const events: TimedEvent[] = [];
+      const span = provider.getTracer('test-pg-pool').startSpan('test span');
+
+      await context.with(trace.setSpan(context.active(), span), async () => {
+        const client = await poolWithConnString.connect();
+        runCallbackTest(span, expectedAttributes, events, unsetStatus, 2, 1);
+        client.release();
+      });
+
+      await poolWithConnString.end();
+    });
+
     // callback - checkout a client
     it('should not return a promise if callback is provided', done => {
       const pgPoolAttributes = {

diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts
@@ -254,4 +254,48 @@ describe('utils.ts', () => {
       );
     });
   });
+
+  describe('.parseAndMaskConnectionString()', () => {
+    it('should remove all auth information from connection string', () => {
+      const connectionString =
+        'postgresql://user:password123@localhost:5432/dbname';
+      assert.strictEqual(
+        utils.parseAndMaskConnectionString(connectionString),
+        'postgresql://localhost:5432/dbname'
+      );
+    });
+
+    it('should remove username when no password is present', () => {
+      const connectionString = 'postgresql://user@localhost:5432/dbname';
+      assert.strictEqual(
+        utils.parseAndMaskConnectionString(connectionString),
+        'postgresql://localhost:5432/dbname'
+      );
+    });
+
+    it('should preserve connection string when no auth is present', () => {
+      const connectionString = 'postgresql://localhost:5432/dbname';
+      assert.strictEqual(
+        utils.parseAndMaskConnectionString(connectionString),
+        'postgresql://localhost:5432/dbname'
+      );
+    });
+
+    it('should preserve query parameters while removing auth', () => {
+      const connectionString =
+        'postgresql://user:pass@localhost/dbname?sslmode=verify-full&application_name=myapp';
+      assert.strictEqual(
+        utils.parseAndMaskConnectionString(connectionString),
+        'postgresql://localhost/dbname?sslmode=verify-full&application_name=myapp'
+      );
+    });
+
+    it('should handle invalid connection string', () => {
+      const connectionString = 'not-a-valid-url';
+      assert.strictEqual(
+        utils.parseAndMaskConnectionString(connectionString),
+        'postgresql://localhost:5432/'
+      );
+    });
+  });
 });