Skip to content

Commit 6dfe93c

Browse files
blumamirtrentm
blumamir
and
trentm
authored
chore: consistent and typed use of instrumentation config (#2289)
* refactor(amqplib): use generic instrumentation config type * chore: consistent and typed use of instrumentation config * fix(amqplib): read config on each invocation of interval * fix(graphql): ts error * fix(undici): tests * chore: lint fix * fix(graphql): remove uneeded non-null assertion operator * fix(graphql): move public type to right file * chore: lint fix --------- Co-authored-by: Trent Mick <[email protected]>
1 parent 8bcb561 commit 6dfe93c

File tree

37 files changed

+282
-397
lines changed

37 files changed

+282
-397
lines changed

‎plugins/node/instrumentation-amqplib/src/amqplib.ts‎

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -78,19 +78,13 @@ import { PACKAGE_NAME, PACKAGE_VERSION } from './version';
7878

7979
const supportedVersions = ['>=0.5.5 <1'];
8080

81-
export class AmqplibInstrumentation extends InstrumentationBase {
82-
protected override _config!: AmqplibInstrumentationConfig;
83-
81+
export class AmqplibInstrumentation extends InstrumentationBase<AmqplibInstrumentationConfig> {
8482
constructor(config: AmqplibInstrumentationConfig = {}) {
85-
super(
86-
PACKAGE_NAME,
87-
PACKAGE_VERSION,
88-
Object.assign({}, DEFAULT_CONFIG, config)
89-
);
83+
super(PACKAGE_NAME, PACKAGE_VERSION, { ...DEFAULT_CONFIG, ...config });
9084
}
9185

9286
override setConfig(config: AmqplibInstrumentationConfig = {}) {
93-
this._config = Object.assign({}, DEFAULT_CONFIG, config);
87+
super.setConfig({ ...DEFAULT_CONFIG, ...config });
9488
}
9589

9690
protected init() {
@@ -394,10 +388,11 @@ export class AmqplibInstrumentation extends InstrumentationBase {
394388
if (
395389
!Object.prototype.hasOwnProperty.call(channel, CHANNEL_SPANS_NOT_ENDED)
396390
) {
397-
if (self._config.consumeTimeoutMs) {
391+
const { consumeTimeoutMs } = self.getConfig();
392+
if (consumeTimeoutMs) {
398393
const timer = setInterval(() => {
399394
self.checkConsumeTimeoutOnChannel(channel);
400-
}, self._config.consumeTimeoutMs);
395+
}, consumeTimeoutMs);
401396
timer.unref();
402397
channel[CHANNEL_CONSUME_TIMEOUT_TIMER] = timer;
403398
}
@@ -455,9 +450,10 @@ export class AmqplibInstrumentation extends InstrumentationBase {
455450
parentContext
456451
);
457452

458-
if (self._config.consumeHook) {
453+
const { consumeHook } = self.getConfig();
454+
if (consumeHook) {
459455
safeExecuteInTheMiddle(
460-
() => self._config.consumeHook!(span, { moduleVersion, msg }),
456+
() => consumeHook(span, { moduleVersion, msg }),
461457
e => {
462458
if (e) {
463459
diag.error('amqplib instrumentation: consumerHook error', e);
@@ -516,10 +512,11 @@ export class AmqplibInstrumentation extends InstrumentationBase {
516512
options
517513
);
518514

519-
if (self._config.publishHook) {
515+
const { publishHook } = self.getConfig();
516+
if (publishHook) {
520517
safeExecuteInTheMiddle(
521518
() =>
522-
self._config.publishHook!(span, {
519+
publishHook(span, {
523520
moduleVersion,
524521
exchange,
525522
routingKey,
@@ -544,10 +541,11 @@ export class AmqplibInstrumentation extends InstrumentationBase {
544541
try {
545542
callback?.call(this, err, ok);
546543
} finally {
547-
if (self._config.publishConfirmHook) {
544+
const { publishConfirmHook } = self.getConfig();
545+
if (publishConfirmHook) {
548546
safeExecuteInTheMiddle(
549547
() =>
550-
self._config.publishConfirmHook!(span, {
548+
publishConfirmHook(span, {
551549
moduleVersion,
552550
exchange,
553551
routingKey,
@@ -616,10 +614,11 @@ export class AmqplibInstrumentation extends InstrumentationBase {
616614
options
617615
);
618616

619-
if (self._config.publishHook) {
617+
const { publishHook } = self.getConfig();
618+
if (publishHook) {
620619
safeExecuteInTheMiddle(
621620
() =>
622-
self._config.publishHook!(span, {
621+
publishHook(span, {
623622
moduleVersion,
624623
exchange,
625624
routingKey,
@@ -731,10 +730,11 @@ export class AmqplibInstrumentation extends InstrumentationBase {
731730
rejected: boolean | null,
732731
endOperation: EndOperation
733732
) {
734-
if (!this._config.consumeEndHook) return;
733+
const { consumeEndHook } = this.getConfig();
734+
if (!consumeEndHook) return;
735735

736736
safeExecuteInTheMiddle(
737-
() => this._config.consumeEndHook!(span, { msg, rejected, endOperation }),
737+
() => consumeEndHook(span, { msg, rejected, endOperation }),
738738
e => {
739739
if (e) {
740740
diag.error('amqplib instrumentation: consumerEndHook error', e);
@@ -748,15 +748,14 @@ export class AmqplibInstrumentation extends InstrumentationBase {
748748
const currentTime = hrTime();
749749
const spansNotEnded = channel[CHANNEL_SPANS_NOT_ENDED] ?? [];
750750
let i: number;
751+
const { consumeTimeoutMs } = this.getConfig();
751752
for (i = 0; i < spansNotEnded.length; i++) {
752753
const currMessage = spansNotEnded[i];
753754
const timeFromConsume = hrTimeDuration(
754755
currMessage.timeOfConsume,
755756
currentTime
756757
);
757-
if (
758-
hrTimeToMilliseconds(timeFromConsume) < this._config.consumeTimeoutMs!
759-
) {
758+
if (hrTimeToMilliseconds(timeFromConsume) < consumeTimeoutMs!) {
760759
break;
761760
}
762761
this.endConsumerSpan(

‎plugins/node/instrumentation-cucumber/src/instrumentation.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ type Step = (typeof steps)[number];
4848

4949
const supportedVersions = ['>=8.0.0 <11'];
5050

51-
export class CucumberInstrumentation extends InstrumentationBase {
51+
export class CucumberInstrumentation extends InstrumentationBase<CucumberInstrumentationConfig> {
5252
private module: Cucumber | undefined;
5353

5454
constructor(config: CucumberInstrumentationConfig = {}) {

‎plugins/node/instrumentation-dataloader/src/instrumentation.ts‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ type DataloaderInternal = typeof Dataloader.prototype & {
4343
type LoadFn = (typeof Dataloader.prototype)['load'];
4444
type LoadManyFn = (typeof Dataloader.prototype)['loadMany'];
4545

46-
export class DataloaderInstrumentation extends InstrumentationBase {
46+
export class DataloaderInstrumentation extends InstrumentationBase<DataloaderInstrumentationConfig> {
4747
constructor(config: DataloaderInstrumentationConfig = {}) {
4848
super(PACKAGE_NAME, PACKAGE_VERSION, config);
4949
}
@@ -72,14 +72,6 @@ export class DataloaderInstrumentation extends InstrumentationBase {
7272
];
7373
}
7474

75-
override getConfig(): DataloaderInstrumentationConfig {
76-
return this._config;
77-
}
78-
79-
override setConfig(config: DataloaderInstrumentationConfig = {}) {
80-
this._config = config;
81-
}
82-
8375
private shouldCreateSpans(): boolean {
8476
const config = this.getConfig();
8577
const hasParentSpan = trace.getSpan(context.active()) !== undefined;

‎plugins/node/instrumentation-fs/src/instrumentation.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ function patchedFunctionWithOriginalProperties<
5151
return Object.assign(patchedFunction, original);
5252
}
5353

54-
export class FsInstrumentation extends InstrumentationBase {
54+
export class FsInstrumentation extends InstrumentationBase<FsInstrumentationConfig> {
5555
constructor(config: FsInstrumentationConfig = {}) {
5656
super(PACKAGE_NAME, PACKAGE_VERSION, config);
5757
}
@@ -438,7 +438,7 @@ export class FsInstrumentation extends InstrumentationBase {
438438
protected _runCreateHook(
439439
...args: Parameters<CreateHook>
440440
): ReturnType<CreateHook> {
441-
const { createHook } = this.getConfig() as FsInstrumentationConfig;
441+
const { createHook } = this.getConfig();
442442
if (typeof createHook === 'function') {
443443
try {
444444
return createHook(...args);
@@ -450,7 +450,7 @@ export class FsInstrumentation extends InstrumentationBase {
450450
}
451451

452452
protected _runEndHook(...args: Parameters<EndHook>): ReturnType<EndHook> {
453-
const { endHook } = this.getConfig() as FsInstrumentationConfig;
453+
const { endHook } = this.getConfig();
454454
if (typeof endHook === 'function') {
455455
try {
456456
endHook(...args);
@@ -467,7 +467,7 @@ export class FsInstrumentation extends InstrumentationBase {
467467
return false;
468468
}
469469

470-
const { requireParentSpan } = this.getConfig() as FsInstrumentationConfig;
470+
const { requireParentSpan } = this.getConfig();
471471
if (requireParentSpan) {
472472
const parentSpan = api.trace.getSpan(context);
473473
if (parentSpan == null) {

‎plugins/node/instrumentation-kafkajs/src/instrumentation.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,7 @@ import {
5353
isWrapped,
5454
} from '@opentelemetry/instrumentation';
5555

56-
export class KafkaJsInstrumentation extends InstrumentationBase {
57-
protected override _config!: KafkaJsInstrumentationConfig;
58-
56+
export class KafkaJsInstrumentation extends InstrumentationBase<KafkaJsInstrumentationConfig> {
5957
constructor(config: KafkaJsInstrumentationConfig = {}) {
6058
super(PACKAGE_NAME, PACKAGE_VERSION, config);
6159
}
@@ -367,9 +365,10 @@ export class KafkaJsInstrumentation extends InstrumentationBase {
367365
context
368366
);
369367

370-
if (this._config?.consumerHook && message) {
368+
const { consumerHook } = this.getConfig();
369+
if (consumerHook && message) {
371370
safeExecuteInTheMiddle(
372-
() => this._config.consumerHook!(span, { topic, message }),
371+
() => consumerHook(span, { topic, message }),
373372
e => {
374373
if (e) this._diag.error('consumerHook error', e);
375374
},
@@ -392,9 +391,10 @@ export class KafkaJsInstrumentation extends InstrumentationBase {
392391
message.headers = message.headers ?? {};
393392
propagation.inject(trace.setSpan(context.active(), span), message.headers);
394393

395-
if (this._config?.producerHook) {
394+
const { producerHook } = this.getConfig();
395+
if (producerHook) {
396396
safeExecuteInTheMiddle(
397-
() => this._config.producerHook!(span, { topic, message }),
397+
() => producerHook(span, { topic, message }),
398398
e => {
399399
if (e) this._diag.error('producerHook error', e);
400400
},

‎plugins/node/instrumentation-mongoose/src/mongoose.ts‎

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -57,17 +57,11 @@ const contextCaptureFunctions = [
5757
// calls. this bypass the unlinked spans issue on thenables await operations.
5858
export const _STORED_PARENT_SPAN: unique symbol = Symbol('stored-parent-span');
5959

60-
export class MongooseInstrumentation extends InstrumentationBase {
61-
protected override _config!: MongooseInstrumentationConfig;
62-
60+
export class MongooseInstrumentation extends InstrumentationBase<MongooseInstrumentationConfig> {
6361
constructor(config: MongooseInstrumentationConfig = {}) {
6462
super(PACKAGE_NAME, PACKAGE_VERSION, config);
6563
}
6664

67-
override setConfig(config: MongooseInstrumentationConfig = {}) {
68-
this._config = Object.assign({}, config);
69-
}
70-
7165
protected init(): InstrumentationModuleDefinition {
7266
const module = new InstrumentationNodeModuleDefinition(
7367
'mongoose',
@@ -140,20 +134,23 @@ export class MongooseInstrumentation extends InstrumentationBase {
140134
return (originalAggregate: Function) => {
141135
return function exec(this: any, callback?: Function) {
142136
if (
143-
self._config.requireParentSpan &&
137+
self.getConfig().requireParentSpan &&
144138
trace.getSpan(context.active()) === undefined
145139
) {
146140
return originalAggregate.apply(this, arguments);
147141
}
148142

149143
const parentSpan = this[_STORED_PARENT_SPAN];
150144
const attributes: Attributes = {};
151-
if (self._config.dbStatementSerializer) {
152-
attributes[SEMATTRS_DB_STATEMENT] =
153-
self._config.dbStatementSerializer('aggregate', {
145+
const { dbStatementSerializer } = self.getConfig();
146+
if (dbStatementSerializer) {
147+
attributes[SEMATTRS_DB_STATEMENT] = dbStatementSerializer(
148+
'aggregate',
149+
{
154150
options: this.options,
155151
aggregatePipeline: this._pipeline,
156-
});
152+
}
153+
);
157154
}
158155

159156
const span = self._startSpan(
@@ -181,22 +178,22 @@ export class MongooseInstrumentation extends InstrumentationBase {
181178
return (originalExec: Function) => {
182179
return function exec(this: any, callback?: Function) {
183180
if (
184-
self._config.requireParentSpan &&
181+
self.getConfig().requireParentSpan &&
185182
trace.getSpan(context.active()) === undefined
186183
) {
187184
return originalExec.apply(this, arguments);
188185
}
189186

190187
const parentSpan = this[_STORED_PARENT_SPAN];
191188
const attributes: Attributes = {};
192-
if (self._config.dbStatementSerializer) {
193-
attributes[SEMATTRS_DB_STATEMENT] =
194-
self._config.dbStatementSerializer(this.op, {
195-
condition: this._conditions,
196-
updates: this._update,
197-
options: this.options,
198-
fields: this._fields,
199-
});
189+
const { dbStatementSerializer } = self.getConfig();
190+
if (dbStatementSerializer) {
191+
attributes[SEMATTRS_DB_STATEMENT] = dbStatementSerializer(this.op, {
192+
condition: this._conditions,
193+
updates: this._update,
194+
options: this.options,
195+
fields: this._fields,
196+
});
200197
}
201198
const span = self._startSpan(
202199
this.mongooseCollection,
@@ -223,7 +220,7 @@ export class MongooseInstrumentation extends InstrumentationBase {
223220
return (originalOnModelFunction: Function) => {
224221
return function method(this: any, options?: any, callback?: Function) {
225222
if (
226-
self._config.requireParentSpan &&
223+
self.getConfig().requireParentSpan &&
227224
trace.getSpan(context.active()) === undefined
228225
) {
229226
return originalOnModelFunction.apply(this, arguments);
@@ -234,9 +231,12 @@ export class MongooseInstrumentation extends InstrumentationBase {
234231
serializePayload.options = options;
235232
}
236233
const attributes: Attributes = {};
237-
if (self._config.dbStatementSerializer) {
238-
attributes[SEMATTRS_DB_STATEMENT] =
239-
self._config.dbStatementSerializer(op, serializePayload);
234+
const { dbStatementSerializer } = self.getConfig();
235+
if (dbStatementSerializer) {
236+
attributes[SEMATTRS_DB_STATEMENT] = dbStatementSerializer(
237+
op,
238+
serializePayload
239+
);
240240
}
241241
const span = self._startSpan(
242242
this.constructor.collection,
@@ -331,7 +331,7 @@ export class MongooseInstrumentation extends InstrumentationBase {
331331
originalThis,
332332
span,
333333
args,
334-
self._config.responseHook,
334+
self.getConfig().responseHook,
335335
moduleVersion
336336
)
337337
);
@@ -342,14 +342,14 @@ export class MongooseInstrumentation extends InstrumentationBase {
342342
return handlePromiseResponse(
343343
response,
344344
span,
345-
self._config.responseHook,
345+
self.getConfig().responseHook,
346346
moduleVersion
347347
);
348348
}
349349
}
350350

351351
private _callOriginalFunction<T>(originalFunction: (...args: any[]) => T): T {
352-
if (this._config?.suppressInternalInstrumentation) {
352+
if (this.getConfig().suppressInternalInstrumentation) {
353353
return context.with(suppressTracing(context.active()), originalFunction);
354354
} else {
355355
return originalFunction();

‎plugins/node/instrumentation-runtime-node/src/instrumentation.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const DEFAULT_CONFIG: RuntimeNodeInstrumentationConfig = {
2626
eventLoopUtilizationMeasurementInterval: 5000,
2727
};
2828

29-
export class RuntimeNodeInstrumentation extends InstrumentationBase {
29+
export class RuntimeNodeInstrumentation extends InstrumentationBase<RuntimeNodeInstrumentationConfig> {
3030
private _ELUs: EventLoopUtilization[] = [];
3131
private _interval: NodeJS.Timeout | undefined;
3232

@@ -79,8 +79,7 @@ export class RuntimeNodeInstrumentation extends InstrumentationBase {
7979
clearInterval(this._interval);
8080
this._interval = setInterval(
8181
() => this._addELU(),
82-
(this._config as RuntimeNodeInstrumentationConfig)
83-
.eventLoopUtilizationMeasurementInterval
82+
this.getConfig().eventLoopUtilizationMeasurementInterval
8483
);
8584

8685
// unref so that it does not keep the process running if disable() is never called

0 commit comments

Comments
 (0)