-
Notifications
You must be signed in to change notification settings - Fork 805
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(instrumentation-grpc): drop support for grpc
#3807
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3807 +/- ##
==========================================
- Coverage 92.98% 92.89% -0.09%
==========================================
Files 300 297 -3
Lines 9104 8836 -268
Branches 1862 1814 -48
==========================================
- Hits 8465 8208 -257
+ Misses 639 628 -11
|
b9d6646
to
5aa59ac
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM % nit
experimental/packages/opentelemetry-instrumentation-grpc/src/grpc-js/clientUtils.ts
Outdated
Show resolved
Hide resolved
Thanks for removing this. 🙂 We discussed this change in the SIG meeting last week, and figured we would add a postinstall script first to the package to alert users that the grpc-native instrumentaiton is being removed as not to break people in an completely unexpected manner. So we will hold off on merging this, add a postinstall script warning users that this is about to happen, release, then merge this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current versions of instrumentation-grpc should now have a warning that this will be removed, so this PR should be safe to merge now - just one nit though + request to create a follow-up issue to refactor as some of the code that became unnecessary with this change.
experimental/packages/opentelemetry-instrumentation-grpc/package.json
Outdated
Show resolved
Hide resolved
0d4ae85
to
5750d9a
Compare
…3807) Co-authored-by: Daniel Dyla <dyladan@users.noreply.github.com>
Which problem is this PR solving?
Dropped support for package
grpc
(not@grpc-js/grpc
) due to deprecation.Fixes #3780
Short description of the changes
grpc
example.grpc
instrumentation.eslint
warnings ininstrumentation-grpc
Type of change
Please delete options that are not relevant.
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
Checklist: