Add otel tracing #45652
Merged
Add otel tracing #45652
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
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
tonistiigi
reviewed
Aug 29, 2023
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{
cpuguy83
force-pushed
the
otel
branch
5 times, most recently
from
29020a4
to
5272256
Compare
tonistiigi
approved these changes
Sep 6, 2023
| 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
approved these changes
Sep 7, 2023
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 <cpuguy83@gmail.com>
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 <cpuguy83@gmail.com>
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 <cpuguy83@gmail.com>
This test ensures that we are able to propagate traces into buildkit's history API. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
|
Updated and all is green. |
neersighted
approved these changes
Sep 7, 2023
tonistiigi
approved these changes
Sep 7, 2023
|
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) |
kzys
added a commit
to superfly/flyctl
that referenced
this pull request
Feb 15, 2024
Since moby/moby#45652, client.NewClientWithOpts wraps its Transport with otelhttp.NewTransport, but *otelhttp.Transport doesn't satisfy http.Transport. Fixes #3274
3 tasks
kzys
added a commit
to superfly/flyctl
that referenced
this pull request
Feb 15, 2024
Since moby/moby#45652, client.NewClientWithOpts wraps its Transport with otelhttp.NewTransport, but *otelhttp.Transport doesn't satisfy http.Transport. Fixes #3274
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.