From 13ed4696c8d4b46f135bf2b5cec5ad11f1bb5989 Mon Sep 17 00:00:00 2001 From: Andrew Aquino Date: Tue, 11 Aug 2026 20:47:04 +0000 Subject: [PATCH] feat: handle session_id in agent middleware Add tracing.SessionIDMiddleware, a log-only middleware that reads the session_id W3C baggage member and attaches it to the request log context, and wire it into the agent HTTP stack before loggermw so agent request logs can be correlated by session. Unlike Middleware, it does not create spans, emit telemetry, or gate on route patterns, per RFC requirement 6.2. Verified with go test ./coderd/tracing/..., go vet, and golangci-lint on the changed packages. --- agent/api.go | 1 + coderd/tracing/httpmw.go | 14 +++++ coderd/tracing/httpmw_test.go | 108 ++++++++++++++++++++++++++++++++++ 3 files changed, 123 insertions(+) diff --git a/agent/api.go b/agent/api.go index 300d92475ed5a..247ce88e97b39 100644 --- a/agent/api.go +++ b/agent/api.go @@ -20,6 +20,7 @@ func (a *agent) apiHandler() http.Handler { r.Use( httpmw.Recover(a.logger), tracing.StatusWriterMiddleware, + tracing.SessionIDMiddleware, loggermw.Logger(a.logger, nil), agentchat.Middleware, ) diff --git a/coderd/tracing/httpmw.go b/coderd/tracing/httpmw.go index 30dd1b1021db5..842907b8114fb 100644 --- a/coderd/tracing/httpmw.go +++ b/coderd/tracing/httpmw.go @@ -98,6 +98,20 @@ func sessionIDFromHeaders(h http.Header) string { return id } +// SessionIDMiddleware reads the session_id baggage member from the request and +// adds it to the log context so downstream request logs can be correlated by +// session. Unlike Middleware, it does not create spans, emit telemetry, or gate +// on route patterns. It is intended for the agent, per the connection-log RFC, +// which for now only requires the session ID on the log context. +func SessionIDMiddleware(next http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + if sessionID := sessionIDFromHeaders(r.Header); sessionID != "" { + r = r.WithContext(slog.With(r.Context(), slog.F("session_id", sessionID))) + } + next.ServeHTTP(rw, r) + }) +} + // validSessionID reports whether s is a 32-character lowercase hexadecimal // string (a 16-byte value), the encoding the RFC mandates for the session ID. // Only lowercase is accepted so that case-sensitive searches correlate diff --git a/coderd/tracing/httpmw_test.go b/coderd/tracing/httpmw_test.go index fbb546cfaff3d..b2842ab0d553f 100644 --- a/coderd/tracing/httpmw_test.go +++ b/coderd/tracing/httpmw_test.go @@ -17,6 +17,7 @@ import ( "go.opentelemetry.io/otel/trace/noop" "cdr.dev/slog/v3" + "github.com/coder/coder/v2/coderd/httpmw/loggermw" "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/testutil" ) @@ -227,6 +228,113 @@ func Test_Middleware_SessionID(t *testing.T) { }) } +func Test_SessionIDMiddleware(t *testing.T) { + t.Parallel() + + // downstreamFields runs a request through SessionIDMiddleware and returns + // the fields a downstream handler logs using the request context. + downstreamFields := func(t *testing.T, header string) []slog.Field { + t.Helper() + + sink := testutil.NewFakeSink(t) + logger := sink.Logger() + + handler := tracing.SessionIDMiddleware(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { + logger.Info(r.Context(), "downstream handler invoked") + rw.WriteHeader(http.StatusNoContent) + })) + + r := httptest.NewRequest(http.MethodGet, "/api/v0/foo", nil) + if header != "" { + r.Header.Set("baggage", header) + } + handler.ServeHTTP(httptest.NewRecorder(), r) + + entries := sink.Entries(func(e slog.SinkEntry) bool { + return e.Message == "downstream handler invoked" + }) + require.Len(t, entries, 1) + return entries[0].Fields + } + + fieldValue := func(fields []slog.Field, name string) (any, bool) { + for _, f := range fields { + if f.Name == name { + return f.Value, true + } + } + return nil, false + } + + t.Run("ValidBaggage", func(t *testing.T) { + t.Parallel() + + val, ok := fieldValue(downstreamFields(t, tracing.SessionIDBaggageKey+"="+testSessionID), "session_id") + require.True(t, ok, "session_id should be on the log context") + require.Equal(t, testSessionID, val) + }) + + t.Run("NoBaggage", func(t *testing.T) { + t.Parallel() + + _, ok := fieldValue(downstreamFields(t, ""), "session_id") + require.False(t, ok, "session_id should be absent when no baggage is sent") + }) + + t.Run("MalformedBaggage", func(t *testing.T) { + t.Parallel() + + _, ok := fieldValue(downstreamFields(t, tracing.SessionIDBaggageKey+"=not-a-valid-session-id"), "session_id") + require.False(t, ok, "malformed session_id should be ignored") + }) + + t.Run("UppercaseRejected", func(t *testing.T) { + t.Parallel() + + upper := strings.ToUpper(testSessionID) + _, ok := fieldValue(downstreamFields(t, tracing.SessionIDBaggageKey+"="+upper), "session_id") + require.False(t, ok, "uppercase session_id should be rejected") + }) +} + +// Test_SessionIDMiddleware_AccessLog verifies that, wired in the same order as +// the agent middleware stack, the session_id lands on loggermw's request +// completion log line, not just on downstream handler logs. +func Test_SessionIDMiddleware_AccessLog(t *testing.T) { + t.Parallel() + + sink := testutil.NewFakeSink(t) + + // StatusWriterMiddleware is required by loggermw; SessionIDMiddleware runs + // before loggermw so the field is on the request context when the access + // log is emitted. This mirrors agent/api.go. + handler := tracing.StatusWriterMiddleware( + tracing.SessionIDMiddleware( + loggermw.Logger(sink.Logger(), nil)( + http.HandlerFunc(func(rw http.ResponseWriter, _ *http.Request) { + rw.WriteHeader(http.StatusNoContent) + }), + ), + ), + ) + + r := httptest.NewRequest(http.MethodGet, "/api/v0/foo", nil) + r.Header.Set("baggage", tracing.SessionIDBaggageKey+"="+testSessionID) + handler.ServeHTTP(httptest.NewRecorder(), r) + + entries := sink.Entries() + require.Len(t, entries, 1) + + var found bool + for _, f := range entries[0].Fields { + if f.Name == "session_id" { + found = true + require.Equal(t, testSessionID, f.Value) + } + } + require.True(t, found, "session_id should be on the request access log") +} + func Test_Middleware(t *testing.T) { t.Parallel()