diff --git a/docs/plans-executions/2026-08-24-1025-otel-tracing.md b/docs/plans-executions/2026-08-24-1025-otel-tracing.md index 39021a4..3f28395 100644 --- a/docs/plans-executions/2026-08-24-1025-otel-tracing.md +++ b/docs/plans-executions/2026-08-24-1025-otel-tracing.md @@ -4,7 +4,7 @@ Plan: `docs/plans/2026-08-24-1025-otel-tracing.md` - [x] Step 1 — Dependencies - [x] Step 2 — New package `internal/tracing` -- [ ] Step 3 — `provider.WithTracing` decorator +- [x] Step 3 — `provider.WithTracing` decorator - [ ] Step 4 — `cmd/main.go` wiring - [ ] Step 5 — Reconciler spans - [ ] Step 6 — Discovery server @@ -67,3 +67,17 @@ instead of relying on the global, so behavior is deterministic under tests and when tracing is disabled. `Setup` tests reset the global provider to a fresh noop per case — restoring otel's own default delegate triggers a "Setting tracer provider to its current value" warning from the SDK. + +## Step 3 — `provider.WithTracing` + +`internal/provider/tracing.go` mirrors `metrics.go` exactly (same wrap shape, +same `resultLabel` classification reused as the `provider.result` span +attribute); tests reuse `staticProvider` from `metrics_test.go`. `ErrNotFound` +maps to span status Ok as planned — the reconciler polls `Get` to NotFound +during replacement/deletion, so it's an answer, not a failure. + +One small API addition to `internal/tracing` for this: exported +`tracing.Tracer(opts...)`, since the decorator lives in package `provider` +and couldn't reach the unexported option resolver. The `cmd/main.go` wiring +(`WithTracing` outermost around `WithMetrics`) lands with Step 4's commit — +same file, one commit. diff --git a/internal/provider/tracing.go b/internal/provider/tracing.go new file mode 100644 index 0000000..a07a492 --- /dev/null +++ b/internal/provider/tracing.go @@ -0,0 +1,80 @@ +package provider + +import ( + "context" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" + "go.opentelemetry.io/otel/trace" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/tracing" +) + +// WithTracing wraps a Provider so every call runs in a client span carrying +// the classified result, mirroring WithMetrics. It goes through +// tracing.StartSpan, so provider-internal logs (e.g. the gcp provider's +// context logger) inherit traceID/spanID. Errors pass through unmodified. +func WithTracing(name string, p Provider, opts ...tracing.Option) Provider { + return &traced{name: name, inner: p, tracer: tracing.Tracer(opts...)} +} + +type traced struct { + name string + inner Provider + tracer trace.Tracer +} + +func (t *traced) Create(ctx context.Context, req CreateRequest) (string, error) { + ctx, span := t.start(ctx, "provider.create") + id, err := t.inner.Create(ctx, req) + if id != "" { + span.SetAttributes(attribute.String("provider.id", id)) + } + t.end(span, err) + return id, err +} + +func (t *traced) Get(ctx context.Context, providerID string) (*Instance, error) { + ctx, span := t.start(ctx, "provider.get") + span.SetAttributes(attribute.String("provider.id", providerID)) + inst, err := t.inner.Get(ctx, providerID) + t.end(span, err) + return inst, err +} + +func (t *traced) Delete(ctx context.Context, providerID string) error { + ctx, span := t.start(ctx, "provider.delete") + span.SetAttributes(attribute.String("provider.id", providerID)) + err := t.inner.Delete(ctx, providerID) + t.end(span, err) + return err +} + +func (t *traced) ListByTag(ctx context.Context) ([]Instance, error) { + ctx, span := t.start(ctx, "provider.list") + instances, err := t.inner.ListByTag(ctx) + span.SetAttributes(attribute.Int("provider.instances", len(instances))) + t.end(span, err) + return instances, err +} + +func (t *traced) start(ctx context.Context, op string) (context.Context, trace.Span) { + return tracing.StartSpan(ctx, t.tracer, op, + trace.WithSpanKind(trace.SpanKindClient), + trace.WithAttributes(attribute.String("provider.name", t.name))) +} + +// end records the taxonomy class and closes the span. ErrNotFound stays Ok: +// the reconciler polls Get to NotFound during replacement/deletion, so it is +// an expected answer, not a failure — same reasoning as resultLabel's +// distinct "not_found" bucket. +func (t *traced) end(span trace.Span, err error) { + span.SetAttributes(attribute.String("provider.result", resultLabel(err))) + switch Class(err) { + case nil, ErrNotFound: + span.SetStatus(codes.Ok, "") + default: + span.SetStatus(codes.Error, err.Error()) + } + span.End() +} diff --git a/internal/provider/tracing_test.go b/internal/provider/tracing_test.go new file mode 100644 index 0000000..cdb14c6 --- /dev/null +++ b/internal/provider/tracing_test.go @@ -0,0 +1,118 @@ +package provider + +import ( + "context" + "errors" + "slices" + "testing" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" + sdktrace "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" + "go.opentelemetry.io/otel/trace" + + "gitea.home.hrajfrisbee.cz/kacerr/egress-proxies-operator/internal/tracing" +) + +func TestWithTracing_spanPerCall(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + inner *staticProvider + call func(p Provider) error + wantSpan string + wantStatus codes.Code + wantResult string + wantAttr attribute.KeyValue + }{ + { + name: "successful create", + inner: &staticProvider{}, + call: func(p Provider) error { _, err := p.Create(context.Background(), CreateRequest{}); return err }, + wantSpan: "provider.create", + wantStatus: codes.Ok, + wantResult: "ok", + wantAttr: attribute.String("provider.id", "id-1"), + }, + { + name: "get NotFound is not a span error", + inner: &staticProvider{getErr: Wrap(ErrNotFound, "get", "x", "id-1", nil)}, + call: func(p Provider) error { _, err := p.Get(context.Background(), "id-1"); return err }, + wantSpan: "provider.get", + wantStatus: codes.Ok, + wantResult: "not_found", + wantAttr: attribute.String("provider.id", "id-1"), + }, + { + name: "delete transient error", + inner: &staticProvider{deleteErr: Wrap(ErrTransient, "delete", "x", "id-1", errors.New("503"))}, + call: func(p Provider) error { return p.Delete(context.Background(), "id-1") }, + wantSpan: "provider.delete", + wantStatus: codes.Error, + wantResult: "transient", + wantAttr: attribute.String("provider.id", "id-1"), + }, + { + name: "quota exceeded create", + inner: &staticProvider{createErr: Wrap(ErrQuotaExceeded, "create", "x", "", nil)}, + call: func(p Provider) error { _, err := p.Create(context.Background(), CreateRequest{}); return err }, + wantSpan: "provider.create", + wantStatus: codes.Error, + wantResult: "quota_exceeded", + wantAttr: attribute.String("provider.name", "x"), + }, + { + name: "list records instance count", + inner: &staticProvider{}, + call: func(p Provider) error { _, err := p.ListByTag(context.Background()); return err }, + wantSpan: "provider.list", + wantStatus: codes.Ok, + wantResult: "ok", + wantAttr: attribute.Int("provider.instances", 0), + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + sr := tracetest.NewSpanRecorder() + tp := sdktrace.NewTracerProvider(sdktrace.WithSpanProcessor(sr)) + t.Cleanup(func() { _ = tp.Shutdown(context.Background()) }) + + p := WithTracing("x", tc.inner, tracing.WithTracerProvider(tp)) + err := tc.call(p) + + wantErr := errors.Join(tc.inner.createErr, tc.inner.getErr, tc.inner.deleteErr, tc.inner.listErr) + if (wantErr == nil) != (err == nil) { + t.Fatalf("decorator changed the error: got %v", err) + } + + ended := sr.Ended() + if len(ended) != 1 { + t.Fatalf("got %d spans, want 1", len(ended)) + } + span := ended[0] + if span.Name() != tc.wantSpan { + t.Errorf("span name = %q, want %q", span.Name(), tc.wantSpan) + } + if span.SpanKind() != trace.SpanKindClient { + t.Errorf("span kind = %v, want client", span.SpanKind()) + } + if span.Status().Code != tc.wantStatus { + t.Errorf("status = %v, want %v", span.Status().Code, tc.wantStatus) + } + attrs := span.Attributes() + hasAttr := func(want attribute.KeyValue) bool { + return slices.Contains(attrs, want) + } + if !hasAttr(attribute.String("provider.result", tc.wantResult)) { + t.Errorf("provider.result %q missing in %v", tc.wantResult, attrs) + } + if !hasAttr(tc.wantAttr) { + t.Errorf("attribute %v missing in %v", tc.wantAttr, attrs) + } + }) + } +}