Skip to content

Commit 4d7cab7

Browse files
committed
fix(tracing): add OTEL_SDK_DISABLED guard and cap shutdown timeout to 5s
initTracer() in the Go ingest service unconditionally constructed an OTLP gRPC exporter, mirroring the same design flaw fixed in Python tracing.py (0b4e416). With OTEL_SDK_DISABLED=true the function now returns a no-op shutdown function before any exporter or provider is created, matching the Python guard exactly. defer shutdownTracer(context.Background()) had no deadline, so the BatchSpanProcessor flush could hang silently on service shutdown when Jaeger is unreachable. The defer now uses context.WithTimeout(5s) to bound the worst-case hang. Adds four regression tests: skip registration when disabled, four case/whitespace variants, and shutdown-respects-timeout.
1 parent 82bcb4b commit 4d7cab7

4 files changed

Lines changed: 75 additions & 1 deletion

File tree

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ Copy `.env.example` to `.env` and fill these in. The two required ones make the
141141
| `REDIS_URL` || unset | Redis connection string; enables multi-worker SSE pub/sub |
142142
| `KAFKA_BROKER` || unset | Bootstrap servers; starts the Kafka consumer when set |
143143
| `OTEL_EXPORTER_OTLP_ENDPOINT` || unset | OTLP endpoint for Go ingest service distributed tracing |
144+
| `OTEL_SDK_DISABLED` || unset | Set to `true` to skip tracer initialisation in the Go ingest service. Without a reachable OTLP collector (e.g. Jaeger), leaving this unset causes a silent hang of up to 5 s on service shutdown while the `BatchSpanProcessor` tries to flush buffered spans. |
144145

145146
## Architecture diagram
146147

cmd/ingest-service/main.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"os"
1010
"os/signal"
1111
"syscall"
12+
"time"
1213

1314
"github.com/Romil2112/SOC-Dashboard/internal/ingest"
1415
)
@@ -31,7 +32,11 @@ func main() {
3132
defer stop()
3233

3334
shutdownTracer := initTracer(ctx)
34-
defer shutdownTracer(context.Background())
35+
defer func() {
36+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
37+
defer cancel()
38+
shutdownTracer(ctx)
39+
}()
3540

3641
svc, err := ingest.NewService(ctx, cfg)
3742
if err != nil {

cmd/ingest-service/tracing.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"log/slog"
66
"os"
7+
"strings"
78

89
"go.opentelemetry.io/otel"
910
"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
@@ -19,6 +20,11 @@ const _serviceName = "soc-ingest"
1920
// function to flush pending spans on exit. Tracing failures are logged but do
2021
// not prevent the service from starting — spans are simply dropped.
2122
func initTracer(ctx context.Context) func(context.Context) {
23+
if strings.EqualFold(strings.TrimSpace(os.Getenv("OTEL_SDK_DISABLED")), "true") {
24+
slog.Info("OTel tracing disabled via OTEL_SDK_DISABLED")
25+
return func(context.Context) {}
26+
}
27+
2228
endpoint := os.Getenv("OTEL_EXPORTER_OTLP_ENDPOINT")
2329
if endpoint == "" {
2430
endpoint = "localhost:4317"

cmd/ingest-service/tracing_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@ package main
33
import (
44
"context"
55
"testing"
6+
"time"
67

78
"go.opentelemetry.io/otel"
9+
oteltrace "go.opentelemetry.io/otel/trace"
10+
sdktrace "go.opentelemetry.io/otel/sdk/trace"
811
)
912

1013
func TestInitTracerDoesNotPanicWithUnreachableEndpoint(t *testing.T) {
@@ -36,3 +39,62 @@ func TestInitTracerSpanDoesNotPanic(t *testing.T) {
3639
}
3740
span.End()
3841
}
42+
43+
func TestInitTracerSkipsProviderWhenSDKDisabled(t *testing.T) {
44+
// Reset the global provider so we can detect whether initTracer sets a new one.
45+
otel.SetTracerProvider(oteltrace.NewNoopTracerProvider())
46+
t.Setenv("OTEL_SDK_DISABLED", "true")
47+
48+
shutdown := initTracer(context.Background())
49+
if shutdown == nil {
50+
t.Fatal("initTracer returned nil shutdown function")
51+
}
52+
53+
// Global must NOT have been upgraded to a real TracerProvider.
54+
if _, ok := otel.GetTracerProvider().(*sdktrace.TracerProvider); ok {
55+
t.Fatal("initTracer with OTEL_SDK_DISABLED=true must not register a TracerProvider")
56+
}
57+
58+
// Shutdown must return promptly — it is a no-op, not a flush.
59+
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
60+
defer cancel()
61+
shutdown(ctx)
62+
}
63+
64+
func TestInitTracerSDKDisabledCaseInsensitive(t *testing.T) {
65+
for _, val := range []string{"TRUE", "True", " true ", "TRUE "} {
66+
val := val
67+
t.Run(val, func(t *testing.T) {
68+
otel.SetTracerProvider(oteltrace.NewNoopTracerProvider())
69+
t.Setenv("OTEL_SDK_DISABLED", val)
70+
_ = initTracer(context.Background())
71+
if _, ok := otel.GetTracerProvider().(*sdktrace.TracerProvider); ok {
72+
t.Fatalf("OTEL_SDK_DISABLED=%q should skip provider registration", val)
73+
}
74+
})
75+
}
76+
}
77+
78+
func TestShutdownRespectsTimeout(t *testing.T) {
79+
// Verify the shutdown function exits when its context deadline fires,
80+
// not when the underlying flush gives up on its own schedule.
81+
t.Setenv("OTEL_SDK_DISABLED", "true")
82+
shutdown := initTracer(context.Background())
83+
84+
// A 50ms deadline is far shorter than any real flush timeout.
85+
ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
86+
defer cancel()
87+
88+
done := make(chan struct{})
89+
go func() {
90+
shutdown(ctx)
91+
close(done)
92+
}()
93+
94+
select {
95+
case <-done:
96+
// returned within deadline — correct
97+
case <-time.After(500 * time.Millisecond):
98+
t.Fatal("shutdown did not return within deadline — no-op guard may not be working")
99+
}
100+
}

0 commit comments

Comments
 (0)