daemon: containerCreate, containerStart: add filtered labels to OTel span#49789
Merged
vvoland merged 2 commits intomoby:masterfrom Apr 11, 2025
Merged
daemon: containerCreate, containerStart: add filtered labels to OTel span#49789vvoland merged 2 commits intomoby:masterfrom
vvoland merged 2 commits intomoby:masterfrom
Conversation
akerouanton
added
status/2-code-review
kind/enhancement
akerouanton
force-pushed
the
trace-containerCreate-labels
branch
from
1a7e867 to
ab02fc6
Compare
robmry
reviewed
Apr 10, 2025
| // | ||
| // Note that, this is an experimental env var that might be removed | ||
| // unceremoniously at any point in time. | ||
| filters := strings.Split(os.Getenv("OTEL_FILTER_CONTAINER_LABELS_ATTRS"), ",") |
Contributor
There was a problem hiding this comment.
Would "DOCKER_OTEL_INCLUDE_CONTAINER_LABEL_ATTRS" be a better name? (It's a Docker variable, not generic-OTEL - and "include" is less ambiguous than "filter" which could mean "include filtered", or "filter out")?
This commit adds a new OTel span to `(*Daemon).containerCreate()` and puts filtered container labels in the span attributes. The filter is based on a comma-separated list of labels provided through the `DOCKER_OTEL_INCLUDE_CONTAINER_LABEL_ATTRS` environment variable. This label might be removed at any point in time if we figure out a better way to filter labels, or if that span becomes unecessary. Signed-off-by: Albin Kerouanton <[email protected]>
Like for containerCreate, filter the list of container labels based on `DOCKER_OTEL_INCLUDE_CONTAINER_LABEL_ATTRS` and put that list in the OTel span. Signed-off-by: Albin Kerouanton <[email protected]>
akerouanton
force-pushed
the
trace-containerCreate-labels
branch
from
ab02fc6 to
099d3ee
Compare
robmry
approved these changes
Apr 10, 2025
Contributor
robmry
left a comment
robmry
left a comment
There was a problem hiding this comment.
A "SOMETHING_FILTER" might still be an allow or deny list, but ok! Thank you for the changes. (The PR description needs a matching update though.)
vvoland
approved these changes
Apr 11, 2025
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
- What I did
First commit adds an OTel span to
(*Daemon).containerCreate()with a list of filtered container labels set as OTel attributes.Second commit adds the list of filtered container labels to the existing OTel span.
Labels are filtered based on the newly introduced env var
DOCKER_OTEL_INCLUDE_CONTAINER_LABEL_ATTRS. As the code states, this env var could be dropped unceremoniously at any point in time if we figure out a better way to support that feature, or if we realize it's not needed anymore.This is a nice-to-have to ease some internal work on Docker Desktop, but could be useful for other use cases too.