Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: align config constructor pattern across instrumentations #2162

Merged
merged 13 commits into from May 2, 2024
2 changes: 1 addition & 1 deletion plugins/node/instrumentation-amqplib/src/amqplib.ts
Expand Up @@ -77,7 +77,7 @@ import { VERSION } from './version';
export class AmqplibInstrumentation extends InstrumentationBase {
protected override _config!: AmqplibInstrumentationConfig;

constructor(config?: AmqplibInstrumentationConfig) {
constructor(config: AmqplibInstrumentationConfig = {}) {
super(
'@opentelemetry/instrumentation-amqplib',
VERSION,
Expand Down
Expand Up @@ -76,7 +76,7 @@ export class DataloaderInstrumentation extends InstrumentationBase {
return this._config;
}

override setConfig(config: DataloaderInstrumentationConfig) {
override setConfig(config: DataloaderInstrumentationConfig = {}) {
this._config = config;
}

Expand Down
2 changes: 1 addition & 1 deletion plugins/node/instrumentation-fs/src/instrumentation.ts
Expand Up @@ -52,7 +52,7 @@ function patchedFunctionWithOriginalProperties<
}

export default class FsInstrumentation extends InstrumentationBase {
constructor(config?: FsInstrumentationConfig) {
constructor(config: FsInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-fs', VERSION, config);
}

Expand Down
Expand Up @@ -23,7 +23,7 @@ import {
import { VERSION } from './version';

export default class LruMemoizerInstrumentation extends InstrumentationBase {
constructor(config?: InstrumentationConfig) {
constructor(config: InstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-lru-memoizer', VERSION, config);
}

Expand Down
6 changes: 1 addition & 5 deletions plugins/node/instrumentation-mongoose/src/mongoose.ts
Expand Up @@ -61,11 +61,7 @@ export class MongooseInstrumentation extends InstrumentationBase {
protected override _config!: MongooseInstrumentationConfig;

constructor(config: MongooseInstrumentationConfig = {}) {
super(
'@opentelemetry/instrumentation-mongoose',
VERSION,
Object.assign({}, config)
);
super('@opentelemetry/instrumentation-mongoose', VERSION, config);
}

override setConfig(config: MongooseInstrumentationConfig = {}) {
Expand Down
Expand Up @@ -70,7 +70,7 @@ function setDatabase(this: ApproxConnection, databaseName: string) {
export class TediousInstrumentation extends InstrumentationBase {
static readonly COMPONENT = 'tedious';

constructor(config?: TediousInstrumentationConfig) {
constructor(config: TediousInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-tedious', VERSION, config);
}

Expand Down
4 changes: 2 additions & 2 deletions plugins/node/instrumentation-undici/src/undici.ts
Expand Up @@ -66,7 +66,7 @@ export class UndiciInstrumentation extends InstrumentationBase {
private _recordFromReq = new WeakMap<UndiciRequest, InstrumentationRecord>();

private _httpClientDurationHistogram!: Histogram;
constructor(config?: UndiciInstrumentationConfig) {
constructor(config: UndiciInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-undici', VERSION, config);
this.setConfig(config);
}
Expand Down Expand Up @@ -111,7 +111,7 @@ export class UndiciInstrumentation extends InstrumentationBase {
this.subscribeToChannel('undici:request:error', this.onError.bind(this));
}

override setConfig(config?: UndiciInstrumentationConfig): void {
override setConfig(config: UndiciInstrumentationConfig = {}): void {
super.setConfig(config);

if (config?.enabled) {
Expand Down
Expand Up @@ -77,8 +77,10 @@ export class AwsLambdaInstrumentation extends InstrumentationBase {
private _traceForceFlusher?: () => Promise<void>;
private _metricForceFlusher?: () => Promise<void>;

constructor(protected override _config: AwsLambdaInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-aws-lambda', VERSION, _config);
protected override _config!: AwsLambdaInstrumentationConfig;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another bit we may want to align, which is the way we type config in our instrumentations. Here we use the override keyword so we tell typescript to treat the property as an AwsLambdaInstrumentationConfig type. And in other instrumentations we have a getter that casts the config property to the type we need

getConfig(): AwsLambdaInstrumentationConfig {
  return this._config as AwsLambdaInstrumentationConfig;
}

Should we encourage one over he other?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once open-telemetry/opentelemetry-js#4659 lands, we can cleanup all the config handling variations as the base class would already be typed correctly. Eager to apply this cleanup in all contrib instrumentations


constructor(config: AwsLambdaInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-aws-lambda', VERSION, config);
if (this._config.disableAwsContextPropagation == null) {
if (
typeof env['OTEL_LAMBDA_DISABLE_AWS_CONTEXT_PROPAGATION'] ===
Expand Down
Expand Up @@ -78,11 +78,7 @@ export class AwsInstrumentation extends InstrumentationBase {
private servicesExtensions: ServicesExtensions = new ServicesExtensions();

constructor(config: AwsSdkInstrumentationConfig = {}) {
super(
'@opentelemetry/instrumentation-aws-sdk',
VERSION,
Object.assign({}, config)
);
super('@opentelemetry/instrumentation-aws-sdk', VERSION, config);
}

override setConfig(config: AwsSdkInstrumentationConfig = {}) {
Expand Down
Expand Up @@ -103,7 +103,7 @@ export class BunyanInstrumentation extends InstrumentationBase {
return this._config;
}

override setConfig(config: BunyanInstrumentationConfig) {
override setConfig(config: BunyanInstrumentationConfig = {}) {
this._config = Object.assign({}, DEFAULT_CONFIG, config);
}

Expand Down
Expand Up @@ -43,11 +43,7 @@ export const ANONYMOUS_NAME = 'anonymous';
/** Connect instrumentation for OpenTelemetry */
export class ConnectInstrumentation extends InstrumentationBase {
constructor(config: InstrumentationConfig = {}) {
super(
'@opentelemetry/instrumentation-connect',
VERSION,
Object.assign({}, config)
);
super('@opentelemetry/instrumentation-connect', VERSION, config);
}

init() {
Expand Down
Expand Up @@ -37,8 +37,10 @@ import {
* Dns instrumentation for Opentelemetry
*/
export class DnsInstrumentation extends InstrumentationBase {
constructor(protected override _config: DnsInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-dns', VERSION, _config);
protected override _config!: DnsInstrumentationConfig;

constructor(config: DnsInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-dns', VERSION, config);
}

init(): (
Expand Down
Expand Up @@ -51,11 +51,7 @@ import {
/** Express instrumentation for OpenTelemetry */
export class ExpressInstrumentation extends InstrumentationBase {
constructor(config: ExpressInstrumentationConfig = {}) {
super(
'@opentelemetry/instrumentation-express',
VERSION,
Object.assign({}, config)
);
super('@opentelemetry/instrumentation-express', VERSION, config);
}

override setConfig(config: ExpressInstrumentationConfig = {}) {
Expand Down
Expand Up @@ -48,11 +48,7 @@ export const ANONYMOUS_NAME = 'anonymous';
/** Fastify instrumentation for OpenTelemetry */
export class FastifyInstrumentation extends InstrumentationBase {
constructor(config: FastifyInstrumentationConfig = {}) {
super(
'@opentelemetry/instrumentation-fastify',
VERSION,
Object.assign({}, config)
);
super('@opentelemetry/instrumentation-fastify', VERSION, config);
}

override setConfig(config: FastifyInstrumentationConfig = {}) {
Expand Down
Expand Up @@ -33,7 +33,7 @@ export default class Instrumentation extends InstrumentationBase {
private _isDisabled = false;

constructor(config: InstrumentationConfig = {}) {
super(`@opentelemetry/instrumentation-${MODULE_NAME}`, VERSION);
super(`@opentelemetry/instrumentation-${MODULE_NAME}`, VERSION, config);
}

init() {
Expand Down
Expand Up @@ -18,7 +18,6 @@ import { context, trace } from '@opentelemetry/api';
import {
isWrapped,
InstrumentationBase,
InstrumentationConfig,
InstrumentationNodeModuleDefinition,
InstrumentationNodeModuleFile,
safeExecuteInTheMiddle,
Expand Down Expand Up @@ -65,9 +64,7 @@ const DEFAULT_CONFIG: GraphQLInstrumentationConfig = {
const supportedVersions = ['>=14 <17'];

export class GraphQLInstrumentation extends InstrumentationBase {
constructor(
config: GraphQLInstrumentationConfig & InstrumentationConfig = {}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed it because GraphQLInstrumentationConfig already extends InstrumentationConfig so it was redundant

) {
constructor(config: GraphQLInstrumentationConfig = {}) {
super(
'@opentelemetry/instrumentation-graphql',
VERSION,
Expand Down
Expand Up @@ -49,7 +49,7 @@ import {

/** Hapi instrumentation for OpenTelemetry */
export class HapiInstrumentation extends InstrumentationBase {
constructor(config?: InstrumentationConfig) {
constructor(config: InstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-hapi', VERSION, config);
}

Expand Down
Expand Up @@ -36,11 +36,13 @@ const DEFAULT_CONFIG: IORedisInstrumentationConfig = {
};

export class IORedisInstrumentation extends InstrumentationBase {
constructor(_config: IORedisInstrumentationConfig = {}) {
protected override _config!: IORedisInstrumentationConfig;

constructor(config: IORedisInstrumentationConfig = {}) {
super(
'@opentelemetry/instrumentation-ioredis',
VERSION,
Object.assign({}, DEFAULT_CONFIG, _config)
Object.assign({}, DEFAULT_CONFIG, config)
);
}

Expand Down
Expand Up @@ -36,11 +36,7 @@ import {
/** Koa instrumentation for OpenTelemetry */
export class KoaInstrumentation extends InstrumentationBase {
constructor(config: KoaInstrumentationConfig = {}) {
super(
'@opentelemetry/instrumentation-koa',
VERSION,
Object.assign({}, config)
);
super('@opentelemetry/instrumentation-koa', VERSION, config);
}

override setConfig(config: KoaInstrumentationConfig = {}) {
Expand Down
Expand Up @@ -58,8 +58,10 @@ export class MongoDBInstrumentation extends InstrumentationBase {
private _connectionsUsage!: UpDownCounter;
private _poolName!: string;

constructor(protected override _config: MongoDBInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-mongodb', VERSION, _config);
protected override _config!: MongoDBInstrumentationConfig;

constructor(config: MongoDBInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-mongodb', VERSION, config);
}

override _updateMetricInstruments() {
Expand Down
Expand Up @@ -57,7 +57,7 @@ export class MySQLInstrumentation extends InstrumentationBase {
};
private _connectionsUsage!: UpDownCounter;

constructor(config?: MySQLInstrumentationConfig) {
constructor(config: MySQLInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-mysql', VERSION, config);
this._setMetricInstruments();
}
Expand Down
Expand Up @@ -44,7 +44,7 @@ export class MySQL2Instrumentation extends InstrumentationBase {
[SEMATTRS_DB_SYSTEM]: DBSYSTEMVALUES_MYSQL,
};

constructor(config?: MySQL2InstrumentationConfig) {
constructor(config: MySQL2InstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-mysql2', VERSION, config);
}

Expand Down
Expand Up @@ -36,7 +36,7 @@ export class Instrumentation extends InstrumentationBase {
};

constructor(config: InstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-nestjs-core', VERSION);
super('@opentelemetry/instrumentation-nestjs-core', VERSION, config);
}

init() {
Expand Down
Expand Up @@ -35,8 +35,8 @@ import { TLSSocket } from 'tls';
import type * as net from 'net';

export class NetInstrumentation extends InstrumentationBase {
constructor(_config?: InstrumentationConfig) {
super('@opentelemetry/instrumentation-net', VERSION, _config);
constructor(config: InstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-net', VERSION, config);
}

init(): InstrumentationNodeModuleDefinition[] {
Expand Down
Expand Up @@ -44,11 +44,7 @@ import { SpanNames } from './enums/SpanNames';

export class PgInstrumentation extends InstrumentationBase {
constructor(config: PgInstrumentationConfig = {}) {
super(
'@opentelemetry/instrumentation-pg',
VERSION,
Object.assign({}, config)
);
super('@opentelemetry/instrumentation-pg', VERSION, config);
}

protected init() {
Expand Down
Expand Up @@ -97,7 +97,7 @@ export class PinoInstrumentation extends InstrumentationBase {
return this._config;
}

override setConfig(config: PinoInstrumentationConfig) {
override setConfig(config: PinoInstrumentationConfig = {}) {
this._config = config;
}

Expand Down
Expand Up @@ -54,8 +54,10 @@ const DEFAULT_CONFIG: RedisInstrumentationConfig = {
export class RedisInstrumentation extends InstrumentationBase {
static readonly COMPONENT = 'redis';

constructor(protected override _config: RedisInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-redis-4', VERSION, _config);
protected override _config!: RedisInstrumentationConfig;

constructor(config: RedisInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-redis-4', VERSION, config);
}

override setConfig(config: RedisInstrumentationConfig = {}) {
Expand Down
Expand Up @@ -35,8 +35,10 @@ const DEFAULT_CONFIG: RedisInstrumentationConfig = {
export class RedisInstrumentation extends InstrumentationBase {
static readonly COMPONENT = 'redis';

constructor(protected override _config: RedisInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-redis', VERSION, _config);
protected override _config!: RedisInstrumentationConfig;

constructor(config: RedisInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-redis', VERSION, config);
}

override setConfig(config: RedisInstrumentationConfig = {}) {
Expand Down
Expand Up @@ -40,7 +40,7 @@ export class RestifyInstrumentation extends InstrumentationBase {
super(
`@opentelemetry/instrumentation-${constants.MODULE_NAME}`,
VERSION,
Object.assign({}, config)
config
);
}

Expand Down
Expand Up @@ -35,7 +35,7 @@ import AttributeNames from './enums/AttributeNames';
import LayerType from './enums/LayerType';

export default class RouterInstrumentation extends InstrumentationBase {
constructor(config?: InstrumentationConfig) {
constructor(config: InstrumentationConfig = {}) {
super(
`@opentelemetry/instrumentation-${constants.MODULE_NAME}`,
VERSION,
Expand Down
Expand Up @@ -116,7 +116,7 @@ export class WinstonInstrumentation extends InstrumentationBase {
return this._config;
}

override setConfig(config: WinstonInstrumentationConfig) {
override setConfig(config: WinstonInstrumentationConfig = {}) {
this._config = config;
}

Expand Down
Expand Up @@ -67,7 +67,7 @@ export class UserInteractionInstrumentation extends InstrumentationBase {
private _eventNames: Set<EventName>;
private _shouldPreventSpanCreation: ShouldPreventSpanCreation;

constructor(config?: UserInteractionInstrumentationConfig) {
constructor(config: UserInteractionInstrumentationConfig = {}) {
super('@opentelemetry/instrumentation-user-interaction', VERSION, config);
this._eventNames = new Set(config?.eventNames ?? DEFAULT_EVENT_NAMES);
this._shouldPreventSpanCreation =
Expand Down