Skip to content

Commit

Permalink
refactor(instrumentation-grpc): fix eslint warnings
Browse files Browse the repository at this point in the history
```
/home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-grpc/src/clientUtils.ts
  105:42  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

/home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts
  129:69  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  134:69  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  139:68  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  144:68  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

/home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-grpc/src/serverUtils.ts
  112:23  warning  Don't use `Function` as a type  @typescript-eslint/ban-types
  152:23  warning  Don't use `Function` as a type  @typescript-eslint/ban-types
  207:31  warning  Don't use `Function` as a type  @typescript-eslint/ban-types
  211:31  warning  Don't use `Function` as a type  @typescript-eslint/ban-types

/home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-grpc/test/helper.ts
  83:11  warning  Don't use `Function` as a type  @typescript-eslint/ban-types
```

This commit does not involve any changes to runtime code and can
be safely merged without any concerns about changing behavior.

The first can be replaced with the more precise type that we are
expecting.

The second just wansn't needed anymore.

The third is highly unusual. After spending significant amount of
time reading and stepping through the code – I am quite confident
that the `.call({}, ...)` (and `.apply({}, ...)` elsewhere in the
tests) are a mistake that we inherited from very old code. However
I don't feel confident enough that I can *explain* what is going
on here to make a change to the runtime code, so in the meantime,
I think the best course of action is to leave it as-is, document
the issue and have someone with more expertise untangle this down
the road.

The last one was a change in test code to use a more precise call
signature.

Ref #5365
  • Loading branch information
chancancode committed Jan 30, 2025
1 parent 29d0da5 commit ffae1a2
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export function patchResponseMetadataEvent(
call: EventEmitter,
metadataCapture: metadataCaptureType
) {
call.on('metadata', (responseMetadata: any) => {
call.on('metadata', (responseMetadata: Metadata) => {
metadataCapture.client.captureResponseMetadata(span, responseMetadata);
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,22 +126,22 @@ export class GrpcInstrumentation extends InstrumentationBase<GrpcInstrumentation
this._wrap(
moduleExports.Client.prototype,
'makeUnaryRequest',
this._patchClientRequestMethod(moduleExports, false) as any
this._patchClientRequestMethod(moduleExports, false)
);
this._wrap(
moduleExports.Client.prototype,
'makeClientStreamRequest',
this._patchClientRequestMethod(moduleExports, false) as any
this._patchClientRequestMethod(moduleExports, false)
);
this._wrap(
moduleExports.Client.prototype,
'makeServerStreamRequest',
this._patchClientRequestMethod(moduleExports, true) as any
this._patchClientRequestMethod(moduleExports, true)
);
this._wrap(
moduleExports.Client.prototype,
'makeBidiStreamRequest',
this._patchClientRequestMethod(moduleExports, true) as any
this._patchClientRequestMethod(moduleExports, true)
);
return moduleExports;
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,10 @@ function serverStreamAndBidiHandler<RequestType, ResponseType>(
endSpan();
});

// Types of parameters 'call' and 'call' are incompatible.
// TODO: Investigate this call/signature – it was inherited from very old
// code and the `this: {}` is highly suspicious, and likely isn't doing
// anything useful. There is probably a more precise cast we can do here.
// eslint-disable-next-line @typescript-eslint/ban-types
return (original as Function).call({}, call);
}

Expand Down Expand Up @@ -149,6 +152,11 @@ function clientStreamAndUnaryHandler<RequestType, ResponseType>(
};

context.bind(context.active(), call);

// TODO: Investigate this call/signature – it was inherited from very old
// code and the `this: {}` is highly suspicious, and likely isn't doing
// anything useful. There is probably a more precise cast we can do here.
// eslint-disable-next-line @typescript-eslint/ban-types
return (original as Function).call({}, call, patchedCallback);
}

Expand Down Expand Up @@ -204,10 +212,18 @@ export function handleUntracedServerFunction<RequestType, ResponseType>(
case 'unary':
case 'clientStream':
case 'client_stream':
// TODO: Investigate this call/signature – it was inherited from very old
// code and the `this: {}` is highly suspicious, and likely isn't doing
// anything useful. There is probably a more precise cast we can do here.
// eslint-disable-next-line @typescript-eslint/ban-types
return (originalFunc as Function).call({}, call, callback);
case 'serverStream':
case 'server_stream':
case 'bidi':
// TODO: Investigate this call/signature – it was inherited from very old
// code and the `this: {}` is highly suspicious, and likely isn't doing
// anything useful. There is probably a more precise cast we can do here.
// eslint-disable-next-line @typescript-eslint/ban-types
return (originalFunc as Function).call({}, call);
default:
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ type TestGrpcClient = Client & {
interface TestGrpcCall {
description: string;
methodName: string;
method: Function;
method: (...args: any[]) => unknown;
request: TestRequestResponse | TestRequestResponse[];
result: TestRequestResponse | TestRequestResponse[];
metadata?: Metadata;
Expand Down

0 comments on commit ffae1a2

Please sign in to comment.