-
Notifications
You must be signed in to change notification settings - Fork 542
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
fix(connect): Skip update HTTP's span name and update RpcMetadata's route instead #1534
Conversation
f5ee4f2
to
7a0306b
Compare
plugins/node/opentelemetry-instrumentation-connect/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
plugins/node/opentelemetry-instrumentation-connect/src/instrumentation.ts
Outdated
Show resolved
Hide resolved
8b2a9d2
to
295b506
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1534 +/- ##
==========================================
- Coverage 96.13% 95.67% -0.47%
==========================================
Files 14 18 +4
Lines 906 1087 +181
Branches 197 221 +24
==========================================
+ Hits 871 1040 +169
- Misses 35 47 +12
|
295b506
to
98f43a5
Compare
…oute instead Signed-off-by: Chi Ma <[email protected]>
98f43a5
to
c2fdffc
Compare
please avoid forced push as it makes review harder. Just merge main into your branch to keep it up to date. Merge to main will do a squash anyway. |
@Flarna seem like there something wrong with last run, it fail at docker pull step. |
@open-telemetry/javascript-maintainers Could someone please take a look on this as connect instrumentation lacks a maintainer but I think this change is useful. |
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.
Thanks for taking care of this - happy to see this behavior aligned with the other instrumentations 🎉
Which problem is this PR solving?
Each framework has a different approach to updating Span's name and HTTP_ROUTE attribute. We want to delegate this to instrumentation-http for consistent's name and attribute
This PR fixes the above issue for connect
Short description of the changes