-
Notifications
You must be signed in to change notification settings - Fork 3
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
Enable tracing in http.Client #896
Conversation
07e56b6
to
17e0cbc
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #896 +/- ##
==========================================
+ Coverage 47.11% 47.24% +0.13%
==========================================
Files 96 95 -1
Lines 5383 5224 -159
==========================================
- Hits 2536 2468 -68
+ Misses 2611 2522 -89
+ Partials 236 234 -2 ☔ View full report in Codecov by Sentry. |
1594992
to
4c5749c
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.
I have one minor change suggestion, but it's not a blocker. 👍
Register a propagator with OpenTelemetry to enable propagation of trace context over the wire. Update HTTP clients with `otelhttp.Transport` to wrap outbound requests with a span and enrich it with metrics. This change enable us to relate spans occuring beyond an individual service, e.g. the activity "move-to-permanent-storage" executed by enduro-a3m-worker uses a HTTP client to request to the Storage Service the long-term storage of the AIP. With this commit we'll start seeing the `HTTP POST` operation performed by the client but also the rest of the details happening on the server side (child spans), e.g.: API handler, SQL queries, workflow, activities, functions...
This function could be more efficient if we introduced insert batches. This commit should make that more apparent, e.g. will read something like: savePreservationTasks (703.14ms) Which is only part of the time that took the entire activity to run, e.g. 5.33s in my development environment.
I'm still trying to figure out what's the best way to share tracers. The SDK encourages to operate with globals but for consistency I'm trying to share them just like we share other values, e.g. context, logger, etc. [skip codecov]
4c5749c
to
8e081c0
Compare
Register a propagator with OpenTelemetry to enable propagation of trace context over the wire 1. Update HTTP clients with
otelhttp.Transport
to wrap outbound requests with a span and enrich it with metrics.This change enable us to relate spans occuring beyond an individual service, e.g. the activity "move-to-permanent-storage" executed by enduro-a3m-worker uses a HTTP client to request to the Storage Service the long-term storage of the AIP. With this commit we'll start seeing the
HTTP POST
operation performed by the client but also the rest of the details happening on the server side, i.e. the API handler, SQL queries, workflow, activities, etc.Footnotes
See the Trace Context spec (W3C Recommendation) for more details. It defines HTTP headers and a value format to standardize how context information is propagated over the wire. ↩