-
Notifications
You must be signed in to change notification settings - Fork 443
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
Opentelemetry environment variables cause the CLI to error without decent error message #2447
Comments
Reproduced on more recent build, updated description |
Thanks for reporting! So, looking at the Exporter Selection section in the OTEL specs, it looks like
That said, the implementation guidelines for Enum values describes that values not recognized must be logged as a warning, but handled gracefully;
This actual error is produced by BuildKit's func detectExporter[T any](envVar string, fn func(d ExporterDetector) (T, bool, error)) (exp T, err error) {
if n := os.Getenv(envVar); n != "" {
d, ok := detectors[n]
if !ok {
return exp, errors.Errorf("unsupported opentelemetry exporter %v", n) That package is currently used both in the docker daemon, and in Doing some testing, and it looks like this error only affects export OTEL_TRACES_EXPORTER=logging
export OTEL_METRICS_EXPORTER=logging
export OTEL_LOGS_EXPORTER=logging
export OTEL_METRIC_EXPORT_INTERVAL=15000
echo 'FROM scratch' | docker build -
ERROR: unsupported opentelemetry exporter logging
docker builder ls
NAME/NODE DRIVER/ENDPOINT STATUS BUILDKIT PLATFORMS
default* docker
\_ default \_ default error
Failed to get status for default (default): unsupported opentelemetry exporter logging
docker builder prune
WARNING! This will remove all dangling build cache. Are you sure you want to continue? [y/N] y
ERROR: unsupported opentelemetry exporter logging Other commands continue to work successfully; docker ps
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES It looks like the docker daemon itself handles this gracefully, although the error appears twice in the daemon logs; BuildKit code is logging it incorrectly as an ERROR (not a WARNING); export OTEL_TRACES_EXPORTER=logging
export OTEL_METRICS_EXPORTER=logging
export OTEL_LOGS_EXPORTER=logging
export OTEL_METRIC_EXPORT_INTERVAL=15000
dockerd
INFO[2024-05-07T07:58:08.815562967Z] Starting up
...
...
WARN[2024-05-07T07:58:09.828211926Z] Failed to initialize tracing, skipping error="unsupported opentelemetry tracer logging"
...
...
INFO[2024-05-07T07:58:10.122811176Z] Docker daemon commit=ac2de55 containerd-snapshotter=false storage-driver=overlay2 version=26.1.1
INFO[2024-05-07T07:58:10.122850051Z] Daemon has completed initialization
ERRO[2024-05-07T07:58:10.135250134Z] Failed to detect trace exporter for buildkit controller error="unsupported opentelemetry tracer logging"
INFO[2024-05-07T07:58:10.140401717Z] API listen on /var/run/docker.sock |
Move this to the buildx repository, but it looks like there's a couple of issues to look at if we want to follow the OTEL specification;
|
On the CLI we currently don't explicitly check for those variables, and we don't use particular So yes, at the moment the CLI gracefully continues when the env vars re set to invalid values, but silently since they just aren't handled explicitly. I'm not sure if the otel SDK uses them internally though, but i see no particular errors |
We can likely move this to buildkit and fix it there since that's where the bug originates. Yes, a future idea is to refactor the detect package so that it can be moved out of buildkit and used more generally across the moby/docker stack. For the CLI, it's just because, as @krissetto mentioned, we haven't implemented user environment variables so these variables aren't even checked. But I agree with turning this into a warning and logging it with |
@jsternberg is this error coming from the daemon, or is it in the vendor code used in buildx? (I guess I assumed it was client side, because there was no "error response from daemon" prefix, but I guess that's only for the Engine API that we add that prefix, and buildx may not do that when using the BuildKit gRPC API |
I believe it's coming from the vendor code used in buildx so it is an error coming from buildx but the fix happens in buildkit. My assumption is this same problem would also occur in buildkit. We have an environment variable ( |
Description
When building with some opentelemetry environment variables set, I get an unhelpful error.
These environment variables are supported by the opentelemetry java agent, but not within whatever docker build uses.
Reproduce
In a bash shell
Expected behavior
Error with a more descriptive error message, or proceed and print a warning without whatever otel integration is included.
docker version
Client: Cloud integration: v1.0.35+desktop.13 Version: 26.1.1 API version: 1.45 Go version: go1.21.9 Git commit: 4cf5afa Built: Tue Apr 30 11:46:57 2024 OS/Arch: linux/amd64 Context: default Server: Docker Desktop Engine: Version: 26.1.1 API version: 1.45 (minimum version 1.24) Go version: go1.21.9 Git commit: ac2de55 Built: Tue Apr 30 11:48:28 2024 OS/Arch: linux/amd64 Experimental: false containerd: Version: 1.6.31 GitCommit: e377cd56a71523140ca6ae87e30244719194a521 runc: Version: 1.1.12 GitCommit: v1.1.12-0-g51d5e94 docker-init: Version: 0.19.0 GitCommit: de40ad0
docker info
Additional Info
No response
The text was updated successfully, but these errors were encountered: