feat(core): add controller log→OTLP bridge (gated OTEL_LOGGING_ENABLED) #2151
+410
−19
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| package telemetry | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
|
|
||
| otelzap "go.opentelemetry.io/contrib/bridges/otelzap" | ||
| "go.opentelemetry.io/contrib/exporters/autoexport" | ||
| logglobal "go.opentelemetry.io/otel/log/global" | ||
| sdklog "go.opentelemetry.io/otel/sdk/log" | ||
| "go.uber.org/zap" | ||
| "go.uber.org/zap/zapcore" | ||
| crzap "sigs.k8s.io/controller-runtime/pkg/log/zap" | ||
|
|
||
| "github.com/kagent-dev/kagent/go/core/pkg/env" | ||
| ) | ||
|
|
||
| const loggerBridgeName = "github.com/kagent-dev/kagent/go/core" | ||
|
|
||
| // InitLoggerProvider configures an OTLP LoggerProvider and registers it as the | ||
| // global OTel logger provider. The exporter type and endpoint are read from the | ||
| // standard OTEL environment variables via autoexport, mirroring | ||
| // InitTracerProvider. The returned shutdown function must be called on process | ||
| // exit to flush in-flight log records. When OTEL_LOGGING_ENABLED is unset the | ||
| // pipeline is not created and a no-op shutdown is returned (default-OFF). | ||
| func InitLoggerProvider(ctx context.Context, serviceVersion string) (func(context.Context) error, error) { | ||
| if !env.OtelLoggingEnabled.Get() { | ||
| return func(context.Context) error { return nil }, nil | ||
| } | ||
|
|
||
| exporter, err := autoexport.NewLogExporter(ctx) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("create log exporter: %w", err) | ||
| } | ||
|
|
||
| res, err := newTelemetryResource(ctx, serviceVersion) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| lp := sdklog.NewLoggerProvider( | ||
| sdklog.WithProcessor(sdklog.NewBatchProcessor(exporter)), | ||
| sdklog.WithResource(res), | ||
| ) | ||
|
|
||
| logglobal.SetLoggerProvider(lp) | ||
|
|
||
| return lp.Shutdown, nil | ||
| } | ||
|
|
||
| // ControllerZapOpts returns controller-runtime zap options. When | ||
| // OTEL_LOGGING_ENABLED is set it additively tees the controller's stdout zap | ||
| // core with an otelzap bridge core, routing the controller's own logs through | ||
| // the global OTLP LoggerProvider while preserving stdout logging. When disabled | ||
| // it returns no options, leaving the logger byte-identical to upstream. | ||
| // | ||
| // InitLoggerProvider must be called first so the bridge core binds to the | ||
| // configured global LoggerProvider. | ||
| func ControllerZapOpts() []crzap.Opts { | ||
| if !env.OtelLoggingEnabled.Get() { | ||
| return nil | ||
| } | ||
| bridgeCore := otelzap.NewCore(loggerBridgeName, | ||
| otelzap.WithLoggerProvider(logglobal.GetLoggerProvider()), | ||
| ) | ||
| return []crzap.Opts{ | ||
| crzap.RawZapOpts(zap.WrapCore(func(core zapcore.Core) zapcore.Core { | ||
| return zapcore.NewTee(core, bridgeCore) | ||
|
Contributor
There was a problem hiding this comment. The OTel log SDK puts severity filtering in a processor, not in the exporter or the bridge, so someone running at error level only expects error+ logs, but when they enable this flag it will ship all their info/debug logs to the backend. We can fix it via https://pkg.go.dev/go.opentelemetry.io/contrib/processors/minsev |
||
| })), | ||
| } | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| package telemetry | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
|
|
||
| crzap "sigs.k8s.io/controller-runtime/pkg/log/zap" | ||
| ) | ||
|
|
||
| // TestControllerZapOpts_DisabledByDefault verifies no zap options are added when | ||
| // OTEL_LOGGING_ENABLED is off, keeping the controller logger unchanged. | ||
| func TestControllerZapOpts_DisabledByDefault(t *testing.T) { | ||
| t.Setenv("OTEL_LOGGING_ENABLED", "false") | ||
| if opts := ControllerZapOpts(); opts != nil { | ||
| t.Fatalf("expected nil opts when logging disabled, got %d", len(opts)) | ||
| } | ||
| } | ||
|
|
||
| // TestControllerZapOpts_EnabledTeesLogger verifies that when OTEL_LOGGING_ENABLED | ||
| // is set, an otelzap bridge option is returned and the resulting logger is usable | ||
| // (the bridge tees onto the stdout core without panicking). | ||
| func TestControllerZapOpts_EnabledTeesLogger(t *testing.T) { | ||
|
Contributor
There was a problem hiding this comment. Let's also test
|
||
| t.Setenv("OTEL_LOGGING_ENABLED", "true") | ||
|
|
||
| opts := ControllerZapOpts() | ||
| if len(opts) != 1 { | ||
| t.Fatalf("expected 1 zap opt when logging enabled, got %d", len(opts)) | ||
| } | ||
|
|
||
| // Building and using the logger must not panic even though no real OTLP | ||
| // LoggerProvider is configured (the global provider defaults to a no-op). | ||
| logger := crzap.New(append([]crzap.Opts{crzap.UseDevMode(true)}, opts...)...) | ||
| logger.Info("tee smoke test") | ||
| } | ||
|
|
||
| // TestInitLoggerProvider_DisabledNoop verifies the returned shutdown is a safe | ||
| // no-op when logging is disabled. | ||
| func TestInitLoggerProvider_DisabledNoop(t *testing.T) { | ||
| t.Setenv("OTEL_LOGGING_ENABLED", "false") | ||
|
|
||
| shutdown, err := InitLoggerProvider(context.Background(), "test") | ||
| if err != nil { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
| if shutdown == nil { | ||
| t.Fatal("expected non-nil shutdown") | ||
| } | ||
| if err := shutdown(context.Background()); err != nil { | ||
| t.Fatalf("no-op shutdown returned error: %v", err) | ||
| } | ||
| } | ||
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
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this actually gets us log-trace correlation as-is, which is what you're trying to deliver?
otelzap.Coreonly adds trace_id/span_id to a record when a context.Context is passed as a zap field. controller-runtime logs through logr -> zapr, andlog.FromContext(ctx)only threads values in viaWithValues, so I am not sure these records contain spans linked to them.