From e36da2dc0a9458b308f11194881baee42031bd1c Mon Sep 17 00:00:00 2001 From: maryliag Date: Tue, 29 Apr 2025 12:10:19 -0400 Subject: [PATCH 01/10] fix pg pool ESM --- .../src/instrumentation.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index 984da9d60d..1c8af9c15a 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -133,6 +133,7 @@ export class PgInstrumentation extends InstrumentationBase=8.0.3 <9']; + const SUPPORTED_PG_POOL_VERSIONS = ['>=2.0.0 <4']; const modulePgNativeClient = new InstrumentationNodeModuleFile( 'pg/lib/native/client.js', @@ -168,8 +169,9 @@ export class PgInstrumentation extends InstrumentationBase=2.0.0 <4'], - (moduleExports: typeof pgPoolTypes) => { + SUPPORTED_PG_POOL_VERSIONS, + (module: any) => { + const moduleExports = extractModuleExports(module); if (isWrapped(moduleExports.prototype.connect)) { this._unwrap(moduleExports.prototype, 'connect'); } @@ -180,7 +182,8 @@ export class PgInstrumentation extends InstrumentationBase { + (module: any) => { + const moduleExports = extractModuleExports(module); if (isWrapped(moduleExports.prototype.connect)) { this._unwrap(moduleExports.prototype, 'connect'); } From 8136008aad14b037ccf101361dbb40c098020491 Mon Sep 17 00:00:00 2001 From: maryliag Date: Tue, 6 May 2025 11:06:47 -0400 Subject: [PATCH 02/10] add test --- .../test/fixtures/use-pg-pool.mjs | 57 +++++++++++++++++++ .../test/pg-pool.test.ts | 29 ++++++++++ 2 files changed, 86 insertions(+) create mode 100644 plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs b/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs new file mode 100644 index 0000000000..59bc598409 --- /dev/null +++ b/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs @@ -0,0 +1,57 @@ +/* + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Use postgres from an ES module: +// node --experimental-loader=@opentelemetry/instrumentation/hook.mjs use-pg.mjs + +import { trace } from '@opentelemetry/api'; +import { createTestNodeSdk } from '@opentelemetry/contrib-test-utils'; +import assert from 'assert'; + +import { PgInstrumentation } from '../../build/src/index.js'; + +const CONFIG = { + user: process.env.POSTGRES_USER || 'postgres', + password: process.env.POSTGRES_PASSWORD || 'postgres', + database: process.env.POSTGRES_DB || 'postgres', + host: process.env.POSTGRES_HOST || 'localhost', + port: process.env.POSTGRES_PORT + ? parseInt(process.env.POSTGRES_PORT, 10) + : 54320, +}; + +const sdk = createTestNodeSdk({ + serviceName: 'use-pg-pool', + instrumentations: [new PgInstrumentation()], +}); +sdk.start(); + +import { Pool as PGPool } from 'pg'; +const pgPool = new PGPool(CONFIG); +const tracer = trace.getTracer(); + +await tracer.startActiveSpan('test-span', async span => { + pgPool.connect((connectErr, _, release) => { + assert.ifError(connectErr) + pgPool.query('SELECT NOW()', (err, res) => { + assert.ok(res); + assert.ifError(err) + }); + release(); + span.end(); + sdk.shutdown(); + }); +}); diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts index 4e8a257d5c..59f63d8e77 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -916,3 +916,32 @@ describe('pg-pool', () => { }); }); }); + +describe('pg-pool (ESM)', () => { + it('should work with ESM usage', async () => { + await testUtils.runTestFixture({ + cwd: __dirname, + argv: ['fixtures/use-pg-pool.mjs'], + env: { + NODE_OPTIONS: + '--experimental-loader=@opentelemetry/instrumentation/hook.mjs', + NODE_NO_WARNINGS: '1', + }, + checkResult: (err, stdout, stderr) => { + assert.ifError(err); + }, + checkCollector: (collector: testUtils.TestCollector) => { + const spans = collector.sortedSpans; + + assert.strictEqual(spans.length, 3); + + assert.strictEqual(spans[0].name, 'pgPool.connect'); + assert.strictEqual(spans[0].kind, 3); + assert.strictEqual(spans[1].name, 'test-span'); + assert.strictEqual(spans[1].kind, 1); + assert.strictEqual(spans[2].name, 'pgPool.query:SELECT NOW()'); + assert.strictEqual(spans[2].kind, 3); + }, + }); + }); +}); From b1be7faf2c7432b9c4751dd9fbb6d81d7e0574bc Mon Sep 17 00:00:00 2001 From: maryliag Date: Tue, 6 May 2025 11:30:08 -0400 Subject: [PATCH 03/10] use different import --- .../test/fixtures/use-pg-pool.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs b/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs index 59bc598409..9b2dbbc200 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs +++ b/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs @@ -39,7 +39,7 @@ const sdk = createTestNodeSdk({ }); sdk.start(); -import { Pool as PGPool } from 'pg'; +import * as PGPool from 'pg-pool'; const pgPool = new PGPool(CONFIG); const tracer = trace.getTracer(); From 0cedfc2d8ff93358afb7820316c197ae97fa2c64 Mon Sep 17 00:00:00 2001 From: maryliag Date: Tue, 6 May 2025 11:43:52 -0400 Subject: [PATCH 04/10] fix --- .../test/fixtures/use-pg-pool.mjs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs b/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs index 9b2dbbc200..3010ed5c9d 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs +++ b/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs @@ -39,8 +39,9 @@ const sdk = createTestNodeSdk({ }); sdk.start(); -import * as PGPool from 'pg-pool'; +const PGPool = require('pg-pool'); const pgPool = new PGPool(CONFIG); + const tracer = trace.getTracer(); await tracer.startActiveSpan('test-span', async span => { From d935e7d810962938728f2c96616f7b6233a27195 Mon Sep 17 00:00:00 2001 From: maryliag Date: Tue, 6 May 2025 12:13:04 -0400 Subject: [PATCH 05/10] fix --- .../test/fixtures/use-pg-pool.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs b/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs index 3010ed5c9d..b210666ef2 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs +++ b/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs @@ -39,8 +39,8 @@ const sdk = createTestNodeSdk({ }); sdk.start(); -const PGPool = require('pg-pool'); -const pgPool = new PGPool(CONFIG); +import pool from 'pg-pool'; +const pgPool = new pool(CONFIG); const tracer = trace.getTracer(); From f8470f113926b0dcf88bb24824f2ff723341d379 Mon Sep 17 00:00:00 2001 From: maryliag Date: Tue, 6 May 2025 13:04:04 -0400 Subject: [PATCH 06/10] add done --- .../opentelemetry-instrumentation-pg/test/pg-pool.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts index 59f63d8e77..b06d89ff39 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -918,6 +918,10 @@ describe('pg-pool', () => { }); describe('pg-pool (ESM)', () => { + after(done => { + done(); + }); + it('should work with ESM usage', async () => { await testUtils.runTestFixture({ cwd: __dirname, From 1e99187ac14a08c8b43ff437c044960eb6344a80 Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Tue, 6 May 2025 15:38:26 -0700 Subject: [PATCH 07/10] fixup pg-pool ESM test; align POSTGRES_DB value used for local and CI testing of instrumentation-pg This also fixes a couple missing 'pool.end()' calls in pg-pool.test.ts that would result in a 10s hang at the end of testing. --- .../src/test-utils.ts | 2 +- .../test/fixtures/use-pg-pool.mjs | 23 ++++----- .../test/fixtures/use-pg.mjs | 2 +- .../test/pg-pool.test.ts | 48 ++++++++++++++----- 4 files changed, 50 insertions(+), 25 deletions(-) diff --git a/packages/opentelemetry-test-utils/src/test-utils.ts b/packages/opentelemetry-test-utils/src/test-utils.ts index a64c9ad56a..28a6aa92af 100644 --- a/packages/opentelemetry-test-utils/src/test-utils.ts +++ b/packages/opentelemetry-test-utils/src/test-utils.ts @@ -43,7 +43,7 @@ const dockerRunCmds = { mysql: 'docker run --rm -d --name otel-mysql -p 33306:3306 -e MYSQL_ROOT_PASSWORD=rootpw -e MYSQL_DATABASE=test_db -e MYSQL_USER=otel -e MYSQL_PASSWORD=secret mysql:5.7 --log_output=TABLE --general_log=ON', postgres: - 'docker run --rm -d --name otel-postgres -p 54320:5432 -e POSTGRES_PASSWORD=postgres postgres:16-alpine', + 'docker run --rm -d --name otel-postgres -p 54320:5432 -e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=otel_pg_database postgres:16-alpine', redis: 'docker run --rm -d --name otel-redis -p 63790:6379 redis:alpine', }; diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs b/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs index b210666ef2..5bdc971a64 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs +++ b/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs @@ -26,7 +26,7 @@ import { PgInstrumentation } from '../../build/src/index.js'; const CONFIG = { user: process.env.POSTGRES_USER || 'postgres', password: process.env.POSTGRES_PASSWORD || 'postgres', - database: process.env.POSTGRES_DB || 'postgres', + database: process.env.POSTGRES_DB || 'otel_pg_database', host: process.env.POSTGRES_HOST || 'localhost', port: process.env.POSTGRES_PORT ? parseInt(process.env.POSTGRES_PORT, 10) @@ -39,20 +39,21 @@ const sdk = createTestNodeSdk({ }); sdk.start(); -import pool from 'pg-pool'; -const pgPool = new pool(CONFIG); +import Pool from 'pg-pool'; +const pgPool = new Pool(CONFIG); const tracer = trace.getTracer(); await tracer.startActiveSpan('test-span', async span => { - pgPool.connect((connectErr, _, release) => { - assert.ifError(connectErr) - pgPool.query('SELECT NOW()', (err, res) => { - assert.ok(res); - assert.ifError(err) - }); - release(); + const client = await pgPool.connect(); + try { + const res = await pgPool.query('SELECT NOW()'); + assert.ok(res); + console.log('rows:', res.rows); + } finally { + client.release(); + pgPool.end(); span.end(); sdk.shutdown(); - }); + } }); diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg.mjs b/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg.mjs index a9c8c5a6a4..619fffce7b 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg.mjs +++ b/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg.mjs @@ -26,7 +26,7 @@ import { PgInstrumentation } from '../../build/src/index.js'; const CONFIG = { user: process.env.POSTGRES_USER || 'postgres', password: process.env.POSTGRES_PASSWORD || 'postgres', - database: process.env.POSTGRES_DB || 'postgres', + database: process.env.POSTGRES_DB || 'otel_pg_database', host: process.env.POSTGRES_HOST || 'localhost', port: process.env.POSTGRES_PORT ? parseInt(process.env.POSTGRES_PORT, 10) diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts index b06d89ff39..ddb90293e1 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -650,6 +650,12 @@ describe('pg-pool', () => { it('should not add duplicate event listeners to PgPool events', done => { const poolAux: pgPool = new pgPool(CONFIG); + + const finish = () => { + poolAux.end(); + done(); + } + let completed = 0; poolAux.connect((err, client, release) => { if (err) { @@ -687,7 +693,7 @@ describe('pg-pool', () => { completed++; if (completed >= 2) { - done(); + finish(); } }); @@ -727,7 +733,7 @@ describe('pg-pool', () => { completed++; if (completed >= 2) { - done(); + finish(); } }); }); @@ -808,6 +814,8 @@ describe('pg-pool', () => { 1, 'expected to have 1 used connection' ); + + poolAux.end(); done(); }); }); @@ -817,6 +825,12 @@ describe('pg-pool', () => { const pool1: pgPool = new pgPool(CONFIG); const pool2: pgPool = new pgPool(CONFIG); + const finish = () => { + pool1.end(); + pool2.end(); + done(); + } + let completed = 0; pool1.connect((err, client, release) => { if (err) { @@ -862,7 +876,7 @@ describe('pg-pool', () => { completed++; if (completed >= 2) { - done(); + finish(); } }); @@ -910,7 +924,7 @@ describe('pg-pool', () => { completed++; if (completed >= 2) { - done(); + finish(); } }); }); @@ -937,14 +951,24 @@ describe('pg-pool (ESM)', () => { checkCollector: (collector: testUtils.TestCollector) => { const spans = collector.sortedSpans; - assert.strictEqual(spans.length, 3); - - assert.strictEqual(spans[0].name, 'pgPool.connect'); - assert.strictEqual(spans[0].kind, 3); - assert.strictEqual(spans[1].name, 'test-span'); - assert.strictEqual(spans[1].kind, 1); - assert.strictEqual(spans[2].name, 'pgPool.query:SELECT NOW()'); - assert.strictEqual(spans[2].kind, 3); + assert.strictEqual(spans.length, 6); + + let span = spans.shift()!; + assert.strictEqual(span.name, 'test-span'); + assert.strictEqual(span.kind, 1 /* OtlpSpanKind.INTERNAL */); + const expectedRemainingSpanNames = [ + // I believe two sets of `*.connect` spans because pg-pool opens + // two connections to start. + 'pg-pool.connect', + 'pg.connect', + 'pg-pool.connect', + 'pg.connect', + 'pg.query:SELECT otel_pg_database' + ]; + for (let expectedName of expectedRemainingSpanNames) { + span = spans.shift()!; + assert.strictEqual(span.name, expectedName); + } }, }); }); From c4056f8d64f10258db3d1a9628c049b21ab32a1e Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Tue, 6 May 2025 15:42:52 -0700 Subject: [PATCH 08/10] fix lint --- .../opentelemetry-instrumentation-pg/test/pg-pool.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts index ddb90293e1..e91000bb1c 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -654,7 +654,7 @@ describe('pg-pool', () => { const finish = () => { poolAux.end(); done(); - } + }; let completed = 0; poolAux.connect((err, client, release) => { @@ -829,7 +829,7 @@ describe('pg-pool', () => { pool1.end(); pool2.end(); done(); - } + }; let completed = 0; pool1.connect((err, client, release) => { @@ -963,9 +963,9 @@ describe('pg-pool (ESM)', () => { 'pg.connect', 'pg-pool.connect', 'pg.connect', - 'pg.query:SELECT otel_pg_database' + 'pg.query:SELECT otel_pg_database', ]; - for (let expectedName of expectedRemainingSpanNames) { + for (const expectedName of expectedRemainingSpanNames) { span = spans.shift()!; assert.strictEqual(span.name, expectedName); } From de6efe4e4afbd31f0598dafb5bd5b407a1aa1afd Mon Sep 17 00:00:00 2001 From: Trent Mick Date: Tue, 6 May 2025 15:48:05 -0700 Subject: [PATCH 09/10] don't need this --- .../opentelemetry-instrumentation-pg/test/pg-pool.test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts index e91000bb1c..bbb2d00513 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -932,10 +932,6 @@ describe('pg-pool', () => { }); describe('pg-pool (ESM)', () => { - after(done => { - done(); - }); - it('should work with ESM usage', async () => { await testUtils.runTestFixture({ cwd: __dirname, From bb00805ae5f128382db9bea8a8dd019f0998476d Mon Sep 17 00:00:00 2001 From: Marylia Gutierrez Date: Wed, 7 May 2025 16:34:34 -0400 Subject: [PATCH 10/10] Update plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs Co-authored-by: Marc Pichler --- .../test/fixtures/use-pg-pool.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs b/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs index 5bdc971a64..559e535e75 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs +++ b/plugins/node/opentelemetry-instrumentation-pg/test/fixtures/use-pg-pool.mjs @@ -14,8 +14,8 @@ * limitations under the License. */ -// Use postgres from an ES module: -// node --experimental-loader=@opentelemetry/instrumentation/hook.mjs use-pg.mjs +// Use pg-pool from an ES module: +// node --experimental-loader=@opentelemetry/instrumentation/hook.mjs use-pg-pool.mjs import { trace } from '@opentelemetry/api'; import { createTestNodeSdk } from '@opentelemetry/contrib-test-utils';