-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Emit traceparent header in gateway responses and add sharness test #9180
Conversation
This should enable trace context propagation from headers in the HTTP Gateway and APIs.
@@ -179,6 +179,7 @@ require ( | |||
github.com/raulk/go-watchdog v1.2.0 // indirect | |||
github.com/spacemonkeygo/spacelog v0.0.0-20180420211403-2296661a0572 // indirect | |||
github.com/spaolacci/murmur3 v1.1.0 // indirect | |||
github.com/stretchr/objx v0.4.0 // indirect |
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.
Needed to avoid a failure in the gotest circleci step that checks whether kubo-as-a-library builds with the current version. Error was
github.com/ipfs/kubo/examples/kubo-as-a-library imports
github.com/ipfs/kubo/core/coreapi imports
github.com/ipfs/kubo/tracing imports
go.opentelemetry.io/otel/exporters/jaeger tested by
go.opentelemetry.io/otel/exporters/jaeger.test imports
github.com/stretchr/testify/mock imports
github.com/stretchr/objx loaded from github.com/stretchr/[email protected],
but go 1.16 would select v0.4.0
go-ipfs-http-client failure looks like an unrelated flake |
sharness failure is in test/sharness/t0310-tracing.sh which is the existing tracing test |
@iand : I saw your mention about engaging on this. I think there is a similar item in bifrost-gateway: ipfs-inactive/bifrost-gateway#68 . Ultimately I assume there will be some shared code in Boxo. |
// Emit tracing context headers if present | ||
prop := otel.GetTextMapPropagator() | ||
prop.Inject(r.Context(), propagation.HeaderCarrier(w.Header())) | ||
|
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.
Do you know the difference with:
gw = otelhttp.NewHandler(gw, "Gateway.Request", otelhttp.WithPropagators(tracing.Propagator()))
We already use this for cmds and this works great:
kubo/core/corehttp/commands.go
Line 150 in d1713ca
cmdHandler = otelhttp.NewHandler(cmdHandler, "corehttp.cmdsHandler", otelhttp.WithPropagators(tracing.Propagator())) |
They seems equivalent to me, except otelhttp
is cleaner and more descriptive.
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 tried that but it didn't set the HTTP headers
c0bb34e
to
44282ce
Compare
The OTel handler extracts incoming trace headers but does not emit them in responses. This has to be done separately and I chose to add it in our gateway handler. The sharness test verifies that no trace header is emitted unless a trace header was sent with the request, that the emitted header keeps the same trace id and that the trace id is collected in traces.
Note that this is to merge into #9169