From 01aa55935c70092486e576377de5affda0ccdc76 Mon Sep 17 00:00:00 2001 From: Mathias Klippinge Date: Fri, 1 Sep 2023 09:22:45 +0200 Subject: [PATCH 1/2] Add API for adding custom metric attributes --- .../opentelemetry-instrumentation-http/README.md | 1 + .../opentelemetry-instrumentation-http/src/http.ts | 12 ++++++++---- .../opentelemetry-instrumentation-http/src/types.ts | 8 +++++++- .../test/functionals/http-metrics.test.ts | 13 ++++++++++++- 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/README.md b/experimental/packages/opentelemetry-instrumentation-http/README.md index 312eb73bd7..951f030449 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/README.md +++ b/experimental/packages/opentelemetry-instrumentation-http/README.md @@ -57,6 +57,7 @@ Http instrumentation has few options available to choose from. You can set the f | [`startOutgoingSpanHook`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L99) | `StartOutgoingSpanCustomAttributeFunction` | Function for adding custom attributes before a span is started in outgoingRequest | | `ignoreIncomingRequestHook` | `IgnoreIncomingRequestFunction` | Http instrumentation will not trace all incoming requests that matched with custom function | | `ignoreOutgoingRequestHook` | `IgnoreOutgoingRequestFunction` | Http instrumentation will not trace all outgoing requests that matched with custom function | +| `customMetricAttributes` | `CustomMetricAttributeFunction` | Function for adding custom metric attributes on all recorded metrics | | [`serverName`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L101) | `string` | The primary server name of the matched virtual host. | | [`requireParentforOutgoingSpans`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L103) | Boolean | Require that is a parent span to create new span for outgoing requests. | | [`requireParentforIncomingSpans`](https://github.com/open-telemetry/opentelemetry-js/blob/main/experimental/packages/opentelemetry-instrumentation-http/src/types.ts#L105) | Boolean | Require that is a parent span to create new span for incoming requests. | diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index 9422bbc9ef..aa43e4c7e4 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -512,8 +512,10 @@ export class HttpInstrumentation extends InstrumentationBase { }; const startTime = hrTime(); - const metricAttributes = - utils.getIncomingRequestMetricAttributes(spanAttributes); + const metricAttributes: MetricAttributes = Object.assign( + utils.getIncomingRequestMetricAttributes(spanAttributes), + instrumentation._getConfig().customMetricAttributes?.() + ); const ctx = propagation.extract(ROOT_CONTEXT, headers); const span = instrumentation._startHttpSpan(method, spanOptions, ctx); @@ -658,8 +660,10 @@ export class HttpInstrumentation extends InstrumentationBase { }); const startTime = hrTime(); - const metricAttributes: MetricAttributes = - utils.getOutgoingRequestMetricAttributes(attributes); + const metricAttributes: MetricAttributes = Object.assign( + utils.getOutgoingRequestMetricAttributes(attributes), + instrumentation._getConfig().customMetricAttributes?.() + ); const spanOptions: SpanOptions = { kind: SpanKind.CLIENT, diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/types.ts b/experimental/packages/opentelemetry-instrumentation-http/src/types.ts index 5cc09341d4..5734e737f5 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/types.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/types.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Span, SpanAttributes } from '@opentelemetry/api'; +import { MetricAttributes, Span, SpanAttributes } from '@opentelemetry/api'; import type * as http from 'http'; import type * as https from 'https'; import { @@ -80,6 +80,10 @@ export interface StartOutgoingSpanCustomAttributeFunction { (request: RequestOptions): SpanAttributes; } +export interface CustomMetricAttributeFunction { + (): MetricAttributes; +} + /** * Options available for the HTTP instrumentation (see [documentation](https://github.com/open-telemetry/opentelemetry-js/tree/main/packages/opentelemetry-instrumentation-http#http-instrumentation-options)) */ @@ -108,6 +112,8 @@ export interface HttpInstrumentationConfig extends InstrumentationConfig { startIncomingSpanHook?: StartIncomingSpanCustomAttributeFunction; /** Function for adding custom attributes before a span is started in outgoingRequest */ startOutgoingSpanHook?: StartOutgoingSpanCustomAttributeFunction; + /** Function for adding custom metric attributes on all recorded metrics */ + customMetricAttributes?: CustomMetricAttributeFunction; /** The primary server name of the matched virtual host. */ serverName?: string; /** Require parent to create span for outgoing requests */ diff --git a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts index d7f55ed846..3b385e679e 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/test/functionals/http-metrics.test.ts @@ -26,7 +26,14 @@ import { HttpInstrumentation } from '../../src/http'; import { httpRequest } from '../utils/httpRequest'; import { TestMetricReader } from '../utils/TestMetricReader'; -const instrumentation = new HttpInstrumentation(); +const instrumentation = new HttpInstrumentation({ + customMetricAttributes: () => { + return { + foo: 'bar', + }; + }, +}); + instrumentation.enable(); instrumentation.disable(); @@ -115,6 +122,8 @@ describe('metrics', () => { metrics[0].dataPoints[0].attributes[SemanticAttributes.NET_HOST_PORT], 22346 ); + // Custom attributes + assert.strictEqual(metrics[0].dataPoints[0].attributes['foo'], 'bar'); assert.strictEqual(metrics[1].dataPointType, DataPointType.HISTOGRAM); assert.strictEqual( @@ -148,5 +157,7 @@ describe('metrics', () => { metrics[1].dataPoints[0].attributes[SemanticAttributes.HTTP_FLAVOR], '1.1' ); + // Custom attributes + assert.strictEqual(metrics[1].dataPoints[0].attributes['foo'], 'bar'); }); }); From bc47738a5dea6491857f0241bac3c56e89fadbaa Mon Sep 17 00:00:00 2001 From: Mathias Klippinge Date: Thu, 21 Sep 2023 11:22:10 +0200 Subject: [PATCH 2/2] Code review. Lint warnings. Stop using deprecated types. --- .../src/http.ts | 36 ++++++++++++------- .../src/types.ts | 8 ++--- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index aa43e4c7e4..a21ebf32ac 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -26,7 +26,7 @@ import { SpanStatusCode, trace, Histogram, - MetricAttributes, + Attributes, ValueType, } from '@opentelemetry/api'; import { @@ -310,7 +310,7 @@ export class HttpInstrumentation extends InstrumentationBase { request: http.ClientRequest, span: Span, startTime: HrTime, - metricAttributes: MetricAttributes + metricAttributes: Attributes ): http.ClientRequest { if (this._getConfig().requestHook) { this._callRequestHook(span, request); @@ -376,7 +376,7 @@ export class HttpInstrumentation extends InstrumentationBase { if (this._getConfig().applyCustomAttributesOnSpan) { safeExecuteInTheMiddle( () => - this._getConfig().applyCustomAttributesOnSpan!( + this._getConfig().applyCustomAttributesOnSpan?.( span, request, response @@ -512,9 +512,14 @@ export class HttpInstrumentation extends InstrumentationBase { }; const startTime = hrTime(); - const metricAttributes: MetricAttributes = Object.assign( + const customMetricAttributes = safeExecuteInTheMiddle( + () => instrumentation._getConfig().customMetricAttributes?.(), + () => {}, + true + ); + const metricAttributes: Attributes = Object.assign( utils.getIncomingRequestMetricAttributes(spanAttributes), - instrumentation._getConfig().customMetricAttributes?.() + customMetricAttributes ); const ctx = propagation.extract(ROOT_CONTEXT, headers); @@ -660,9 +665,14 @@ export class HttpInstrumentation extends InstrumentationBase { }); const startTime = hrTime(); - const metricAttributes: MetricAttributes = Object.assign( + const customMetricAttributes = safeExecuteInTheMiddle( + () => instrumentation._getConfig().customMetricAttributes?.(), + () => {}, + true + ); + const metricAttributes: Attributes = Object.assign( utils.getOutgoingRequestMetricAttributes(attributes), - instrumentation._getConfig().customMetricAttributes?.() + customMetricAttributes ); const spanOptions: SpanOptions = { @@ -723,7 +733,7 @@ export class HttpInstrumentation extends InstrumentationBase { request: http.IncomingMessage, response: http.ServerResponse, span: Span, - metricAttributes: MetricAttributes, + metricAttributes: Attributes, startTime: HrTime ) { const attributes = utils.getIncomingRequestAttributesOnResponse( @@ -751,7 +761,7 @@ export class HttpInstrumentation extends InstrumentationBase { if (this._getConfig().applyCustomAttributesOnSpan) { safeExecuteInTheMiddle( () => - this._getConfig().applyCustomAttributesOnSpan!( + this._getConfig().applyCustomAttributesOnSpan?.( span, request, response @@ -766,7 +776,7 @@ export class HttpInstrumentation extends InstrumentationBase { private _onServerResponseError( span: Span, - metricAttributes: MetricAttributes, + metricAttributes: Attributes, startTime: HrTime, error: Err ) { @@ -806,7 +816,7 @@ export class HttpInstrumentation extends InstrumentationBase { span: Span, spanKind: SpanKind, startTime: HrTime, - metricAttributes: MetricAttributes + metricAttributes: Attributes ) { if (!this._spanNotEnded.has(span)) { return; @@ -829,7 +839,7 @@ export class HttpInstrumentation extends InstrumentationBase { response: http.IncomingMessage | http.ServerResponse ) { safeExecuteInTheMiddle( - () => this._getConfig().responseHook!(span, response), + () => this._getConfig().responseHook?.(span, response), () => {}, true ); @@ -840,7 +850,7 @@ export class HttpInstrumentation extends InstrumentationBase { request: http.ClientRequest | http.IncomingMessage ) { safeExecuteInTheMiddle( - () => this._getConfig().requestHook!(span, request), + () => this._getConfig().requestHook?.(span, request), () => {}, true ); diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/types.ts b/experimental/packages/opentelemetry-instrumentation-http/src/types.ts index 5734e737f5..b1de0d7b3d 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/types.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/types.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { MetricAttributes, Span, SpanAttributes } from '@opentelemetry/api'; +import { Attributes, Span } from '@opentelemetry/api'; import type * as http from 'http'; import type * as https from 'https'; import { @@ -73,15 +73,15 @@ export interface HttpResponseCustomAttributeFunction { } export interface StartIncomingSpanCustomAttributeFunction { - (request: IncomingMessage): SpanAttributes; + (request: IncomingMessage): Attributes; } export interface StartOutgoingSpanCustomAttributeFunction { - (request: RequestOptions): SpanAttributes; + (request: RequestOptions): Attributes; } export interface CustomMetricAttributeFunction { - (): MetricAttributes; + (): Attributes; } /**