Skip to content

Commit b37633c

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 b37633c

7 files changed

Lines changed: 484 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: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package observability
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"sync"
7+
"testing"
8+
9+
"github.com/sirupsen/logrus"
10+
"github.com/stretchr/testify/require"
11+
"github.com/supabase/auth/internal/conf"
12+
)
13+
14+
func TestConfigureLoggingWithFile(t *testing.T) {
15+
// loggingOnce must be reset for this branch to run; we can only do this
16+
// safely from inside the package's own tests.
17+
loggingOnce = sync.Once{}
18+
19+
dir := t.TempDir()
20+
logFile := filepath.Join(dir, "test.log")
21+
22+
require.NoError(t, ConfigureLogging(&conf.LoggingConfig{
23+
File: logFile,
24+
Level: "info",
25+
Fields: map[string]interface{}{
26+
"env": "test",
27+
"region": "us-east-1",
28+
},
29+
SQL: LOG_SQL_ALL,
30+
}))
31+
32+
// The configure path opens the log file before writing the "Set output
33+
// file to ..." entry; the file must exist after the call returns.
34+
_, err := os.Stat(logFile)
35+
require.NoError(t, err)
36+
}
37+
38+
func TestNewCustomFormatterFormatsTimeAsUTC(t *testing.T) {
39+
f := NewCustomFormatter()
40+
require.NotNil(t, f)
41+
42+
entry := logrus.NewEntry(logrus.New())
43+
entry.Message = "hello"
44+
out, err := f.Format(entry)
45+
require.NoError(t, err)
46+
require.Contains(t, string(out), "hello")
47+
}
48+
49+
func TestSetPopLoggerExecutesAllSQLConfigs(t *testing.T) {
50+
// setPopLogger registers a closure with pop.SetLogger; calling it for
51+
// each documented SQL log mode covers the three branches that select
52+
// shouldLogSQL and shouldLogSQLArgs.
53+
for _, mode := range []string{LOG_SQL_NONE, LOG_SQL_STATEMENT, LOG_SQL_ALL, ""} {
54+
setPopLogger(mode)
55+
}
56+
}
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package observability
2+
3+
import (
4+
"context"
5+
"sync"
6+
"testing"
7+
"time"
8+
9+
"github.com/stretchr/testify/require"
10+
"github.com/supabase/auth/internal/conf"
11+
)
12+
13+
func TestMeter(t *testing.T) {
14+
require.NotNil(t, Meter("test-meter"))
15+
}
16+
17+
func TestObtainMetricCounter(t *testing.T) {
18+
require.NotNil(t, ObtainMetricCounter("test_counter", "a counter for tests"))
19+
}
20+
21+
func TestConfigureMetricsNilContextPanics(t *testing.T) {
22+
require.PanicsWithValue(t, "context must not be nil", func() {
23+
_ = ConfigureMetrics(nil, &conf.MetricsConfig{})
24+
})
25+
}
26+
27+
func TestConfigureMetricsDisabled(t *testing.T) {
28+
// metricsOnce is a package-level *sync.Once used to ensure ConfigureMetrics
29+
// runs exactly once per process. Reset it for this isolated test so we can
30+
// exercise the disabled-config branch without coupling to other tests.
31+
metricsOnce = &sync.Once{}
32+
require.NoError(t, ConfigureMetrics(context.Background(), &conf.MetricsConfig{Enabled: false}))
33+
}
34+
35+
func TestEnableOpenTelemetryMetricsRejectsUnsupportedExporter(t *testing.T) {
36+
err := enableOpenTelemetryMetrics(context.Background(), &conf.MetricsConfig{ExporterProtocol: "http/json"})
37+
require.Error(t, err)
38+
require.Contains(t, err.Error(), "unsupported OpenTelemetry exporter protocol")
39+
}
40+
41+
func TestEnablePrometheusMetricsStartsAndShutsDown(t *testing.T) {
42+
// Coverage of the synchronous setup path; the goroutine that calls
43+
// ListenAndServe shuts down when the context is cancelled. The function
44+
// returns nil whether or not the listener binds, so we only assert the
45+
// happy synchronous outcome.
46+
ctx, cancel := context.WithCancel(context.Background())
47+
port := freeLocalPort(t)
48+
require.NoError(t, enablePrometheusMetrics(ctx, &conf.MetricsConfig{
49+
PrometheusListenHost: "127.0.0.1",
50+
PrometheusListenPort: port,
51+
}))
52+
cancel()
53+
time.Sleep(150 * time.Millisecond)
54+
}
55+
56+
func TestEnableOpenTelemetryMetricsGRPC(t *testing.T) {
57+
ctx, cancel := context.WithCancel(context.Background())
58+
require.NoError(t, enableOpenTelemetryMetrics(ctx, &conf.MetricsConfig{ExporterProtocol: "grpc"}))
59+
cancel()
60+
time.Sleep(150 * time.Millisecond)
61+
}
62+
63+
func TestEnableOpenTelemetryMetricsHTTPProtobuf(t *testing.T) {
64+
ctx, cancel := context.WithCancel(context.Background())
65+
require.NoError(t, enableOpenTelemetryMetrics(ctx, &conf.MetricsConfig{ExporterProtocol: "http/protobuf"}))
66+
cancel()
67+
time.Sleep(150 * time.Millisecond)
68+
}
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
package observability
2+
3+
import (
4+
"context"
5+
"net"
6+
"net/http"
7+
"net/http/httptest"
8+
"strconv"
9+
"testing"
10+
"time"
11+
12+
"github.com/stretchr/testify/require"
13+
"github.com/supabase/auth/internal/conf"
14+
)
15+
16+
// freeLocalPort returns the string form of a free TCP port on 127.0.0.1.
17+
// Used by tests that need to start a real listener (profiler, prometheus)
18+
// without colliding with other services or other tests running in parallel.
19+
func freeLocalPort(t *testing.T) string {
20+
t.Helper()
21+
l, err := net.Listen("tcp", "127.0.0.1:0")
22+
require.NoError(t, err)
23+
port := l.Addr().(*net.TCPAddr).Port
24+
require.NoError(t, l.Close())
25+
return strconv.Itoa(port)
26+
}
27+
28+
func TestConfigureProfilerDisabled(t *testing.T) {
29+
require.NoError(t, ConfigureProfiler(context.Background(), &conf.ProfilerConfig{Enabled: false}))
30+
}
31+
32+
func TestConfigureProfilerStartsAndShutsDown(t *testing.T) {
33+
// Start the profiler on a free localhost port and immediately cancel the
34+
// context; the cleanup goroutine should run server.Shutdown without
35+
// hanging the test. The function returns nil regardless of whether the
36+
// underlying listener bound successfully, so coverage focuses on the
37+
// synchronous setup path.
38+
ctx, cancel := context.WithCancel(context.Background())
39+
port := freeLocalPort(t)
40+
require.NoError(t, ConfigureProfiler(ctx, &conf.ProfilerConfig{
41+
Enabled: true,
42+
Host: "127.0.0.1",
43+
Port: port,
44+
}))
45+
cancel()
46+
// Give the cleanup goroutine a brief moment to run.
47+
time.Sleep(150 * time.Millisecond)
48+
}
49+
50+
func TestProfilerHandlerServeHTTP(t *testing.T) {
51+
tests := []struct {
52+
name string
53+
path string
54+
expectedStatus int
55+
}{
56+
{name: "pprof index", path: "/debug/pprof/", expectedStatus: http.StatusOK},
57+
{name: "pprof cmdline", path: "/debug/pprof/cmdline", expectedStatus: http.StatusOK},
58+
{name: "pprof symbol", path: "/debug/pprof/symbol", expectedStatus: http.StatusOK},
59+
{name: "pprof goroutine", path: "/debug/pprof/goroutine", expectedStatus: http.StatusOK},
60+
{name: "pprof heap", path: "/debug/pprof/heap", expectedStatus: http.StatusOK},
61+
{name: "pprof allocs", path: "/debug/pprof/allocs", expectedStatus: http.StatusOK},
62+
{name: "pprof threadcreate", path: "/debug/pprof/threadcreate", expectedStatus: http.StatusOK},
63+
{name: "pprof block", path: "/debug/pprof/block", expectedStatus: http.StatusOK},
64+
{name: "pprof mutex", path: "/debug/pprof/mutex", expectedStatus: http.StatusOK},
65+
{name: "unknown path returns 404", path: "/debug/pprof/unknown", expectedStatus: http.StatusNotFound},
66+
{name: "non-pprof path returns 404", path: "/something/else", expectedStatus: http.StatusNotFound},
67+
}
68+
69+
handler := &ProfilerHandler{}
70+
for _, tt := range tests {
71+
t.Run(tt.name, func(t *testing.T) {
72+
req := httptest.NewRequest(http.MethodGet, tt.path, nil)
73+
w := httptest.NewRecorder()
74+
handler.ServeHTTP(w, req)
75+
require.Equal(t, tt.expectedStatus, w.Code)
76+
})
77+
}
78+
}

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+
}

0 commit comments

Comments
 (0)