Add tracing manifests and docs; clean up branch lint findings

Manager env block (downward-API resource attrs, commented OTLP
examples), architecture §10 + Decisions entries, README section.
Lint: goconst constants, gofmt, logcheck (Setup now takes its logger
from ctx via logf.FromContext).

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
2026-08-24 11:30:27 +02:00
parent 53c0d77ef5
commit aeb4115c72
10 changed files with 207 additions and 44 deletions

View File

@@ -192,6 +192,25 @@ Test against a fake API seam rather than the real cloud — see the
[internal/provider/gcp/gcp.go](internal/provider/gcp/gcp.go) for the
pattern.
## Tracing
OpenTelemetry tracing is built in but **off by default** — the operator
only exports spans when pointed at an OTLP receiver:
```bash
# In config/manager/manager.yaml (commented-out block is already there):
# OTEL_EXPORTER_OTLP_ENDPOINT=http://jaeger-collector.observability:4318
# Local dev: print spans to stdout instead
OTEL_TRACES_EXPORTER=console go run ./cmd --providers-config hack/providers-dev.yaml
```
Every reconcile, discovery API request, and GC sweep becomes a trace, with
child spans for provider and Kubernetes API calls; log lines inside a traced
operation carry matching `traceID`/`spanID` fields (point Grafana/Loki
derived fields at `traceID`). Sampling and endpoints follow the standard
`OTEL_*` env vars; `--trace-health-probes` additionally traces each health
probe (high volume). Details in `docs/architecture.md` §10.
## Caveats — read these two
**Changing a proxy changes its IP.** Proxies are immutable cattle: editing

View File

