Skip to content

Commit e7265dd

Browse files
test(observability): add tests for previously-uncovered helpers
internal/observability had 17.7% statement coverage. Most production helpers were untested, including the pprof handler, the chi-aware tracing helpers, the intercepting response writer, the request-logger helpers, the cleanup wrapper, and the disabled-config and error branches of the configure-* setup functions. Adds focused, infra-free unit tests across cleanup, profiler, metrics, tracing, request-tracing, and request-logger files. The metrics, tracing, and metrics-config tests intentionally cover only the disabled and protocol-unsupported branches; the success branches require a real prometheus or OTLP endpoint and are out of scope for a coverage PR. Coverage for the package goes from 17.7% to 53.4% with no behaviour changes. The remaining gap is in functions that actually start listeners or connect to exporters; those need integration-style fixtures that this PR deliberately doesn't introduce. Signed-off-by: Manas Srivastava <mastermanas805@gmail.com>
1 parent 4ec0d5c commit e7265dd

6 files changed

Lines changed: 309 additions & 0 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package observability
2+
3+
import (
4+
"context"
5+
"testing"
6+
"time"
7+
)
8+
9+
func TestWaitForCleanup(t *testing.T) {
10+
ctx, cancel := context.WithCancel(context.Background())
11+
cancel()
12+
13+
done := make(chan struct{})
14+
go func() {
15+
defer close(done)
16+
WaitForCleanup(ctx)
17+
}()
18+
19+
select {
20+
case <-done:
21+
case <-time.After(time.Second):
22+
t.Fatal("WaitForCleanup did not return after context cancellation")
23+
}
24+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package observability
2+
3+
import (
4+
"context"
5+
"sync"
6+
"testing"
7+
8+
"github.com/stretchr/testify/require"
9+
"github.com/supabase/auth/internal/conf"
10+
)
11+
12+
func TestMeter(t *testing.T) {
13+
require.NotNil(t, Meter("test-meter"))
14+
}
15+
16+
func TestObtainMetricCounter(t *testing.T) {
17+
require.NotNil(t, ObtainMetricCounter("test_counter", "a counter for tests"))
18+
}
19+
20+
func TestConfigureMetricsNilContextPanics(t *testing.T) {
21+
require.PanicsWithValue(t, "context must not be nil", func() {
22+
_ = ConfigureMetrics(nil, &conf.MetricsConfig{})
23+
})
24+
}
25+
26+
func TestConfigureMetricsDisabled(t *testing.T) {
27+
// metricsOnce is a package-level *sync.Once used to ensure ConfigureMetrics
28+
// runs exactly once per process. Reset it for this isolated test so we can
29+
// exercise the disabled-config branch without coupling to other tests.
30+
metricsOnce = &sync.Once{}
31+
require.NoError(t, ConfigureMetrics(context.Background(), &conf.MetricsConfig{Enabled: false}))
32+
}
33+
34+
func TestEnableOpenTelemetryMetricsRejectsUnsupportedExporter(t *testing.T) {
35+
err := enableOpenTelemetryMetrics(context.Background(), &conf.MetricsConfig{ExporterProtocol: "http/json"})
36+
require.Error(t, err)
37+
require.Contains(t, err.Error(), "unsupported OpenTelemetry exporter protocol")
38+
}
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
package observability
2+
3+
import (
4+
"context"
5+
"net/http"
6+
"net/http/httptest"
7+
"testing"
8+
9+
"github.com/stretchr/testify/require"
10+
"github.com/supabase/auth/internal/conf"
11+
)
12+
13+
func TestConfigureProfilerDisabled(t *testing.T) {
14+
require.NoError(t, ConfigureProfiler(context.Background(), &conf.ProfilerConfig{Enabled: false}))
15+
}
16+
17+
func TestProfilerHandlerServeHTTP(t *testing.T) {
18+
tests := []struct {
19+
name string
20+
path string
21+
expectedStatus int
22+
}{
23+
{name: "pprof index", path: "/debug/pprof/", expectedStatus: http.StatusOK},
24+
{name: "pprof cmdline", path: "/debug/pprof/cmdline", expectedStatus: http.StatusOK},
25+
{name: "pprof symbol", path: "/debug/pprof/symbol", expectedStatus: http.StatusOK},
26+
{name: "pprof goroutine", path: "/debug/pprof/goroutine", expectedStatus: http.StatusOK},
27+
{name: "pprof heap", path: "/debug/pprof/heap", expectedStatus: http.StatusOK},
28+
{name: "pprof allocs", path: "/debug/pprof/allocs", expectedStatus: http.StatusOK},
29+
{name: "pprof threadcreate", path: "/debug/pprof/threadcreate", expectedStatus: http.StatusOK},
30+
{name: "pprof block", path: "/debug/pprof/block", expectedStatus: http.StatusOK},
31+
{name: "pprof mutex", path: "/debug/pprof/mutex", expectedStatus: http.StatusOK},
32+
{name: "unknown path returns 404", path: "/debug/pprof/unknown", expectedStatus: http.StatusNotFound},
33+
{name: "non-pprof path returns 404", path: "/something/else", expectedStatus: http.StatusNotFound},
34+
}
35+
36+
handler := &ProfilerHandler{}
37+
for _, tt := range tests {
38+
t.Run(tt.name, func(t *testing.T) {
39+
req := httptest.NewRequest(http.MethodGet, tt.path, nil)
40+
w := httptest.NewRecorder()
41+
handler.ServeHTTP(w, req)
42+
require.Equal(t, tt.expectedStatus, w.Code)
43+
})
44+
}
45+
}

internal/observability/request-logger_test.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,77 @@ func TestContext(t *testing.T) {
9393
}
9494
}
9595
}
96+
97+
func TestNewLogEntry(t *testing.T) {
98+
le := NewLogEntry(logrus.NewEntry(logrus.StandardLogger()))
99+
require.NotNil(t, le)
100+
// NewLogEntry returns a chimiddleware.LogEntry; verify the underlying type
101+
// is the package's *logEntry so downstream casts in GetLogEntry work.
102+
_, ok := le.(*logEntry)
103+
require.True(t, ok, "expected NewLogEntry to return *logEntry")
104+
}
105+
106+
func TestGetLogEntryFallback(t *testing.T) {
107+
// No log entry is attached to this request's context, so GetLogEntry
108+
// should return a fresh fallback entry rather than panic or nil.
109+
req := httptest.NewRequest(http.MethodGet, "/", nil)
110+
got := GetLogEntry(req)
111+
require.NotNil(t, got)
112+
require.NotNil(t, got.Entry)
113+
}
114+
115+
func TestGetLogEntryReturnsAttachedEntry(t *testing.T) {
116+
want := &logEntry{Entry: logrus.NewEntry(logrus.StandardLogger())}
117+
req := httptest.NewRequest(http.MethodGet, "/", nil)
118+
ctx := SetLogEntryWithContext(req.Context(), want)
119+
req = req.WithContext(ctx)
120+
121+
got := GetLogEntry(req)
122+
require.Same(t, want, got)
123+
}
124+
125+
func TestLogEntrySetField(t *testing.T) {
126+
le := &logEntry{Entry: logrus.NewEntry(logrus.StandardLogger())}
127+
req := httptest.NewRequest(http.MethodGet, "/", nil)
128+
req = req.WithContext(SetLogEntryWithContext(req.Context(), le))
129+
130+
LogEntrySetField(req, "user_id", "abc-123")
131+
require.Equal(t, "abc-123", le.Entry.Data["user_id"])
132+
}
133+
134+
func TestLogEntrySetFieldsMerges(t *testing.T) {
135+
le := &logEntry{Entry: logrus.NewEntry(logrus.StandardLogger())}
136+
req := httptest.NewRequest(http.MethodGet, "/", nil)
137+
req = req.WithContext(SetLogEntryWithContext(req.Context(), le))
138+
139+
LogEntrySetFields(req, logrus.Fields{
140+
"session": "s1",
141+
"trace": "t1",
142+
})
143+
require.Equal(t, "s1", le.Entry.Data["session"])
144+
require.Equal(t, "t1", le.Entry.Data["trace"])
145+
}
146+
147+
func TestLogEntrySetFieldNoLogEntryInContextIsNoop(t *testing.T) {
148+
req := httptest.NewRequest(http.MethodGet, "/", nil)
149+
require.NotPanics(t, func() {
150+
LogEntrySetField(req, "k", "v")
151+
LogEntrySetFields(req, logrus.Fields{"k": "v"})
152+
})
153+
}
154+
155+
func TestLogEntryPanicWritesPanicAndStackFields(t *testing.T) {
156+
var buf bytes.Buffer
157+
logger := logrus.New()
158+
logger.SetOutput(&buf)
159+
logger.SetFormatter(&logrus.JSONFormatter{})
160+
161+
le := &logEntry{Entry: logrus.NewEntry(logger)}
162+
le.Panic("boom", []byte("fake-stack-trace"))
163+
164+
var out map[string]interface{}
165+
require.NoError(t, json.Unmarshal(buf.Bytes(), &out))
166+
require.Equal(t, "request panicked", out["msg"])
167+
require.Equal(t, "fake-stack-trace", out["stack"])
168+
require.Contains(t, out["panic"], "boom")
169+
}
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
package observability
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"testing"
7+
8+
"github.com/go-chi/chi/v5"
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
func TestGetChiRoutePatternNoRouteContext(t *testing.T) {
13+
req := httptest.NewRequest(http.MethodGet, "/anything", nil)
14+
require.Equal(t, "noroute", getChiRoutePattern(req))
15+
}
16+
17+
func TestGetChiRoutePatternWithRouter(t *testing.T) {
18+
r := chi.NewRouter()
19+
var captured string
20+
r.Get("/users/{id}", func(w http.ResponseWriter, r *http.Request) {
21+
captured = getChiRoutePattern(r)
22+
w.WriteHeader(http.StatusOK)
23+
})
24+
25+
req := httptest.NewRequest(http.MethodGet, "/users/42", nil)
26+
w := httptest.NewRecorder()
27+
r.ServeHTTP(w, req)
28+
29+
require.Equal(t, http.StatusOK, w.Code)
30+
require.Equal(t, "/users/{id}", captured)
31+
}
32+
33+
func TestTraceChiRoutesSafelyDoesNotPanicWithoutRouteContext(t *testing.T) {
34+
req := httptest.NewRequest(http.MethodGet, "/no/route/context", nil)
35+
require.NotPanics(t, func() { traceChiRoutesSafely(req) })
36+
}
37+
38+
func TestTraceChiRouteURLParamsSafelyDoesNotPanicWithoutRouteContext(t *testing.T) {
39+
req := httptest.NewRequest(http.MethodGet, "/no/route/context", nil)
40+
require.NotPanics(t, func() { traceChiRouteURLParamsSafely(req) })
41+
}
42+
43+
func TestAddMetricAttributesIncludesRoutePattern(t *testing.T) {
44+
req := httptest.NewRequest(http.MethodGet, "/something", nil)
45+
attrs := addMetricAttributes(req)
46+
require.NotEmpty(t, attrs)
47+
// The first attribute should be the route key. Without a chi RouteContext
48+
// the value falls back to "noroute".
49+
require.Equal(t, "noroute", attrs[0].Value.AsString())
50+
}
51+
52+
func TestInterceptingResponseWriterDelegates(t *testing.T) {
53+
inner := httptest.NewRecorder()
54+
w := &interceptingResponseWriter{writer: inner}
55+
56+
w.Header().Set("X-Test", "value")
57+
require.Equal(t, "value", inner.Header().Get("X-Test"))
58+
59+
w.WriteHeader(http.StatusTeapot)
60+
require.Equal(t, http.StatusTeapot, inner.Code)
61+
require.Equal(t, http.StatusTeapot, w.statusCode)
62+
63+
n, err := w.Write([]byte("hello"))
64+
require.NoError(t, err)
65+
require.Equal(t, 5, n)
66+
require.Equal(t, "hello", inner.Body.String())
67+
}
68+
69+
func TestCountStatusCodesSafelyNilCounterIsNoop(t *testing.T) {
70+
req := httptest.NewRequest(http.MethodGet, "/path", nil)
71+
writer := &interceptingResponseWriter{statusCode: http.StatusOK}
72+
require.NotPanics(t, func() { countStatusCodesSafely(writer, req, nil) })
73+
}
74+
75+
func TestRequestTracingMiddlewareInvokesNext(t *testing.T) {
76+
called := false
77+
next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
78+
called = true
79+
w.WriteHeader(http.StatusOK)
80+
})
81+
82+
handler := RequestTracing()(next)
83+
req := httptest.NewRequest(http.MethodGet, "/ping", nil)
84+
req.Header.Set("User-Agent", "agent-under-test")
85+
w := httptest.NewRecorder()
86+
handler.ServeHTTP(w, req)
87+
88+
require.True(t, called, "next handler should have been invoked")
89+
require.Equal(t, http.StatusOK, w.Code)
90+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package observability
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/stretchr/testify/require"
8+
"github.com/supabase/auth/internal/conf"
9+
)
10+
11+
func TestTracer(t *testing.T) {
12+
require.NotNil(t, Tracer("test-tracer"))
13+
}
14+
15+
func TestOpenTelemetryResource(t *testing.T) {
16+
resource := openTelemetryResource()
17+
require.NotNil(t, resource)
18+
// The resource should at minimum carry the gotrue.version attribute that
19+
// openTelemetryResource merges in. We don't assert the exact value because
20+
// utilities.Version is a build-time string; presence is enough.
21+
require.NotEmpty(t, resource.Attributes())
22+
}
23+
24+
func TestConfigureTracingNilContextPanics(t *testing.T) {
25+
require.PanicsWithValue(t, "context must not be nil", func() {
26+
_ = ConfigureTracing(nil, &conf.TracingConfig{})
27+
})
28+
}
29+
30+
func TestConfigureTracingDisabled(t *testing.T) {
31+
require.NoError(t, ConfigureTracing(context.Background(), &conf.TracingConfig{Enabled: false}))
32+
}
33+
34+
func TestEnableOpenTelemetryTracingRejectsUnsupportedExporter(t *testing.T) {
35+
err := enableOpenTelemetryTracing(context.Background(), &conf.TracingConfig{ExporterProtocol: "http/json"})
36+
require.Error(t, err)
37+
require.Contains(t, err.Error(), "unsupported OpenTelemetry exporter protocol")
38+
}

0 commit comments

Comments
 (0)