-
Notifications
You must be signed in to change notification settings - Fork 587
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
otelhttp: add WithClientTrace to connect with otelhttptrace #875
Conversation
@tonistiigi #873 was merged; guess this needs to be rebased and moved out of draft |
#874 was merged as well; this ready to be moved out of draft? |
aaa9769
to
81547d2
Compare
Codecov Report
@@ Coverage Diff @@
## main #875 +/- ##
=====================================
Coverage 69.3% 69.3%
=====================================
Files 127 127
Lines 5424 5432 +8
=====================================
+ Hits 3760 3768 +8
Misses 1521 1521
Partials 143 143
|
Transport can be created with `otelhttp.NewTransport(base, otelhttp.WithClientTrace(otelhttptrace.NewClientTrace))` for additional httptrace tracing. Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
81547d2
to
4d49d7b
Compare
@pellared @MadVikingGod @Aneurysm9 ptal; this is the remaining one for us to get rid of our fork ❤️ |
Friendly ping, is it ok to merge this one? Thx! |
Looks like it may need another rebase |
This still requires another approval. @open-telemetry/go-approvers can anyone else weigh in on this? |
LGTM 👍 |
Thank you! |
I couldn't quite figure out how
otelhttp
andotelhttptrace
are supposed to be used together. From they being in separate packages I assume maintainers don't want them to depend on each other.This adds a new option so caller can initialize
Transport
withotelhttp.NewTransport(base, otelhttp.WithClientTrace(otelhttptrace.NewClientTrace))
(or any otherhttptrace
implementation) so thatotelhttp
spans will haveotelhttptrace
spans as children.Depends on #873