feat(otel): logs are exported with otel now

[Cali0707]

With this PR, our logs are exported with otel now.

As a follow up, once #1228 lands I will add a test to verify that logs and traces are actually correlated after tool calls in an otel backend

[manusa]

For the logs/traces correlation follow-up: I dug into how otellogr picks up the active span, and it's worth deciding the approach up front, because logr's LogSink interface doesn't carry a context.Context. The bridge only reads a span from a context.Context that appears as a value in the log call's key/values (convertKVs uses the last context it finds, and strips it so it never becomes an attribute). klog.FromContext(ctx).Info(...) doesn't forward that ctx to the sink, so with the current wiring records emit with context.Background() and carry no trace/span IDs. I confirmed it with a quick test: a log inside an active span comes out with an all-zero TraceID.

Three ways to close that gap:

1. Bind the ctx at the span boundary (good starting point)
Right after tracer.Start in the tool-call middleware (and the HTTP middleware), enrich the contextual logger once:

ctx = logr.NewContext(ctx, klog.FromContext(ctx).WithValues(otelCtxKey, ctx))

Every downstream klog.FromContext(ctx) log then correlates, with no changes to tool handlers. It needs the tee sink to strip context-valued pairs from the text branch so the file logger doesn't render a raw context (otellogr already strips it on its side).

• Pro: centralized to the two span-start sites, zero call-site churn, clean text logs.
• Con: snapshots the span active at the boundary, so logs won't attach to deeper child spans.

2. Pass ctx per call
klog.FromContext(ctx).Info(msg, otelCtxKey, ctx) at the sites that should correlate, with the same tee strip on the text branch.

• Pro: most precise, attaches to the innermost span.
• Con: touches every relevant call site and is easy to forget, more churn.

3. Dedicated correlated-log emit path
Skip logr for correlation and emit OTel log records directly with ctx via a small helper (or the otel log API) at the points that matter, keeping klog for text.

• Pro: sidesteps the logr interface limitation entirely, full control over ctx and attributes.
• Con: two logging APIs to keep in sync, and any log still going through klog elsewhere won't correlate.

What won't work, to save time: bumping otellogr (the limitation is the logr sink signature, upstream of the bridge) or a custom sdklog processor (otellogr calls Emit(context.Background(), record), so OnEmit never sees the real ctx).

For transparency, these three options were generated by Claude Opus 4.8.

[github-actions]

👋 Heads up — this pull request changes files owned by @aljesusg @josunect @ksimon1 @lyarwood @mjudeikis.

You are listed as an owner of one or more of the changed areas in .github/CODEOWNERS. GitHub cannot auto-request review from owners without write access, so this comment is the notification instead. A review when you have a moment would be appreciated 🙏

[Cali0707]

@manusa what I went for is switching klog.FromContext to a klogutil.FromContext that sets the context as a value on the logger

[manusa]

Review summary

Thanks for this. The log export works and the trace/log correlation is real: otelCtx satisfies context.Context, otellogr extracts the active span, and the SDK stamps the trace/span IDs onto the record (exercised by TestOtelLogRecordCarriesTraceContext). The migration to klogutil.FromContext is complete, go build/go vet are clean, and the new pkg/logging/pkg/telemetry tests pass race-clean. The otelCtx placeholder is also a good safety net: I checked both branches and no bearer token or header can leak into either the text logs or the exported OTel attributes.

Two things I think are worth addressing before merge, both left inline:

1. The context strip never runs, so every text log line now carries ctx="<otel-ctx>", including the default telemetry-off config. There is a failing test and a small validated fix in the inline comment on pkg/klogutil/util.go.
2. There is no OTEL_LOGS_EXPORTER=none opt-out, so enabling telemetry now also ships the entire klog stream off-box. Inline on pkg/telemetry/logs.go.

I also have a few minor notes that are not blockers (the discarded NewLogProvider error, buffered records not flushed on stdio SIGTERM, a per-call allocation in FromContext on hot paths, and a couple of test-coverage gaps). Happy to add those if you want them, but I left them out to keep this focused.

[manusa]

The context strip this relies on never runs, so every text log line now carries ctx="<otel-ctx>" (including the default telemetry-off config).

logging.New wires klog with SetLoggerWithOptions(...) and no ContextualLogger(true), so klog.FromContext(ctx) returns klog's internal klogger, not the teeSink. The WithValues("ctx", otelCtx{ctx}) here lands on the klogger, which forwards "ctx" as a per-record key/value into teeSink.Info, and teeSink.Info passes it to the primary text sink unmodified. stripContextValues only runs inside teeSink.WithValues, which this path never calls, so stripContextValues/teeSink.WithValues are effectively dead code and the otelCtx placeholder is the only thing keeping the raw context out of the text output.

Observed (both telemetry off and on):

I0707 ... "hello world" ctx="<otel-ctx>" k="v"

It is not a data leak (the placeholder holds and otellogr drops the ctx from exported attributes), but it changes the default log format for everyone and will break exact-match log assertions and log parsers.

A test that showcases it (fails on the current branch, both subtests):

