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

shippingservice: add http client request span #610

Merged

Conversation

styblope
Copy link
Contributor

@styblope styblope commented Nov 29, 2022

Changes

I propose to add a dedicated http client span for the outgoing calls to /getquote. This change will provide tracing separation between the server part and the downstream requests client part inside shippingservice. A client span can also better match with the peer quoteservice server span. The client middleware-based instrumentation uses the available reqwests-tracing and tracing-opentelementry crates.

Merge Requirements

For new features contributions please make sure you have completed the following
essential items:

  • CHANGELOG.md updated to document new feature additions
  • Appropriate documentation updates in the docs folder

The commit adds new http client span for outgoing calls to `/getquote`
endpoint of quoteservice via the `reqwests` client middleware instrumentation.
This provides a bit of the server/client tasks separation visibility
as well as enables span pairing with the downstream quoteservice server span.
The instrumentation uses the available `reqwests-tracing` and `tracing-opentelementry` crates.
@styblope styblope requested a review from a team November 29, 2022 16:53
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 29, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link
Member

@julianocosta89 julianocosta89 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @styblope!
A lot of new attributes and events were added automatically. Good one!

Could you also update the doc for the Shipping Service to reflect your changes?
https://github.com/open-telemetry/opentelemetry-demo/blob/main/docs/services/shippingservice.md

@julianocosta89
Copy link
Member

Hey @GaryPWhite, as you were the main responsible for the shippingservice implementation, would you mind taking a look at this one?

Copy link
Contributor

@GaryPWhite GaryPWhite left a comment

Choose a reason for hiding this comment

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

this is really cool. I want to point out that this is a way of using even downstream dependencies in a really adept way and that's a massive contribution of SME to this project. Thanks much for your input here.

left some comments but by no means blocking.

src/shippingservice/src/shipping_service/quote.rs Outdated Show resolved Hide resolved
src/shippingservice/src/main.rs Outdated Show resolved Hide resolved
@cartersocha cartersocha merged commit 48892f5 into open-telemetry:main Dec 5, 2022
jmichalak9 pushed a commit to jmichalak9/opentelemetry-demo that referenced this pull request Mar 22, 2024
* shippingservice: Add http client request span

The commit adds new http client span for outgoing calls to `/getquote`
endpoint of quoteservice via the `reqwests` client middleware instrumentation.
This provides a bit of the server/client tasks separation visibility
as well as enables span pairing with the downstream quoteservice server span.
The instrumentation uses the available `reqwests-tracing` and `tracing-opentelementry` crates.

* update shippingservice docs

* update changelog

* update doc section position

* apply syntax change suggestion

Co-authored-by: Juliano Costa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants