Add otel tracing#45652
Conversation
cpuguy83
commented
May 30, 2023
cpuguy83
commented
- Adds basic otel instrumentation to http transports/handlers
- Adds OTLP/HTTP exporter
- Updates tests to configure traces/spans automatically (as automatically as we can, anyway)
- Exposes options in Makefile to pass through info needed to export traces from test daemon
|
cpuguy83
force-pushed
the
otel
branch
2 times, most recently
from
2537a93 to
9cc176d
Compare
cpuguy83
force-pushed
the
otel
branch
25 times, most recently
from
be86f14 to
831d6cf
Compare
|
@cpuguy83 I pushed a branch that rebases this PR on top of my separate "fix TestDaemonProxy integration tests" PR;
Let me know if you're ok with merging that one (#46126) first |
This comment was marked as resolved.
This comment was marked as resolved.
|
Fixed. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
tonistiigi
left a comment
tonistiigi
left a comment
There was a problem hiding this comment.
Tested this and it now logs "no trace recorder found, skipping" for every build. It happens because detect.Exporter() returns nil.
(This can be a follow-up as not directly related, but) I don't think client-side traces can work unless TraceCollector is set in control.NewController(control.Opt{
| HistoryConfig: historyConf, | ||
| LeaseManager: wo.LeaseManager, | ||
| ContentStore: wo.ContentStore, | ||
| TraceCollector: getTraceExporter(ctx), |
There was a problem hiding this comment.
Note that the client trace delegation still doesn't work yet, but the issue is in buildx side where it is not enabled for all drivers. I'll make a patch for this tomorrow. Moby side looks correct.
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
had a discussion in the maintainers call;
- the integration tests already set the SERVICE_NAME (so for our own tests, it's not an issue to remove the daemon iD)
- the current code moved "generating" the daemon-id only for OTEL (not ideal)
- let's remove the ID for now, and work on other options in a follow-up
after that, "LGTM" (let's get this in!)
This uses otel standard environment variables to configure tracing in the daemon. It also adds support for propagating trace contexts in the client and reading those from the API server. See https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/ for details on otel environment variables. Signed-off-by: Brian Goff <[email protected]>
Integration tests will now configure clients to propagate traces as well as create spans for all tests. Some extra changes were needed (or desired for trace propagation) in the test helpers to pass through tracing spans via context. Signed-off-by: Brian Goff <[email protected]>
This wires up the integration tests to export spans to a jager instance. After tests are finished it exports the data out of jaeger and uploads as an artifact to the action run. Signed-off-by: Brian Goff <[email protected]>
This test ensures that we are able to propagate traces into buildkit's history API. Signed-off-by: Brian Goff <[email protected]>
|
Updated and all is green. |
|
w000000000t! thank so much for this, @cpuguy83 ! |
|
@cpuguy83 I assume you'll be opening the GHA followup PRs? |
|
Now that this one's merged, I rebased, moved this one out of draft 😅 (wink wink, nudge nudge, say no more) |
4 participants
