diff --git a/logctx/handler.go b/logctx/handler.go index 2ad15d6..a351f19 100644 --- a/logctx/handler.go +++ b/logctx/handler.go @@ -3,7 +3,8 @@ package logctx import ( "context" "log/slog" - "os" + + "instant.dev/common/buildinfo" ) // Field-name constants — never inline these strings in tests or callers. @@ -17,18 +18,18 @@ const ( FieldTeamID = "team_id" ) -// commitID returns the build's git SHA. Track 1 of the observability rollout -// adds a real `instant.dev/common/buildinfo` package whose GitSHA var is set -// via `-ldflags -X`. Until that package merges, we fall back to the -// COMMIT_ID env var (set by the Dockerfile / k8s deployment) so this package -// does not block on track 1. The sentinel "dev" matches the buildinfo -// package's planned default so log readers see a single consistent value -// across both implementations. +// commitID returns the build's git SHA, sourced from +// `instant.dev/common/buildinfo.GitSHA`. The buildinfo var is set at link +// time via `-ldflags -X` by the Dockerfiles / CI; un-flagged local builds +// fall back to the buildinfo sentinel ("dev"). +// +// Historical note: this used to read os.Getenv("COMMIT_ID") as a +// decoupling shim from when logctx shipped before buildinfo. Both packages +// now live on the same module, so we collapse to a direct import. This +// eliminates the divergence where /healthz returned the real SHA (from +// buildinfo) but slog lines emitted commit_id="dev" (env var unset). func commitID() string { - if v := os.Getenv("COMMIT_ID"); v != "" { - return v - } - return "dev" + return buildinfo.GitSHA } // Handler wraps an underlying slog.Handler and injects the five mandatory diff --git a/logctx/handler_test.go b/logctx/handler_test.go index 4a3b744..9429ca2 100644 --- a/logctx/handler_test.go +++ b/logctx/handler_test.go @@ -8,6 +8,8 @@ import ( "strings" "testing" "time" + + "instant.dev/common/buildinfo" ) // newTestHandler builds a logctx Handler over a fresh JSON handler writing to @@ -142,6 +144,26 @@ func TestHandler_EnabledPassthrough(t *testing.T) { } } +// Test 6: commit_id is sourced from instant.dev/common/buildinfo.GitSHA. +// Confirms the logctx <-> buildinfo wiring: when the buildinfo var is +// patched (in production this happens via `-ldflags -X` at link time), +// every emitted log line carries that same SHA — keeping slog output in +// lock-step with /healthz and /api/v1/buildinfo. +func TestHandler_CommitIDFromBuildinfo(t *testing.T) { + prev := buildinfo.GitSHA + t.Cleanup(func() { buildinfo.GitSHA = prev }) + buildinfo.GitSHA = "test-sha-abc" + + buf, h := newTestHandler(t, "api") + if err := h.Handle(context.Background(), newRecord("hello")); err != nil { + t.Fatalf("Handle: %v", err) + } + rec := decode(t, buf) + if rec[FieldCommitID] != "test-sha-abc" { + t.Errorf("commit_id = %v, want test-sha-abc", rec[FieldCommitID]) + } +} + // Bonus: WithAttrs / WithGroup preserve the injected service+commit_id on // the returned child handler. Belt-and-braces guard against regressions // where someone refactors the struct and forgets to copy the fields.