test(otel-e2e): fix Thufir-flagged correctness issues in E2E harness

Three correctness fixes (all IMPORTANT from Thufir's review) + one MINOR: 1. Per-run isolated collector output dir (mktemp -d): stale data from a prior run can no longer false-green the OTLP trace assertions. The dir is passed into compose via BUZZ_OTEL_OUTPUT_DIR env var (replaces the hard-coded /tmp/buzz-otel-e2e-output bind-mount). Cleaned on success; preserved on failure for inspection. 2. Delete test_otlp_metrics_tagged_service_name_buzz_relay: descoped per Tyler/Max decision — relay drops OTLP metrics export entirely; metrics go out the Prometheus :9102 scrape only; OTLP carries traces only. 3. Remove OTEL_SERVICE_NAME from enabled relay start so the harness exercises the relay's Finding-A fallback exactly as staging does. Replace blob.contains("buzz-relay") substring check with a structural has_resource_attr_in_spans() assertion that walks ResourceSpans[]. resource.attributes[]{key, value.stringValue} — a span/scope name containing "buzz-relay" can no longer satisfy this check. 4. README: note Rust tests are harness internals, not standalone; update manual-run example to match new env var shape. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
2026-11-01 23:11:58 +00:00 · 2026-06-30 14:14:45 -04:00
parent fd59e6660f
commit deee33ba60
4 changed files with 113 additions and 77 deletions
--- a/test/otel-e2e/README.md
+++ b/test/otel-e2e/README.md
@@ -8,10 +8,11 @@ Local end-to-end validation for the Buzz relay's observability export surfaces
 | Assertion | What's checked |
 |-----------|---------------|
 | **(a) Prometheus scrape** | `GET :9102/metrics` contains `buzz_ws_connections_total`, `buzz_events_received_total`, `buzz_auth_attempts_total` with **non-zero values**; does **not** contain `target_info` (suppressed via `.without_target_info()`) |
-| **(b) OTLP traces** | The otel-collector received spans named `ws.auth` and `ws.event` carrying a `conn_id` attribute, tagged `service.name=buzz-relay` |
-| **(c) OTLP metrics** | The otel-collector received OTLP metric data tagged `service.name=buzz-relay` |
+| **(b) OTLP traces** | The otel-collector received spans named `ws.auth` and `ws.event` carrying a `conn_id` attribute; `service.name=buzz-relay` verified **structurally** via `resource.attributes` key/value (not substring) |
 | **(d) OTLP-disabled control** | With `OTEL_EXPORTER_OTLP_ENDPOINT` unset the relay still serves `/metrics` correctly; the collector receives **nothing** |

+OTLP metrics export is out of scope — the relay emits metrics via the Prometheus `:9102` scrape only; OTLP carries traces only.
+
 ## Prerequisites

 - Docker (for compose services: postgres, redis, minio, otel-collector)
@@ -65,7 +66,7 @@ RELAY_BINARY=/path/to/buzz/.worktrees/duncan-otel-migration/target/ci/buzz-relay
          ▼                           ▼
 ┌─────── DOCKER (buzz-net) ──────────────────────────────────────┐
 │                                                                 │
-│  otel-collector :4317  ←── relay pushes traces + metrics here  │
+│  otel-collector :4317  ←── relay pushes traces here            │
 │    exports to: debug stdout + /tmp/otelcol-output/             │
 │                                                                 │
 │  postgres :5432  redis :6379  minio :9000                      │
@@ -96,24 +97,36 @@ All four tests are `#[ignore]` by default and selected by `run.sh`.

 ## Running individual tests manually

+> **Note:** The Rust tests in `crates/buzz-test-client/tests/e2e_otel.rs` are
+> **harness internals**.  They depend on the two-relay sequence orchestrated by
+> `run.sh` (OTLP-enabled run first, then OTLP-disabled run), and the OTLP trace
+> assertions read collector output written during the first relay's run.  Running
+> the tests in isolation will fail or produce misleading results unless you
+> manually replicate the same sequence.  Use `just otel-e2e` as the canonical
+> entry point.
+
+For development/debugging, you can reproduce the harness sequence manually:
+
 ```bash
-# Start the stack first:
+# Start the stack first (per-run output dir is required):
+export BUZZ_OTEL_OUTPUT_DIR=$(mktemp -d)
 docker compose -f docker-compose.yml -f test/otel-e2e/compose.otel-e2e.yml \
  up -d postgres redis minio minio-init otel-collector

-# Start relay with OTLP enabled:
+# Start relay with OTLP enabled (no OTEL_SERVICE_NAME — relay defaults it):
 OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4317 \
-OTEL_SERVICE_NAME=buzz-relay \
 DATABASE_URL=postgres://buzz:buzz_dev@localhost:5432/buzz \
 REDIS_URL=redis://localhost:6379 \
 RELAY_URL=ws://localhost:3000 \
 BUZZ_REQUIRE_AUTH_TOKEN=false \
  ./target/ci/buzz-relay &

-# Run all four tests:
+# Run the Prometheus + OTLP trace assertions:
 RELAY_URL=ws://localhost:3000 \
 METRICS_URL=http://localhost:9102/metrics \
-  cargo test -p buzz-test-client --test e2e_otel -- --ignored
+OTEL_COLLECTOR_OUTPUT=${BUZZ_OTEL_OUTPUT_DIR}/telemetry.json \
+  cargo test -p buzz-test-client --test e2e_otel \
+    test_prometheus_contains_buzz_metrics test_otlp_traces -- --ignored

 # Teardown:
 docker compose -f docker-compose.yml -f test/otel-e2e/compose.otel-e2e.yml down -v
--- a/test/otel-e2e/compose.otel-e2e.yml
+++ b/test/otel-e2e/compose.otel-e2e.yml
@@ -17,6 +17,10 @@
 # The relay is pointed at the collector via:
 #   OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4317
 # (the collector's gRPC port is published to the host).
+#
+# BUZZ_OTEL_OUTPUT_DIR must be set to a per-run directory created by run.sh
+# (via mktemp -d) before docker compose up.  This prevents stale data from a
+# prior run from satisfying the OTLP trace assertions.
 name: buzz

 services:
@@ -26,9 +30,10 @@ services:
    command: ["--config=/etc/otelcol-config.yml"]
    volumes:
      - ./test/otel-e2e/otelcol-config.yml:/etc/otelcol-config.yml:ro
-      # Bind-mount to the host so assertions can read the file directly —
-      # the collector image is distroless (no shell/cat available via docker exec).
-      - /tmp/buzz-otel-e2e-output:/tmp/otelcol-output
+      # Per-run bind-mount set by run.sh (BUZZ_OTEL_OUTPUT_DIR=$(mktemp -d)).
+      # The collector image is distroless — no shell available via docker exec,
+      # so assertions read from the host path directly.
+      - ${BUZZ_OTEL_OUTPUT_DIR}:/tmp/otelcol-output
    ports:
      # OTLP gRPC — relay (on host) dials this
      - "4317:4317"
--- a/test/otel-e2e/run.sh
+++ b/test/otel-e2e/run.sh
@@ -8,11 +8,14 @@
 #   (a) Prometheus /metrics contains expected buzz_* series with non-zero
 #       values and does NOT contain a target_info series.
 #   (b) OTLP traces: the collector received ws.auth and ws.event spans
-#       carrying a conn_id attribute tagged service.name=buzz-relay.
-#   (c) OTLP metrics: the collector received data tagged service.name=buzz-relay.
+#       carrying a conn_id attribute; service.name=buzz-relay verified
+#       structurally via resource.attributes (not substring).
 #   (d) OTLP-disabled control: with OTEL_EXPORTER_OTLP_ENDPOINT unset the
 #       relay still serves /metrics; the collector receives nothing.
 #
+# Note: OTLP metrics export is out of scope — relay emits metrics via the
+# Prometheus :9102 scrape only; OTLP carries traces only.
+#
 # Usage:
 #   just otel-e2e        # canonical one-command entry point
 #   ./test/otel-e2e/run.sh [--skip-build]
@@ -72,6 +75,7 @@ cleanup() {
  fi
  # Bring down compose stack (including collector) and remove volumes.
  cd "${REPO_ROOT}"
+  BUZZ_OTEL_OUTPUT_DIR="${BUZZ_OTEL_OUTPUT_DIR:-/tmp/buzz-otel-e2e-missing}" \
  docker compose -f docker-compose.yml -f test/otel-e2e/compose.otel-e2e.yml \
    down -v --remove-orphans 2>/dev/null || true
  ok "Teardown complete"
@@ -81,6 +85,14 @@ trap cleanup EXIT
 # ── Step 1: Start backing services + otel-collector ──────────────────────────
 cd "${REPO_ROOT}"

+# Create a per-run isolated collector output dir so stale data from a prior run
+# can never satisfy the OTLP trace assertions.  Cleaned on success; preserved on
+# failure so the output is inspectable.
+BUZZ_OTEL_OUTPUT_DIR=$(mktemp -d)
+export BUZZ_OTEL_OUTPUT_DIR
+log "Collector output dir: ${BUZZ_OTEL_OUTPUT_DIR}"
+COLLECTOR_OUTPUT_HOST="${BUZZ_OTEL_OUTPUT_DIR}/telemetry.json"
+
 log "Starting backing services and otel-collector..."
 docker compose -f docker-compose.yml -f test/otel-e2e/compose.otel-e2e.yml \
  up -d postgres redis minio minio-init otel-collector
@@ -161,12 +173,10 @@ else
 fi

 # ── Step 4: Set up collector output readback ─────────────────────────────────
-# The file exporter writes to /tmp/otelcol-output inside the collector container,
-# which is bind-mounted to /tmp/buzz-otel-e2e-output on the host (see compose overlay).
-# The collector image is distroless so we read directly from the host path.
-COLLECTOR_OUTPUT_DIR=/tmp/buzz-otel-e2e-output
-mkdir -p "${COLLECTOR_OUTPUT_DIR}"
-COLLECTOR_OUTPUT_HOST="${COLLECTOR_OUTPUT_DIR}/telemetry.json"
+# COLLECTOR_OUTPUT_HOST is already set to ${BUZZ_OTEL_OUTPUT_DIR}/telemetry.json
+# from Step 1 (per-run mktemp -d).  The file exporter writes
+# /tmp/otelcol-output/telemetry.json inside the container, which maps to that
+# host path via the compose bind-mount.

 # ── Step 5: Start relay WITH OTLP enabled ────────────────────────────────────
 log "Starting relay (OTLP enabled → http://localhost:4317)..."
@@ -180,7 +190,6 @@ nohup env \
  BUZZ_REQUIRE_AUTH_TOKEN=false \
  BUZZ_RECONCILE_CHANNELS=true \
  OTEL_EXPORTER_OTLP_ENDPOINT="http://localhost:4317" \
-  OTEL_SERVICE_NAME="buzz-relay" \
  RUST_LOG="buzz_relay=info" \
  "${RELAY_BIN}" > "${RELAY_LOG}" 2>&1 &
 RELAY_PID=$!
@@ -206,14 +215,14 @@ for attempt in $(seq 1 60); do
  fi
 done

-# ── Step 6: Run OTLP-enabled tests (a), (b), (c) ────────────────────────────
+# ── Step 6: Run OTLP-enabled tests (a), (b) ─────────────────────────────────
 log "Waiting for OTLP exports to flush (batch processor 1s timeout + buffer)..."
 sleep 5  # Give the relay's batch exporter time to flush after readiness

 COLLECTOR_LINE_COUNT=$(wc -l < "${COLLECTOR_OUTPUT_HOST}" 2>/dev/null || echo 0)
-log "Collector has ${COLLECTOR_LINE_COUNT} lines so far (may be low — OTLP metrics are periodic)"
+log "Collector has ${COLLECTOR_LINE_COUNT} lines so far"

-log "Running OTLP-enabled tests (a)(b)(c)..."
+log "Running OTLP-enabled tests (a)(b)..."
 RELAY_URL="ws://localhost:3000" \
 METRICS_URL="http://localhost:9102/metrics" \
 OTEL_COLLECTOR_OUTPUT="${COLLECTOR_OUTPUT_HOST}" \
@@ -229,7 +238,7 @@ RELAY_URL="ws://localhost:3000" \
 METRICS_URL="http://localhost:9102/metrics" \
 OTEL_COLLECTOR_OUTPUT="${COLLECTOR_OUTPUT_HOST}" \
  cargo test -p buzz-test-client --test e2e_otel \
-    test_otlp \
+    test_otlp_traces \
    -- --ignored 2>&1
 ok "OTLP-enabled assertions passed"

@@ -327,6 +336,9 @@ if [[ -n "${DISABLED_OUTPUT}" ]]; then
 fi
 ok "OTLP-disabled: collector received nothing ✓"

+# Clean up the per-run collector output dir on success.
+rm -rf "${BUZZ_OTEL_OUTPUT_DIR}"
+
 # ── Done ──────────────────────────────────────────────────────────────────────
 echo ""
 echo -e "${GREEN}╔══════════════════════════════════════════════════════════╗${NC}"
@@ -334,7 +346,7 @@ echo -e "${GREEN}║  OTEL E2E HARNESS — ALL ASSERTIONS PASSED
 echo -e "${GREEN}╠══════════════════════════════════════════════════════════╣${NC}"
 echo -e "${GREEN}║  (a) Prometheus buzz_* series present, non-zero, no      ║${NC}"
 echo -e "${GREEN}║      target_info                                          ║${NC}"
-echo -e "${GREEN}║  (b) OTLP traces: ws.auth + ws.event with conn_id        ║${NC}"
-echo -e "${GREEN}║  (c) OTLP metrics: service.name=buzz-relay present       ║${NC}"
+echo -e "${GREEN}║  (b) OTLP traces: ws.auth + ws.event with conn_id,       ║${NC}"
+echo -e "${GREEN}║      service.name=buzz-relay verified structurally        ║${NC}"
 echo -e "${GREEN}║  (d) OTLP-disabled: /metrics works, collector silent     ║${NC}"
 echo -e "${GREEN}╚══════════════════════════════════════════════════════════╝${NC}"