func (s *SinkSuite) TestKlogutilFromContextTextIsClean() {
	s.Run("OTel disabled (default): text log carries no ctx field", func() {
		path := filepath.Join(s.tempDir, "disabled.log")
		sink, err := logging.New(&config.StaticConfig{LogLevel: 1, LogFile: path}, s.httpOut, s.errOut)
		s.Require().NoError(err)
		s.T().Cleanup(func() { _ = sink.Close() })

		klogutil.FromContext(s.T().Context()).Info("default-line", "k", "v")
		klog.Flush()

		content, err := os.ReadFile(path)
		s.Require().NoError(err)
		s.NotContains(string(content), "otel-ctx", "text log must not carry the correlation context")
	})

	s.Run("OTel enabled: text log clean, OTel record still correlates", func() {
		recorder := logtest.NewRecorder()
		otelSink := telemetry.NewLogSink("svc", "1.0.0", recorder)
		path := filepath.Join(s.tempDir, "enabled.log")
		sink, err := logging.New(&config.StaticConfig{LogLevel: 1, LogFile: path}, s.httpOut, s.errOut, logging.WithOtelLogSink(otelSink, nil))
		s.Require().NoError(err)
		s.T().Cleanup(func() { _ = sink.Close() })

		tp := sdktrace.NewTracerProvider(sdktrace.WithSampler(sdktrace.AlwaysSample()))
		s.T().Cleanup(func() { _ = tp.Shutdown(s.T().Context()) })
		ctx, span := tp.Tracer("test").Start(s.T().Context(), "span")
		defer span.End()

		klogutil.FromContext(ctx).Info("enabled-line", "k", "v")
		klog.Flush()

		content, err := os.ReadFile(path)
		s.Require().NoError(err)
		s.NotContains(string(content), "otel-ctx")

		var records []logtest.Record
		for _, recs := range recorder.Result() {
			records = append(records, recs...)
		}
		s.Require().NotEmpty(records)
		s.True(trace.SpanContextFromContext(records[0].Context).HasTraceID(), "OTel record must still carry the trace context")
	})
}

Heads up that the fix is not simply flipping ContextualLogger(true): that routes Background() through the teeSink whose Enabled() is always-true (textlogger Verbosity(9) + otellogr), which breaks -v gating. A small fix that keeps correlation, cleans the text branch, and also drops the per-call allocation in the default path:

 // pkg/logging/tee_sink.go
 func (t *teeSink) Info(level int, msg string, keysAndValues ...any) {
 	if t.primary.Enabled(level) {
-		t.primary.Info(level, msg, keysAndValues...)
+		t.primary.Info(level, msg, stripContextValues(keysAndValues)...)
 	}
 	if t.secondary.Enabled(level) {
 		t.secondary.Info(level, msg, keysAndValues...)
 	}
 }

 func (t *teeSink) Error(err error, msg string, keysAndValues ...any) {
-	t.primary.Error(err, msg, keysAndValues...)
+	t.primary.Error(err, msg, stripContextValues(keysAndValues)...)
 	t.secondary.Error(err, msg, keysAndValues...)
 }

 // pkg/klogutil/util.go
+var otelLogSinkActive atomic.Bool
+
+// SetOtelLogSinkActive records whether the OTel log bridge is active.
+func SetOtelLogSinkActive(active bool) { otelLogSinkActive.Store(active) }
+
 func FromContext(ctx context.Context) klog.Logger {
-	return klog.FromContext(ctx).WithValues("ctx", otelCtx{ctx})
+	logger := klog.FromContext(ctx)
+	if !otelLogSinkActive.Load() {
+		return logger
+	}
+	return logger.WithValues("ctx", otelCtx{ctx})
 }

 // pkg/logging/sink.go (in New, where the sink is wired)
 	if o.otelSink != nil {
 		klog.SetLoggerWithOptions(logr.New(&teeSink{primary: textLogger.GetSink(), secondary: o.otelSink}))
 		s.logProvider = o.otelProvider
+		klogutil.SetOtelLogSinkActive(true)
 	} else {
 		klog.SetLoggerWithOptions(textLogger)
+		klogutil.SetOtelLogSinkActive(false)
 	}

With this, the showcase test passes and the existing pkg/logging/pkg/telemetry suites stay green (verified locally, race included).

[manusa]

No per-signal opt-out for logs.

createLogExporter does not check OTEL_LOGS_EXPORTER, unlike metrics which honors OTEL_METRICS_EXPORTER=none (pkg/metrics/otel_stats_collector.go). Because log export auto-enables whenever an endpoint is set (shared IsEnabled()), an operator who wants traces and metrics but not the log stream (the highest-volume and most sensitive signal) has no way to disable only logs short of turning telemetry off entirely.

Worth adding the standard OTEL_LOGS_EXPORTER=none escape hatch, or at least documenting that enabling telemetry now also ships all klog output off-box.

[manusa]

LGTM. thx!

Unfortunately there are some merge conflicts, I think they should be easy to deal with in a rebase.

[Cali0707]

@manusa I rebased on main and fixed the conflicts