feat(tracing): add COMPOSE_OTEL_DEBUG to surface OTel internals - #14152
feat(tracing): add COMPOSE_OTEL_DEBUG to surface OTel internals#14152htoyoda18 wants to merge 1 commit into
Conversation
Signed-off-by: hiroto.toyoda <hiroto.toyoda@dena.com>
glours
left a comment
There was a problem hiding this comment.
Thanks for the PR, the diagnosability goal makes sense, but we don't think a new environment variable is needed here: Compose already has a debug mechanism with --debug that surfaces this exact kind of tracing diagnostic elsewhere in the codebase.
Could you route the OTel error printing through that instead of introducing COMPOSE_OTEL_DEBUG? Should end up simpler, with a single debug switch instead of two overlapping ones.
|
|
||
| cmd.SetContext(ctx) | ||
| wrapRunE(cmd, cmdSpan, tracingShutdown) | ||
| wrapRunE(cmd, cmdSpan, tracingShutdown, dockerCli.Err()) |
There was a problem hiding this comment.
No needed , we should use logrus to write the error which is already configured to write in the right IO
| wrapRunE(cmd, cmdSpan, tracingShutdown, dockerCli.Err()) | |
| wrapRunE(cmd, cmdSpan, tracingShutdown) |
| // Unfortunately, PersistentPostRun(E) can't be used for this purpose because it | ||
| // only runs if RunE does _not_ return an error, but this should run unconditionally. | ||
| func wrapRunE(c *cobra.Command, cmdSpan trace.Span, tracingShutdown tracing.ShutdownFunc) { | ||
| func wrapRunE(c *cobra.Command, cmdSpan trace.Span, tracingShutdown tracing.ShutdownFunc, errOut io.Writer) { |
There was a problem hiding this comment.
| func wrapRunE(c *cobra.Command, cmdSpan trace.Span, tracingShutdown tracing.ShutdownFunc, errOut io.Writer) { | |
| func wrapRunE(c *cobra.Command, cmdSpan trace.Span, tracingShutdown tracing.ShutdownFunc) { |
| // OTel components for debugging purposes | ||
| _ = tracingShutdown(ctx) | ||
| if err := tracingShutdown(ctx); err != nil && tracing.DebugEnabled() { | ||
| fmt.Fprintln(errOut, "otel: shutdown:", err) |
There was a problem hiding this comment.
Use logrus
| fmt.Fprintln(errOut, "otel: shutdown:", err) | |
| logrus.Debugf("otel: shutdown: %v", err) |
| // DebugEnabled reports whether OTel SDK/exporter internals should print | ||
| // their diagnostics, controlled by the COMPOSE_OTEL_DEBUG environment | ||
| // variable. It defaults to false so tracing plumbing never leaks into | ||
| // ordinary CLI output. | ||
| func DebugEnabled() bool { | ||
| enabled, _ := strconv.ParseBool(os.Getenv("COMPOSE_OTEL_DEBUG")) | ||
| return enabled | ||
| } |
There was a problem hiding this comment.
no need for a dedicated env var; logrus.Debugf is already a no-op unless --debug is set, so the check is redundant.
| if DebugEnabled() { | ||
| fmt.Fprintln(os.Stderr, "otel:", err) | ||
| } |
There was a problem hiding this comment.
reuses the same debug mechanism cmd/main.go:55 already uses for tracing setup failures, instead of a second, parallel gate for the same diagnostic category.
| if DebugEnabled() { | |
| fmt.Fprintln(os.Stderr, "otel:", err) | |
| } | |
| logrus.Debugf("otel: %v", err) |
| detect.ServiceName = "compose" | ||
| // do not log tracing errors to stdio | ||
| otel.SetErrorHandler(skipErrors{}) | ||
| // do not log tracing errors to stdio, unless COMPOSE_OTEL_DEBUG is set |
There was a problem hiding this comment.
| // do not log tracing errors to stdio, unless COMPOSE_OTEL_DEBUG is set | |
| // do not log tracing errors to stdio, unless `--debug` is set |
What I did
OTel SDK/exporter internal errors (
otel.ErrorHandler) and the tracer shutdown/flush error inwrapRunEwere both always discarded, making it impossible to diagnose tracing issues (e.g. a misconfigured or unreachable OTLP endpoint) without instrumenting the code.Added
tracing.DebugEnabled(), gated on a newCOMPOSE_OTEL_DEBUGenvironment variable, and used it in both places:internal/tracing'sotel.ErrorHandlernow prints to stderr when enabled instead of always being a no-optracingShutdown(ctx)error incmd/cmdtrace/cmd_span.gois now printed to the command's stderr (dockerCli.Err()) when enabledDefault behavior (
COMPOSE_OTEL_DEBUGunset) is unchanged — tracing plumbing still never leaks into ordinary CLI output.Related issue
N/A
(not mandatory) A picture of a cute animal, if possible in relation to what you did
