chore(instrumentation-grpc): cleanup remnants of grpc-native support

[llc1123]

Which problem is this PR solving?

Fixes #3869

Short description of the changes

• Removed all import * from '@grpc/grpc-js' and use destructuring imports instead.
• Removed passing grpc instance as a parameter.
  • grpcClient in getMetadata()
  • grpcClient in _patchClient()
  • grpcClient in _patchLoadPackageDefinition()
  • grpcClient in _getPatchedClientMethods()
  • grpcClient in _patchLoadedPackage()
  • metadata in shouldNotTraceServerCall()
    *idk if this one should be removed but metadata is not used in this function and it's not exported outside the package.
• Removed related tests.

Type of change

Please delete options that are not relevant.

• Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

• Existing Tests

Checklist:

• Followed the style guidelines of this project
• Unit tests have been added

[codecov]

Codecov Report

Merging #3886 (6caaa95) into main (c51c9eb) will increase coverage by 1.98%.
The diff coverage is 96.96%.

❗ Current head 6caaa95 differs from pull request most recent head faf08ca. Consider uploading reports for the commit faf08ca to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3886      +/-   ##
==========================================
+ Coverage   91.15%   93.14%   +1.98%     
==========================================
  Files         127      298     +171     
  Lines        2747     8868    +6121     
  Branches      548     1826    +1278     
==========================================
+ Hits         2504     8260    +5756     
- Misses        243      608     +365     

Impacted Files	Coverage Δ
...emetry-instrumentation-grpc/src/instrumentation.ts	72.72% <50.00%> (ø)
...ry-instrumentation-grpc/src/grpc-js/clientUtils.ts	92.85% <100.00%> (ø)
...elemetry-instrumentation-grpc/src/grpc-js/index.ts	92.68% <100.00%> (ø)
...ry-instrumentation-grpc/src/grpc-js/serverUtils.ts	91.78% <100.00%> (ø)
...es/opentelemetry-instrumentation-grpc/src/utils.ts	82.69% <100.00%> (ø)

... and 184 files with indirect coverage changes

[dyladan]

Generally looks good to me. I think removing metadata from shouldNotTraceServerCall is fine since it is unused and unexported

[llc1123]

cc @pichlermarc

[pichlermarc]

Thanks for taking care of this. 🙂
Just one possible caveat: I'm wondering what happens when we wrap two different versions of @grpc/grpc-js in the same app. 🤔

[llc1123]

I reverted the import * from "@grpc/grpc-js" part.

But the problem still exists:
In src/grpc-js/index.ts, to pass grpc-js instance to the wrapping methods, you still need to do import * from "@grpc/grpc-js". import type doesn't work.

In the output JS code, there is still something like const grpcJs = require("@grpc/grpc-js");.
Maybe we should avoid using new Metadata() in clientUtils.ts.

[pichlermarc]

I reverted the import * from "@grpc/grpc-js" part.

But the problem still exists: In src/grpc-js/index.ts, to pass grpc-js instance to the wrapping methods, you still need to do import * from "@grpc/grpc-js". import type doesn't work.

In the output JS code, there is still something like const grpcJs = require("@grpc/grpc-js");. Maybe we should avoid using new Metadata() in clientUtils.ts.

You can pass moduleExports instead of grpcJs as was done in the original code - then you should be able to use type imports only. 🙂

Unfortunately, we need to construct Metadata if it is not provided by the user. Otherwise, we won't be able to inject context into it, which will result in broken traces.

[llc1123]

You can pass moduleExports instead of grpcJs as was done in the original code.

Nice.

[pichlermarc]

Looks good; thanks for addressing the comments 🙂

[llc1123]

cc @dyladan