@@ -42,6 +42,7 @@ import (
"sigs.k8s.io/controller-runtime/pkg/cache"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/healthz"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/log/zap"
ctrlmetrics "sigs.k8s.io/controller-runtime/pkg/metrics"
"sigs.k8s.io/controller-runtime/pkg/metrics/filters"
@@ -157,7 +158,7 @@ func main() {
// Off (no-op spans, unchanged logs) unless OTEL_* env opts in; see
// internal/tracing. Shutdown is called explicitly after mgr.Start
// returns — the os.Exit paths below skip defers.
tracingShutdown, err := tracing.Setup(context.Background(), setupLog,
tracingShutdown, err := tracing.Setup(logf.IntoContext(context.Background(), setupLog),
"egress-proxies-operator", version.Resolve())
if err != nil {
setupLog.Error(err, "Failed to set up tracing")

View File

@@ -64,6 +64,8 @@ spec:
- --leader-elect
- --health-probe-bind-address=:8081
- --providers-config=/etc/proxy-operator/providers.yaml
# Add --trace-health-probes to emit one span per health probe
# (high volume: ~1/s per proxy).
env:
# Bearer token for the discovery API. Optional: without the
# Secret the API serves unauthenticated (with a loud warning).
@@ -76,6 +78,33 @@ spec:
name: discovery-token
key: token
optional: true
# --- OpenTelemetry tracing ---
# Tracing is OFF until OTEL_EXPORTER_OTLP_ENDPOINT (or
# OTEL_TRACES_EXPORTER) is set; without it the operator runs
# exactly as before. The downward-API vars must stay listed
# before OTEL_RESOURCE_ATTRIBUTES: $(VAR) expansion only sees
# earlier-listed vars.
- name: POD_NAME
valueFrom:
fieldRef:
fieldPath: metadata.name
- name: POD_NAMESPACE
valueFrom:
fieldRef:
fieldPath: metadata.namespace
- name: OTEL_SERVICE_NAME
value: egress-proxies-operator
- name: OTEL_RESOURCE_ATTRIBUTES
value: k8s.pod.name=$(POD_NAME),k8s.namespace.name=$(POD_NAMESPACE)
# Point at an OTLP receiver (Tempo, Jaeger, otel-collector) to
# enable tracing; http/protobuf (4318) is the default protocol,
# set OTEL_EXPORTER_OTLP_PROTOCOL=grpc for 4317.
# - name: OTEL_EXPORTER_OTLP_ENDPOINT
# value: http://jaeger-collector.observability:4318
# - name: OTEL_TRACES_SAMPLER
# value: parentbased_traceidratio
# - name: OTEL_TRACES_SAMPLER_ARG
# value: "0.1"
image: controller:latest
name: manager
ports:

View File

@@ -300,6 +300,45 @@ Each consuming package defines its own small recorder interface
`metrics.Metrics` satisfies all of them structurally, so no package other
than `cmd/main.go` imports the metrics package.
### 10. Tracing (`internal/tracing/`)
OpenTelemetry tracing, integrated with — not replacing — the logr/zap
logging. Everything hangs off standard `OTEL_*` env vars: with no
`OTEL_EXPORTER_OTLP_ENDPOINT` / `OTEL_TRACES_EXPORTER` set, no SDK is
installed, spans are no-ops, and logs are byte-for-byte what they were.
`OTEL_SDK_DISABLED=true` and `OTEL_TRACES_EXPORTER=none` force it off;
`console` prints spans to stdout for local dev; sampling follows
`OTEL_TRACES_SAMPLER(_ARG)`.
Span topology — each unit of work is a new trace (watch events carry no
incoming trace context):
- `Reconcile Proxy` (root, via a `reconcile.Reconciler` decorator) →
`reconcile.managed` / `reconcile.replaceInstance` / `reconcile.delete`,
plus `status.patch` from the deferred flush. `reconcileExternal` does no
I/O and is unspanned.
- `provider.create|get|delete|list` (client spans, `provider.WithTracing`
decorator wired outermost around `WithMetrics`); the GCP SDK's own
otelhttp transport contributes HTTP child spans automatically.
- Kubernetes API calls become client child spans via a wrapped
`rest.Config` transport — gated on an existing parent span, so informer
list/watch long-polls and leader-election renewals never create root
spans. The kube-apiserver ignores incoming `traceparent` by design;
these spans are leaves.
- Discovery API: one server span per request (named from the route
pattern), incoming W3C `traceparent` honored so clients' traces continue
into the operator; `/healthz` excluded.
- `gc.sweep` per GC pass; `health.probe` per probe **only** with
`--trace-health-probes` (default off — probes run ~1/s per proxy).
Log correlation: logr sinks never see a context, so trace IDs ride on the
logger — `tracing.Start` re-derives the ctx logger from a captured base
with `traceID`/`spanID` values (lowerCamel, matching `reconcileID`;
Grafana/Loki derived-field regexes must match `traceID`, not `trace_id`).
Re-deriving from the base rather than layering keeps zap from emitting
duplicate keys on nested spans. GCP V(5) wire logs get the same keys from
the slog handler's ctx.
## Decisions
Judgment calls the spec left open, and deliberate deviations — recorded so
@@ -404,3 +443,24 @@ they read as choices, not accidents. Chronological by build step.
logger to everything running under the manager — fighting that would
mean two logging systems in one process. Noted as a deviation rather
than silently ignored.
- **Tracing is env-gated, not flag-gated:** it activates only when
`OTEL_EXPORTER_OTLP_ENDPOINT` / `OTEL_TRACES_EXPORTER` is set (user
decision). No collector configured means no SDK installed, no-op spans,
unchanged logs — the safe default for every existing deployment. The
one flag is `--trace-health-probes`, off by default, because probes at
~1/s per proxy would dominate trace volume.
- **Trace keys are `traceID`/`spanID`**, lowerCamel like `reconcileID` and
`providerID`, deliberately not the `trace_id` many Grafana derived-field
examples assume — configure the derived-field regex accordingly. logr
sinks can't read ctx, so the IDs ride on the ctx logger, re-derived from
a captured base per span so zap never emits duplicate keys.
- **Probe→reconcile trace links are not attempted:** the health engine's
`GenericEvent` carries only namespace/name (no ctx), and the workqueue
coalesces events, so any link would be a guess. A health-triggered
reconcile starts a fresh trace; the probe that caused it is findable via
its own (opt-in) span and shared proxy attributes.
- **No `traceparent` toward probe targets:** probe transports stay
uninstrumented so trace headers can never leak through a proxy to
external sites. The kube-apiserver ignores incoming `traceparent` by
design (public endpoint), so k8s client spans are leaves — in-process
traces, not cross-process ones.

View File

@@ -10,7 +10,7 @@ Plan: `docs/plans/2026-08-24-1025-otel-tracing.md`
- [x] Step 6 — Discovery server
- [x] Step 7 — GC + health
- [x] Step 8 — GCP wire-log enrichment
- [ ] Step 9 — Manifests + docs
- [x] Step 9 — Manifests + docs
## Step 1 — Dependencies
@@ -155,3 +155,29 @@ before the elision pass (both values are well under the 1KiB threshold).
The new `wirelog_test.go` covers the enrichment, that the JWT/token security
filter still drops non-wire Debug records even with a span present, and that
spanless records stay unchanged.
## Step 9 — Manifests, docs, lint
`config/manager/manager.yaml` gained the OTel env block (downward-API
`POD_NAME`/`POD_NAMESPACE` deliberately listed *before*
`OTEL_RESOURCE_ATTRIBUTES``$(VAR)` expansion only sees earlier vars),
with the OTLP endpoint and sampler left as commented examples so deployments
stay tracing-off by default. `docs/architecture.md` got "### 10. Tracing"
plus five Decisions entries; README got a Tracing section.
`make lint` surfaced 16 new-on-branch issues (checked with
`golangci-lint run --new-from-rev main`; the other ~43 pre-date this work):
- one gofmt slip, 13 goconst repeats — fixed with `env*`/`exporter*`
constants in `internal/tracing/tracing.go` shared by the tests;
- two logcheck hits ("function takes both a context and a logger"):
`Setup` now reads its logger from ctx (`logf.FromContext`, caller seeds
via `logf.IntoContext` — the linter's sanctioned pattern), and
`ContextWithLogger` carries a justified `//nolint:logcheck`, since being
the IntoContext-analogue is its purpose.
Worth noting: pre-existing lint debt (50 goconst etc. on `main`) was left
untouched — `make lint` still fails overall; only branch-introduced issues
were cleaned. The lint fixes changed `Setup`'s signature after Step 2/4 had
landed, so those commits show the two-arg version; this commit is where it
settles.

View File

@@ -50,17 +50,25 @@ func (i *instrumented) record(op string, err error) {
i.rec.ProviderRequest(i.name, op, resultLabel(err))
}
const (
resultOK = "ok"
resultNotFound = "not_found"
resultQuotaExceeded = "quota_exceeded"
resultPermanent = "permanent"
resultTransient = "transient"
)
func resultLabel(err error) string {
switch Class(err) {
case nil:
return "ok"
return resultOK
case ErrNotFound:
return "not_found"
return resultNotFound
case ErrQuotaExceeded:
return "quota_exceeded"
return resultQuotaExceeded
case ErrPermanent:
return "permanent"
return resultPermanent
default:
return "transient"
return resultTransient
}
}

View File

@@ -33,7 +33,7 @@ func TestWithTracing_spanPerCall(t *testing.T) {
call: func(p Provider) error { _, err := p.Create(context.Background(), CreateRequest{}); return err },
wantSpan: "provider.create",
wantStatus: codes.Ok,
wantResult: "ok",
wantResult: resultOK,
wantAttr: attribute.String("provider.id", "id-1"),
},
{
@@ -42,7 +42,7 @@ func TestWithTracing_spanPerCall(t *testing.T) {
call: func(p Provider) error { _, err := p.Get(context.Background(), "id-1"); return err },
wantSpan: "provider.get",
wantStatus: codes.Ok,
wantResult: "not_found",
wantResult: resultNotFound,
wantAttr: attribute.String("provider.id", "id-1"),
},
{
@@ -51,7 +51,7 @@ func TestWithTracing_spanPerCall(t *testing.T) {
call: func(p Provider) error { return p.Delete(context.Background(), "id-1") },
wantSpan: "provider.delete",
wantStatus: codes.Error,
wantResult: "transient",
wantResult: resultTransient,
wantAttr: attribute.String("provider.id", "id-1"),
},
{
@@ -60,7 +60,7 @@ func TestWithTracing_spanPerCall(t *testing.T) {
call: func(p Provider) error { _, err := p.Create(context.Background(), CreateRequest{}); return err },
wantSpan: "provider.create",
wantStatus: codes.Error,
wantResult: "quota_exceeded",
wantResult: resultQuotaExceeded,
wantAttr: attribute.String("provider.name", "x"),
},
{
@@ -69,7 +69,7 @@ func TestWithTracing_spanPerCall(t *testing.T) {
call: func(p Provider) error { _, err := p.ListByTag(context.Background()); return err },
wantSpan: "provider.list",
wantStatus: codes.Ok,
wantResult: "ok",
wantResult: resultOK,
wantAttr: attribute.Int("provider.instances", 0),
},
}

View File

@@ -47,6 +47,8 @@ func StartSpan(ctx context.Context, tracer trace.Tracer, name string, opts ...tr
// base for span enrichment. If ctx already carries a valid span (the HTTP
// middleware calls this inside the otelhttp handler), the logger is
// enriched immediately.
//
//nolint:logcheck // IntoContext-analogue: taking ctx and the logger to seed it with is the point.
func ContextWithLogger(ctx context.Context, base logr.Logger) context.Context {
ctx = context.WithValue(ctx, baseLoggerKey{}, base)
if sc := trace.SpanContextFromContext(ctx); sc.IsValid() {

View File

@@ -10,7 +10,6 @@ import (
"os"
"strings"
"github.com/go-logr/logr"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc"
"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp"
@@ -19,6 +18,20 @@ import (
"go.opentelemetry.io/otel/sdk/resource"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
semconv "go.opentelemetry.io/otel/semconv/v1.43.0"
logf "sigs.k8s.io/controller-runtime/pkg/log"
)
const (
envSDKDisabled = "OTEL_SDK_DISABLED"
envTracesExporter = "OTEL_TRACES_EXPORTER"
envOTLPEndpoint = "OTEL_EXPORTER_OTLP_ENDPOINT"
envOTLPTracesEndpoint = "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT"
envOTLPProtocol = "OTEL_EXPORTER_OTLP_PROTOCOL"
envOTLPTracesProtocol = "OTEL_EXPORTER_OTLP_TRACES_PROTOCOL"
exporterOTLP = "otlp"
exporterConsole = "console"
exporterNone = "none"
)
// maxQueueSize bounds the batch processor's span buffer; the default 2048
@@ -35,15 +48,19 @@ const maxQueueSize = 512
// OTEL_TRACES_EXPORTER=none force it off (the Go SDK does not implement
// OTEL_SDK_DISABLED itself). Sampling, endpoints, TLS, and headers follow
// the standard SDK/exporter env vars (OTEL_TRACES_SAMPLER, OTEL_EXPORTER_OTLP_*).
func Setup(ctx context.Context, log logr.Logger, service, version string) (func(context.Context) error, error) {
//
// Setup logs via the logger in ctx (logf.FromContext) rather than a
// parameter — the caller seeds it with logf.IntoContext.
func Setup(ctx context.Context, service, version string) (func(context.Context) error, error) {
log := logf.FromContext(ctx)
noop := func(context.Context) error { return nil }
exporterEnv := strings.ToLower(strings.TrimSpace(os.Getenv("OTEL_TRACES_EXPORTER")))
endpointSet := os.Getenv("OTEL_EXPORTER_OTLP_ENDPOINT") != "" ||
os.Getenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT") != ""
sdkDisabled := strings.EqualFold(strings.TrimSpace(os.Getenv("OTEL_SDK_DISABLED")), "true")
exporterEnv := strings.ToLower(strings.TrimSpace(os.Getenv(envTracesExporter)))
endpointSet := os.Getenv(envOTLPEndpoint) != "" ||
os.Getenv(envOTLPTracesEndpoint) != ""
sdkDisabled := strings.EqualFold(strings.TrimSpace(os.Getenv(envSDKDisabled)), "true")
if sdkDisabled || exporterEnv == "none" || (exporterEnv == "" && !endpointSet) {
if sdkDisabled || exporterEnv == exporterNone || (exporterEnv == "" && !endpointSet) {
log.Info("tracing disabled",
"reason", disabledReason(sdkDisabled, exporterEnv))
return noop, nil
@@ -87,7 +104,7 @@ func disabledReason(sdkDisabled bool, exporterEnv string) string {
switch {
case sdkDisabled:
return "OTEL_SDK_DISABLED=true"
case exporterEnv == "none":
case exporterEnv == exporterNone:
return "OTEL_TRACES_EXPORTER=none"
default:
return "no OTEL_TRACES_EXPORTER or OTLP endpoint configured"
@@ -99,10 +116,10 @@ func disabledReason(sdkDisabled bool, exporterEnv string) string {
// metric/log/prometheus exporters into a trace-only binary.
func newExporter(ctx context.Context, kind string) (sdktrace.SpanExporter, string, error) {
switch kind {
case "", "otlp":
proto := strings.ToLower(strings.TrimSpace(os.Getenv("OTEL_EXPORTER_OTLP_TRACES_PROTOCOL")))
case "", exporterOTLP:
proto := strings.ToLower(strings.TrimSpace(os.Getenv(envOTLPTracesProtocol)))
if proto == "" {
proto = strings.ToLower(strings.TrimSpace(os.Getenv("OTEL_EXPORTER_OTLP_PROTOCOL")))
proto = strings.ToLower(strings.TrimSpace(os.Getenv(envOTLPProtocol)))
}
switch proto {
case "", "http/protobuf":
@@ -114,9 +131,9 @@ func newExporter(ctx context.Context, kind string) (sdktrace.SpanExporter, strin
default:
return nil, "", fmt.Errorf("unsupported OTLP protocol %q (supported: http/protobuf, grpc)", proto)
}
case "console":
case exporterConsole:
exp, err := stdouttrace.New()
return exp, "console", err
return exp, exporterConsole, err
default:
return nil, "", fmt.Errorf("unsupported OTEL_TRACES_EXPORTER %q (supported: otlp, console, none)", kind)
}

View File

@@ -5,24 +5,25 @@ import (
"testing"
"time"
"github.com/go-logr/logr"
"go.opentelemetry.io/otel"
sdktrace "go.opentelemetry.io/otel/sdk/trace"
"go.opentelemetry.io/otel/trace/noop"
)
const testOTLPEndpoint = "http://localhost:4318"
// clearOTelEnv pins every env var Setup reads, so developer shells with
// OTEL_* set can't change test outcomes. Not parallel-safe by design
// (t.Setenv forbids t.Parallel).
func clearOTelEnv(t *testing.T) {
t.Helper()
for _, k := range []string{
"OTEL_SDK_DISABLED",
"OTEL_TRACES_EXPORTER",
"OTEL_EXPORTER_OTLP_ENDPOINT",
"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT",
"OTEL_EXPORTER_OTLP_PROTOCOL",
"OTEL_EXPORTER_OTLP_TRACES_PROTOCOL",
envSDKDisabled,
envTracesExporter,
envOTLPEndpoint,
envOTLPTracesEndpoint,
envOTLPProtocol,
envOTLPTracesProtocol,
} {
t.Setenv(k, "")
}
@@ -38,50 +39,50 @@ func TestSetup_envGating(t *testing.T) {
{name: "no env means disabled", env: nil, wantEnabled: false},
{
name: "endpoint enables otlp",
env: map[string]string{"OTEL_EXPORTER_OTLP_ENDPOINT": "http://localhost:4318"},
env: map[string]string{envOTLPEndpoint: testOTLPEndpoint},
wantEnabled: true,
},
{
name: "traces endpoint enables otlp",
env: map[string]string{"OTEL_EXPORTER_OTLP_TRACES_ENDPOINT": "http://localhost:4318"},
env: map[string]string{envOTLPTracesEndpoint: testOTLPEndpoint},
wantEnabled: true,
},
{
name: "explicit console exporter",
env: map[string]string{"OTEL_TRACES_EXPORTER": "console"},
env: map[string]string{envTracesExporter: exporterConsole},
wantEnabled: true,
},
{
name: "grpc protocol",
env: map[string]string{"OTEL_TRACES_EXPORTER": "otlp", "OTEL_EXPORTER_OTLP_PROTOCOL": "grpc"},
env: map[string]string{envTracesExporter: exporterOTLP, envOTLPProtocol: "grpc"},
wantEnabled: true,
},
{
name: "exporter none wins over endpoint",
env: map[string]string{
"OTEL_TRACES_EXPORTER": "none",
"OTEL_EXPORTER_OTLP_ENDPOINT": "http://localhost:4318",
envTracesExporter: exporterNone,
envOTLPEndpoint: testOTLPEndpoint,
},
wantEnabled: false,
},
{
name: "OTEL_SDK_DISABLED wins over everything",
env: map[string]string{
"OTEL_SDK_DISABLED": "true",
"OTEL_EXPORTER_OTLP_ENDPOINT": "http://localhost:4318",
envSDKDisabled: "true",
envOTLPEndpoint: testOTLPEndpoint,
},
wantEnabled: false,
},
{
name: "unsupported exporter errors",
env: map[string]string{"OTEL_TRACES_EXPORTER": "jaeger"},
env: map[string]string{envTracesExporter: "jaeger"},
wantErr: true,
},
{
name: "unsupported protocol errors",
env: map[string]string{
"OTEL_TRACES_EXPORTER": "otlp",
"OTEL_EXPORTER_OTLP_PROTOCOL": "http/json",
envTracesExporter: exporterOTLP,
envOTLPProtocol: "http/json",
},
wantErr: true,
},
@@ -100,7 +101,7 @@ func TestSetup_envGating(t *testing.T) {
before := noop.NewTracerProvider()
otel.SetTracerProvider(before)
shutdown, err := Setup(context.Background(), logr.Discard(), "test-svc", "abc123")
shutdown, err := Setup(context.Background(), "test-svc", "abc123")
if tc.wantErr {
if err == nil {
t.Fatal("want error, got nil")