Skip to content

Commit

Permalink
fix(amqplib): stop importing from amqplib directly in compiled types (
Browse files Browse the repository at this point in the history
open-telemetry#1394)

* fix(amqplib): make all instrumentation methods private

Previously the `amqplib.d.ts` file would import `./utils` (resolved as
`utils.d.ts`) which imports `amqplib` directly. This can cause
typechecking issues for users who don't use `amqplib` but still import
the `amqplib` instrumentation through the node autoinstrumentation.

The `./utils` import was generated by typescript because some of the
protected methods have their argument and return types defined in
`utils.ts`. Now that these methods are private, typescript no longer
needs to generate the type information for those methods and omits the
`./utils` import.

Signed-off-by: Boris Bera <bbera@coveo.com>

* fix(amqplib): reduce the number of vendored types

Signed-off-by: Boris Bera <bbera@coveo.com>

---------

Signed-off-by: Boris Bera <bbera@coveo.com>
  • Loading branch information
dotboris authored Feb 14, 2023
1 parent 6d08157 commit 9d0198c
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 202 deletions.
16 changes: 9 additions & 7 deletions plugins/node/instrumentation-amqplib/src/amqplib.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,17 @@ import {
MessagingOperationValues,
MessagingDestinationKindValues,
} from '@opentelemetry/semantic-conventions';
import {
AmqplibInstrumentationConfig,
import type {
Connection,
ConsumeMessage,
DEFAULT_CONFIG,
EndOperation,
Message,
Options,
Replies,
} from 'amqplib';
import {
AmqplibInstrumentationConfig,
DEFAULT_CONFIG,
EndOperation,
} from './types';
import {
CHANNEL_CONSUME_TIMEOUT_TIMER,
Expand Down Expand Up @@ -368,7 +370,7 @@ export class AmqplibInstrumentation extends InstrumentationBase {
};
}

protected getConsumePatch(
private getConsumePatch(
moduleVersion: string | undefined,
original: Function
) {
Expand Down Expand Up @@ -466,7 +468,7 @@ export class AmqplibInstrumentation extends InstrumentationBase {
};
}

protected getConfirmedPublishPatch(
private getConfirmedPublishPatch(
moduleVersion: string | undefined,
original: Function
) {
Expand Down Expand Up @@ -563,7 +565,7 @@ export class AmqplibInstrumentation extends InstrumentationBase {
};
}

protected getPublishPatch(
private getPublishPatch(
moduleVersion: string | undefined,
original: Function
) {
Expand Down
219 changes: 24 additions & 195 deletions plugins/node/instrumentation-amqplib/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export interface PublishInfo {
exchange: string;
routingKey: string;
content: Buffer;
options?: Options.Publish;
options?: AmqplibPublishOptions;
isConfirmChannel?: boolean;
}

Expand Down Expand Up @@ -104,192 +104,29 @@ export const DEFAULT_CONFIG: AmqplibInstrumentationConfig = {

// The following types are vendored from `@types/amqplib@0.10.1` - commit SHA: 4205e03127692a40b4871709a7134fe4e2ed5510

// Vendored from: /~https://github.com/DefinitelyTyped/DefinitelyTyped/blob/4205e03127692a40b4871709a7134fe4e2ed5510/types/amqplib/properties.d.ts#LL23C18-L23C25
// eslint-disable-next-line @typescript-eslint/no-namespace
export namespace Options {
export interface Connect {
/**
* The to be used protocol
*
* Default value: 'amqp'
*/
protocol?: string;
/**
* Hostname used for connecting to the server.
*
* Default value: 'localhost'
*/
hostname?: string;
/**
* Port used for connecting to the server.
*
* Default value: 5672
*/
port?: number;
/**
* Username used for authenticating against the server.
*
* Default value: 'guest'
*/
username?: string;
/**
* Password used for authenticating against the server.
*
* Default value: 'guest'
*/
password?: string;
/**
* The desired locale for error messages. RabbitMQ only ever uses en_US
*
* Default value: 'en_US'
*/
locale?: string;
/**
* The size in bytes of the maximum frame allowed over the connection. 0 means
* no limit (but since frames have a size field which is an unsigned 32 bit integer, it’s perforce 2^32 - 1).
*
* Default value: 0x1000 (4kb) - That's the allowed minimum, it will fit many purposes
*/
frameMax?: number;
/**
* The period of the connection heartbeat in seconds.
*
* Default value: 0
*/
heartbeat?: number;
/**
* What VHost shall be used.
*
* Default value: '/'
*/
vhost?: string;
}

export interface AssertQueue {
exclusive?: boolean;
durable?: boolean;
autoDelete?: boolean;
arguments?: any;
messageTtl?: number;
expires?: number;
deadLetterExchange?: string;
deadLetterRoutingKey?: string;
maxLength?: number;
maxPriority?: number;
}
export interface DeleteQueue {
ifUnused?: boolean;
ifEmpty?: boolean;
}
export interface AssertExchange {
durable?: boolean;
internal?: boolean;
autoDelete?: boolean;
alternateExchange?: string;
arguments?: any;
}
export interface DeleteExchange {
ifUnused?: boolean;
}
export interface Publish {
expiration?: string | number;
userId?: string;
CC?: string | string[];

mandatory?: boolean;
persistent?: boolean;
deliveryMode?: boolean | number;
BCC?: string | string[];

contentType?: string;
contentEncoding?: string;
headers?: any;
priority?: number;
correlationId?: string;
replyTo?: string;
messageId?: string;
timestamp?: number;
type?: string;
appId?: string;
}

export interface Consume {
consumerTag?: string;
noLocal?: boolean;
noAck?: boolean;
exclusive?: boolean;
priority?: number;
arguments?: any;
}

export interface Get {
noAck?: boolean;
}
}

// Vendored from: /~https://github.com/DefinitelyTyped/DefinitelyTyped/blob/4205e03127692a40b4871709a7134fe4e2ed5510/types/amqplib/properties.d.ts#L214
interface ServerProperties {
host: string;
product: string;
version: string;
platform: string;
copyright?: string;
information: string;
[key: string]: string | undefined;
}

// Vendored from: /~https://github.com/DefinitelyTyped/DefinitelyTyped/blob/4205e03127692a40b4871709a7134fe4e2ed5510/types/amqplib/properties.d.ts#L1
// eslint-disable-next-line @typescript-eslint/no-namespace
export namespace Replies {
// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface Empty {}

export interface AssertQueue {
queue: string;
messageCount: number;
consumerCount: number;
}
export interface PurgeQueue {
messageCount: number;
}
export interface DeleteQueue {
messageCount: number;
}
export interface AssertExchange {
exchange: string;
}
export interface Consume {
consumerTag: string;
}
}

// Vendored from: /~https://github.com/DefinitelyTyped/DefinitelyTyped/blob/4205e03127692a40b4871709a7134fe4e2ed5510/types/amqplib/callback_api.d.ts#L55
export interface ConfirmChannel {
publish(
exchange: string,
routingKey: string,
content: Buffer,
options?: Options.Publish,
callback?: (err: any, ok: Replies.Empty) => void
): boolean;
sendToQueue(
queue: string,
content: Buffer,
options?: Options.Publish,
callback?: (err: any, ok: Replies.Empty) => void
): boolean;

waitForConfirms(): Promise<void>;
}

// Vendored from: /~https://github.com/DefinitelyTyped/DefinitelyTyped/blob/4205e03127692a40b4871709a7134fe4e2ed5510/types/amqplib/callback_api.d.ts#L5
export interface Connection {
close(): Promise<void>;
createChannel(): Promise<any>;
createConfirmChannel(): Promise<ConfirmChannel>;
connection: {
serverProperties: ServerProperties;
};
// Vendored from: /~https://github.com/DefinitelyTyped/DefinitelyTyped/blob/4205e03127692a40b4871709a7134fe4e2ed5510/types/amqplib/properties.d.ts#L108
// This exists in `@types/amqplib` as `Options.Publish`. We're renaming things
// here to avoid importing the whole Options namespace.
export interface AmqplibPublishOptions {
expiration?: string | number;
userId?: string;
CC?: string | string[];

mandatory?: boolean;
persistent?: boolean;
deliveryMode?: boolean | number;
BCC?: string | string[];

contentType?: string;
contentEncoding?: string;
headers?: any;
priority?: number;
correlationId?: string;
replyTo?: string;
messageId?: string;
timestamp?: number;
type?: string;
appId?: string;
}

// Vendored from: /~https://github.com/DefinitelyTyped/DefinitelyTyped/blob/4205e03127692a40b4871709a7134fe4e2ed5510/types/amqplib/properties.d.ts#L142
Expand All @@ -299,10 +136,6 @@ export interface Message {
properties: MessageProperties;
}

export interface GetMessage extends Message {
fields: GetMessageFields;
}

export interface ConsumeMessage extends Message {
fields: ConsumeMessageFields;
}
Expand All @@ -319,10 +152,6 @@ export interface MessageFields extends CommonMessageFields {
consumerTag?: string;
}

export interface GetMessageFields extends CommonMessageFields {
messageCount: number;
}

export interface ConsumeMessageFields extends CommonMessageFields {
deliveryTag: number;
}
Expand Down

0 comments on commit 9d0198c

Please sign in to comment.