From 19567fd1ba97e8d5bd3b17694385e1393be88759 Mon Sep 17 00:00:00 2001 From: maryliag Date: Wed, 24 Jul 2024 09:57:19 -0400 Subject: [PATCH 01/34] initial implementation of prototype --- package-lock.json | 16 ++-- .../package.json | 2 +- .../src/instrumentation.ts | 86 ++++++++++++++++++- .../src/utils.ts | 46 ++++++++++ 4 files changed, 140 insertions(+), 10 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8e4f861947..7a0e12c1c9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -28679,9 +28679,9 @@ } }, "node_modules/pg-pool": { - "version": "3.4.1", - "resolved": "https://registry.npmjs.org/pg-pool/-/pg-pool-3.4.1.tgz", - "integrity": "sha512-TVHxR/gf3MeJRvchgNHxsYsTCHQ+4wm3VIHSS19z8NC0+gioEhq1okDY1sm/TYbfoP6JLFx01s0ShvZ3puP/iQ==", + "version": "3.6.2", + "resolved": "https://registry.npmjs.org/pg-pool/-/pg-pool-3.6.2.tgz", + "integrity": "sha512-Htjbg8BlwXqSBQ9V8Vjtc+vzf/6fVUuak/3/XXKA9oxZprwW3IMDQTGHP+KDmVL7rtd+R1QjbnCFPuTHm3G4hg==", "dev": true, "peerDependencies": { "pg": ">=8.0" @@ -38940,7 +38940,7 @@ "mocha": "7.2.0", "nyc": "15.1.0", "pg": "8.7.1", - "pg-pool": "3.4.1", + "pg-pool": "3.6.2", "rimraf": "5.0.5", "safe-stable-stringify": "^2.4.1", "sinon": "15.2.0", @@ -47935,7 +47935,7 @@ "mocha": "7.2.0", "nyc": "15.1.0", "pg": "8.7.1", - "pg-pool": "3.4.1", + "pg-pool": "3.6.2", "rimraf": "5.0.5", "safe-stable-stringify": "^2.4.1", "sinon": "15.2.0", @@ -64688,9 +64688,9 @@ "integrity": "sha512-WCtabS6t3c8SkpDBUlb1kjOs7l66xsGdKpIPZsg4wR+B3+u9UAum2odSsF9tnvxg80h4ZxLWMy4pRjOsFIqQpw==" }, "pg-pool": { - "version": "3.4.1", - "resolved": "https://registry.npmjs.org/pg-pool/-/pg-pool-3.4.1.tgz", - "integrity": "sha512-TVHxR/gf3MeJRvchgNHxsYsTCHQ+4wm3VIHSS19z8NC0+gioEhq1okDY1sm/TYbfoP6JLFx01s0ShvZ3puP/iQ==", + "version": "3.6.2", + "resolved": "https://registry.npmjs.org/pg-pool/-/pg-pool-3.6.2.tgz", + "integrity": "sha512-Htjbg8BlwXqSBQ9V8Vjtc+vzf/6fVUuak/3/XXKA9oxZprwW3IMDQTGHP+KDmVL7rtd+R1QjbnCFPuTHm3G4hg==", "dev": true, "requires": {} }, diff --git a/plugins/node/opentelemetry-instrumentation-pg/package.json b/plugins/node/opentelemetry-instrumentation-pg/package.json index 18c0de2ca1..8fd6284758 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/package.json +++ b/plugins/node/opentelemetry-instrumentation-pg/package.json @@ -62,7 +62,7 @@ "mocha": "7.2.0", "nyc": "15.1.0", "pg": "8.7.1", - "pg-pool": "3.4.1", + "pg-pool": "3.6.2", "rimraf": "5.0.5", "safe-stable-stringify": "^2.4.1", "sinon": "15.2.0", diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index c6bee00999..21e1c84201 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -26,6 +26,8 @@ import { Span, SpanStatusCode, SpanKind, + MeterProvider, + UpDownCounter, } from '@opentelemetry/api'; import type * as pgTypes from 'pg'; import type * as pgPoolTypes from 'pg-pool'; @@ -43,8 +45,30 @@ import { PACKAGE_NAME, PACKAGE_VERSION } from './version'; import { SpanNames } from './enums/SpanNames'; export class PgInstrumentation extends InstrumentationBase { + private _connectionsCount!: UpDownCounter; + private _connectionsCounter: utils.poolConnectionsCounter = { + used: 0, idle: 0 + }; + constructor(config: PgInstrumentationConfig = {}) { super(PACKAGE_NAME, PACKAGE_VERSION, config); + this._setMetricInstruments(); + } + + override setMeterProvider(meterProvider: MeterProvider) { + super.setMeterProvider(meterProvider); + this._setMetricInstruments(); + } + + private _setMetricInstruments() { + this._connectionsCount = this.meter.createUpDownCounter( + 'db.client.connection.count', + { + description: + 'The number of connections that are currently in state described by the state attribute.', + unit: '{connection}', + } + ); } protected init() { @@ -96,17 +120,29 @@ export class PgInstrumentation extends InstrumentationBase { if (isWrapped(moduleExports.prototype.connect)) { this._unwrap(moduleExports.prototype, 'connect'); } + if (isWrapped(moduleExports.prototype.end)) { + this._unwrap(moduleExports.prototype, 'end'); + } + this._wrap( moduleExports.prototype, 'connect', this._getPoolConnectPatch() as any ); + this._wrap( + moduleExports.prototype, + 'end', + this._getPoolEndPatch() as any + ); return moduleExports; }, (moduleExports: typeof pgPoolTypes) => { if (isWrapped(moduleExports.prototype.connect)) { this._unwrap(moduleExports.prototype, 'connect'); } + if (isWrapped(moduleExports.prototype.end)) { + this._unwrap(moduleExports.prototype, 'end'); + } } ); @@ -323,7 +359,6 @@ export class PgInstrumentation extends InstrumentationBase { }); }); } - // else returns void return result; // void }; @@ -344,6 +379,39 @@ export class PgInstrumentation extends InstrumentationBase { attributes: utils.getSemanticAttributesFromPool(this.options), }); + this.on('connect', connection => { + plugin._connectionsCounter = utils.updateCounter( + this, + plugin._connectionsCount, + plugin._connectionsCounter, + ) + }); + + this.on('acquire', connection => { + plugin._connectionsCounter = utils.updateCounter( + this, + plugin._connectionsCount, + plugin._connectionsCounter, + ) + }); + + this.on('remove', connection => { + plugin._connectionsCounter = utils.updateCounter( + this, + plugin._connectionsCount, + plugin._connectionsCounter, + ) + }); + + // TODO check why is not recognizing release (e.g. version) + // this.on('release', connection => { + // plugin._connectionsCounter = utils.updateCounter( + // this, + // plugin._connectionsCount, + // plugin._connectionsCounter, + // ) + // }); + if (callback) { const parentSpan = trace.getSpan(context.active()); callback = utils.patchCallbackPGPool( @@ -367,6 +435,22 @@ export class PgInstrumentation extends InstrumentationBase { }; }; } + + private _getPoolEndPatch() { + const plugin = this; + return (originalPoolEnd: typeof pgPoolTypes.prototype.end) => { + return function end(this: PgPoolExtended, callback?: PgPoolCallback) { + if (utils.shouldSkipInstrumentation(plugin.getConfig())) { + return originalPoolEnd.call(this, callback as any); + } + plugin._connectionsCounter = utils.updateCounter( + this, + plugin._connectionsCount, + plugin._connectionsCounter, + ) + } + }; + } } function handleConnectResult(span: Span, connectResult: unknown) { diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts index a16ccc9cd0..0fc93b10d7 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts @@ -22,6 +22,7 @@ import { Tracer, SpanKind, diag, + UpDownCounter, } from '@opentelemetry/api'; import { AttributeNames } from './enums/AttributeNames'; import { @@ -258,6 +259,51 @@ export function patchCallback( }; } +export function getPoolName(pool: PgPoolOptionsParams): string { + if (!pool) { + return "unknown_pool"; + } + let poolName = ''; + poolName += pool.user ? `${pool.user}@` : 'unknown_user'; + poolName += pool.host ? `${pool.host}:` : 'unknown_host'; + poolName += pool.port ? `${pool.port}/` : 'unknown_port'; + poolName += pool.database ? `${pool.database}` : 'unknown_database'; + + return poolName.trim(); +} + +export interface poolConnectionsCounter { + used: number; + idle: number; +} + +export function updateCounter( + pool: PgPoolExtended, + connectionsCount: UpDownCounter, + latestCounter: poolConnectionsCounter, +): poolConnectionsCounter { + const poolName = getPoolName(pool.options); + const all = pool.totalCount; + const idle = pool.waitingCount; + const used = all - idle; + + if (used != latestCounter.used) { + connectionsCount.add(used - latestCounter.used, { + state: 'used', + name: poolName, + }); + } + + if (idle != latestCounter.idle) { + connectionsCount.add(idle - latestCounter.idle, { + state: 'idle', + name: poolName, + }); + } + + return {used: used, idle: idle}; +} + export function patchCallbackPGPool( span: Span, cb: PgPoolCallback From 99fc72df1a892dcaa19a24d1910dfaae348fa27f Mon Sep 17 00:00:00 2001 From: maryliag Date: Wed, 24 Jul 2024 10:06:14 -0400 Subject: [PATCH 02/34] update after rebase --- package-lock.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package-lock.json b/package-lock.json index 38c9e692ac..aa453223f4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -282,6 +282,7 @@ "dev": true }, "metapackages/auto-configuration-propagators": { + "name": "@opentelemetry/auto-configuration-propagators", "version": "0.1.0", "license": "Apache-2.0", "dependencies": { From 87da206bca23e9cd11a770e488635178020a2153 Mon Sep 17 00:00:00 2001 From: maryliag Date: Wed, 24 Jul 2024 10:50:11 -0400 Subject: [PATCH 03/34] fix lint --- .../node/opentelemetry-instrumentation-pg/src/utils.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts index 0fc93b10d7..febb509790 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts @@ -261,7 +261,7 @@ export function patchCallback( export function getPoolName(pool: PgPoolOptionsParams): string { if (!pool) { - return "unknown_pool"; + return 'unknown_pool'; } let poolName = ''; poolName += pool.user ? `${pool.user}@` : 'unknown_user'; @@ -280,28 +280,28 @@ export interface poolConnectionsCounter { export function updateCounter( pool: PgPoolExtended, connectionsCount: UpDownCounter, - latestCounter: poolConnectionsCounter, + latestCounter: poolConnectionsCounter ): poolConnectionsCounter { const poolName = getPoolName(pool.options); const all = pool.totalCount; const idle = pool.waitingCount; const used = all - idle; - if (used != latestCounter.used) { + if (used !== latestCounter.used) { connectionsCount.add(used - latestCounter.used, { state: 'used', name: poolName, }); } - if (idle != latestCounter.idle) { + if (idle !== latestCounter.idle) { connectionsCount.add(idle - latestCounter.idle, { state: 'idle', name: poolName, }); } - return {used: used, idle: idle}; + return { used: used, idle: idle }; } export function patchCallbackPGPool( From db622c0039d6c82bcde80ed0730e5a2f423ef82a Mon Sep 17 00:00:00 2001 From: maryliag Date: Wed, 24 Jul 2024 10:51:42 -0400 Subject: [PATCH 04/34] fix lint --- .../src/instrumentation.ts | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index 3169bcf717..f0a6d39002 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -45,10 +45,10 @@ import { PACKAGE_NAME, PACKAGE_VERSION } from './version'; import { SpanNames } from './enums/SpanNames'; export class PgInstrumentation extends InstrumentationBase { - private _connectionsCount!: UpDownCounter; private _connectionsCounter: utils.poolConnectionsCounter = { - used: 0, idle: 0 + used: 0, + idle: 0 }; constructor(config: PgInstrumentationConfig = {}) { @@ -374,24 +374,24 @@ export class PgInstrumentation extends InstrumentationBase { plugin._connectionsCounter = utils.updateCounter( this, plugin._connectionsCount, - plugin._connectionsCounter, - ) + plugin._connectionsCounter + ); }); this.on('remove', connection => { plugin._connectionsCounter = utils.updateCounter( this, plugin._connectionsCount, - plugin._connectionsCounter, - ) + plugin._connectionsCounter + ); }); // TODO check why is not recognizing release (e.g. version) @@ -437,9 +437,9 @@ export class PgInstrumentation extends InstrumentationBase Date: Wed, 24 Jul 2024 11:41:26 -0400 Subject: [PATCH 05/34] fix lint --- .../opentelemetry-instrumentation-pg/src/instrumentation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index f0a6d39002..24f6344ab7 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -48,7 +48,7 @@ export class PgInstrumentation extends InstrumentationBase Date: Wed, 24 Jul 2024 14:01:36 -0400 Subject: [PATCH 06/34] add poolname test --- .../src/instrumentation.ts | 1 + .../src/utils.ts | 11 ++++------ .../test/utils.test.ts | 20 ++++++++++++++++++- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index 24f6344ab7..a082b077a3 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -439,6 +439,7 @@ export class PgInstrumentation extends InstrumentationBase { ); }); }); + + describe('.getPoolName()', () => { + it('creation of pool name based on pool config', () => { + const dummyPool: PgPoolOptionsParams = { + host: 'host_name', + port: 1234, + user: 'username', + database: 'database_name', + idleTimeoutMillis: 10, + maxClient: 5, + } + + assert.strictEqual( + utils.getPoolName(dummyPool), + 'username@host_name:1234/database_name' + ) + }); + }); }); From 052637541a2ddb5d62d074a65567c23d23912a94 Mon Sep 17 00:00:00 2001 From: maryliag Date: Wed, 24 Jul 2024 14:24:45 -0400 Subject: [PATCH 07/34] add callback for release --- .../src/instrumentation.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index a082b077a3..d8a20b735f 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -395,13 +395,14 @@ export class PgInstrumentation extends InstrumentationBase { - // plugin._connectionsCounter = utils.updateCounter( - // this, - // plugin._connectionsCount, - // plugin._connectionsCounter, - // ) - // }); + this.on('release' as any, connection => { + console.log("RELEASE FROM PLUGIN!!"); + plugin._connectionsCounter = utils.updateCounter( + this, + plugin._connectionsCount, + plugin._connectionsCounter, + ) + }); if (callback) { const parentSpan = trace.getSpan(context.active()); From 9b479995dcd8cdd0e305b19e96e9df31233b8ac8 Mon Sep 17 00:00:00 2001 From: maryliag Date: Wed, 24 Jul 2024 14:30:30 -0400 Subject: [PATCH 08/34] remove no longer valid comment --- .../node/opentelemetry-instrumentation-pg/src/instrumentation.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index d8a20b735f..39fd0e5993 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -394,7 +394,6 @@ export class PgInstrumentation extends InstrumentationBase { console.log("RELEASE FROM PLUGIN!!"); plugin._connectionsCounter = utils.updateCounter( From 7d502d364f7c77e9dd51c7e9ae043a8b5c42031b Mon Sep 17 00:00:00 2001 From: maryliag Date: Wed, 24 Jul 2024 15:11:26 -0400 Subject: [PATCH 09/34] fix parameter used for idle --- .../opentelemetry-instrumentation-pg/src/instrumentation.ts | 1 - plugins/node/opentelemetry-instrumentation-pg/src/utils.ts | 2 +- .../node/opentelemetry-instrumentation-pg/test/utils.test.ts | 4 ++-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index 39fd0e5993..e03e89679f 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -395,7 +395,6 @@ export class PgInstrumentation extends InstrumentationBase { - console.log("RELEASE FROM PLUGIN!!"); plugin._connectionsCounter = utils.updateCounter( this, plugin._connectionsCount, diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts index 28fbbb62a1..ca092d39b0 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts @@ -281,7 +281,7 @@ export function updateCounter( ): poolConnectionsCounter { const poolName = getPoolName(pool.options); const all = pool.totalCount; - const idle = pool.waitingCount; + const idle = pool.idleCount; const used = all - idle; if (used !== latestCounter.used) { diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts index 826b444949..43113ac667 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts @@ -248,12 +248,12 @@ describe('utils.ts', () => { database: 'database_name', idleTimeoutMillis: 10, maxClient: 5, - } + }; assert.strictEqual( utils.getPoolName(dummyPool), 'username@host_name:1234/database_name' - ) + ); }); }); }); From 66bb46370001332c7c5aaf8345fa78116f8a144a Mon Sep 17 00:00:00 2001 From: maryliag Date: Thu, 25 Jul 2024 08:33:10 -0400 Subject: [PATCH 10/34] fix lint --- .../opentelemetry-instrumentation-pg/src/instrumentation.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index e03e89679f..022f09376e 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -398,8 +398,8 @@ export class PgInstrumentation extends InstrumentationBase Date: Fri, 26 Jul 2024 09:24:14 -0400 Subject: [PATCH 11/34] add tests for new metric --- .../src/instrumentation.ts | 8 +- .../test/pg-pool.test.ts | 115 ++++++++++++++++-- 2 files changed, 106 insertions(+), 17 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index 022f09376e..8933faab90 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -370,7 +370,7 @@ export class PgInstrumentation extends InstrumentationBase { + this.on('connect', () => { plugin._connectionsCounter = utils.updateCounter( this, plugin._connectionsCount, @@ -378,7 +378,7 @@ export class PgInstrumentation extends InstrumentationBase { + this.on('acquire', () => { plugin._connectionsCounter = utils.updateCounter( this, plugin._connectionsCount, @@ -386,7 +386,7 @@ export class PgInstrumentation extends InstrumentationBase { + this.on('remove', () => { plugin._connectionsCounter = utils.updateCounter( this, plugin._connectionsCount, @@ -394,7 +394,7 @@ export class PgInstrumentation extends InstrumentationBase { + this.on('release' as any, () => { plugin._connectionsCounter = utils.updateCounter( this, plugin._connectionsCount, 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 62bfaf1711..5aed564556 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -35,6 +35,13 @@ import { InMemorySpanExporter, SimpleSpanProcessor, } from '@opentelemetry/sdk-trace-base'; +import { + AggregationTemporality, + InMemoryMetricExporter, + MeterProvider, + PeriodicExportingMetricReader, + ResourceMetrics, +} from '@opentelemetry/sdk-metrics'; import * as assert from 'assert'; import * as pg from 'pg'; import * as pgPool from 'pg-pool'; @@ -110,6 +117,22 @@ describe('pg-pool', () => { instrumentation.enable(); } + async function waitForNumberOfExports( + exporter: InMemoryMetricExporter, + numberOfExports: number + ): Promise { + if (numberOfExports <= 0) { + throw new Error('numberOfExports must be greater than or equal to 0'); + } + let totalExports = 0; + while (totalExports < numberOfExports) { + await new Promise(resolve => setTimeout(resolve, 20)); + const exportedMetrics = exporter.getMetrics(); + totalExports = exportedMetrics.length; + } + return exporter.getMetrics(); + } + let pool: pgPool; let contextManager: AsyncHooksContextManager; let instrumentation: PgInstrumentation; @@ -180,7 +203,7 @@ describe('pg-pool', () => { describe('#pool.connect()', () => { // promise - checkout a client it('should intercept pool.connect()', async () => { - const pgPoolattributes = { + const pgPoolAttributes = { ...DEFAULT_PGPOOL_ATTRIBUTES, }; const pgAttributes = { @@ -191,7 +214,7 @@ describe('pg-pool', () => { const span = provider.getTracer('test-pg-pool').startSpan('test span'); await context.with(trace.setSpan(context.active(), span), async () => { const client = await pool.connect(); - runCallbackTest(span, pgPoolattributes, events, unsetStatus, 2, 1); + runCallbackTest(span, pgPoolAttributes, events, unsetStatus, 2, 1); const [connectSpan, poolConnectSpan] = memoryExporter.getFinishedSpans(); @@ -212,7 +235,7 @@ describe('pg-pool', () => { // callback - checkout a client it('should not return a promise if callback is provided', done => { - const pgPoolattributes = { + const pgPoolAttributes = { ...DEFAULT_PGPOOL_ATTRIBUTES, }; const pgAttributes = { @@ -237,7 +260,7 @@ describe('pg-pool', () => { assert.ok(client); runCallbackTest( parentSpan, - pgPoolattributes, + pgPoolAttributes, events, unsetStatus, 1, @@ -285,7 +308,7 @@ describe('pg-pool', () => { describe('#pool.query()', () => { // promise it('should call patched client.query()', async () => { - const pgPoolattributes = { + const pgPoolAttributes = { ...DEFAULT_PGPOOL_ATTRIBUTES, }; const pgAttributes = { @@ -296,7 +319,7 @@ describe('pg-pool', () => { const span = provider.getTracer('test-pg-pool').startSpan('test span'); await context.with(trace.setSpan(context.active(), span), async () => { const result = await pool.query('SELECT NOW()'); - runCallbackTest(span, pgPoolattributes, events, unsetStatus, 2, 0); + runCallbackTest(span, pgPoolAttributes, events, unsetStatus, 2, 0); runCallbackTest(span, pgAttributes, events, unsetStatus, 2, 1); assert.ok(result, 'pool.query() returns a promise'); }); @@ -304,7 +327,7 @@ describe('pg-pool', () => { // callback it('should not return a promise if callback is provided', done => { - const pgPoolattributes = { + const pgPoolAttributes = { ...DEFAULT_PGPOOL_ATTRIBUTES, }; const pgAttributes = { @@ -322,7 +345,7 @@ describe('pg-pool', () => { } runCallbackTest( parentSpan, - pgPoolattributes, + pgPoolAttributes, events, unsetStatus, 2, @@ -341,7 +364,7 @@ describe('pg-pool', () => { const events: TimedEvent[] = []; describe('AND valid responseHook', () => { - const pgPoolattributes = { + const pgPoolAttributes = { ...DEFAULT_PGPOOL_ATTRIBUTES, }; const pgAttributes = { @@ -375,7 +398,7 @@ describe('pg-pool', () => { } runCallbackTest( parentSpan, - pgPoolattributes, + pgPoolAttributes, events, unsetStatus, 2, @@ -409,7 +432,7 @@ describe('pg-pool', () => { const result = await pool.query(query); runCallbackTest( span, - pgPoolattributes, + pgPoolAttributes, events, unsetStatus, 2, @@ -423,7 +446,7 @@ describe('pg-pool', () => { }); describe('AND invalid responseHook', () => { - const pgPoolattributes = { + const pgPoolAttributes = { ...DEFAULT_PGPOOL_ATTRIBUTES, }; const pgAttributes = { @@ -456,7 +479,7 @@ describe('pg-pool', () => { runCallbackTest( parentSpan, - pgPoolattributes, + pgPoolAttributes, events, unsetStatus, 2, @@ -482,4 +505,70 @@ describe('pg-pool', () => { }); }); }); + + describe('pg metrics', () => { + let otelTestingMeterProvider; + let inMemoryMetricsExporter: InMemoryMetricExporter; + + function initMeterProvider() { + otelTestingMeterProvider = new MeterProvider(); + inMemoryMetricsExporter = new InMemoryMetricExporter( + AggregationTemporality.CUMULATIVE + ); + const metricReader = new PeriodicExportingMetricReader({ + exporter: inMemoryMetricsExporter, + exportIntervalMillis: 100, + exportTimeoutMillis: 100, + }); + + otelTestingMeterProvider.addMetricReader(metricReader); + instrumentation.setMeterProvider(otelTestingMeterProvider); + } + + beforeEach(() => { + initMeterProvider(); + inMemoryMetricsExporter.reset(); + }); + + it('should generate `db.client.connection.count` metric', async () => { + const span = provider.getTracer('test-pg-pool').startSpan('test span'); + context.with(trace.setSpan(context.active(), span), () => { + pool.connect((err, client, release) => { + if (err) { + throw new Error(err.message); + } + if (!release) { + throw new Error('Did not receive release function'); + } + if (!client) { + throw new Error('No client received'); + } + assert.ok(client); + + client.query('SELECT NOW()', async (err, ret) => { + release(); + if (err) { + throw new Error(err.message); + } + assert.ok(ret); + + const exportedMetrics = await waitForNumberOfExports( + inMemoryMetricsExporter, + 2 + ); + const metrics = exportedMetrics[1].scopeMetrics[0].metrics; + + assert.strictEqual( + metrics[0].descriptor.name, + 'db.client.connection.count' + ); + assert.strictEqual( + metrics[0].descriptor.description, + 'The number of connections that are currently in state described by the state attribute.' + ); + }); + }); + }); + }); + }); }); From c785caffcc5dd83898d41aab6f44acea80f03b63 Mon Sep 17 00:00:00 2001 From: maryliag Date: Fri, 26 Jul 2024 09:33:51 -0400 Subject: [PATCH 12/34] add comments --- package-lock.json | 22 +++++++++++++------ .../package.json | 2 +- .../src/instrumentation.ts | 4 ++++ 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/package-lock.json b/package-lock.json index aa453223f4..c5584287a2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -282,7 +282,6 @@ "dev": true }, "metapackages/auto-configuration-propagators": { - "name": "@opentelemetry/auto-configuration-propagators", "version": "0.1.0", "license": "Apache-2.0", "dependencies": { @@ -30439,9 +30438,9 @@ } }, "node_modules/pg-pool": { - "version": "3.6.2", + "version": "3.4.1", "resolved": "https://registry.npmjs.org/pg-pool/-/pg-pool-3.6.2.tgz", - "integrity": "sha512-Htjbg8BlwXqSBQ9V8Vjtc+vzf/6fVUuak/3/XXKA9oxZprwW3IMDQTGHP+KDmVL7rtd+R1QjbnCFPuTHm3G4hg==", + "integrity": "sha512-TVHxR/gf3MeJRvchgNHxsYsTCHQ+4wm3VIHSS19z8NC0+gioEhq1okDY1sm/TYbfoP6JLFx01s0ShvZ3puP/iQ==", "dev": true, "peerDependencies": { "pg": ">=8.0" @@ -39412,7 +39411,7 @@ "mocha": "7.2.0", "nyc": "15.1.0", "pg": "8.7.1", - "pg-pool": "3.6.2", + "pg-pool": "3.4.1", "rimraf": "5.0.5", "safe-stable-stringify": "^2.4.1", "sinon": "15.2.0", @@ -39427,6 +39426,15 @@ "@opentelemetry/api": "^1.3.0" } }, + "plugins/node/opentelemetry-instrumentation-pg/node_modules/pg-pool": { + "version": "3.4.1", + "resolved": "https://registry.npmjs.org/pg-pool/-/pg-pool-3.4.1.tgz", + "integrity": "sha512-TVHxR/gf3MeJRvchgNHxsYsTCHQ+4wm3VIHSS19z8NC0+gioEhq1okDY1sm/TYbfoP6JLFx01s0ShvZ3puP/iQ==", + "dev": true, + "peerDependencies": { + "pg": ">=8.0" + } + }, "plugins/node/opentelemetry-instrumentation-pino": { "name": "@opentelemetry/instrumentation-pino", "version": "0.41.0", @@ -52999,7 +53007,7 @@ "mocha": "7.2.0", "nyc": "15.1.0", "pg": "8.7.1", - "pg-pool": "3.6.2", + "pg-pool": "3.4.1", "rimraf": "5.0.5", "safe-stable-stringify": "^2.4.1", "sinon": "15.2.0", @@ -71026,9 +71034,9 @@ "integrity": "sha512-WCtabS6t3c8SkpDBUlb1kjOs7l66xsGdKpIPZsg4wR+B3+u9UAum2odSsF9tnvxg80h4ZxLWMy4pRjOsFIqQpw==" }, "pg-pool": { - "version": "3.6.2", + "version": "3.4.1", "resolved": "https://registry.npmjs.org/pg-pool/-/pg-pool-3.6.2.tgz", - "integrity": "sha512-Htjbg8BlwXqSBQ9V8Vjtc+vzf/6fVUuak/3/XXKA9oxZprwW3IMDQTGHP+KDmVL7rtd+R1QjbnCFPuTHm3G4hg==", + "integrity": "sha512-TVHxR/gf3MeJRvchgNHxsYsTCHQ+4wm3VIHSS19z8NC0+gioEhq1okDY1sm/TYbfoP6JLFx01s0ShvZ3puP/iQ==", "dev": true, "requires": {} }, diff --git a/plugins/node/opentelemetry-instrumentation-pg/package.json b/plugins/node/opentelemetry-instrumentation-pg/package.json index b43e7bd383..aee343bade 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/package.json +++ b/plugins/node/opentelemetry-instrumentation-pg/package.json @@ -63,7 +63,7 @@ "mocha": "7.2.0", "nyc": "15.1.0", "pg": "8.7.1", - "pg-pool": "3.6.2", + "pg-pool": "3.4.1", "rimraf": "5.0.5", "safe-stable-stringify": "^2.4.1", "sinon": "15.2.0", diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index 8933faab90..51c11bd305 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -46,6 +46,10 @@ import { SpanNames } from './enums/SpanNames'; export class PgInstrumentation extends InstrumentationBase { private _connectionsCount!: UpDownCounter; + // Pool events connect, acquire, release and remove can be called + // multiple times without changing the values of total, idle and waiting + // connections. The _connectionsCounter is used to keep track of latest + // values and only update the metric _connectionsCount when the value change. private _connectionsCounter: utils.poolConnectionsCounter = { used: 0, idle: 0, From ce4ae04507b584e2ddc308a929107a989eb6a741 Mon Sep 17 00:00:00 2001 From: maryliag Date: Fri, 26 Jul 2024 09:35:29 -0400 Subject: [PATCH 13/34] update package-lock --- package-lock.json | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/package-lock.json b/package-lock.json index c5584287a2..edd02152bc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -30439,7 +30439,7 @@ }, "node_modules/pg-pool": { "version": "3.4.1", - "resolved": "https://registry.npmjs.org/pg-pool/-/pg-pool-3.6.2.tgz", + "resolved": "https://registry.npmjs.org/pg-pool/-/pg-pool-3.4.1.tgz", "integrity": "sha512-TVHxR/gf3MeJRvchgNHxsYsTCHQ+4wm3VIHSS19z8NC0+gioEhq1okDY1sm/TYbfoP6JLFx01s0ShvZ3puP/iQ==", "dev": true, "peerDependencies": { @@ -39426,15 +39426,6 @@ "@opentelemetry/api": "^1.3.0" } }, - "plugins/node/opentelemetry-instrumentation-pg/node_modules/pg-pool": { - "version": "3.4.1", - "resolved": "https://registry.npmjs.org/pg-pool/-/pg-pool-3.4.1.tgz", - "integrity": "sha512-TVHxR/gf3MeJRvchgNHxsYsTCHQ+4wm3VIHSS19z8NC0+gioEhq1okDY1sm/TYbfoP6JLFx01s0ShvZ3puP/iQ==", - "dev": true, - "peerDependencies": { - "pg": ">=8.0" - } - }, "plugins/node/opentelemetry-instrumentation-pino": { "name": "@opentelemetry/instrumentation-pino", "version": "0.41.0", @@ -71035,7 +71026,7 @@ }, "pg-pool": { "version": "3.4.1", - "resolved": "https://registry.npmjs.org/pg-pool/-/pg-pool-3.6.2.tgz", + "resolved": "https://registry.npmjs.org/pg-pool/-/pg-pool-3.4.1.tgz", "integrity": "sha512-TVHxR/gf3MeJRvchgNHxsYsTCHQ+4wm3VIHSS19z8NC0+gioEhq1okDY1sm/TYbfoP6JLFx01s0ShvZ3puP/iQ==", "dev": true, "requires": {} From 55f8fb964e54b58e058c83deedbe6bd52c2d4dee Mon Sep 17 00:00:00 2001 From: maryliag Date: Mon, 29 Jul 2024 09:32:10 -0400 Subject: [PATCH 14/34] add metric db.client.connection.pending_requests --- .../src/instrumentation.ts | 18 ++++++++- .../src/utils.ts | 31 +++++++++------- .../test/pg-pool.test.ts | 37 ++++++++++++++++++- 3 files changed, 70 insertions(+), 16 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index 51c11bd305..5a5b020ab7 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -46,13 +46,16 @@ import { SpanNames } from './enums/SpanNames'; export class PgInstrumentation extends InstrumentationBase { private _connectionsCount!: UpDownCounter; + private _connectionPendingRequests!: UpDownCounter; // Pool events connect, acquire, release and remove can be called // multiple times without changing the values of total, idle and waiting // connections. The _connectionsCounter is used to keep track of latest - // values and only update the metric _connectionsCount when the value change. + // values and only update the metrics _connectionsCount and _connectionPendingRequests + // when the value change. private _connectionsCounter: utils.poolConnectionsCounter = { used: 0, idle: 0, + pending: 0, }; constructor(config: PgInstrumentationConfig = {}) { @@ -74,6 +77,14 @@ export class PgInstrumentation extends InstrumentationBase { inMemoryMetricsExporter.reset(); }); - it('should generate `db.client.connection.count` metric', async () => { + it('should generate `db.client.connection.count` and `db.client.connection.pending_requests` metrics', async () => { const span = provider.getTracer('test-pg-pool').startSpan('test span'); context.with(trace.setSpan(context.active(), span), () => { pool.connect((err, client, release) => { @@ -566,6 +566,41 @@ describe('pg-pool', () => { metrics[0].descriptor.description, 'The number of connections that are currently in state described by the state attribute.' ); + assert.strictEqual( + metrics[0].dataPoints[0].attributes['state'], + 'used' + ); + assert.equal( + metrics[0].dataPoints[0].attributes['pool.name'] + ?.toString() + .match(/postgres@[a-zA-Z]+:[\d]+\/postgres/gm), + true + ); + assert.strictEqual( + metrics[0].dataPoints[1].attributes['state'], + 'idle' + ); + assert.equal( + metrics[0].dataPoints[1].attributes['pool.name'] + ?.toString() + .match(/postgres@[a-zA-Z]+:[\d]+\/postgres/gm), + true + ); + + assert.strictEqual( + metrics[1].descriptor.name, + 'db.client.connection.pending_requests' + ); + assert.equal( + metrics[1].dataPoints[0].attributes['pool.name'] + ?.toString() + .match(/postgres@[a-zA-Z]+:[\d]+\/postgres/gm), + true + ); + assert.strictEqual( + metrics[1].descriptor.description, + 'The number of current pending requests for an open connection.' + ); }); }); }); From 21db3349f4c152a4617631b715493e495357276c Mon Sep 17 00:00:00 2001 From: maryliag Date: Mon, 29 Jul 2024 10:34:04 -0400 Subject: [PATCH 15/34] remove regex --- .../test/pg-pool.test.ts | 18 ------------------ 1 file changed, 18 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 da3cb86a2c..0ef2159287 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -570,33 +570,15 @@ describe('pg-pool', () => { metrics[0].dataPoints[0].attributes['state'], 'used' ); - assert.equal( - metrics[0].dataPoints[0].attributes['pool.name'] - ?.toString() - .match(/postgres@[a-zA-Z]+:[\d]+\/postgres/gm), - true - ); assert.strictEqual( metrics[0].dataPoints[1].attributes['state'], 'idle' ); - assert.equal( - metrics[0].dataPoints[1].attributes['pool.name'] - ?.toString() - .match(/postgres@[a-zA-Z]+:[\d]+\/postgres/gm), - true - ); assert.strictEqual( metrics[1].descriptor.name, 'db.client.connection.pending_requests' ); - assert.equal( - metrics[1].dataPoints[0].attributes['pool.name'] - ?.toString() - .match(/postgres@[a-zA-Z]+:[\d]+\/postgres/gm), - true - ); assert.strictEqual( metrics[1].descriptor.description, 'The number of current pending requests for an open connection.' From 87af5fbb4ac9f72c773f2b24e1743e60feb7e5c0 Mon Sep 17 00:00:00 2001 From: maryliag Date: Wed, 31 Jul 2024 11:28:49 -0400 Subject: [PATCH 16/34] remove username from poolname --- plugins/node/opentelemetry-instrumentation-pg/src/utils.ts | 1 - .../node/opentelemetry-instrumentation-pg/test/utils.test.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts index e5cf85d8ef..053b18ae1e 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts @@ -261,7 +261,6 @@ export function patchCallback( export function getPoolName(pool: PgPoolOptionsParams): string { let poolName = ''; - poolName += pool?.user ? `${pool.user}@` : 'unknown_user'; poolName += pool?.host ? `${pool.host}:` : 'unknown_host'; poolName += pool?.port ? `${pool.port}/` : 'unknown_port'; poolName += pool?.database ? `${pool.database}` : 'unknown_database'; diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts index 43113ac667..a76bb61988 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/utils.test.ts @@ -252,7 +252,7 @@ describe('utils.ts', () => { assert.strictEqual( utils.getPoolName(dummyPool), - 'username@host_name:1234/database_name' + 'host_name:1234/database_name' ); }); }); From d0d5574fbb76504c5eee5e380e17a8a7ee3f04e9 Mon Sep 17 00:00:00 2001 From: maryliag Date: Thu, 1 Aug 2024 11:59:18 -0400 Subject: [PATCH 17/34] use sem conv for attribute names --- .../opentelemetry-instrumentation-pg/src/utils.ts | 15 ++++++++++----- .../test/pg-pool.test.ts | 12 ++++++++++-- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts index 053b18ae1e..ee192fdc16 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts @@ -48,6 +48,11 @@ import type * as pgTypes from 'pg'; import { safeExecuteInTheMiddle } from '@opentelemetry/instrumentation'; import { SpanNames } from './enums/SpanNames'; +// TODO: Replace these constants once a new version of the semantic conventions +// package is created with https://github.com/open-telemetry/opentelemetry-js/pull/4891 +const SEMATTRS_CLIENT_CONNECTION_POOL_NAME = 'db.client.connection.pool.name'; +const SEMATTRS_CLIENT_CONNECTION_STATE = 'db.client.connection.state'; + /** * Helper function to get a low cardinality span name from whatever info we have * about the query. @@ -287,17 +292,17 @@ export function updateCounter( const used = all - idle; connectionCount.add(used - latestCounter.used, { - state: 'used', - 'pool.name': poolName, + [SEMATTRS_CLIENT_CONNECTION_STATE]: 'used', + [SEMATTRS_CLIENT_CONNECTION_POOL_NAME]: poolName, }); connectionCount.add(idle - latestCounter.idle, { - state: 'idle', - 'pool.name': poolName, + [SEMATTRS_CLIENT_CONNECTION_STATE]: 'idle', + [SEMATTRS_CLIENT_CONNECTION_POOL_NAME]: poolName, }); connectionPendingRequests.add(pending - latestCounter.pending, { - 'pool.name': poolName, + [SEMATTRS_CLIENT_CONNECTION_POOL_NAME]: poolName, }); return { used: used, idle: idle, pending: pending }; 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 0ef2159287..aa23d59d2c 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -58,6 +58,10 @@ import { SEMATTRS_DB_STATEMENT, } from '@opentelemetry/semantic-conventions'; +// TODO: Replace these constants once a new version of the semantic conventions +// package is created with https://github.com/open-telemetry/opentelemetry-js/pull/4891 +const SEMATTRS_CLIENT_CONNECTION_STATE = 'db.client.connection.state'; + const memoryExporter = new InMemorySpanExporter(); const CONFIG = { @@ -567,11 +571,15 @@ describe('pg-pool', () => { 'The number of connections that are currently in state described by the state attribute.' ); assert.strictEqual( - metrics[0].dataPoints[0].attributes['state'], + metrics[0].dataPoints[0].attributes[ + SEMATTRS_CLIENT_CONNECTION_STATE + ], 'used' ); assert.strictEqual( - metrics[0].dataPoints[1].attributes['state'], + metrics[0].dataPoints[1].attributes[ + SEMATTRS_CLIENT_CONNECTION_STATE + ], 'idle' ); From 24ba86313d30e3bce86e8128d900764296038884 Mon Sep 17 00:00:00 2001 From: maryliag Date: Mon, 12 Aug 2024 11:58:29 -0400 Subject: [PATCH 18/34] fix separator for unkown case --- plugins/node/opentelemetry-instrumentation-pg/src/utils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts index ee192fdc16..48d6099cdc 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts @@ -266,8 +266,8 @@ export function patchCallback( export function getPoolName(pool: PgPoolOptionsParams): string { let poolName = ''; - poolName += pool?.host ? `${pool.host}:` : 'unknown_host'; - poolName += pool?.port ? `${pool.port}/` : 'unknown_port'; + poolName += (pool?.host ? `${pool.host}` : 'unknown_host') + ':'; + poolName += (pool?.port ? `${pool.port}` : 'unknown_port') + '/'; poolName += pool?.database ? `${pool.database}` : 'unknown_database'; return poolName.trim(); From 88d7a77700ef415af5fd9a44f7a1bd9d97f18089 Mon Sep 17 00:00:00 2001 From: maryliag Date: Tue, 13 Aug 2024 14:20:28 -0400 Subject: [PATCH 19/34] add assert to test --- .../test/pg-pool.test.ts | 12 ++++++++++++ 1 file changed, 12 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 aa23d59d2c..495d899f16 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -576,12 +576,20 @@ describe('pg-pool', () => { ], 'used' ); + assert.strictEqual( + metrics[0].dataPoints[0].value, + 1 + ); assert.strictEqual( metrics[0].dataPoints[1].attributes[ SEMATTRS_CLIENT_CONNECTION_STATE ], 'idle' ); + assert.strictEqual( + metrics[0].dataPoints[1].value, + 0 + ); assert.strictEqual( metrics[1].descriptor.name, @@ -591,6 +599,10 @@ describe('pg-pool', () => { metrics[1].descriptor.description, 'The number of current pending requests for an open connection.' ); + assert.strictEqual( + metrics[1].dataPoints[0].value, + 0 + ); }); }); }); From 524fed93df0a36176091d2aa442abefc5bada565 Mon Sep 17 00:00:00 2001 From: maryliag Date: Tue, 13 Aug 2024 14:28:39 -0400 Subject: [PATCH 20/34] fix lint --- .../test/pg-pool.test.ts | 15 +++------------ 1 file changed, 3 insertions(+), 12 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 495d899f16..19482afc49 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -576,20 +576,14 @@ describe('pg-pool', () => { ], 'used' ); - assert.strictEqual( - metrics[0].dataPoints[0].value, - 1 - ); + assert.strictEqual(metrics[0].dataPoints[0].value, 1); assert.strictEqual( metrics[0].dataPoints[1].attributes[ SEMATTRS_CLIENT_CONNECTION_STATE ], 'idle' ); - assert.strictEqual( - metrics[0].dataPoints[1].value, - 0 - ); + assert.strictEqual(metrics[0].dataPoints[1].value, 0); assert.strictEqual( metrics[1].descriptor.name, @@ -599,10 +593,7 @@ describe('pg-pool', () => { metrics[1].descriptor.description, 'The number of current pending requests for an open connection.' ); - assert.strictEqual( - metrics[1].dataPoints[0].value, - 0 - ); + assert.strictEqual(metrics[1].dataPoints[0].value, 0); }); }); }); From 9cee17f28cdcadc4bd3b4bfe9410eb1eaca2fcb3 Mon Sep 17 00:00:00 2001 From: maryliag Date: Tue, 13 Aug 2024 15:01:17 -0400 Subject: [PATCH 21/34] remove not needed update --- .../opentelemetry-instrumentation-pg/src/instrumentation.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index 5a5b020ab7..372870b2bb 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -452,12 +452,6 @@ export class PgInstrumentation extends InstrumentationBase Date: Mon, 19 Aug 2024 11:12:14 -0400 Subject: [PATCH 22/34] test updates --- .../test/pg-pool.test.ts | 88 ++++++++++--------- 1 file changed, 46 insertions(+), 42 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 19482afc49..594798958c 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -36,11 +36,10 @@ import { SimpleSpanProcessor, } from '@opentelemetry/sdk-trace-base'; import { - AggregationTemporality, InMemoryMetricExporter, MeterProvider, - PeriodicExportingMetricReader, ResourceMetrics, + MetricReader, } from '@opentelemetry/sdk-metrics'; import * as assert from 'assert'; import * as pg from 'pg'; @@ -57,6 +56,7 @@ import { SEMATTRS_DB_USER, SEMATTRS_DB_STATEMENT, } from '@opentelemetry/semantic-conventions'; +import { InstrumentationBase } from '@opentelemetry/instrumentation'; // TODO: Replace these constants once a new version of the semantic conventions // package is created with https://github.com/open-telemetry/opentelemetry-js/pull/4891 @@ -116,27 +116,12 @@ const runCallbackTest = ( }; describe('pg-pool', () => { + function create(config: PgInstrumentationConfig = {}) { instrumentation.setConfig(config); instrumentation.enable(); } - async function waitForNumberOfExports( - exporter: InMemoryMetricExporter, - numberOfExports: number - ): Promise { - if (numberOfExports <= 0) { - throw new Error('numberOfExports must be greater than or equal to 0'); - } - let totalExports = 0; - while (totalExports < numberOfExports) { - await new Promise(resolve => setTimeout(resolve, 20)); - const exportedMetrics = exporter.getMetrics(); - totalExports = exportedMetrics.length; - } - return exporter.getMetrics(); - } - let pool: pgPool; let contextManager: AsyncHooksContextManager; let instrumentation: PgInstrumentation; @@ -511,27 +496,32 @@ describe('pg-pool', () => { }); describe('pg metrics', () => { - let otelTestingMeterProvider; - let inMemoryMetricsExporter: InMemoryMetricExporter; - - function initMeterProvider() { - otelTestingMeterProvider = new MeterProvider(); - inMemoryMetricsExporter = new InMemoryMetricExporter( - AggregationTemporality.CUMULATIVE - ); - const metricReader = new PeriodicExportingMetricReader({ - exporter: inMemoryMetricsExporter, - exportIntervalMillis: 100, - exportTimeoutMillis: 100, + // TODO replace once a new version of opentelemetry-test-utils is created + class TestMetricReader extends MetricReader { + constructor() { + super(); + } + + protected async onForceFlush(): Promise {} + + protected async onShutdown(): Promise {} + } + const initMeterProvider = ( + instrumentation: InstrumentationBase + ): TestMetricReader => { + const metricReader = new TestMetricReader(); + const meterProvider = new MeterProvider({ + readers: [metricReader], }); + instrumentation.setMeterProvider(meterProvider); + + return metricReader; + }; - otelTestingMeterProvider.addMetricReader(metricReader); - instrumentation.setMeterProvider(otelTestingMeterProvider); - } + let metricReader: TestMetricReader; beforeEach(() => { - initMeterProvider(); - inMemoryMetricsExporter.reset(); + metricReader = initMeterProvider(instrumentation); }); it('should generate `db.client.connection.count` and `db.client.connection.pending_requests` metrics', async () => { @@ -556,12 +546,14 @@ describe('pg-pool', () => { } assert.ok(ret); - const exportedMetrics = await waitForNumberOfExports( - inMemoryMetricsExporter, - 2 + const { resourceMetrics, errors } = await metricReader.collect(); + assert.deepEqual( + errors, + [], + 'expected no errors from the callback during metric collection' ); - const metrics = exportedMetrics[1].scopeMetrics[0].metrics; + const metrics = resourceMetrics.scopeMetrics[0].metrics; assert.strictEqual( metrics[0].descriptor.name, 'db.client.connection.count' @@ -576,14 +568,22 @@ describe('pg-pool', () => { ], 'used' ); - assert.strictEqual(metrics[0].dataPoints[0].value, 1); + assert.strictEqual( + metrics[0].dataPoints[0].value, + 1, + "expected to have 1 used connection" + ); assert.strictEqual( metrics[0].dataPoints[1].attributes[ SEMATTRS_CLIENT_CONNECTION_STATE ], 'idle' ); - assert.strictEqual(metrics[0].dataPoints[1].value, 0); + assert.strictEqual( + metrics[0].dataPoints[1].value, + 0, + "expected to have 0 idle connections" + ); assert.strictEqual( metrics[1].descriptor.name, @@ -593,7 +593,11 @@ describe('pg-pool', () => { metrics[1].descriptor.description, 'The number of current pending requests for an open connection.' ); - assert.strictEqual(metrics[1].dataPoints[0].value, 0); + assert.strictEqual( + metrics[1].dataPoints[0].value, + 0, + "expected to have 0 pending requests" + ); }); }); }); From 18a9befb82cc5338d0eeb9fdc5cecb7dd9d5a5b2 Mon Sep 17 00:00:00 2001 From: maryliag Date: Mon, 19 Aug 2024 12:03:33 -0400 Subject: [PATCH 23/34] fix lint --- .../test/pg-pool.test.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 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 594798958c..e5a7259e3c 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -116,7 +116,6 @@ const runCallbackTest = ( }; describe('pg-pool', () => { - function create(config: PgInstrumentationConfig = {}) { instrumentation.setConfig(config); instrumentation.enable(); @@ -501,9 +500,7 @@ describe('pg-pool', () => { constructor() { super(); } - protected async onForceFlush(): Promise {} - protected async onShutdown(): Promise {} } const initMeterProvider = ( @@ -514,7 +511,6 @@ describe('pg-pool', () => { readers: [metricReader], }); instrumentation.setMeterProvider(meterProvider); - return metricReader; }; @@ -571,7 +567,7 @@ describe('pg-pool', () => { assert.strictEqual( metrics[0].dataPoints[0].value, 1, - "expected to have 1 used connection" + 'expected to have 1 used connection' ); assert.strictEqual( metrics[0].dataPoints[1].attributes[ @@ -582,7 +578,7 @@ describe('pg-pool', () => { assert.strictEqual( metrics[0].dataPoints[1].value, 0, - "expected to have 0 idle connections" + 'expected to have 0 idle connections' ); assert.strictEqual( @@ -596,7 +592,7 @@ describe('pg-pool', () => { assert.strictEqual( metrics[1].dataPoints[0].value, 0, - "expected to have 0 pending requests" + 'expected to have 0 pending requests' ); }); }); From d6c8a97c083390915cc8862274ffb74aa6b03214 Mon Sep 17 00:00:00 2001 From: maryliag Date: Mon, 19 Aug 2024 12:10:45 -0400 Subject: [PATCH 24/34] remove unused imports --- .../node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts | 2 -- 1 file changed, 2 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 e5a7259e3c..963ee275e6 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -36,9 +36,7 @@ import { SimpleSpanProcessor, } from '@opentelemetry/sdk-trace-base'; import { - InMemoryMetricExporter, MeterProvider, - ResourceMetrics, MetricReader, } from '@opentelemetry/sdk-metrics'; import * as assert from 'assert'; From 427e0be2d12bd450dd162eee333ae08b63ff1129 Mon Sep 17 00:00:00 2001 From: maryliag Date: Mon, 19 Aug 2024 12:17:34 -0400 Subject: [PATCH 25/34] fix lint --- .../opentelemetry-instrumentation-pg/test/pg-pool.test.ts | 5 +---- 1 file changed, 1 insertion(+), 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 963ee275e6..581f89103b 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -35,10 +35,7 @@ import { InMemorySpanExporter, SimpleSpanProcessor, } from '@opentelemetry/sdk-trace-base'; -import { - MeterProvider, - MetricReader, -} from '@opentelemetry/sdk-metrics'; +import { MeterProvider, MetricReader } from '@opentelemetry/sdk-metrics'; import * as assert from 'assert'; import * as pg from 'pg'; import * as pgPool from 'pg-pool'; From c72f39227a3bd34c69b97f41bcdada6f784867fb Mon Sep 17 00:00:00 2001 From: maryliag Date: Mon, 19 Aug 2024 12:53:22 -0400 Subject: [PATCH 26/34] cleanup --- .../src/instrumentation.ts | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index 372870b2bb..b5bd6e683f 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -136,20 +136,11 @@ export class PgInstrumentation extends InstrumentationBase { @@ -444,18 +435,6 @@ export class PgInstrumentation extends InstrumentationBase { - return function end(this: PgPoolExtended, callback?: PgPoolCallback) { - if (utils.shouldSkipInstrumentation(plugin.getConfig())) { - return originalPoolEnd.call(this, callback as any); - } - return originalPoolEnd.call(this, callback as any); - }; - }; - } } function handleConnectResult(span: Span, connectResult: unknown) { From 7850e2edc715c18924260fc5ffea870190800f12 Mon Sep 17 00:00:00 2001 From: maryliag Date: Mon, 19 Aug 2024 12:57:03 -0400 Subject: [PATCH 27/34] cleanup --- .../opentelemetry-instrumentation-pg/src/instrumentation.ts | 4 +--- plugins/node/opentelemetry-instrumentation-pg/src/utils.ts | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index b5bd6e683f..cddfc87951 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -147,9 +147,6 @@ export class PgInstrumentation extends InstrumentationBase Date: Tue, 20 Aug 2024 09:18:06 -0400 Subject: [PATCH 28/34] reset counters --- .../opentelemetry-instrumentation-pg/src/instrumentation.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index cddfc87951..ae496da61a 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -69,6 +69,11 @@ export class PgInstrumentation extends InstrumentationBase Date: Wed, 21 Aug 2024 08:50:43 -0400 Subject: [PATCH 29/34] remove unused span on test --- .../test/pg-pool.test.ts | 133 +++++++++--------- 1 file changed, 65 insertions(+), 68 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 581f89103b..86a814a214 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -516,80 +516,77 @@ describe('pg-pool', () => { }); it('should generate `db.client.connection.count` and `db.client.connection.pending_requests` metrics', async () => { - const span = provider.getTracer('test-pg-pool').startSpan('test span'); - context.with(trace.setSpan(context.active(), span), () => { - pool.connect((err, client, release) => { + pool.connect((err, client, release) => { + if (err) { + throw new Error(err.message); + } + if (!release) { + throw new Error('Did not receive release function'); + } + if (!client) { + throw new Error('No client received'); + } + assert.ok(client); + + client.query('SELECT NOW()', async (err, ret) => { + release(); if (err) { throw new Error(err.message); } - if (!release) { - throw new Error('Did not receive release function'); - } - if (!client) { - throw new Error('No client received'); - } - assert.ok(client); + assert.ok(ret); - client.query('SELECT NOW()', async (err, ret) => { - release(); - if (err) { - throw new Error(err.message); - } - assert.ok(ret); - - const { resourceMetrics, errors } = await metricReader.collect(); - assert.deepEqual( - errors, - [], - 'expected no errors from the callback during metric collection' - ); + const { resourceMetrics, errors } = await metricReader.collect(); + assert.deepEqual( + errors, + [], + 'expected no errors from the callback during metric collection' + ); - const metrics = resourceMetrics.scopeMetrics[0].metrics; - assert.strictEqual( - metrics[0].descriptor.name, - 'db.client.connection.count' - ); - assert.strictEqual( - metrics[0].descriptor.description, - 'The number of connections that are currently in state described by the state attribute.' - ); - assert.strictEqual( - metrics[0].dataPoints[0].attributes[ - SEMATTRS_CLIENT_CONNECTION_STATE - ], - 'used' - ); - assert.strictEqual( - metrics[0].dataPoints[0].value, - 1, - 'expected to have 1 used connection' - ); - assert.strictEqual( - metrics[0].dataPoints[1].attributes[ - SEMATTRS_CLIENT_CONNECTION_STATE - ], - 'idle' - ); - assert.strictEqual( - metrics[0].dataPoints[1].value, - 0, - 'expected to have 0 idle connections' - ); + const metrics = resourceMetrics.scopeMetrics[0].metrics; + assert.strictEqual( + metrics[0].descriptor.name, + 'db.client.connection.count' + ); + assert.strictEqual( + metrics[0].descriptor.description, + 'The number of connections that are currently in state described by the state attribute.' + ); + assert.strictEqual( + metrics[0].dataPoints[0].attributes[ + SEMATTRS_CLIENT_CONNECTION_STATE + ], + 'used' + ); + assert.strictEqual( + metrics[0].dataPoints[0].value, + 1, + 'expected to have 1 used connection' + ); + assert.strictEqual( + metrics[0].dataPoints[1].attributes[ + SEMATTRS_CLIENT_CONNECTION_STATE + ], + 'idle' + ); + assert.strictEqual( + metrics[0].dataPoints[1].value, + 0, + 'expected to have 0 idle connections' + ); - assert.strictEqual( - metrics[1].descriptor.name, - 'db.client.connection.pending_requests' - ); - assert.strictEqual( - metrics[1].descriptor.description, - 'The number of current pending requests for an open connection.' - ); - assert.strictEqual( - metrics[1].dataPoints[0].value, - 0, - 'expected to have 0 pending requests' - ); - }); + assert.strictEqual( + metrics[1].descriptor.name, + 'db.client.connection.pending_requests' + ); + assert.strictEqual( + metrics[1].descriptor.description, + 'The number of current pending requests for an open connection.' + ); + assert.strictEqual( + metrics[1].dataPoints[0].value, + 0, + 'expected to have 0 pending requests' + ); }); }); }); From c60fed0bba4921755cd9b0b86d59e9962d7ed6ff Mon Sep 17 00:00:00 2001 From: maryliag Date: Wed, 21 Aug 2024 09:08:36 -0400 Subject: [PATCH 30/34] use updateMetric instead of setMetric --- .../src/instrumentation.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index ae496da61a..f98ed3315c 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -60,15 +60,9 @@ export class PgInstrumentation extends InstrumentationBase Date: Wed, 21 Aug 2024 09:20:12 -0400 Subject: [PATCH 31/34] fix lint --- .../node/opentelemetry-instrumentation-pg/src/instrumentation.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index f98ed3315c..7757ea7736 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -26,7 +26,6 @@ import { Span, SpanStatusCode, SpanKind, - MeterProvider, UpDownCounter, } from '@opentelemetry/api'; import type * as pgTypes from 'pg'; From b91bc2739497d9209835d4ff2a724c37b1aaa5c9 Mon Sep 17 00:00:00 2001 From: maryliag Date: Thu, 29 Aug 2024 14:30:21 -0400 Subject: [PATCH 32/34] use semantic conventions 1.27 --- .../src/utils.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts index b6cea09204..29399fd6cb 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts @@ -33,6 +33,10 @@ import { SEMATTRS_NET_PEER_PORT, SEMATTRS_DB_USER, SEMATTRS_DB_STATEMENT, + ATTR_DB_CLIENT_CONNECTION_POOL_NAME, + ATTR_DB_CLIENT_CONNECTION_STATE, + DB_CLIENT_CONNECTION_STATE_VALUE_USED, + DB_CLIENT_CONNECTION_STATE_VALUE_IDLE, DBSYSTEMVALUES_POSTGRESQL, } from '@opentelemetry/semantic-conventions'; import { @@ -50,8 +54,8 @@ import { SpanNames } from './enums/SpanNames'; // TODO: Replace these constants once a new version of the semantic conventions // package is created -const SEMATTRS_CLIENT_CONNECTION_POOL_NAME = 'db.client.connection.pool.name'; -const SEMATTRS_CLIENT_CONNECTION_STATE = 'db.client.connection.state'; +// const SEMATTRS_CLIENT_CONNECTION_POOL_NAME = 'db.client.connection.pool.name'; +// const SEMATTRS_CLIENT_CONNECTION_STATE = 'db.client.connection.state'; /** * Helper function to get a low cardinality span name from whatever info we have @@ -292,17 +296,17 @@ export function updateCounter( const used = all - idle; connectionCount.add(used - latestCounter.used, { - [SEMATTRS_CLIENT_CONNECTION_STATE]: 'used', - [SEMATTRS_CLIENT_CONNECTION_POOL_NAME]: poolName, + [ATTR_DB_CLIENT_CONNECTION_STATE]: DB_CLIENT_CONNECTION_STATE_VALUE_USED, + [ATTR_DB_CLIENT_CONNECTION_POOL_NAME]: poolName, }); connectionCount.add(idle - latestCounter.idle, { - [SEMATTRS_CLIENT_CONNECTION_STATE]: 'idle', - [SEMATTRS_CLIENT_CONNECTION_POOL_NAME]: poolName, + [ATTR_DB_CLIENT_CONNECTION_STATE]: DB_CLIENT_CONNECTION_STATE_VALUE_IDLE, + [ATTR_DB_CLIENT_CONNECTION_POOL_NAME]: poolName, }); connectionPendingRequests.add(pending - latestCounter.pending, { - [SEMATTRS_CLIENT_CONNECTION_POOL_NAME]: poolName, + [ATTR_DB_CLIENT_CONNECTION_POOL_NAME]: poolName, }); return { used: used, idle: idle, pending: pending }; From 23efae88ee12ed1aa27bcb430007a0bf275ffa26 Mon Sep 17 00:00:00 2001 From: maryliag Date: Wed, 4 Sep 2024 12:24:45 -0400 Subject: [PATCH 33/34] use sem conv --- .../src/instrumentation.ts | 8 ++++++-- .../opentelemetry-instrumentation-pg/src/utils.ts | 11 ++++------- .../test/pg-pool.test.ts | 9 +++------ 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index 7757ea7736..d1f0a3f0cb 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -42,6 +42,10 @@ import * as utils from './utils'; import { addSqlCommenterComment } from '@opentelemetry/sql-common'; import { PACKAGE_NAME, PACKAGE_VERSION } from './version'; import { SpanNames } from './enums/SpanNames'; +import { + METRIC_DB_CLIENT_CONNECTION_COUNT, + METRIC_DB_CLIENT_CONNECTION_PENDING_REQUESTS, +} from '@opentelemetry/semantic-conventions/incubating'; export class PgInstrumentation extends InstrumentationBase { private _connectionsCount!: UpDownCounter; @@ -68,7 +72,7 @@ export class PgInstrumentation extends InstrumentationBase { ); assert.strictEqual( metrics[0].dataPoints[0].attributes[ - SEMATTRS_CLIENT_CONNECTION_STATE + ATTR_DB_CLIENT_CONNECTION_STATE ], 'used' ); @@ -564,7 +561,7 @@ describe('pg-pool', () => { ); assert.strictEqual( metrics[0].dataPoints[1].attributes[ - SEMATTRS_CLIENT_CONNECTION_STATE + ATTR_DB_CLIENT_CONNECTION_STATE ], 'idle' ); From 22c41127db168f66027a1221222c29f95cd3d43f Mon Sep 17 00:00:00 2001 From: maryliag Date: Wed, 4 Sep 2024 12:42:20 -0400 Subject: [PATCH 34/34] use test metrics utils --- .../test/pg-pool.test.ts | 25 ++----------------- 1 file changed, 2 insertions(+), 23 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 bc9d95e758..ab57a0fb05 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg-pool.test.ts @@ -35,7 +35,6 @@ import { InMemorySpanExporter, SimpleSpanProcessor, } from '@opentelemetry/sdk-trace-base'; -import { MeterProvider, MetricReader } from '@opentelemetry/sdk-metrics'; import * as assert from 'assert'; import * as pg from 'pg'; import * as pgPool from 'pg-pool'; @@ -52,7 +51,6 @@ import { SEMATTRS_DB_STATEMENT, } from '@opentelemetry/semantic-conventions'; import { ATTR_DB_CLIENT_CONNECTION_STATE } from '@opentelemetry/semantic-conventions/incubating'; -import { InstrumentationBase } from '@opentelemetry/instrumentation'; const memoryExporter = new InMemorySpanExporter(); @@ -487,29 +485,10 @@ describe('pg-pool', () => { }); describe('pg metrics', () => { - // TODO replace once a new version of opentelemetry-test-utils is created - class TestMetricReader extends MetricReader { - constructor() { - super(); - } - protected async onForceFlush(): Promise {} - protected async onShutdown(): Promise {} - } - const initMeterProvider = ( - instrumentation: InstrumentationBase - ): TestMetricReader => { - const metricReader = new TestMetricReader(); - const meterProvider = new MeterProvider({ - readers: [metricReader], - }); - instrumentation.setMeterProvider(meterProvider); - return metricReader; - }; - - let metricReader: TestMetricReader; + let metricReader: testUtils.TestMetricReader; beforeEach(() => { - metricReader = initMeterProvider(instrumentation); + metricReader = testUtils.initMeterProvider(instrumentation); }); it('should generate `db.client.connection.count` and `db.client.connection.pending_requests` metrics', async () => {