From 143c81bf9b6ffbfa3b6d757596d9930bd6904d72 Mon Sep 17 00:00:00 2001 From: maryliag Date: Wed, 31 Jul 2024 11:27:43 -0400 Subject: [PATCH 01/15] operation duration metric --- .../src/instrumentation.ts | 86 ++++++++++++++++++- .../src/utils.ts | 20 ++++- 2 files changed, 101 insertions(+), 5 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index 2df376d29f..a760047d8d 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -26,6 +26,11 @@ import { Span, SpanStatusCode, SpanKind, + Histogram, + MeterProvider, + ValueType, + Attributes, + HrTime, } from '@opentelemetry/api'; import type * as pgTypes from 'pg'; import type * as pgPoolTypes from 'pg-pool'; @@ -41,10 +46,39 @@ import * as utils from './utils'; import { addSqlCommenterComment } from '@opentelemetry/sql-common'; import { PACKAGE_NAME, PACKAGE_VERSION } from './version'; import { SpanNames } from './enums/SpanNames'; +import { + hrTime, + hrTimeDuration, + hrTimeToMilliseconds, +} from '@opentelemetry/core'; +import { DBSYSTEMVALUES_POSTGRESQL, SEMATTRS_DB_SYSTEM } from '@opentelemetry/semantic-conventions'; export class PgInstrumentation extends InstrumentationBase { + private _operationDuration!: Histogram; constructor(config: PgInstrumentationConfig = {}) { super(PACKAGE_NAME, PACKAGE_VERSION, config); + this._setMetricInstruments(); + } + + override setMeterProvider(meterProvider: MeterProvider) { + super.setMeterProvider(meterProvider); + this._setMetricInstruments(); + } + + private _setMetricInstruments() { + this._operationDuration = this.meter.createHistogram( + 'db.client.operation.duration', + { + description: 'Duration of database client operations.', + unit: 's', + valueType: ValueType.DOUBLE, + advice: { + explicitBucketBoundaries: [ + 0.001, 0.005, 0.01, 0.05, 0.1, 0.5, 1, 5, 10, + ], + }, + } + ); } protected init() { @@ -146,6 +180,37 @@ export class PgInstrumentation extends InstrumentationBase { + if (key in attributes) { + metricsAttributes[key] = attributes[key]; + } + }); + console.log("METRICS ATTR: ", metricsAttributes); + + const durationSeconds = + hrTimeToMilliseconds(hrTimeDuration(startTime, hrTime())) / 1000; + this._operationDuration.record( + durationSeconds, + metricsAttributes + ); + } + private _getClientQueryPatch() { const plugin = this; return (original: typeof pgTypes.Client.prototype.query) => { @@ -154,6 +219,7 @@ export class PgInstrumentation extends InstrumentationBase { + plugin.recordOperationDuration(attributes, startTime); + }); + const instrumentationConfig = plugin.getConfig(); const span = utils.handleConfigQuery.call( @@ -209,7 +288,8 @@ export class PgInstrumentation extends InstrumentationBase Date: Wed, 31 Jul 2024 19:19:26 -0400 Subject: [PATCH 02/15] use temporary const for sem attr --- .../src/instrumentation.ts | 30 ++++++++++--------- .../src/utils.ts | 20 +++++++------ 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index a760047d8d..dfb4161ad2 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -53,6 +53,13 @@ import { } from '@opentelemetry/core'; import { DBSYSTEMVALUES_POSTGRESQL, SEMATTRS_DB_SYSTEM } 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_DB_NAMESPACE = 'db.namespace'; +const SEMATTRS_ERROR_TYPE = 'error.type'; +const SEMATTRS_SERVER_PORT = 'server.port'; +const SEMATTRS_SERVER_ADDRESS = 'server.address'; + export class PgInstrumentation extends InstrumentationBase { private _operationDuration!: Histogram; constructor(config: PgInstrumentationConfig = {}) { @@ -186,14 +193,10 @@ export class PgInstrumentation extends InstrumentationBase { @@ -245,14 +248,11 @@ export class PgInstrumentation extends InstrumentationBase { plugin.recordOperationDuration(attributes, startTime); @@ -368,6 +368,7 @@ export class PgInstrumentation extends InstrumentationBase Date: Tue, 13 Aug 2024 15:06:44 -0400 Subject: [PATCH 03/15] fix lint --- .../src/instrumentation.ts | 31 +++++++++---------- .../src/utils.ts | 2 +- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index dfb4161ad2..29cc919dfb 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -51,10 +51,13 @@ import { hrTimeDuration, hrTimeToMilliseconds, } from '@opentelemetry/core'; -import { DBSYSTEMVALUES_POSTGRESQL, SEMATTRS_DB_SYSTEM } from '@opentelemetry/semantic-conventions'; +import { + DBSYSTEMVALUES_POSTGRESQL, + SEMATTRS_DB_SYSTEM, +} 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 +// package is created const SEMATTRS_DB_NAMESPACE = 'db.namespace'; const SEMATTRS_ERROR_TYPE = 'error.type'; const SEMATTRS_SERVER_PORT = 'server.port'; @@ -204,14 +207,10 @@ export class PgInstrumentation extends InstrumentationBase { - plugin.recordOperationDuration(attributes, startTime); - }); + [SEMATTRS_DB_SYSTEM]: DBSYSTEMVALUES_POSTGRESQL, + [SEMATTRS_DB_NAMESPACE]: this.database, + [SEMATTRS_SERVER_PORT]: this.connectionParameters.port, + [SEMATTRS_SERVER_ADDRESS]: this.connectionParameters.host, + }; + this.on('end', () => { + plugin.recordOperationDuration(attributes, startTime); + }); const instrumentationConfig = plugin.getConfig(); @@ -289,7 +288,7 @@ export class PgInstrumentation extends InstrumentationBase Date: Tue, 13 Aug 2024 15:11:41 -0400 Subject: [PATCH 04/15] remove unsed type --- plugins/node/opentelemetry-instrumentation-pg/src/utils.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts index ce5757e4aa..0c025a2247 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts @@ -330,8 +330,3 @@ export type ObjectWithText = { text: string; [k: string]: unknown; }; - -export interface operationInfo { - collectionName: string; - name: string; -} From eaabf7528d5a701c507b3bac44f102c7ee10034c Mon Sep 17 00:00:00 2001 From: maryliag Date: Tue, 20 Aug 2024 09:14:34 -0400 Subject: [PATCH 05/15] add test --- .../src/instrumentation.ts | 18 +-- .../src/utils.ts | 6 +- .../test/pg.test.ts | 111 ++++++++++++++++++ 3 files changed, 124 insertions(+), 11 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index 29cc919dfb..18d74bdd6c 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -19,7 +19,6 @@ import { InstrumentationNodeModuleDefinition, safeExecuteInTheMiddle, } from '@opentelemetry/instrumentation'; - import { context, trace, @@ -191,9 +190,7 @@ export class PgInstrumentation extends InstrumentationBase { + + const recordDuration = () => { plugin.recordOperationDuration(attributes, startTime); - }); + }; const instrumentationConfig = plugin.getConfig(); @@ -288,7 +286,8 @@ export class PgInstrumentation extends InstrumentationBase { }); }); }); + + 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; + + beforeEach(() => { + metricReader = initMeterProvider(instrumentation); + }); + + it('should generate db.client.operation.duration metric', done => { + const span = tracer.startSpan('test span'); + context.with(trace.setSpan(context.active(), span), () => { + client.query('SELECT NOW()', async (_, ret) => { + assert.ok(ret, 'query should be executed'); + + 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.operation.duration' + ); + assert.strictEqual( + metrics[0].descriptor.description, + 'Duration of database client operations.' + ); + const dataPoint = metrics[0].dataPoints[0]; + assert.strictEqual( + dataPoint.attributes[SEMATTRS_DB_SYSTEM], + DBSYSTEMVALUES_POSTGRESQL + ); + assert.strictEqual( + dataPoint.attributes[SEMATTRS_DB_NAMESPACE], + 'postgres' + ); + assert.strictEqual( + dataPoint.attributes[SEMATTRS_ERROR_TYPE], + undefined + ); + }); + }); + }); + + it('should generate db.client.operation.duration metric with error attribute', done => { + const span = tracer.startSpan('test span'); + context.with(trace.setSpan(context.active(), span), () => { + client.query('SELECT test()', async (err, ret) => { + assert.notEqual(err, null); + 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.operation.duration' + ); + assert.strictEqual( + metrics[0].descriptor.description, + 'Duration of database client operations.' + ); + const dataPoint = metrics[0].dataPoints[0]; + assert.strictEqual( + dataPoint.attributes[SEMATTRS_DB_SYSTEM], + DBSYSTEMVALUES_POSTGRESQL + ); + assert.strictEqual( + dataPoint.attributes[SEMATTRS_DB_NAMESPACE], + 'postgres' + ); + assert.strictEqual( + dataPoint.attributes[SEMATTRS_ERROR_TYPE], + 'function test() does not exist' + ); + }); + }); + }); + }); }); From c344db505cc282101c844f90478c84af2243fb61 Mon Sep 17 00:00:00 2001 From: maryliag Date: Tue, 20 Aug 2024 14:32:10 -0400 Subject: [PATCH 06/15] add more tests --- .../test/pg.test.ts | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts index 2be5b2d919..a768ae54d8 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts @@ -30,7 +30,12 @@ import { InMemorySpanExporter, SimpleSpanProcessor, } from '@opentelemetry/sdk-trace-base'; -import { MeterProvider, MetricReader } from '@opentelemetry/sdk-metrics'; +import { + DataPoint, + Histogram, + MeterProvider, + MetricReader, +} from '@opentelemetry/sdk-metrics'; import * as assert from 'assert'; import type * as pg from 'pg'; import * as sinon from 'sinon'; @@ -1029,6 +1034,20 @@ describe('pg', () => { dataPoint.attributes[SEMATTRS_ERROR_TYPE], undefined ); + + const v = (dataPoint as DataPoint).value; + v.min = v.min ? v.min : 0; + v.max = v.max ? v.max : 0; + assert.equal( + v.min > 0, + true, + 'expect min value for Histogram to be greater than 0' + ); + assert.equal( + v.max > 0, + true, + 'expect max value for Histogram to be greater than 0' + ); }); }); }); @@ -1067,6 +1086,20 @@ describe('pg', () => { dataPoint.attributes[SEMATTRS_ERROR_TYPE], 'function test() does not exist' ); + + const v = (dataPoint as DataPoint).value; + v.min = v.min ? v.min : 0; + v.max = v.max ? v.max : 0; + assert.equal( + v.min > 0, + true, + 'expect min value for Histogram to be greater than 0' + ); + assert.equal( + v.max > 0, + true, + 'expect max value for Histogram to be greater than 0' + ); }); }); }); From a66144d28f1bba9b58ac0486594aec5cc487076f Mon Sep 17 00:00:00 2001 From: maryliag Date: Tue, 20 Aug 2024 14:41:23 -0400 Subject: [PATCH 07/15] fix test --- .../opentelemetry-instrumentation-pg/test/pg.test.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts index a768ae54d8..72c4088379 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts @@ -62,7 +62,6 @@ import { InstrumentationBase } from '@opentelemetry/instrumentation'; // TODO: Replace these constants once a new version of the semantic conventions // package is created -const SEMATTRS_DB_NAMESPACE = 'db.namespace'; const SEMATTRS_ERROR_TYPE = 'error.type'; const memoryExporter = new InMemorySpanExporter(); @@ -1026,10 +1025,6 @@ describe('pg', () => { dataPoint.attributes[SEMATTRS_DB_SYSTEM], DBSYSTEMVALUES_POSTGRESQL ); - assert.strictEqual( - dataPoint.attributes[SEMATTRS_DB_NAMESPACE], - 'postgres' - ); assert.strictEqual( dataPoint.attributes[SEMATTRS_ERROR_TYPE], undefined @@ -1078,10 +1073,6 @@ describe('pg', () => { dataPoint.attributes[SEMATTRS_DB_SYSTEM], DBSYSTEMVALUES_POSTGRESQL ); - assert.strictEqual( - dataPoint.attributes[SEMATTRS_DB_NAMESPACE], - 'postgres' - ); assert.strictEqual( dataPoint.attributes[SEMATTRS_ERROR_TYPE], 'function test() does not exist' From 4adc2a6d300693b27363fd6fec83c605308e2463 Mon Sep 17 00:00:00 2001 From: maryliag Date: Tue, 20 Aug 2024 16:22:40 -0400 Subject: [PATCH 08/15] cleanup --- .../opentelemetry-instrumentation-pg/src/utils.ts | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts index f9ce62ad62..2ba201c079 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts @@ -310,17 +310,6 @@ export function getErrorMessage(e: unknown) { : undefined; } -/** - * Attempt to get a name string from a thrown value, while being quite - * defensive, to recognize the fact that, in JS, any kind of value (even - * primitives) can be thrown. - */ -export function getErrorName(e: unknown) { - return typeof e === 'object' && e !== null && 'name' in e - ? String((e as { name?: unknown }).name) - : undefined; -} - export function isObjectWithTextString(it: unknown): it is ObjectWithText { return ( typeof it === 'object' && From 3be63a397284bd5b12593ba16a450b5b06f556d9 Mon Sep 17 00:00:00 2001 From: maryliag Date: Wed, 21 Aug 2024 09:24:58 -0400 Subject: [PATCH 09/15] cleanup --- .../src/instrumentation.ts | 9 +- .../test/pg.test.ts | 162 +++++++++--------- 2 files changed, 79 insertions(+), 92 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index 18d74bdd6c..ca30ba7179 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -26,7 +26,6 @@ import { SpanStatusCode, SpanKind, Histogram, - MeterProvider, ValueType, Attributes, HrTime, @@ -66,15 +65,9 @@ export class PgInstrumentation extends InstrumentationBase { }); it('should generate db.client.operation.duration metric', done => { - const span = tracer.startSpan('test span'); - context.with(trace.setSpan(context.active(), span), () => { - client.query('SELECT NOW()', async (_, ret) => { - assert.ok(ret, 'query should be executed'); + client.query('SELECT NOW()', async (_, ret) => { + assert.ok(ret, 'query should be executed'); - 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.operation.duration' - ); - assert.strictEqual( - metrics[0].descriptor.description, - 'Duration of database client operations.' - ); - const dataPoint = metrics[0].dataPoints[0]; - assert.strictEqual( - dataPoint.attributes[SEMATTRS_DB_SYSTEM], - DBSYSTEMVALUES_POSTGRESQL - ); - assert.strictEqual( - dataPoint.attributes[SEMATTRS_ERROR_TYPE], - undefined - ); + const metrics = resourceMetrics.scopeMetrics[0].metrics; + assert.strictEqual( + metrics[0].descriptor.name, + 'db.client.operation.duration' + ); + assert.strictEqual( + metrics[0].descriptor.description, + 'Duration of database client operations.' + ); + const dataPoint = metrics[0].dataPoints[0]; + assert.strictEqual( + dataPoint.attributes[SEMATTRS_DB_SYSTEM], + DBSYSTEMVALUES_POSTGRESQL + ); + assert.strictEqual( + dataPoint.attributes[SEMATTRS_ERROR_TYPE], + undefined + ); - const v = (dataPoint as DataPoint).value; - v.min = v.min ? v.min : 0; - v.max = v.max ? v.max : 0; - assert.equal( - v.min > 0, - true, - 'expect min value for Histogram to be greater than 0' - ); - assert.equal( - v.max > 0, - true, - 'expect max value for Histogram to be greater than 0' - ); - }); + const v = (dataPoint as DataPoint).value; + v.min = v.min ? v.min : 0; + v.max = v.max ? v.max : 0; + assert.equal( + v.min > 0, + true, + 'expect min value for Histogram to be greater than 0' + ); + assert.equal( + v.max > 0, + true, + 'expect max value for Histogram to be greater than 0' + ); }); }); it('should generate db.client.operation.duration metric with error attribute', done => { - const span = tracer.startSpan('test span'); - context.with(trace.setSpan(context.active(), span), () => { - client.query('SELECT test()', async (err, ret) => { - assert.notEqual(err, null); - const { resourceMetrics, errors } = await metricReader.collect(); - assert.deepEqual( - errors, - [], - 'expected no errors from the callback during metric collection' - ); + client.query('SELECT test()', async (err, ret) => { + assert.notEqual(err, null); + 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.operation.duration' - ); - assert.strictEqual( - metrics[0].descriptor.description, - 'Duration of database client operations.' - ); - const dataPoint = metrics[0].dataPoints[0]; - assert.strictEqual( - dataPoint.attributes[SEMATTRS_DB_SYSTEM], - DBSYSTEMVALUES_POSTGRESQL - ); - assert.strictEqual( - dataPoint.attributes[SEMATTRS_ERROR_TYPE], - 'function test() does not exist' - ); + const metrics = resourceMetrics.scopeMetrics[0].metrics; + assert.strictEqual( + metrics[0].descriptor.name, + 'db.client.operation.duration' + ); + assert.strictEqual( + metrics[0].descriptor.description, + 'Duration of database client operations.' + ); + const dataPoint = metrics[0].dataPoints[0]; + assert.strictEqual( + dataPoint.attributes[SEMATTRS_DB_SYSTEM], + DBSYSTEMVALUES_POSTGRESQL + ); + assert.strictEqual( + dataPoint.attributes[SEMATTRS_ERROR_TYPE], + 'function test() does not exist' + ); - const v = (dataPoint as DataPoint).value; - v.min = v.min ? v.min : 0; - v.max = v.max ? v.max : 0; - assert.equal( - v.min > 0, - true, - 'expect min value for Histogram to be greater than 0' - ); - assert.equal( - v.max > 0, - true, - 'expect max value for Histogram to be greater than 0' - ); - }); + const v = (dataPoint as DataPoint).value; + v.min = v.min ? v.min : 0; + v.max = v.max ? v.max : 0; + assert.equal( + v.min > 0, + true, + 'expect min value for Histogram to be greater than 0' + ); + assert.equal( + v.max > 0, + true, + 'expect max value for Histogram to be greater than 0' + ); }); }); }); From ceb6fadc0a6cb40025b80d5df8693f1ce12ceb74 Mon Sep 17 00:00:00 2001 From: maryliag Date: Tue, 27 Aug 2024 14:51:23 -0400 Subject: [PATCH 10/15] add timeout for test --- plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts index 174d011d70..e5579d9df5 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts @@ -994,6 +994,10 @@ describe('pg', () => { let metricReader: TestMetricReader; + before(function () { + this.timeout(5000); + }); + beforeEach(() => { metricReader = initMeterProvider(instrumentation); }); From 1aa18388d7754a4c351d0455783e81af251d36c2 Mon Sep 17 00:00:00 2001 From: maryliag Date: Tue, 27 Aug 2024 15:05:48 -0400 Subject: [PATCH 11/15] add missing done --- .../node/opentelemetry-instrumentation-pg/test/pg.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts index e5579d9df5..e69d4487b0 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts @@ -994,10 +994,6 @@ describe('pg', () => { let metricReader: TestMetricReader; - before(function () { - this.timeout(5000); - }); - beforeEach(() => { metricReader = initMeterProvider(instrumentation); }); @@ -1045,6 +1041,7 @@ describe('pg', () => { true, 'expect max value for Histogram to be greater than 0' ); + done(); }); }); @@ -1090,6 +1087,7 @@ describe('pg', () => { true, 'expect max value for Histogram to be greater than 0' ); + done(); }); }); }); From d7a78d010f9d53a0013f455111a2ce514a492180 Mon Sep 17 00:00:00 2001 From: maryliag Date: Wed, 4 Sep 2024 12:40:09 -0400 Subject: [PATCH 12/15] use sem conv --- .../src/instrumentation.ts | 34 +++++++++--------- .../test/pg.test.ts | 35 +++---------------- 2 files changed, 22 insertions(+), 47 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index ca30ba7179..6a18984540 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -52,14 +52,14 @@ import { import { DBSYSTEMVALUES_POSTGRESQL, SEMATTRS_DB_SYSTEM, + ATTR_ERROR_TYPE, + ATTR_SERVER_PORT, + ATTR_SERVER_ADDRESS, } from '@opentelemetry/semantic-conventions'; - -// TODO: Replace these constants once a new version of the semantic conventions -// package is created -const SEMATTRS_DB_NAMESPACE = 'db.namespace'; -const SEMATTRS_ERROR_TYPE = 'error.type'; -const SEMATTRS_SERVER_PORT = 'server.port'; -const SEMATTRS_SERVER_ADDRESS = 'server.address'; +import { + METRIC_DB_CLIENT_OPERATION_DURATION, + ATTR_DB_NAMESPACE, +} from '@opentelemetry/semantic-conventions/incubating'; export class PgInstrumentation extends InstrumentationBase { private _operationDuration!: Histogram; @@ -69,7 +69,7 @@ export class PgInstrumentation extends InstrumentationBase { @@ -239,9 +239,9 @@ export class PgInstrumentation extends InstrumentationBase { @@ -360,7 +360,7 @@ export class PgInstrumentation extends InstrumentationBase { }); 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.operation.duration metric', done => { @@ -1024,7 +999,7 @@ describe('pg', () => { DBSYSTEMVALUES_POSTGRESQL ); assert.strictEqual( - dataPoint.attributes[SEMATTRS_ERROR_TYPE], + dataPoint.attributes[ATTR_ERROR_TYPE], undefined ); @@ -1070,7 +1045,7 @@ describe('pg', () => { DBSYSTEMVALUES_POSTGRESQL ); assert.strictEqual( - dataPoint.attributes[SEMATTRS_ERROR_TYPE], + dataPoint.attributes[ATTR_ERROR_TYPE], 'function test() does not exist' ); From 6af47fade4f442b6725d8ed8262056f276e1551c Mon Sep 17 00:00:00 2001 From: maryliag Date: Wed, 4 Sep 2024 12:52:17 -0400 Subject: [PATCH 13/15] fix lint --- .../opentelemetry-instrumentation-pg/test/pg.test.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts index 5284487b9f..15d9b37a33 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts @@ -30,10 +30,7 @@ import { InMemorySpanExporter, SimpleSpanProcessor, } from '@opentelemetry/sdk-trace-base'; -import { - DataPoint, - Histogram, -} from '@opentelemetry/sdk-metrics'; +import { DataPoint, Histogram } from '@opentelemetry/sdk-metrics'; import * as assert from 'assert'; import type * as pg from 'pg'; import * as sinon from 'sinon'; @@ -998,10 +995,7 @@ describe('pg', () => { dataPoint.attributes[SEMATTRS_DB_SYSTEM], DBSYSTEMVALUES_POSTGRESQL ); - assert.strictEqual( - dataPoint.attributes[ATTR_ERROR_TYPE], - undefined - ); + assert.strictEqual(dataPoint.attributes[ATTR_ERROR_TYPE], undefined); const v = (dataPoint as DataPoint).value; v.min = v.min ? v.min : 0; From aed321d8504cdd9c24b4a0f1eace047ff77deb76 Mon Sep 17 00:00:00 2001 From: maryliag Date: Fri, 13 Sep 2024 11:22:43 -0400 Subject: [PATCH 14/15] remove error attribute --- .../src/instrumentation.ts | 4 -- .../src/utils.ts | 3 +- .../test/pg-pool.test.ts | 18 ++++---- .../test/pg.test.ts | 46 ------------------- 4 files changed, 10 insertions(+), 61 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts index a0c7a8abe9..e7c3da7a5f 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/instrumentation.ts @@ -317,7 +317,6 @@ export class PgInstrumentation extends InstrumentationBase { const metrics = resourceMetrics.scopeMetrics[0].metrics; assert.strictEqual( - metrics[0].descriptor.name, + metrics[1].descriptor.name, 'db.client.connection.count' ); assert.strictEqual( - metrics[0].descriptor.description, + metrics[1].descriptor.description, 'The number of connections that are currently in state described by the state attribute.' ); assert.strictEqual( - metrics[0].dataPoints[0].attributes[ + metrics[1].dataPoints[0].attributes[ ATTR_DB_CLIENT_CONNECTION_STATE ], 'used' ); assert.strictEqual( - metrics[0].dataPoints[0].value, + metrics[1].dataPoints[0].value, 1, 'expected to have 1 used connection' ); assert.strictEqual( - metrics[0].dataPoints[1].attributes[ + metrics[1].dataPoints[1].attributes[ ATTR_DB_CLIENT_CONNECTION_STATE ], 'idle' ); assert.strictEqual( - metrics[0].dataPoints[1].value, + metrics[1].dataPoints[1].value, 0, 'expected to have 0 idle connections' ); assert.strictEqual( - metrics[1].descriptor.name, + metrics[2].descriptor.name, 'db.client.connection.pending_requests' ); assert.strictEqual( - metrics[1].descriptor.description, + metrics[2].descriptor.description, 'The number of current pending requests for an open connection.' ); assert.strictEqual( - metrics[1].dataPoints[0].value, + metrics[2].dataPoints[0].value, 0, 'expected to have 0 pending requests' ); diff --git a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts index 15d9b37a33..b394893399 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/test/pg.test.ts @@ -1013,51 +1013,5 @@ describe('pg', () => { done(); }); }); - - it('should generate db.client.operation.duration metric with error attribute', done => { - client.query('SELECT test()', async (err, ret) => { - assert.notEqual(err, null); - 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.operation.duration' - ); - assert.strictEqual( - metrics[0].descriptor.description, - 'Duration of database client operations.' - ); - const dataPoint = metrics[0].dataPoints[0]; - assert.strictEqual( - dataPoint.attributes[SEMATTRS_DB_SYSTEM], - DBSYSTEMVALUES_POSTGRESQL - ); - assert.strictEqual( - dataPoint.attributes[ATTR_ERROR_TYPE], - 'function test() does not exist' - ); - - const v = (dataPoint as DataPoint).value; - v.min = v.min ? v.min : 0; - v.max = v.max ? v.max : 0; - assert.equal( - v.min > 0, - true, - 'expect min value for Histogram to be greater than 0' - ); - assert.equal( - v.max > 0, - true, - 'expect max value for Histogram to be greater than 0' - ); - done(); - }); - }); }); }); From bc87a34abc813b5494bcd99cc22fcc1edfc3107e Mon Sep 17 00:00:00 2001 From: maryliag Date: Fri, 13 Sep 2024 11:24:06 -0400 Subject: [PATCH 15/15] cleanup --- plugins/node/opentelemetry-instrumentation-pg/src/utils.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts index c7bce1ca73..f3d65f5b82 100644 --- a/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-pg/src/utils.ts @@ -22,12 +22,10 @@ import { Tracer, SpanKind, diag, - Attributes, UpDownCounter, } from '@opentelemetry/api'; import { AttributeNames } from './enums/AttributeNames'; import { - ATTR_ERROR_TYPE, SEMATTRS_DB_SYSTEM, SEMATTRS_DB_NAME, SEMATTRS_DB_CONNECTION_STRING,