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

refactor(instrumentation-grpc): clean up remnants of 'grpc' package instrumentation #4420

Conversation

pichlermarc
Copy link
Member

Which problem is this PR solving?

We had some unused code left over from back when @opentelemetry/instrumentation-grpc still supported the grpc package (now only @grpc/gprc-js is supported). This PR gets rid of the wrapper that used to make two instrumentation look like one.

Fixes #4201

Type of change

  • refactor

How Has This Been Tested?

  • Tested locally on a typescript project without @grpc/grpc-js to ensure we don't rely on @grpc/grpc-js types in the public interface

Copy link

codecov bot commented Jan 16, 2024

Codecov Report

Merging #4420 (4aec62e) into main (efa6307) will increase coverage by 0.04%.
The diff coverage is 92.22%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4420      +/-   ##
==========================================
+ Coverage   92.37%   92.41%   +0.04%     
==========================================
  Files         331      330       -1     
  Lines        9518     9497      -21     
  Branches     2021     2021              
==========================================
- Hits         8792     8777      -15     
+ Misses        726      720       -6     
Files Coverage Δ
...ntelemetry-instrumentation-grpc/src/clientUtils.ts 96.70% <100.00%> (ø)
...ntelemetry-instrumentation-grpc/src/serverUtils.ts 91.78% <100.00%> (ø)
...emetry-instrumentation-grpc/src/instrumentation.ts 92.09% <92.04%> (+19.36%) ⬆️

Copy link
Member

@Flarna Flarna left a comment

Choose a reason for hiding this comment

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

Seems it's mostly a code move/adaption to move.
I haven't looked into each and every line.

@pichlermarc pichlermarc merged commit 2df6310 into open-telemetry:main Jan 31, 2024
20 checks passed
@pichlermarc pichlermarc deleted the refactor/instrumentation-grpc branch January 31, 2024 15:41
Zirak pushed a commit to Zirak/opentelemetry-js that referenced this pull request Sep 14, 2024
…nstrumentation (open-telemetry#4420)

* refactor(instrumentation-grpc): clean up remnants of 'grpc' package instrumentation

* fix(changelog): add changelog entry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GrpcInstrumentation does not implement @opentelemetry/instrumentation InstrumentationBase
3 participants