From e6eea0ac56805d92d905f989591d073781fd7ffd Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Thu, 25 Apr 2024 09:00:37 +0300 Subject: [PATCH 01/13] feat!(instrumentation): generic config type and no default config value --- .../src/fetch.ts | 20 +++----- .../src/instrumentation.ts | 15 ++---- .../src/http.ts | 50 +++++++++---------- .../src/xhr.ts | 20 +++----- .../src/instrumentation.ts | 11 ++-- .../src/platform/browser/instrumentation.ts | 7 +-- .../src/platform/node/instrumentation.ts | 8 +-- 7 files changed, 56 insertions(+), 75 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts index edf6f50cf7..85aca36bf8 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts @@ -72,23 +72,19 @@ export interface FetchInstrumentationConfig extends InstrumentationConfig { /** * This class represents a fetch plugin for auto instrumentation */ -export class FetchInstrumentation extends InstrumentationBase { +export class FetchInstrumentation extends InstrumentationBase { readonly component: string = 'fetch'; readonly version: string = VERSION; moduleName = this.component; private _usedResources = new WeakSet(); private _tasksCount = 0; - constructor(config?: FetchInstrumentationConfig) { + constructor(config: FetchInstrumentationConfig = {}) { super('@opentelemetry/instrumentation-fetch', VERSION, config); } init(): void {} - private _getConfig(): FetchInstrumentationConfig { - return this._config; - } - /** * Add cors pre flight child span * @param span @@ -105,7 +101,7 @@ export class FetchInstrumentation extends InstrumentationBase { }, api.trace.setSpan(api.context.active(), span) ); - if (!this._getConfig().ignoreNetworkEvents) { + if (!this.getConfig().ignoreNetworkEvents) { web.addSpanNetworkEvents(childSpan, corsPreFlightRequest); } childSpan.end( @@ -149,7 +145,7 @@ export class FetchInstrumentation extends InstrumentationBase { if ( !web.shouldPropagateTraceHeaders( spanUrl, - this._getConfig().propagateTraceHeaderCorsUrls + this.getConfig().propagateTraceHeaderCorsUrls ) ) { const headers: Partial> = {}; @@ -186,7 +182,7 @@ export class FetchInstrumentation extends InstrumentationBase { * @private */ private _clearResources() { - if (this._tasksCount === 0 && this._getConfig().clearTimingResources) { + if (this._tasksCount === 0 && this.getConfig().clearTimingResources) { performance.clearResourceTimings(); this._usedResources = new WeakSet(); } @@ -201,7 +197,7 @@ export class FetchInstrumentation extends InstrumentationBase { url: string, options: Partial = {} ): api.Span | undefined { - if (core.isUrlIgnored(url, this._getConfig().ignoreUrls)) { + if (core.isUrlIgnored(url, this.getConfig().ignoreUrls)) { this._diag.debug('ignoring span as url matches ignored url'); return; } @@ -258,7 +254,7 @@ export class FetchInstrumentation extends InstrumentationBase { this._addChildSpan(span, corsPreFlightRequest); this._markResourceAsUsed(corsPreFlightRequest); } - if (!this._getConfig().ignoreNetworkEvents) { + if (!this.getConfig().ignoreNetworkEvents) { web.addSpanNetworkEvents(span, mainRequest); } } @@ -419,7 +415,7 @@ export class FetchInstrumentation extends InstrumentationBase { result: Response | FetchError ) { const applyCustomAttributesOnSpan = - this._getConfig().applyCustomAttributesOnSpan; + this.getConfig().applyCustomAttributesOnSpan; if (applyCustomAttributesOnSpan) { safeExecuteInTheMiddle( () => applyCustomAttributesOnSpan(span, request, result), diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts index 504fb4fd5f..d8dfde3f38 100644 --- a/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts @@ -88,10 +88,10 @@ import { import { AttributeValues } from './enums/AttributeValues'; import { VERSION } from './version'; -export class GrpcInstrumentation extends InstrumentationBase { +export class GrpcInstrumentation extends InstrumentationBase { private _metadataCapture: metadataCaptureType; - constructor(config?: GrpcInstrumentationConfig) { + constructor(config: GrpcInstrumentationConfig = {}) { super('@opentelemetry/instrumentation-grpc', VERSION, config); this._metadataCapture = this._createMetadataCapture(); } @@ -195,16 +195,7 @@ export class GrpcInstrumentation extends InstrumentationBase { ]; } - /** - * @internal - * Public reference to the protected BaseInstrumentation `_config` instance to be used by this - * plugin's external helper functions - */ - override getConfig(): GrpcInstrumentationConfig { - return super.getConfig(); - } - - override setConfig(config?: GrpcInstrumentationConfig): void { + override setConfig(config: GrpcInstrumentationConfig = {}): void { super.setConfig(config); this._metadataCapture = this._createMetadataCapture(); } diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index c13736e4da..7d63a9d771 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -63,14 +63,14 @@ import { SEMATTRS_HTTP_ROUTE } from '@opentelemetry/semantic-conventions'; /** * Http instrumentation instrumentation for Opentelemetry */ -export class HttpInstrumentation extends InstrumentationBase { +export class HttpInstrumentation extends InstrumentationBase { /** keep track on spans not ended */ private readonly _spanNotEnded: WeakSet = new WeakSet(); private _headerCapture; private _httpServerDurationHistogram!: Histogram; private _httpClientDurationHistogram!: Histogram; - constructor(config?: HttpInstrumentationConfig) { + constructor(config: HttpInstrumentationConfig = {}) { super('@opentelemetry/instrumentation-http', VERSION, config); this._headerCapture = this._createHeaderCapture(); } @@ -94,11 +94,7 @@ export class HttpInstrumentation extends InstrumentationBase { ); } - private _getConfig(): HttpInstrumentationConfig { - return this._config; - } - - override setConfig(config?: HttpInstrumentationConfig): void { + override setConfig(config: HttpInstrumentationConfig = {}): void { super.setConfig(config); this._headerCapture = this._createHeaderCapture(); } @@ -306,7 +302,7 @@ export class HttpInstrumentation extends InstrumentationBase { startTime: HrTime, metricAttributes: MetricAttributes ): http.ClientRequest { - if (this._getConfig().requestHook) { + if (this.getConfig().requestHook) { this._callRequestHook(span, request); } @@ -335,7 +331,7 @@ export class HttpInstrumentation extends InstrumentationBase { utils.getOutgoingRequestMetricAttributesOnResponse(responseAttributes) ); - if (this._getConfig().responseHook) { + if (this.getConfig().responseHook) { this._callResponseHook(span, response); } @@ -370,10 +366,10 @@ export class HttpInstrumentation extends InstrumentationBase { span.setStatus(status); - if (this._getConfig().applyCustomAttributesOnSpan) { + if (this.getConfig().applyCustomAttributesOnSpan) { safeExecuteInTheMiddle( () => - this._getConfig().applyCustomAttributesOnSpan!( + this.getConfig().applyCustomAttributesOnSpan!( span, request, response @@ -467,13 +463,13 @@ export class HttpInstrumentation extends InstrumentationBase { if ( utils.isIgnored( pathname, - instrumentation._getConfig().ignoreIncomingPaths, + instrumentation.getConfig().ignoreIncomingPaths, (e: unknown) => instrumentation._diag.error('caught ignoreIncomingPaths error: ', e) ) || safeExecuteInTheMiddle( () => - instrumentation._getConfig().ignoreIncomingRequestHook?.(request), + instrumentation.getConfig().ignoreIncomingRequestHook?.(request), (e: unknown) => { if (e != null) { instrumentation._diag.error( @@ -496,10 +492,10 @@ export class HttpInstrumentation extends InstrumentationBase { const spanAttributes = utils.getIncomingRequestAttributes(request, { component: component, - serverName: instrumentation._getConfig().serverName, + serverName: instrumentation.getConfig().serverName, hookAttributes: instrumentation._callStartSpanHook( request, - instrumentation._getConfig().startIncomingSpanHook + instrumentation.getConfig().startIncomingSpanHook ), }); @@ -525,10 +521,10 @@ export class HttpInstrumentation extends InstrumentationBase { context.bind(context.active(), request); context.bind(context.active(), response); - if (instrumentation._getConfig().requestHook) { + if (instrumentation.getConfig().requestHook) { instrumentation._callRequestHook(span, request); } - if (instrumentation._getConfig().responseHook) { + if (instrumentation.getConfig().responseHook) { instrumentation._callResponseHook(span, response); } @@ -619,14 +615,14 @@ export class HttpInstrumentation extends InstrumentationBase { if ( utils.isIgnored( origin + pathname, - instrumentation._getConfig().ignoreOutgoingUrls, + instrumentation.getConfig().ignoreOutgoingUrls, (e: unknown) => instrumentation._diag.error('caught ignoreOutgoingUrls error: ', e) ) || safeExecuteInTheMiddle( () => instrumentation - ._getConfig() + .getConfig() .ignoreOutgoingRequestHook?.(optionsParsed), (e: unknown) => { if (e != null) { @@ -650,7 +646,7 @@ export class HttpInstrumentation extends InstrumentationBase { hostname, hookAttributes: instrumentation._callStartSpanHook( optionsParsed, - instrumentation._getConfig().startOutgoingSpanHook + instrumentation.getConfig().startOutgoingSpanHook ), }); @@ -745,10 +741,10 @@ export class HttpInstrumentation extends InstrumentationBase { span.updateName(`${request.method || 'GET'} ${route}`); } - if (this._getConfig().applyCustomAttributesOnSpan) { + if (this.getConfig().applyCustomAttributesOnSpan) { safeExecuteInTheMiddle( () => - this._getConfig().applyCustomAttributesOnSpan!( + this.getConfig().applyCustomAttributesOnSpan!( span, request, response @@ -782,8 +778,8 @@ export class HttpInstrumentation extends InstrumentationBase { */ const requireParent = options.kind === SpanKind.CLIENT - ? this._getConfig().requireParentforOutgoingSpans - : this._getConfig().requireParentforIncomingSpans; + ? this.getConfig().requireParentforOutgoingSpans + : this.getConfig().requireParentforIncomingSpans; let span: Span; const currentSpan = trace.getSpan(ctx); @@ -826,7 +822,7 @@ export class HttpInstrumentation extends InstrumentationBase { response: http.IncomingMessage | http.ServerResponse ) { safeExecuteInTheMiddle( - () => this._getConfig().responseHook!(span, response), + () => this.getConfig().responseHook!(span, response), () => {}, true ); @@ -837,7 +833,7 @@ export class HttpInstrumentation extends InstrumentationBase { request: http.ClientRequest | http.IncomingMessage ) { safeExecuteInTheMiddle( - () => this._getConfig().requestHook!(span, request), + () => this.getConfig().requestHook!(span, request), () => {}, true ); @@ -857,7 +853,7 @@ export class HttpInstrumentation extends InstrumentationBase { } private _createHeaderCapture() { - const config = this._getConfig(); + const config = this.getConfig(); return { client: { diff --git a/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts b/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts index 4d1e11df70..111d68272b 100644 --- a/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts +++ b/experimental/packages/opentelemetry-instrumentation-xml-http-request/src/xhr.ts @@ -81,7 +81,7 @@ export interface XMLHttpRequestInstrumentationConfig /** * This class represents a XMLHttpRequest plugin for auto instrumentation */ -export class XMLHttpRequestInstrumentation extends InstrumentationBase { +export class XMLHttpRequestInstrumentation extends InstrumentationBase { readonly component: string = 'xml-http-request'; readonly version: string = VERSION; moduleName = this.component; @@ -90,16 +90,12 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase { private _xhrMem = new WeakMap(); private _usedResources = new WeakSet(); - constructor(config?: XMLHttpRequestInstrumentationConfig) { + constructor(config: XMLHttpRequestInstrumentationConfig = {}) { super('@opentelemetry/instrumentation-xml-http-request', VERSION, config); } init() {} - private _getConfig(): XMLHttpRequestInstrumentationConfig { - return this._config; - } - /** * Adds custom headers to XMLHttpRequest * @param xhr @@ -111,7 +107,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase { if ( !shouldPropagateTraceHeaders( url, - this._getConfig().propagateTraceHeaderCorsUrls + this.getConfig().propagateTraceHeaderCorsUrls ) ) { const headers: Partial> = {}; @@ -142,7 +138,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase { const childSpan = this.tracer.startSpan('CORS Preflight', { startTime: corsPreFlightRequest[PTN.FETCH_START], }); - if (!this._getConfig().ignoreNetworkEvents) { + if (!this.getConfig().ignoreNetworkEvents) { addSpanNetworkEvents(childSpan, corsPreFlightRequest); } childSpan.end(corsPreFlightRequest[PTN.RESPONSE_END]); @@ -182,7 +178,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase { private _applyAttributesAfterXHR(span: api.Span, xhr: XMLHttpRequest) { const applyCustomAttributesOnSpan = - this._getConfig().applyCustomAttributesOnSpan; + this.getConfig().applyCustomAttributesOnSpan; if (typeof applyCustomAttributesOnSpan === 'function') { safeExecuteInTheMiddle( () => applyCustomAttributesOnSpan(span, xhr), @@ -244,7 +240,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase { * @private */ private _clearResources() { - if (this._tasksCount === 0 && this._getConfig().clearTimingResources) { + if (this._tasksCount === 0 && this.getConfig().clearTimingResources) { (otperformance as unknown as Performance).clearResourceTimings(); this._xhrMem = new WeakMap(); this._usedResources = new WeakSet(); @@ -296,7 +292,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase { this._addChildSpan(span, corsPreFlightRequest); this._markResourceAsUsed(corsPreFlightRequest); } - if (!this._getConfig().ignoreNetworkEvents) { + if (!this.getConfig().ignoreNetworkEvents) { addSpanNetworkEvents(span, mainRequest); } } @@ -331,7 +327,7 @@ export class XMLHttpRequestInstrumentation extends InstrumentationBase { url: string, method: string ): api.Span | undefined { - if (isUrlIgnored(url, this._getConfig().ignoreUrls)) { + if (isUrlIgnored(url, this.getConfig().ignoreUrls)) { this._diag.debug('ignoring span as url matches ignored url'); return; } diff --git a/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts index 851b38c231..3aa6376769 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts @@ -32,11 +32,12 @@ import { InstrumentationConfig, } from './types'; + /** * Base abstract internal class for instrumenting node and web plugins */ -export abstract class InstrumentationAbstract implements Instrumentation { - protected _config: InstrumentationConfig; +export abstract class InstrumentationAbstract implements Instrumentation { + protected _config: ConfigType; private _tracer: Tracer; private _meter: Meter; @@ -46,7 +47,7 @@ export abstract class InstrumentationAbstract implements Instrumentation { constructor( public readonly instrumentationName: string, public readonly instrumentationVersion: string, - config: InstrumentationConfig = {} + config: ConfigType, ) { this._config = { enabled: true, @@ -131,7 +132,7 @@ export abstract class InstrumentationAbstract implements Instrumentation { } /* Returns InstrumentationConfig */ - public getConfig(): InstrumentationConfig { + public getConfig(): ConfigType { return this._config; } @@ -139,7 +140,7 @@ export abstract class InstrumentationAbstract implements Instrumentation { * Sets InstrumentationConfig to this plugin * @param InstrumentationConfig */ - public setConfig(config: InstrumentationConfig = {}): void { + public setConfig(config: ConfigType): void { this._config = Object.assign({}, config); } diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/browser/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/browser/instrumentation.ts index 0458357a73..7aad8a4e8a 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/platform/browser/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/browser/instrumentation.ts @@ -16,18 +16,19 @@ import { InstrumentationAbstract } from '../../instrumentation'; import * as types from '../../types'; +import { InstrumentationConfig } from '../../types'; /** * Base abstract class for instrumenting web plugins */ -export abstract class InstrumentationBase - extends InstrumentationAbstract +export abstract class InstrumentationBase + extends InstrumentationAbstract implements types.Instrumentation { constructor( instrumentationName: string, instrumentationVersion: string, - config: types.InstrumentationConfig = {} + config: ConfigType ) { super(instrumentationName, instrumentationVersion, config); diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts index 095e6339ae..c5dca33972 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts @@ -26,7 +26,7 @@ import { } from './RequireInTheMiddleSingleton'; import type { HookFn } from 'import-in-the-middle'; import * as ImportInTheMiddle from 'import-in-the-middle'; -import { InstrumentationModuleDefinition } from '../../types'; +import { InstrumentationConfig, InstrumentationModuleDefinition } from '../../types'; import { diag } from '@opentelemetry/api'; import type { OnRequireFn } from 'require-in-the-middle'; import { Hook } from 'require-in-the-middle'; @@ -35,8 +35,8 @@ import { readFileSync } from 'fs'; /** * Base abstract class for instrumenting node plugins */ -export abstract class InstrumentationBase - extends InstrumentationAbstract +export abstract class InstrumentationBase + extends InstrumentationAbstract implements types.Instrumentation { private _modules: InstrumentationModuleDefinition[]; @@ -48,7 +48,7 @@ export abstract class InstrumentationBase constructor( instrumentationName: string, instrumentationVersion: string, - config: types.InstrumentationConfig = {} + config: ConfigType ) { super(instrumentationName, instrumentationVersion, config); From d97f38b7936caec36cdec27540a38a3d90ba2ba7 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Thu, 25 Apr 2024 09:26:23 +0300 Subject: [PATCH 02/13] fix: apply type in base Instrumentation interface --- .../opentelemetry-instrumentation/src/autoLoaderUtils.ts | 2 +- .../opentelemetry-instrumentation/src/instrumentation.ts | 4 ++-- .../src/platform/browser/instrumentation.ts | 4 ++-- .../src/platform/node/instrumentation.ts | 4 ++-- .../packages/opentelemetry-instrumentation/src/types.ts | 8 ++++---- .../test/common/autoLoader.test.ts | 2 +- .../test/node/InstrumentationBase.test.ts | 4 ++-- .../test/node/scoped-package-instrumentation.test.ts | 4 ++-- 8 files changed, 16 insertions(+), 16 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation/src/autoLoaderUtils.ts b/experimental/packages/opentelemetry-instrumentation/src/autoLoaderUtils.ts index 91ef7a653d..4ac3f99e08 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/autoLoaderUtils.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/autoLoaderUtils.ts @@ -71,7 +71,7 @@ export function enableInstrumentations( // so enable only if user prevented that by setting enabled to false // this is to prevent double enabling but when calling register all // instrumentations should be now enabled - if (!instrumentation.getConfig().enabled) { + if (!instrumentation.getConfig().disabled) { instrumentation.enable(); } } diff --git a/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts index 3aa6376769..71105172fb 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts @@ -36,7 +36,7 @@ import { /** * Base abstract internal class for instrumenting node and web plugins */ -export abstract class InstrumentationAbstract implements Instrumentation { +export abstract class InstrumentationAbstract implements Instrumentation { protected _config: ConfigType; private _tracer: Tracer; @@ -50,7 +50,7 @@ export abstract class InstrumentationAbstract extends InstrumentationAbstract - implements types.Instrumentation + implements types.Instrumentation { constructor( instrumentationName: string, @@ -32,7 +32,7 @@ export abstract class InstrumentationBase extends InstrumentationAbstract - implements types.Instrumentation + implements types.Instrumentation { private _modules: InstrumentationModuleDefinition[]; private _hooks: (Hooked | Hook)[] = []; @@ -68,7 +68,7 @@ export abstract class InstrumentationBase { /** Instrumentation Name */ instrumentationName: string; @@ -48,10 +48,10 @@ export interface Instrumentation { setLoggerProvider?(loggerProvider: LoggerProvider): void; /** Method to set instrumentation config */ - setConfig(config: InstrumentationConfig): void; + setConfig(config: ConfigType): void; /** Method to get instrumentation config */ - getConfig(): InstrumentationConfig; + getConfig(): ConfigType; /** * Contains all supported versions. @@ -67,7 +67,7 @@ export interface InstrumentationConfig { * Whether to enable the plugin. * @default true */ - enabled?: boolean; + disabled?: boolean; } /** diff --git a/experimental/packages/opentelemetry-instrumentation/test/common/autoLoader.test.ts b/experimental/packages/opentelemetry-instrumentation/test/common/autoLoader.test.ts index e5532f2e5f..102862b29f 100644 --- a/experimental/packages/opentelemetry-instrumentation/test/common/autoLoader.test.ts +++ b/experimental/packages/opentelemetry-instrumentation/test/common/autoLoader.test.ts @@ -102,7 +102,7 @@ describe('autoLoader', () => { unload = undefined; } instrumentation = new FooInstrumentation('foo', '1', { - enabled: false, + disabled: false, }); enableSpy = sinon.spy(instrumentation, 'enable'); setTracerProviderSpy = sinon.stub(instrumentation, 'setTracerProvider'); diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/InstrumentationBase.test.ts b/experimental/packages/opentelemetry-instrumentation/test/node/InstrumentationBase.test.ts index a26d2c818a..8cc2341964 100644 --- a/experimental/packages/opentelemetry-instrumentation/test/node/InstrumentationBase.test.ts +++ b/experimental/packages/opentelemetry-instrumentation/test/node/InstrumentationBase.test.ts @@ -296,7 +296,7 @@ describe('InstrumentationBase', () => { class TestInstrumentation extends InstrumentationBase { constructor() { super('@opentelemetry/instrumentation-net-test', '0.0.0', { - enabled: false, + disabled: false, }); } init(): InstrumentationNodeModuleDefinition[] { @@ -338,7 +338,7 @@ describe('InstrumentationBase', () => { class TestInstrumentation extends InstrumentationBase { constructor() { super('@opentelemetry/instrumentation-absolute-path-test', '0.0.0', { - enabled: false, + disabled: false, }); } init(): InstrumentationNodeModuleDefinition[] { diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/scoped-package-instrumentation.test.ts b/experimental/packages/opentelemetry-instrumentation/test/node/scoped-package-instrumentation.test.ts index b3478cfe46..7c6809849e 100644 --- a/experimental/packages/opentelemetry-instrumentation/test/node/scoped-package-instrumentation.test.ts +++ b/experimental/packages/opentelemetry-instrumentation/test/node/scoped-package-instrumentation.test.ts @@ -71,7 +71,7 @@ describe('instrumenting a scoped package', function () { */ it('should patch internal file and main module', function () { const instrumentation = new TestInstrumentationSimple({ - enabled: false, + disabled: false, }); instrumentation.enable(); @@ -92,7 +92,7 @@ describe('instrumenting a scoped package', function () { */ it('should patch main module', function () { const instrumentation = new TestInstrumentationSimple({ - enabled: false, + disabled: false, }); instrumentation.enable(); From 192d3eb339720b5556d2d031872c7c920a3a0d42 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Thu, 25 Apr 2024 09:49:59 +0300 Subject: [PATCH 03/13] revert: enabled flag rename --- .../opentelemetry-instrumentation/src/autoLoaderUtils.ts | 2 +- .../opentelemetry-instrumentation/src/instrumentation.ts | 2 +- .../src/platform/browser/instrumentation.ts | 2 +- .../src/platform/node/instrumentation.ts | 2 +- .../packages/opentelemetry-instrumentation/src/types.ts | 2 +- .../test/common/autoLoader.test.ts | 2 +- .../test/node/InstrumentationBase.test.ts | 6 +++--- .../test/node/scoped-package-instrumentation.test.ts | 4 ++-- 8 files changed, 11 insertions(+), 11 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation/src/autoLoaderUtils.ts b/experimental/packages/opentelemetry-instrumentation/src/autoLoaderUtils.ts index 4ac3f99e08..91ef7a653d 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/autoLoaderUtils.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/autoLoaderUtils.ts @@ -71,7 +71,7 @@ export function enableInstrumentations( // so enable only if user prevented that by setting enabled to false // this is to prevent double enabling but when calling register all // instrumentations should be now enabled - if (!instrumentation.getConfig().disabled) { + if (!instrumentation.getConfig().enabled) { instrumentation.enable(); } } diff --git a/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts index 71105172fb..d3705e0653 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts @@ -50,7 +50,7 @@ export abstract class InstrumentationAbstract { unload = undefined; } instrumentation = new FooInstrumentation('foo', '1', { - disabled: false, + enabled: false, }); enableSpy = sinon.spy(instrumentation, 'enable'); setTracerProviderSpy = sinon.stub(instrumentation, 'setTracerProvider'); diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/InstrumentationBase.test.ts b/experimental/packages/opentelemetry-instrumentation/test/node/InstrumentationBase.test.ts index 8cc2341964..0a36f4e277 100644 --- a/experimental/packages/opentelemetry-instrumentation/test/node/InstrumentationBase.test.ts +++ b/experimental/packages/opentelemetry-instrumentation/test/node/InstrumentationBase.test.ts @@ -33,7 +33,7 @@ const CORE_MODULE = 'random_core'; class TestInstrumentation extends InstrumentationBase { constructor() { - super(MODULE_NAME, MODULE_VERSION); + super(MODULE_NAME, MODULE_VERSION, {}); } init() {} @@ -296,7 +296,7 @@ describe('InstrumentationBase', () => { class TestInstrumentation extends InstrumentationBase { constructor() { super('@opentelemetry/instrumentation-net-test', '0.0.0', { - disabled: false, + enabled: false, }); } init(): InstrumentationNodeModuleDefinition[] { @@ -338,7 +338,7 @@ describe('InstrumentationBase', () => { class TestInstrumentation extends InstrumentationBase { constructor() { super('@opentelemetry/instrumentation-absolute-path-test', '0.0.0', { - disabled: false, + enabled: false, }); } init(): InstrumentationNodeModuleDefinition[] { diff --git a/experimental/packages/opentelemetry-instrumentation/test/node/scoped-package-instrumentation.test.ts b/experimental/packages/opentelemetry-instrumentation/test/node/scoped-package-instrumentation.test.ts index 7c6809849e..b3478cfe46 100644 --- a/experimental/packages/opentelemetry-instrumentation/test/node/scoped-package-instrumentation.test.ts +++ b/experimental/packages/opentelemetry-instrumentation/test/node/scoped-package-instrumentation.test.ts @@ -71,7 +71,7 @@ describe('instrumenting a scoped package', function () { */ it('should patch internal file and main module', function () { const instrumentation = new TestInstrumentationSimple({ - disabled: false, + enabled: false, }); instrumentation.enable(); @@ -92,7 +92,7 @@ describe('instrumenting a scoped package', function () { */ it('should patch main module', function () { const instrumentation = new TestInstrumentationSimple({ - disabled: false, + enabled: false, }); instrumentation.enable(); From ff3d60d70cec7aaa405ec04ef29ceb8bf4293ec4 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Thu, 25 Apr 2024 10:09:52 +0300 Subject: [PATCH 04/13] fix: autoloader types --- .../test/common/autoLoaderUtils.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-instrumentation/test/common/autoLoaderUtils.test.ts b/experimental/packages/opentelemetry-instrumentation/test/common/autoLoaderUtils.test.ts index 2477225004..cdf7e5fd9c 100644 --- a/experimental/packages/opentelemetry-instrumentation/test/common/autoLoaderUtils.test.ts +++ b/experimental/packages/opentelemetry-instrumentation/test/common/autoLoaderUtils.test.ts @@ -38,7 +38,7 @@ describe('autoLoaderUtils', () => { describe('parseInstrumentationOptions', () => { it('should create a new instrumentation from class', () => { const { instrumentations } = parseInstrumentationOptions([ - FooInstrumentation, + FooInstrumentation as typeof InstrumentationBase, ]); assert.strictEqual(instrumentations.length, 1); const instrumentation = instrumentations[0]; From f6205706f8332cfd7ad003c92817318788fd3abc Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Thu, 25 Apr 2024 10:23:33 +0300 Subject: [PATCH 05/13] chore: lint fix --- .../opentelemetry-instrumentation/src/instrumentation.ts | 8 +++++--- .../src/platform/browser/instrumentation.ts | 4 +++- .../src/platform/node/instrumentation.ts | 9 +++++++-- .../packages/opentelemetry-instrumentation/src/types.ts | 4 +++- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts index d3705e0653..58b01629d5 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts @@ -32,11 +32,13 @@ import { InstrumentationConfig, } from './types'; - /** * Base abstract internal class for instrumenting node and web plugins */ -export abstract class InstrumentationAbstract implements Instrumentation { +export abstract class InstrumentationAbstract< + ConfigType extends InstrumentationConfig, +> implements Instrumentation +{ protected _config: ConfigType; private _tracer: Tracer; @@ -47,7 +49,7 @@ export abstract class InstrumentationAbstract +export abstract class InstrumentationBase< + ConfigType extends InstrumentationConfig = InstrumentationConfig, + > extends InstrumentationAbstract implements types.Instrumentation { diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts index f10e0e01a4..573d8f32a9 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts @@ -26,7 +26,10 @@ import { } from './RequireInTheMiddleSingleton'; import type { HookFn } from 'import-in-the-middle'; import * as ImportInTheMiddle from 'import-in-the-middle'; -import { InstrumentationConfig, InstrumentationModuleDefinition } from '../../types'; +import { + InstrumentationConfig, + InstrumentationModuleDefinition, +} from '../../types'; import { diag } from '@opentelemetry/api'; import type { OnRequireFn } from 'require-in-the-middle'; import { Hook } from 'require-in-the-middle'; @@ -35,7 +38,9 @@ import { readFileSync } from 'fs'; /** * Base abstract class for instrumenting node plugins */ -export abstract class InstrumentationBase +export abstract class InstrumentationBase< + ConfigType extends InstrumentationConfig = InstrumentationConfig, + > extends InstrumentationAbstract implements types.Instrumentation { diff --git a/experimental/packages/opentelemetry-instrumentation/src/types.ts b/experimental/packages/opentelemetry-instrumentation/src/types.ts index dcbe6aec8b..d204c3225d 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/types.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/types.ts @@ -18,7 +18,9 @@ import { TracerProvider, MeterProvider } from '@opentelemetry/api'; import { LoggerProvider } from '@opentelemetry/api-logs'; /** Interface Instrumentation to apply patch. */ -export interface Instrumentation { +export interface Instrumentation< + ConfigType extends InstrumentationConfig = InstrumentationConfig, +> { /** Instrumentation Name */ instrumentationName: string; From e64f9f8e94d83a29fea6c1e9705193826fb86367 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Thu, 25 Apr 2024 11:00:23 +0300 Subject: [PATCH 06/13] revert: default config in constructor to empty object --- .../packages/opentelemetry-instrumentation-fetch/src/fetch.ts | 2 +- .../src/platform/browser/instrumentation.ts | 2 +- .../src/platform/node/instrumentation.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts index 85aca36bf8..d459d3884f 100644 --- a/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts +++ b/experimental/packages/opentelemetry-instrumentation-fetch/src/fetch.ts @@ -79,7 +79,7 @@ export class FetchInstrumentation extends InstrumentationBase(); private _tasksCount = 0; - constructor(config: FetchInstrumentationConfig = {}) { + constructor(config?: FetchInstrumentationConfig) { super('@opentelemetry/instrumentation-fetch', VERSION, config); } diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/browser/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/browser/instrumentation.ts index e5b13b80bb..46f3b2cc72 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/platform/browser/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/browser/instrumentation.ts @@ -30,7 +30,7 @@ export abstract class InstrumentationBase< constructor( instrumentationName: string, instrumentationVersion: string, - config: ConfigType + config: ConfigType = {} as ConfigType // The cast here may be wrong as ConfigType may contain required fields ) { super(instrumentationName, instrumentationVersion, config); diff --git a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts index 573d8f32a9..674b1b963f 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/platform/node/instrumentation.ts @@ -53,7 +53,7 @@ export abstract class InstrumentationBase< constructor( instrumentationName: string, instrumentationVersion: string, - config: ConfigType + config: ConfigType = {} as ConfigType // The cast here may be wrong as ConfigType may contain required fields ) { super(instrumentationName, instrumentationVersion, config); From ed3589b80b701c852f2ac388b9bb493add4ee5c3 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Thu, 25 Apr 2024 11:06:49 +0300 Subject: [PATCH 07/13] revert: make constructor config default empty object --- .../opentelemetry-instrumentation-grpc/src/instrumentation.ts | 2 +- .../packages/opentelemetry-instrumentation-http/src/http.ts | 2 +- .../opentelemetry-instrumentation-xml-http-request/src/xhr.ts | 2 +- .../opentelemetry-instrumentation/src/instrumentation.ts | 4 +++- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts index d8dfde3f38..ae0ac8f7a4 100644 --- a/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts @@ -91,7 +91,7 @@ import { VERSION } from './version'; export class GrpcInstrumentation extends InstrumentationBase { private _metadataCapture: metadataCaptureType; - constructor(config: GrpcInstrumentationConfig = {}) { + constructor(config?: GrpcInstrumentationConfig) { super('@opentelemetry/instrumentation-grpc', VERSION, config); this._metadataCapture = this._createMetadataCapture(); } diff --git a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts index 7d63a9d771..7b6e91bf2d 100644 --- a/experimental/packages/opentelemetry-instrumentation-http/src/http.ts +++ b/experimental/packages/opentelemetry-instrumentation-http/src/http.ts @@ -70,7 +70,7 @@ export class HttpInstrumentation extends InstrumentationBase(); private _usedResources = new WeakSet(); - constructor(config: XMLHttpRequestInstrumentationConfig = {}) { + constructor(config?: XMLHttpRequestInstrumentationConfig) { super('@opentelemetry/instrumentation-xml-http-request', VERSION, config); } diff --git a/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts index 58b01629d5..51d3f0dae1 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts @@ -142,7 +142,9 @@ export abstract class InstrumentationAbstract< * Sets InstrumentationConfig to this plugin * @param InstrumentationConfig */ - public setConfig(config: ConfigType): void { + public setConfig(config: ConfigType = {} as ConfigType): void { + // the assertion that {} is compatible with ConfigType may not be correct, + // ConfigType should contain only optional fields, but there is no enforcement in place for that this._config = Object.assign({}, config); } From 93f0c55c8a84d19690f52862f1b59a92ae7f862e Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Thu, 25 Apr 2024 11:10:05 +0300 Subject: [PATCH 08/13] docs: note that instrumentation config fields are optional --- .../packages/opentelemetry-instrumentation/src/types.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/experimental/packages/opentelemetry-instrumentation/src/types.ts b/experimental/packages/opentelemetry-instrumentation/src/types.ts index d204c3225d..695cb7f0f7 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/types.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/types.ts @@ -64,6 +64,12 @@ export interface Instrumentation< supportedVersions?: string[]; } +/** + * Base interface for configuration options common to all instrumentations. + * This interface can be extended by individual instrumentations to include + * additional configuration options specific to that instrumentation. + * All configuration options must be optional. + */ export interface InstrumentationConfig { /** * Whether to enable the plugin. From fbb948f9a4183c577d05958f929d6c479b1447b6 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Thu, 25 Apr 2024 11:29:31 +0300 Subject: [PATCH 09/13] revert: deftaul type for generic --- .../opentelemetry-instrumentation/src/instrumentation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts index 51d3f0dae1..882322b9c1 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts @@ -36,7 +36,7 @@ import { * Base abstract internal class for instrumenting node and web plugins */ export abstract class InstrumentationAbstract< - ConfigType extends InstrumentationConfig, + ConfigType extends InstrumentationConfig = InstrumentationConfig, > implements Instrumentation { protected _config: ConfigType; From 74b812edfef2356f38b8d2a01862077c590b45c8 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Thu, 25 Apr 2024 11:31:50 +0300 Subject: [PATCH 10/13] revert: default object in instrumentation abstract constructor --- .../opentelemetry-instrumentation/src/instrumentation.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts b/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts index 882322b9c1..bd4a280fc8 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/instrumentation.ts @@ -49,7 +49,7 @@ export abstract class InstrumentationAbstract< constructor( public readonly instrumentationName: string, public readonly instrumentationVersion: string, - config: ConfigType + config: ConfigType = {} as ConfigType // assuming ConfigType is an object with optional fields only ) { this._config = { enabled: true, From 14023db1f1381a8bc910536039649fe914705ac5 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Thu, 25 Apr 2024 12:40:31 +0300 Subject: [PATCH 11/13] chore: lint fix --- .../packages/opentelemetry-instrumentation/src/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experimental/packages/opentelemetry-instrumentation/src/types.ts b/experimental/packages/opentelemetry-instrumentation/src/types.ts index 695cb7f0f7..5e74b06b5a 100644 --- a/experimental/packages/opentelemetry-instrumentation/src/types.ts +++ b/experimental/packages/opentelemetry-instrumentation/src/types.ts @@ -64,7 +64,7 @@ export interface Instrumentation< supportedVersions?: string[]; } -/** +/** * Base interface for configuration options common to all instrumentations. * This interface can be extended by individual instrumentations to include * additional configuration options specific to that instrumentation. From c5db3f8cd301aaa2a6d48bf45187221c93caaeb6 Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Thu, 25 Apr 2024 12:46:09 +0300 Subject: [PATCH 12/13] chore: changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32875a6c4f..881e236e0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/ ### :rocket: (Enhancement) +* feat(instrumentation): generic config type in instrumentation base [#4659](https://github.com/open-telemetry/opentelemetry-js/pull/4659) @blumamir + ### :bug: (Bug Fix) ### :books: (Refine Doc) From 4b344182a12697839de588d813d8a477dc56907b Mon Sep 17 00:00:00 2001 From: Amir Blum Date: Tue, 30 Apr 2024 20:19:33 +0300 Subject: [PATCH 13/13] fix: changelog in merge --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 881e236e0f..6aee4da9b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ For experimental package changes, see the [experimental CHANGELOG](experimental/ ### :rocket: (Enhancement) * feat(instrumentation): generic config type in instrumentation base [#4659](https://github.com/open-telemetry/opentelemetry-js/pull/4659) @blumamir +* feat: support node 22 [#4666](https://github.com/open-telemetry/opentelemetry-js/pull/4666) @dyladan ### :bug: (Bug Fix)