From c7f22645eaf622323a9a72d1ca75a0f2cd6c9c89 Mon Sep 17 00:00:00 2001 From: Jeff Larson Date: Tue, 28 Jul 2026 20:45:44 -0700 Subject: [PATCH] chore: remove Linear ticket references so the repo is self-contained Strips ~150 JEF-#### comment/doc tags across server, ui, docs, and CI scripts so the codebase only references in-repo material. Where a ticket documented a decision captured in an ADR, the reference is replaced with the ADR number (e.g. JEF-580 -> ADR 0021, JEF-473/493 -> ADR 0013/0019); bare trailing tags with no further in-repo context are just deleted. server/migrations/*.sql is deliberately untouched -- those files are checksum-locked by sqlx::migrate! and editing even a comment byte would crashloop the pod on boot. No behavior change: only comments, docstrings, and doc files were edited. Runtime string literals (e.g. test table names like jef_590_lock_test) and the online-DDL advisory-lock constant (0x004A_4546_5F35_3830) are untouched byte-for-byte -- only its comment was reworded to drop the ticket reference. Tests: existing server (cargo test --locked, 78+67 passed) and UI (vitest, 33 passed) suites cover all touched files; no new tests needed since this is a pure prose/comment cleanup. Verified cargo fmt/check/clippy --all-targets -D warnings and npm run lint/build all green. Co-authored-by: Claude Sonnet 5 --- .github/workflows/ci.yml | 12 ++-- CLAUDE.md | 2 +- docs/adr/0007-retention-by-deletion.md | 2 +- docs/adr/0013-auth-at-the-edge.md | 4 +- ...0014-self-monitoring-in-process-metrics.md | 2 +- docs/adr/0016-self-log-instrumentation.md | 4 +- ...7-self-trace-instrumentation-in-process.md | 4 +- .../0018-read-only-mcp-server-in-process.md | 9 ++- .../0019-mcp-auth-cloudflare-access-oidc.md | 12 ++-- ...020-on-ingest-per-series-metric-rollups.md | 6 +- docs/adr/0021-online-ddl-lane.md | 4 +- docs/architecture.md | 2 +- scripts/lint-migrations.sh | 2 +- scripts/start-sccache-docker.sh | 6 +- server/Dockerfile | 4 +- server/migrations/README.md | 4 +- server/src/access_jwt.rs | 9 ++- server/src/alerts.rs | 23 ++++---- server/src/api.rs | 55 +++++++++---------- server/src/db.rs | 13 +++-- server/src/lib.rs | 16 +++--- server/src/main.rs | 20 +++---- server/src/mcp.rs | 10 ++-- server/src/mcp_auth.rs | 10 ++-- server/src/online_ddl.rs | 23 ++++---- server/src/otlp.rs | 20 +++---- server/src/retention.rs | 6 +- server/src/selflog.rs | 2 +- server/src/selfmon.rs | 16 +++--- server/src/selftrace.rs | 4 +- server/tests/smoke.rs | 52 +++++++++--------- ui/eslint.config.js | 2 +- ui/src/api.ts | 6 +- ui/src/components/Alerts.chartHref.test.ts | 2 +- ui/src/components/MetricChart.tsx | 6 +- ui/src/components/TraceList.tsx | 2 +- .../components/TraceWaterfall.logic.test.ts | 2 +- ui/src/empty.ts | 2 +- ui/src/links.test.ts | 2 +- ui/src/links.ts | 2 +- ui/src/metricLabels.test.ts | 2 +- ui/src/metricLabels.ts | 2 +- ui/src/routes.a11y.test.tsx | 14 ++--- ui/src/styles.css | 10 ++-- 44 files changed, 205 insertions(+), 207 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d21a967..64f87c3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,7 +10,7 @@ permissions: jobs: migrations-lint: - # Guardrail (JEF-592): migration 0017 shipped a plain (non-CONCURRENTLY) + # Guardrail: migration 0017 shipped a plain (non-CONCURRENTLY) # CREATE INDEX on a 10.4M-row hot table with nothing flagging it as risky. # This fails the build on that pattern (and a couple of other # table-locking-DDL shapes) in any newly-added migration; see @@ -37,7 +37,7 @@ jobs: # Secret via envFrom, so none of it belongs in this workflow and no credential is # committed here. # - # The redis backend is deliberately NOT set here any more (JEF-564): sccache selects + # The redis backend is deliberately NOT set here any more: sccache selects # ONE backend from its env, so setting both would make which one you get an # implementation detail of sccache's precedence -- the R2 cutover could read as done # while every build still went to redis. If the pod env is missing, sccache degrades @@ -98,10 +98,10 @@ jobs: cache: npm cache-dependency-path: ui/package-lock.json - run: npm ci - # Accessibility floor (JEF-431): eslint-plugin-jsx-a11y gates keyboard- + # Accessibility floor: eslint-plugin-jsx-a11y gates keyboard- # operable rows, labeled charts, and no silent-live tables from regressing. - run: npm run lint - # Runtime a11y route-smoke (JEF-442) runs inside `npm test`: each top-level + # Runtime a11y route-smoke runs inside `npm test`: each top-level # route is mounted (jsdom, API mocked) and scanned by axe-core, failing on # serious/critical structural violations. Complements the static lint above — # it catches ARIA that only resolves against the rendered tree. @@ -183,8 +183,8 @@ jobs: # cargo-chef the dep-compile layer cache-hits here until Cargo.lock moves, # so a cold node skips it. mode=max also caches intermediate (build-stage) # layers, which is what carries the cook layer — at the cost of a larger - # WAN transfer (see JEF-87 / measure in JEF-82). - # sccache's R2 backend for the in-image cargo build (JEF-584, ADR-0020). + # WAN transfer (measured in practice against the alternative). + # sccache's R2 backend for the in-image cargo build (ADR-0020). # `secret-envs` (key=envname) reads these straight out of the RUNNER POD's # env, where the `sccache-r2` Secret is injected via envFrom (cluster repo: # charts/actions/runners/values-watcher.yaml) — the repo has no Actions secret diff --git a/CLAUDE.md b/CLAUDE.md index cebde72..438d5e6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -67,7 +67,7 @@ digest and rolls it. ARM images are required for Raspberry Pi nodes (CI builds t Traces, logs, metrics (latest-value table **and** time-series charts), OTLP HTTP + gRPC, service map, retention (global default + optional per-table -spans/logs/metrics windows, JEF-434), metric downsampling/rollups, threshold +spans/logs/metrics windows), metric downsampling/rollups, threshold alerting (rules + events + email/webhook). Alert rules are **declarative**: a JSON config (rendered from the chart's `server.alerts` values, env `WATCHER_ALERTS_CONFIG`) is reconciled into the DB on startup and the diff --git a/docs/adr/0007-retention-by-deletion.md b/docs/adr/0007-retention-by-deletion.md index 4f237f6..bb03795 100644 --- a/docs/adr/0007-retention-by-deletion.md +++ b/docs/adr/0007-retention-by-deletion.md @@ -23,7 +23,7 @@ For v0, a background task prunes rows older than `WATCHER_RETENTION_DAYS` continuous aggregates, which slots in under the same schema ([0001](0001-postgres-only-no-clickhouse.md)). - Deletes are coarse (whole-row, by age); no per-service policy yet. Spans, logs, and metric rollups can each be given their own window via - `WATCHER_RETENTION_{SPANS,LOGS,METRICS}_DAYS` (JEF-434, an omitted override + `WATCHER_RETENTION_{SPANS,LOGS,METRICS}_DAYS` (an omitted override falls back to `WATCHER_RETENTION_DAYS`) — per-service is still out of scope, since a per-service delete over these tables would need `ctid`-batching like the raw-metrics prune to avoid the statement-timeout failure mode. diff --git a/docs/adr/0013-auth-at-the-edge.md b/docs/adr/0013-auth-at-the-edge.md index d3970fe..eecd618 100644 --- a/docs/adr/0013-auth-at-the-edge.md +++ b/docs/adr/0013-auth-at-the-edge.md @@ -2,7 +2,7 @@ - Status: Accepted - Date: 2026-05-30 -- Amended: 2026-07-21 (JEF-473 — add origin-side Access JWT verification) +- Amended: 2026-07-21 (add origin-side Access JWT verification) - Supersedes: [0008](0008-optional-bearer-auth.md) ## Context @@ -39,7 +39,7 @@ the chart's token Secret are all gone. - For environments without Cloudflare, equivalent edge auth (Traefik forwardAuth + an SSO proxy) is the substitute; the app deliberately holds no auth of its own. -## Amendment — origin-side Access JWT verification (JEF-473, 2026-07-21) +## Amendment — origin-side Access JWT verification (2026-07-21) Edge-only auth trusts that every request to `/api` and the UI arrived through Access. An Access-policy slip, a tunnel/ingress misconfig, or direct in-cluster access to the diff --git a/docs/adr/0014-self-monitoring-in-process-metrics.md b/docs/adr/0014-self-monitoring-in-process-metrics.md index 6786e21..1744de0 100644 --- a/docs/adr/0014-self-monitoring-in-process-metrics.md +++ b/docs/adr/0014-self-monitoring-in-process-metrics.md @@ -7,7 +7,7 @@ ## Context watcher had no visibility into its own health. A silently stalled retention sweep let -the `metrics` table grow to tens of GB un-paged (JEF-425). We want watcher's own +the `metrics` table grow to tens of GB un-paged. We want watcher's own operational signals (ingest throughput, drop counts, per-table on-disk bytes, retention recency, rollup lag, pool utilisation) to be visible and alertable — using the machinery watcher already has, on a Raspberry Pi, in a single binary. diff --git a/docs/adr/0016-self-log-instrumentation.md b/docs/adr/0016-self-log-instrumentation.md index 826aebf..cb651b0 100644 --- a/docs/adr/0016-self-log-instrumentation.md +++ b/docs/adr/0016-self-log-instrumentation.md @@ -7,12 +7,12 @@ ## Context watcher self-instruments its own **traces** (OTLP `SpanExporter` + the -`tracing-opentelemetry` layer) and its own **metrics** (JEF-425 / ADR 0014: +`tracing-opentelemetry` layer) and its own **metrics** (ADR 0014: `selfmon` hands ops gauges/counters straight to `otlp::store_metrics`, tagged `service.name=watcher`). But its own **logs** only went to stdout via the `fmt` layer — there was no `tracing`→logs bridge, so watcher's log lines never landed in its own `logs` table. You could open a watcher self-trace but couldn't jump to the -correlated self-logs (the span→logs drill, JEF-429). +correlated self-logs (the span→logs drill). Two shapes were possible, mirroring the ADR 0014 metrics decision: diff --git a/docs/adr/0017-self-trace-instrumentation-in-process.md b/docs/adr/0017-self-trace-instrumentation-in-process.md index cd5ad17..e543421 100644 --- a/docs/adr/0017-self-trace-instrumentation-in-process.md +++ b/docs/adr/0017-self-trace-instrumentation-in-process.md @@ -12,12 +12,12 @@ watcher self-instruments its own **metrics** (ADR 0014) and **logs** (ADR 0016) original network path: an `opentelemetry-otlp` batch `SpanExporter` POSTing OTLP over HTTP back to watcher's own `:4318/v1/traces`. -That path was **dead** (JEF-462). The batch processor had wedged into a shut-down +That path was **dead**. The batch processor had wedged into a shut-down state and logged "Spans are being emitted even after Shutdown ... Spans will not be exported" on every span. Evidence from prod: **0** watcher spans in the `spans` table ever, while self-metrics and self-logs (in-process) worked; and ~74.8k of the ~75.1k self-log rows were *that one warning* — the broken exporter flooding the `logs` table -via the JEF-452 self-log capture. Consequences: watcher never appeared in the Services +via the self-log capture. Consequences: watcher never appeared in the Services pulldown (`/api/services` reads `spans`), no self-traces to correlate, and ~75k junk rows. diff --git a/docs/adr/0018-read-only-mcp-server-in-process.md b/docs/adr/0018-read-only-mcp-server-in-process.md index 094b1af..455eacf 100644 --- a/docs/adr/0018-read-only-mcp-server-in-process.md +++ b/docs/adr/0018-read-only-mcp-server-in-process.md @@ -11,8 +11,7 @@ watcher already exposes its telemetry through a same-origin query API (`/api/... consumed by the embedded UI. LLM agents (Claude Code, MCP Inspector, …) increasingly speak the **Model Context Protocol** (MCP): given an MCP endpoint they can search traces, read logs/metrics, and inspect services/alerts as tools. We want watcher to -be that endpoint without standing up a second process or duplicating query logic -(JEF-471). +be that endpoint without standing up a second process or duplicating query logic. Two shapes were possible: @@ -50,8 +49,8 @@ mature. - **Opt-in, default OFF, unauthenticated for now.** `/mcp` mounts only when `WATCHER_MCP_ENABLED` is truthy. It is deliberately mounted *outside* the edge auth that fronts the UI/`/api` (Cloudflare Access, ADR 0013): an MCP client is not a - browser and carries no Access cookie. Its own auth is a **separate** ticket - (JEF-472); until that lands the endpoint must not be exposed, so it defaults off and + browser and carries no Access cookie. Its own auth is a **separate** effort + (see ADR 0019); until that lands the endpoint must not be exposed, so it defaults off and the flag's doc comment says so. The transport's default loopback-only Host allow-list (a DNS-rebinding guard aimed at locally-run servers reached by a browser) is disabled here, since watcher's MCP is a server-to-server endpoint reached through a public @@ -65,7 +64,7 @@ mature. - Query behavior stays identical across the HTTP and MCP surfaces because they share the `query_*` functions; a future change to a clamp or window applies to both. - The endpoint is inert until an operator sets `WATCHER_MCP_ENABLED` **and** (once - JEF-472 lands) configures its auth. Enabling it before then exposes read access to + ADR 0019's auth lands) configures its auth. Enabling it before then exposes read access to anyone who can reach the host — the flag default and the ADR make that ordering explicit, mirroring the "create the Access app first" runbook rule of ADR 0013. - `rmcp` (and, for tests, its client + `reqwest` 0.13) enters the dependency tree; the diff --git a/docs/adr/0019-mcp-auth-cloudflare-access-oidc.md b/docs/adr/0019-mcp-auth-cloudflare-access-oidc.md index 8d5a264..909c986 100644 --- a/docs/adr/0019-mcp-auth-cloudflare-access-oidc.md +++ b/docs/adr/0019-mcp-auth-cloudflare-access-oidc.md @@ -3,7 +3,7 @@ - Status: Accepted - Date: 2026-07-22 - Related: [0013](0013-auth-at-the-edge.md) (edge auth + origin verify), [0018](0018-read-only-mcp-server-in-process.md) (the read-only MCP server) -- Revises: the initial JEF-472 design recorded in this ADR (raw-`Bearer` JWT validation + self-served OAuth metadata), superseded by JEF-493. +- Revises: the initial design recorded in this ADR (raw-`Bearer` JWT validation + self-served OAuth metadata), superseded by the spike finding below. ## Context @@ -20,19 +20,19 @@ the client obtains a token from an authorization server and presents it; the res server validates it. The open question was *who is the authorization server* and *what does the origin actually receive*. -**Initial design (JEF-472, now revised).** The first cut had watcher itself act as the +**Initial design (now revised).** The first cut had watcher itself act as the OAuth-aware resource server: it validated the raw `Authorization: Bearer ` as a Cloudflare Access **OIDC** JWT and *self-served* the RFC 9728 protected-resource metadata (`/.well-known/oauth-protected-resource`) pointing clients at the Access OIDC authorization server. -**Spike finding (JEF-493).** The mechanism Cloudflare actually provides for this is +**Spike finding.** The mechanism Cloudflare actually provides for this is Access **Managed OAuth**: Cloudflare is the OAuth authorization server the client needs (including the dynamic client registration — DCR — that claude.ai's connector performs), it issues the client an **opaque** access token, resolves that token at its **edge**, and forwards the origin the standard **`Cf-Access-Jwt-Assertion`** JWT — the *same* -header, issuer, and team JWKS that `/api` already validates (JEF-473). Under this model -the JEF-472 design is wrong in two ways: the origin would receive an *opaque* token in +header, issuer, and team JWKS that `/api` already validates (ADR 0013). Under this model +the initial design above is wrong in two ways: the origin would receive an *opaque* token in `Authorization: Bearer` (not a JWT — it would fail JWT validation), and OAuth discovery/metadata is owned by Cloudflare, not the origin. @@ -43,7 +43,7 @@ discovery/metadata is owned by Cloudflare, not the origin. edge sets after resolving the client's opaque Managed-OAuth token — via the shared [`access_jwt::Verifier`](../../server/src/access_jwt.rs) (RS256 via the team's JWKS, `iss` = team domain, `aud`, and expiry). This is the **same** assertion model as - JEF-473's `/api` `access_guard`; the header-extraction + verify step is factored into + ADR 0013's `/api` `access_guard`; the header-extraction + verify step is factored into one shared `check_access_assertion` helper both guards call. The origin only ever **validates**, never mints (the [0013](0013-auth-at-the-edge.md) invariant), and never parses the opaque OAuth token. diff --git a/docs/adr/0020-on-ingest-per-series-metric-rollups.md b/docs/adr/0020-on-ingest-per-series-metric-rollups.md index cbfe9ab..138c1c3 100644 --- a/docs/adr/0020-on-ingest-per-series-metric-rollups.md +++ b/docs/adr/0020-on-ingest-per-series-metric-rollups.md @@ -29,8 +29,8 @@ writes the raw point — there is no background sweep and no `rollup.rs`. via `ON CONFLICT (name, series_key, bucket) DO UPDATE`, accumulating `count`/`sum`/`min`/`max`/`avg` (and, for histograms, `bucket_bounds` / `bucket_counts`, summed element-wise by the `array_sum` aggregate from - `array_add`). The whole batch write goes through `write_with_failover_retry` - (JEF-496), so a Patroni failover retries the statement on a fresh connection. + `array_add`). The whole batch write goes through `write_with_failover_retry`, + so a Patroni failover retries the statement on a fresh connection. - **Series identity is preserved, not collapsed.** `series_key` is `metric_series_key(service, attrs)` — `md5(coalesce(service,'') || '|' || attrs::text)` (`0009_metric_sql_helpers.sql`) — a stable hash of the service plus @@ -80,7 +80,7 @@ current, still-filling bucket. highest-volume table (see `retention.rs`'s batched delete, added after it once grew to tens of GB unpruned). - The extra aggregation work (`GROUP BY` + upsert) now happens inline on every - ingest batch rather than off-peak; batching (JEF-495) and the fixed lock order + ingest batch rather than off-peak; batching and the fixed lock order keep it from becoming an ingest bottleneck, but it does mean ingest latency and rollup-write cost are coupled. - Still the same trend-off as [0011](0011-metric-rollups.md): history beyond the diff --git a/docs/adr/0021-online-ddl-lane.md b/docs/adr/0021-online-ddl-lane.md index 91198b9..2652642 100644 --- a/docs/adr/0021-online-ddl-lane.md +++ b/docs/adr/0021-online-ddl-lane.md @@ -16,9 +16,9 @@ advisory lock for the whole run. That makes it the wrong place for heavy or non-transactional DDL, and we hit **both** failure modes on `metric_series_rollups` (~10.4M rows / 5.8 GB in production): -- **JEF-548 (first attempt):** `CREATE INDEX CONCURRENTLY` cannot run inside a +- **First attempt:** `CREATE INDEX CONCURRENTLY` cannot run inside a transaction and deadlocked against the migrator's advisory lock. -- **JEF-580 (second attempt):** a plain, transactional `CREATE INDEX` took a `SHARE` +- **Second attempt:** a plain, transactional `CREATE INDEX` took a `SHARE` lock on the table and ran past the pool's 60 s `statement_timeout`, so every boot's migration was cancelled → CrashLoopBackOff, and each attempt re-locked the table and stalled cluster-wide OTLP ingest for ~25 minutes until the index was built by hand diff --git a/docs/architecture.md b/docs/architecture.md index 9d21ad3..0dc8fb3 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -82,7 +82,7 @@ last rollup bucket, so it stays continuous after pruning. | `WATCHER_RETENTION_METRICS_DAYS` | — | per-table override of `WATCHER_RETENTION_DAYS` for `metric_series_rollups`; unset falls back to the default | | `WATCHER_METRICS_RAW_DAYS` | `2` | prune age for raw metric points (rollups keep history); `0` = same as retention | | `WATCHER_ROLLUP_BUCKET_SECS` | `300` | downsample bucket width; `0` disables rollups | -| `WATCHER_MAX_QUERY_HOURS` | `168` (7d), clamped to `[1, 8760]` | max look-back for `/api/traces`, `/api/services`, and `/api/logs`; an explicit `from` (or its absence) is clamped to this ceiling, not honored verbatim (JEF-532, JEF-546) | +| `WATCHER_MAX_QUERY_HOURS` | `168` (7d), clamped to `[1, 8760]` | max look-back for `/api/traces`, `/api/services`, and `/api/logs`; an explicit `from` (or its absence) is clamped to this ceiling, not honored verbatim | | `WATCHER_ALERT_INTERVAL_SECS` | `30` | how often alert rules are evaluated (min 5) | | `WATCHER_ALERT_WEBHOOK` | — | optional URL to POST on alert fire/resolve | | `WATCHER_ALERT_SMTP_HOST` | — | SMTP relay host; setting it enables emailing alert fire/resolve (STARTTLS) | diff --git a/scripts/lint-migrations.sh b/scripts/lint-migrations.sh index 83e5f8a..94ed4e4 100755 --- a/scripts/lint-migrations.sh +++ b/scripts/lint-migrations.sh @@ -1,5 +1,5 @@ #!/bin/sh -# CI guardrail (JEF-592): fail the build on migration DDL that can stall the big +# CI guardrail: fail the build on migration DDL that can stall the big # telemetry tables (spans, logs, metric_series_rollups) at deploy time. # # Migration 0017 shipped a plain (non-CONCURRENTLY) `CREATE INDEX` on diff --git a/scripts/start-sccache-docker.sh b/scripts/start-sccache-docker.sh index b2abcda..e1b54c2 100755 --- a/scripts/start-sccache-docker.sh +++ b/scripts/start-sccache-docker.sh @@ -1,13 +1,13 @@ #!/bin/sh -# Fail-soft sccache backend selection INSIDE the Rust image builds (JEF-584). +# Fail-soft sccache backend selection INSIDE the Rust image builds. # The in-build twin of .github/scripts/start-sccache.sh — same shape, different # transport for the config: the CI script reads the runner pod's env directly, # this one reads BuildKit build secrets mounted at /run/secrets by the RUN that # calls it. # # Adopted verbatim (bar the overridable LOCAL_CACHE_DIR below) from the murmurify -# repo's scripts/start-sccache-docker.sh, which landed this pattern first in -# JEF-589 — same reasoning, same fixtures. Keep them in step when either changes. +# repo's scripts/start-sccache-docker.sh, which landed this pattern first — +# same reasoning, same fixtures. Keep them in step when either changes. # # Usage: # sh scripts/start-sccache-docker.sh # start a server, never fail diff --git a/server/Dockerfile b/server/Dockerfile index 6abeee2..fe870bc 100644 --- a/server/Dockerfile +++ b/server/Dockerfile @@ -22,7 +22,7 @@ RUN VITE_API_BASE="" npm run build # chef's `cook` leaves the shared target dir in a state where those extern `.rmeta` # files don't survive into the final `cargo build`, so sccache fatals ("Failed to # open file for hashing: …/lib*.rmeta: No such file or directory") and aborts -# (JEF-389, confirmed real and backend-independent). Collapsed to one build stage: +# (confirmed real and backend-independent). Collapsed to one build stage: # COPY source, one `cargo build`. Migrations are embedded via sqlx::migrate! and # the UI via rust-embed, so the runtime image needs only the binary + CA certs. FROM rust:1-bookworm AS build @@ -42,7 +42,7 @@ COPY server/migrations ./migrations COPY server/src ./src COPY --from=ui /ui/dist /ui/dist # sccache backend = the shared Cloudflare R2 bucket (cluster repo: charts/sccache, -# ADR-0020, JEF-584), replacing the in-cluster Redis this used to hardcode. The config +# ADR-0020), replacing the in-cluster Redis this used to hardcode. The config # and the bucket-scoped token arrive as BuildKit build SECRETS — never ENV or a # build-arg, both of which persist in `docker history` on every image we push to ghcr. # diff --git a/server/migrations/README.md b/server/migrations/README.md index 047b841..8203873 100644 --- a/server/migrations/README.md +++ b/server/migrations/README.md @@ -16,7 +16,7 @@ one of them briefly blocks writes to that table — fine for a genuinely small o brand-new table, a real stall risk once one of these three has millions of rows (see `0017_metric_series_rollups_covering_idx.sql`, which hit exactly this). Online/heavy DDL against `spans`, `logs`, or `metric_series_rollups` belongs in -the JEF-580 online-DDL lane (ADR 0021) — run out-of-band from `sqlx::migrate!`'s +the online-DDL lane (ADR 0021) — run out-of-band from `sqlx::migrate!`'s advisory lock, not a boot migration. ## `-- no-transaction` migrations: one statement per file @@ -26,7 +26,7 @@ transaction (needed for `CREATE INDEX CONCURRENTLY`, which cannot run inside one). It only supports **one statement per no-transaction file** — split a multi-step no-transaction change across several numbered migrations instead. -## CI lint: `scripts/lint-migrations.sh` (JEF-592) +## CI lint: `scripts/lint-migrations.sh` CI runs `scripts/lint-migrations.sh` against every file in this directory and fails the build on: diff --git a/server/src/access_jwt.rs b/server/src/access_jwt.rs index d64a395..be7388d 100644 --- a/server/src/access_jwt.rs +++ b/server/src/access_jwt.rs @@ -1,4 +1,4 @@ -//! Origin-side verification of Cloudflare Access JWTs (JEF-473). +//! Origin-side verification of Cloudflare Access JWTs (ADR 0013). //! //! watcher's public read surface (the UI shell + `/api`) is gated at the edge by //! Cloudflare Access (ADR 0013). This module lets the *origin* independently verify @@ -9,7 +9,7 @@ //! The verifier is deliberately **transport-agnostic**: [`Verifier::verify`] takes a //! raw token string and checks it against the configured issuer/audience. The axum //! middleware in [`crate::app`] pulls the token out of the `Cf-Access-Jwt-Assertion` -//! header; a future `/mcp` Bearer-token guard (JEF-471/472) reuses the same verifier +//! header; the `/mcp` guard (ADR 0019) reuses the same verifier //! with the token taken from `Authorization: Bearer`. //! //! ## Fail-open when unconfigured / on JWKS trouble @@ -154,7 +154,7 @@ impl Verifier { /// The team domain may be given with or without a scheme /// (`team.cloudflareaccess.com`); Cloudflare's `iss` is that host with an /// `https://` scheme and no trailing slash. Shared by [`from_env`](Self::from_env) - /// (the browser Access app) and the `/mcp` Bearer guard (JEF-472), which points at + /// (the browser Access app) and the `/mcp` Bearer guard (ADR 0019), which points at /// the same team but a **separate** Access application AUD. pub fn for_team(team_domain: &str, audience: impl Into) -> Self { let host = team_domain @@ -168,8 +168,7 @@ impl Verifier { } /// The expected issuer (`iss`) — the team domain as an `https://` URL. Also the - /// Cloudflare Access OIDC authorization-server identifier the `/mcp` resource - /// metadata advertises (JEF-472). + /// Cloudflare Access OIDC authorization-server identifier. pub fn issuer(&self) -> &str { &self.issuer } diff --git a/server/src/alerts.rs b/server/src/alerts.rs index de3c527..b0c4603 100644 --- a/server/src/alerts.rs +++ b/server/src/alerts.rs @@ -314,7 +314,7 @@ fn agg_expr(agg: &str) -> &'static str { /// a per-second rate *before* aggregating, reset-safe like `/api/metrics/facet`: /// a level that drops (counter reset) yields 0 for that interval, never a spike. /// -/// `agg == "increase"` is a third, self-contained shape (JEF-463): "how much did +/// `agg == "increase"` is a third, self-contained shape: "how much did /// this monotonic counter go up within the window", e.g. "container restarted >N /// times in the last 10m" rather than `max` of its lifetime total (which never /// resolves once a pod has ever restarted a lot). It reuses the same per-series @@ -328,7 +328,7 @@ fn agg_expr(agg: &str) -> &'static str { /// it's what "increase" means, so `agg_expr` (avg/max/min/sum/last) never applies. fn eval_sql(agg: &str, rate: bool) -> String { // Residual JSONB predicates ride on top of the (name, time) index narrowing; - // a NULL operand disables its clause so pre-JEF-426 rules are unaffected. + // a NULL operand disables its clause so rules that predate this feature are unaffected. let filtered = "SELECT time, service, attributes, value FROM metrics WHERE name = $1 @@ -617,16 +617,17 @@ async fn fire( /// Ceiling on each sink's *total* delivery time inside `notify()`. This is /// distinct from — and layered on top of — each sink's own per-operation /// bound (the reqwest client's `.timeout(10s)` and `Mailer`'s -/// `.timeout(Some(10s))`, both from JEF-497): lettre's SMTP timeout only +/// `.timeout(Some(10s))`): lettre's SMTP timeout only /// bounds each individual network operation in the conversation (connect, /// EHLO, MAIL, RCPT, DATA, body, QUIT, ...), not the conversation as a whole. /// A relay that's slow-but-responsive at every step can still sum well past /// that per-step bound — this is what left `alert.notify` spans at ~12s even -/// after JEF-497 (error_count=0: it was delivering, just slowly). Wrapping +/// with that per-operation bound in place (error_count=0: it was delivering, +/// just slowly). Wrapping /// each sink's whole send in this timeout bounds its total regardless of how /// many round trips the underlying protocol takes. For the webhook this is -/// redundant with reqwest's own total timeout (defense-in-depth, per the -/// ticket); for SMTP it's the actual fix. +/// redundant with reqwest's own total timeout (defense-in-depth); for SMTP +/// it's the actual fix. const NOTIFY_SINK_TIMEOUT: Duration = Duration::from_secs(10); /// Log and notify a firing/resolved transition on every configured sink. @@ -776,7 +777,7 @@ mod tests { #[test] fn validate_accepts_increase_agg() { - // JEF-463: "restarts increased by >N in window_secs" — a rule keying on + // "restarts increased by >N in window_secs" — a rule keying on // window delta rather than lifetime level must validate like any other agg. assert!(validate(&cfg("crashlooping", "gt", "increase")).is_ok()); } @@ -903,7 +904,7 @@ mod tests { #[test] fn config_defaults_new_fields_to_none() { - // A pre-JEF-426 rule (none of the new keys) parses with them all absent. + // An older rule config (none of the new keys) parses with them all absent. let r = &serde_json::from_str::>( r#"[{"name":"r","metric":"m","comparator":"gt","threshold":1}]"#, ) @@ -1010,8 +1011,8 @@ mod tests { // A fake SMTP relay that is slow but never hangs: it answers every step of the // conversation (greeting, EHLO, MAIL, RCPT, DATA, body) after `step_delay`, // never leaving a single read/write outstanding for long. This is exactly the - // JEF-537 scenario — lettre's per-operation SMTP timeout never trips because - // no single step is slow, but the *sum* of six such steps blows past the + // scenario `NOTIFY_SINK_TIMEOUT` guards against — lettre's per-operation SMTP + // timeout never trips because no single step is slow, but the *sum* of six such steps blows past the // total notify budget. Uses `builder_dangerous` (plaintext, no STARTTLS/AUTH) // so the conversation is short enough to hand-roll deterministically. Any I/O // error (most likely the client giving up once its own timeout fires) just @@ -1115,7 +1116,7 @@ mod tests { #[test] fn eval_sql_increase_sums_reset_safe_positive_steps() { - // JEF-463: window-increase of a monotonic counter, not its lifetime `max`. + // Window-increase of a monotonic counter, not its lifetime `max`. // `rate` is irrelevant to this agg — pass both to confirm it's ignored. for rate in [false, true] { let sql = eval_sql("increase", rate); diff --git a/server/src/api.rs b/server/src/api.rs index 895ce0e..973a645 100644 --- a/server/src/api.rs +++ b/server/src/api.rs @@ -14,7 +14,7 @@ use tracing::Instrument; type ApiError = (StatusCode, String); /// Map an internal failure to a 500 — and always log it. This is an observability -/// tool: a silent 5xx (as JEF-494's decode error was) is undiagnosable from +/// tool: a silent 5xx (as a past decode error was) is undiagnosable from /// watcher's own logs, so every internal error is recorded at ERROR here. The /// active tracing span (each handler is `#[tracing::instrument]`ed) carries the /// route, so the log line is attributable without threading it through by hand. @@ -27,7 +27,7 @@ fn internal(e: impl std::fmt::Display) -> ApiError { /// GET /healthz — deep readiness probe: 200 only when the DB is reachable AND /// retention isn't stalled past the configured age; otherwise 503. This gates /// *readiness* (traffic), not liveness — a stalled retention or a DB outage -/// should stop new traffic and page, not kill the process (JEF-425). +/// should stop new traffic and page, not kill the process. pub async fn healthz(State(pool): State) -> impl IntoResponse { let h = crate::selfmon::health(&pool).await; let status = if h.healthy() { @@ -87,13 +87,13 @@ pub async fn list_traces( } /// Hard ceiling (hours) on how far back a `spans`/`logs`-table query may -/// reach, even when the caller passes an explicit `from` (JEF-532). Without +/// reach, even when the caller passes an explicit `from`. Without /// this, the `COALESCE($n, now() - interval '24 hours')` default-window floor /// below only applies when `from` is unset — an arbitrarily-far-past explicit /// `from` would defeat it and full-scan the retention-deep table. Defaults to /// the same 7-day ceiling most metric endpoints use (`resolve_window`'s /// `max_hours` for facet/histogram); override with `WATCHER_MAX_QUERY_HOURS`. -/// Clamped to `[1, 8760]` (1 hour .. 1 year, JEF-546) so a misconfigured env +/// Clamped to `[1, 8760]` (1 hour .. 1 year) so a misconfigured env /// value can't disable the ceiling (0/negative) or resolve it to the distant /// past and re-open the full-scan it exists to prevent (an absurdly large /// value). @@ -140,7 +140,7 @@ pub async fn query_traces(pool: &PgPool, q: TraceQuery) -> Result= GREATEST( COALESCE($2::timestamptz, now() - interval '24 hours'), @@ -273,8 +273,7 @@ pub async fn query_logs(pool: &PgPool, q: LogQuery) -> Result, sqlx: -- default to a recent window when unbounded, and clamp an explicit -- `from` to the max-lookback ceiling too, so a body ILIKE search for -- a rare/absent term can't full-scan the retention-deep `logs` table - -- to satisfy ORDER BY time DESC LIMIT (JEF-546, mirrors JEF-532's - -- query_traces clamp). + -- to satisfy ORDER BY time DESC LIMIT (mirrors query_traces' clamp above). AND time >= GREATEST( COALESCE($5::timestamptz, now() - interval '24 hours'), now() - make_interval(hours => $9::int) @@ -414,7 +413,7 @@ fn rollup_bucket_secs() -> f64 { } /// Point count a chart plausibly renders usefully; wide-window rollup reads -/// coarsen their output bucket to stay near this bound (JEF-561) rather than +/// coarsen their output bucket to stay near this bound rather than /// returning one point per raw rollup bucket regardless of window width — a /// 90-day series at the base 5-min bucket is ~26k points. const TARGET_POINTS: f64 = 1000.0; @@ -439,7 +438,7 @@ fn output_bucket_secs(span_secs: f64, base: f64) -> f64 { } /// `resolve_window` plus the adaptive output-bucket width for the resolved -/// span (JEF-561) — shared by every series/facet/histogram rollup read below. +/// span — shared by every series/facet/histogram rollup read below. fn resolve_window_and_width( hours: Option, default_hours: i32, @@ -453,7 +452,7 @@ fn resolve_window_and_width( } /// Resolve a metric endpoint's window to concrete bounds, shared by every -/// series/facet/histogram/exemplar query below (JEF-433). An absolute `from`/`to` +/// series/facet/histogram/exemplar query below. An absolute `from`/`to` /// takes precedence; either end may be omitted, in which case it falls back to /// the `hours`-relative bound anchored on the *other* end (so `to` alone still /// yields an `hours`-wide window ending at `to`, not at the real "now"). With @@ -461,7 +460,7 @@ fn resolve_window_and_width( /// /// The relative-hours path was already capped to `max_hours` (via `.clamp` /// above); the absolute `from`/`to` path was not, so an over-wide explicit -/// window could out-scan it (JEF-532). Clamp the resolved span to the same +/// window could out-scan it. Clamp the resolved span to the same /// `max_hours` ceiling regardless of which path produced it, so an absolute /// window can never load more than a relative one could. fn resolve_window( @@ -490,7 +489,7 @@ pub struct SeriesQuery { pub hours: Option, /// Absolute window (RFC3339), taking precedence over `hours` when given — /// lets "metrics around this trace" render the trace's exact moment instead - /// of an hours-back-from-now window (JEF-433). Either end may be omitted; + /// of an hours-back-from-now window. Either end may be omitted; /// an omitted end falls back to the `hours`-relative bound. pub from: Option>, pub to: Option>, @@ -514,7 +513,7 @@ pub async fn metric_series( } /// Hard ceiling (hours) on `query_metric_series`'s window, sourced from -/// `WATCHER_RETENTION_DAYS` (JEF-593) — the same knob `retention::prune_once` +/// `WATCHER_RETENTION_DAYS` — the same knob `retention::prune_once` /// prunes `metric_series_rollups` against — rather than a hard-wired constant, /// so raising retention widens the queryable window without a code change. /// Falls back to the sibling facet/histogram/hist_facet queries' fixed @@ -537,12 +536,12 @@ fn parse_retention_max_hours(raw: Option<&str>) -> i32 { /// One metric's collapsed time series, shared by the HTTP handler and the MCP /// `metric_series` tool. Applies the same hours clamp, and honors an absolute /// `from`/`to` window when given (see `SeriesQuery`). The ceiling used to be a -/// flat 90 days on the theory that the covering index (JEF-548) makes the read +/// flat 90 days on the theory that the covering index makes the read /// cheap and the adaptive output bucket below keeps the *result* bounded /// regardless of window width — but `metric_series_rollups` is itself pruned at /// `WATCHER_RETENTION_DAYS` (default 7), so anything past that structurally /// holds no rows: widening the scan range there was pure waste. Capped to -/// `metric_series_max_hours()` instead (JEF-593), which tracks retention. +/// `metric_series_max_hours()` instead, which tracks retention. pub async fn query_metric_series( pool: &PgPool, q: SeriesQuery, @@ -636,7 +635,7 @@ pub async fn metric_series_grouped( Ok(Json(rows)) } -// --- Exemplars (metric -> trace correlation, JEF-433) ----------------------- +// --- Exemplars (metric -> trace correlation) -------------------------------- #[derive(Deserialize)] pub struct ExemplarQuery { @@ -755,7 +754,7 @@ pub async fn metric_facet( // `kind` is decoded as Option: metric_series_rollups.kind is a nullable column // (unlike metrics.kind), so a rollup row can carry a NULL kind. Decoding it as a // bare String made this handler 500 with "unexpected null" for any metric whose - // latest meta row is such a rollup (JEF-494) — a null kind just means "unknown". + // latest meta row is such a rollup — a null kind just means "unknown". let meta: Option<(Option, Option, Option)> = sqlx::query_as( "SELECT kind, is_monotonic, unit FROM ( (SELECT kind, is_monotonic, unit, time AS t FROM metrics @@ -787,7 +786,7 @@ pub async fn metric_facet( // Per-series points straight from the downsampled rollup — index-fast, no // raw scan. Rollups are maintained on ingest, so the current (still-filling) // bucket is present, just partial. avg = bucket mean (gauges), re-aggregated - // as a count-weighted average across the adaptive output bucket (JEF-561, same + // as a count-weighted average across the adaptive output bucket (same // fold as `query_metric_series`); last = cumulative level (counters, for rate // differencing) — monotonic non-decreasing outside a reset, so the max seen // within the output bucket is its most recent value. @@ -919,7 +918,7 @@ pub async fn metric_histogram( let (lo, hi, width) = resolve_window_and_width(q.hours, 6, 24 * 7, q.from, q.to); // Per-series rollup counts, first re-aggregated per series into the adaptive - // output bucket (JEF-561: `array_sum`/`min(bounds)`, the same fold ingest + // output bucket (`array_sum`/`min(bounds)`, the same fold ingest // uses to combine raw points into a rollup row — see `insert_histograms`), // then the Rust pass below sums across series per time bucket. let rows: Vec = sqlx::query_as( @@ -1061,7 +1060,7 @@ pub async fn metric_hist_facet( ) -> Result, ApiError> { let (lo, hi, width) = resolve_window_and_width(q.hours, 6, 24 * 7, q.from, q.to); - // Re-aggregated per series into the adaptive output bucket (JEF-561), same + // Re-aggregated per series into the adaptive output bucket, same // fold as `metric_histogram` above. let rows: Vec = sqlx::query_as( "SELECT attrs, metric_bucket(bucket, $4) AS t, min(bucket_bounds) AS bounds, @@ -1178,7 +1177,7 @@ pub async fn query_service_red(pool: &PgPool, q: RedQuery) -> Result= GREATEST( COALESCE($1::timestamptz, now() - interval '24 hours'), now() - make_interval(hours => $3::int) @@ -1404,7 +1403,7 @@ mod tests { } } - // JEF-494: every 5xx must be diagnosable from watcher's own logs. `internal()` + // Every 5xx must be diagnosable from watcher's own logs. `internal()` // maps to a 500 AND logs the error at ERROR — this proves it does both. #[test] fn internal_logs_the_error_and_returns_500() { @@ -1427,7 +1426,7 @@ mod tests { ); } - // JEF-532: an explicit `from`/`to` bypassed the relative-hours `max_hours` + // An explicit `from`/`to` used to bypass the relative-hours `max_hours` // clamp entirely — this proves the absolute path is now capped to the same // ceiling. #[test] @@ -1466,8 +1465,8 @@ mod tests { assert_eq!(lo, hi - Duration::hours(48)); } - // JEF-546: a pathologically large WATCHER_MAX_QUERY_HOURS must not resolve - // the ceiling to the distant past and re-open the full-scan JEF-532 closed. + // A pathologically large WATCHER_MAX_QUERY_HOURS must not resolve + // the ceiling to the distant past and re-open the full-scan the clamp above closed. #[test] fn parse_max_lookback_hours_clamps_huge_value_to_one_year() { assert_eq!(parse_max_lookback_hours(Some("999999999")), 8760); @@ -1491,7 +1490,7 @@ mod tests { assert_eq!(parse_max_lookback_hours(Some("48")), 48); } - // JEF-593: at the default 7-day retention, the series ceiling must match + // At the default 7-day retention, the series ceiling must match // the sibling facet/histogram/hist_facet queries' fixed `24 * 7` — the old // 90-day ceiling scanned 83 days of range that retention had already // pruned to nothing. @@ -1520,7 +1519,7 @@ mod tests { } // Acceptance criterion: a >retention window request is clamped to the - // retention bound, exactly like JEF-532's absolute-window clamp test above + // retention bound, exactly like the absolute-window clamp test above // — but driven by the retention-derived ceiling instead of a literal. #[test] fn resolve_window_clamps_series_window_to_retention_ceiling() { @@ -1541,7 +1540,7 @@ mod tests { ); } - // JEF-561: a narrow window (well under TARGET_POINTS * base) must collapse + // A narrow window (well under TARGET_POINTS * base) must collapse // to exactly `base` — unwidened, full rollup resolution — so a short-range // chart's points are unaffected. #[test] diff --git a/server/src/db.rs b/server/src/db.rs index 7a6aa3e..5dc4aad 100644 --- a/server/src/db.rs +++ b/server/src/db.rs @@ -12,7 +12,7 @@ pub async fn connect(url: &str) -> anyhow::Result { let opts = PgConnectOptions::from_str(url)?.options([("statement_timeout", "60s")]); let pool = PgPoolOptions::new() .max_connections(10) - // Defense-in-depth for a Patroni/postgres-operator failover (JEF-496). When + // Defense-in-depth for a Patroni/postgres-operator failover. When // the leader is demoted, the pool keeps live connections to what is now a // read-only replica; writes on them fail with SQLSTATE 25006 until the // connections recycle and the `-master` DNS re-resolves to the new leader. @@ -40,8 +40,9 @@ pub async fn connect(url: &str) -> anyhow::Result { /// Run pending migrations on a single dedicated connection — deliberately /// NOT the query pool from [`connect`] above, so a migration never inherits /// that pool's 60s `statement_timeout` or waits behind ingest traffic for a -/// lock. (JEF-580: migration 0017's `CREATE INDEX` ran on the shared pool, -/// inherited the 60s bound, and was killed mid-run — crashlooping the pod.) +/// lock. (Migration 0017's `CREATE INDEX` ran on the shared pool, +/// inherited the 60s bound, and was killed mid-run — crashlooping the pod; +/// see ADR 0021.) /// /// * `lock_timeout=3s`: a migration that can't acquire the lock it needs /// (e.g. blocked behind a long-running transaction on the target table) @@ -71,7 +72,7 @@ fn migrate_connect_options(url: &str) -> anyhow::Result { .options([("lock_timeout", "3s"), ("statement_timeout", "0")])) } -/// Connect options for the online-DDL lane (ADR 0021, JEF-580): a dedicated +/// Connect options for the online-DDL lane (ADR 0021): a dedicated /// connection, not the query pool, so a `CREATE`/`DROP INDEX CONCURRENTLY` build /// that runs for minutes on a big table is never cancelled by the pool's 60s /// `statement_timeout` (exactly the failure a plain, transactional `CREATE INDEX` @@ -95,9 +96,9 @@ mod tests { use sqlx::Connection; use std::time::{Duration, Instant}; - /// JEF-580/590: a migration that can't get the lock it needs must abort within + /// A migration that can't get the lock it needs must abort within /// `lock_timeout`, not queue behind the holder and head-of-line-block the - /// table. Proven at the connection level per the ticket (no real migration + /// table. Proven at the connection level (no real migration /// file needed): a competing transaction takes an ACCESS EXCLUSIVE lock on a /// throwaway table, and a trivial DDL statement on a connection configured /// exactly like `migrate`'s must fail fast with a lock_timeout error rather diff --git a/server/src/lib.rs b/server/src/lib.rs index 244f860..e7aa283 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -32,9 +32,9 @@ use crate::access_jwt::{Verifier, VerifyError}; use crate::mcp_auth::McpAuth; /// The header Cloudflare Access sets on requests that cleared its edge policy, -/// carrying the signed identity JWT the origin re-verifies (JEF-473). Under +/// carrying the signed identity JWT the origin re-verifies (ADR 0013). Under /// Managed OAuth this is also what the edge forwards after resolving an MCP -/// client's opaque OAuth token (JEF-493), so `/api` and `/mcp` verify the same +/// client's opaque OAuth token (ADR 0019), so `/api` and `/mcp` verify the same /// header. Cloudflare strips any client-supplied `Cf-Access-*` header, so the /// origin can trust it as edge-set. const ACCESS_JWT_HEADER: &str = "Cf-Access-Jwt-Assertion"; @@ -58,7 +58,7 @@ pub(crate) enum Assertion { /// Verify a request's `Cf-Access-Jwt-Assertion` header against `verifier`. The /// single place the header name and the `VerifyError` → outcome mapping live, so -/// the `/api` and `/mcp` guards share exactly one verification path (JEF-493). +/// the `/api` and `/mcp` guards share exactly one verification path (ADR 0019). pub(crate) async fn check_access_assertion( verifier: &Verifier, headers: &axum::http::HeaderMap, @@ -165,7 +165,7 @@ async fn ui_handler(uri: Uri) -> Response { } /// Middleware that re-verifies the Cloudflare Access JWT on the read surface -/// (JEF-473). Origin-side defense-in-depth on top of the edge Access policy: a +/// (ADR 0013). Origin-side defense-in-depth on top of the edge Access policy: a /// request missing or carrying an invalid `Cf-Access-Jwt-Assertion` is rejected /// `401` before reaching a handler. Wired **only** onto the UI shell + `/api` /// (never `/v1` ingest or `/healthz`) and only when Access is configured — see @@ -203,13 +203,13 @@ pub fn app(pool: PgPool) -> Router { } /// Build the HTTP router, optionally enforcing Cloudflare Access JWT verification -/// (JEF-473) on the read surface; MCP auth is taken from the environment. +/// (ADR 0013) on the read surface; MCP auth is taken from the environment. pub fn app_with_access(pool: PgPool, access: Option>) -> Router { app_with_auth(pool, access, McpAuth::from_env()) } /// Build the HTTP router, optionally enforcing Cloudflare Access JWT verification -/// (JEF-473) on the read surface and Managed-OAuth assertion auth (JEF-493) on `/mcp`. +/// (ADR 0013) on the read surface and Managed-OAuth assertion auth (ADR 0019) on `/mcp`. /// /// The server holds no app-layer auth by default — auth lives at the edge /// (Cloudflare Access for the public read surface) and ingest is only reachable @@ -278,10 +278,10 @@ pub fn app_with_auth( .merge(ingest) .merge(guarded); - // Read-only MCP server (JEF-471), opt-in via WATCHER_MCP_ENABLED (default OFF). + // Read-only MCP server (ADR 0018), opt-in via WATCHER_MCP_ENABLED (default OFF). // Nested as its own tower service *outside* the `/api` router — and therefore // outside the browser Access guard above, since an MCP client is not a browser - // and carries no Access cookie. Under Cloudflare Managed OAuth (JEF-493) the edge + // and carries no Access cookie. Under Cloudflare Managed OAuth (ADR 0019) the edge // resolves the client's opaque OAuth token and forwards a `Cf-Access-Jwt-Assertion`; // `mcp_auth::assertion_guard` validates that assertion (its own AUD, fail-closed). // Cloudflare owns OAuth discovery, so no `.well-known` metadata is self-served. diff --git a/server/src/main.rs b/server/src/main.rs index 3a526de..ed6a26f 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -15,7 +15,7 @@ use watcher_server::{ /// `service.name=watcher`, so they land in its own `spans` table and it shows up in /// its own UI. Like self-metrics (ADR 0014) and self-logs (ADR 0016), traces go /// straight to the ingest path ([`selftrace`]) — no network hop, no OTLP self-POST, -/// no batch-to-self that can wedge in a shut-down state (JEF-462). Opt out with +/// no batch-to-self that can wedge in a shut-down state. Opt out with /// `WATCHER_SELF_TELEMETRY=0`. /// /// Returns the provider (kept alive for the process lifetime; dropping it shuts the @@ -53,7 +53,7 @@ async fn main() -> anyhow::Result<()> { .with_tracer(provider.tracer("watcher-server")) .with_filter(dynamic_filter_fn(|_meta, _cx| !selflog::suppressed())) }); - // Self-log capture (JEF-452): a layer that mirrors watcher's own events into its + // Self-log capture: a layer that mirrors watcher's own events into its // own `logs` table. Built here, before the pool exists, so startup events buffer // in its channel; `main` spawns the drain task once the DB is up. The receiver // rides alongside the (Option) layer so both share the enabled() decision. @@ -82,8 +82,8 @@ async fn main() -> anyhow::Result<()> { .init(); // Keep the tracer provider alive for the whole process (dropping it shuts the - // batch processor down — the very failure mode JEF-462 fixes), and take the - // receiver so the drain task can be spawned once the pool is up. + // batch processor down — the very failure mode this in-process capture avoids), + // and take the receiver so the drain task can be spawned once the pool is up. let (_self_trace_provider, selftrace_rx) = self_traces.unzip(); let database_url = std::env::var("DATABASE_URL") @@ -104,7 +104,7 @@ async fn main() -> anyhow::Result<()> { // Raw metric points are aggregated into per-series rollups on ingest, so raw // is kept only as a short full-resolution window for inspection. let metrics_raw_hours = env_i32("WATCHER_METRICS_RAW_HOURS", 6); - // Per-signal retention windows (JEF-434): each is optional and falls back to + // Per-signal retention windows: each is optional and falls back to // WATCHER_RETENTION_DAYS above when unset, so omitting these is a no-op. let retention_windows = retention::Windows { spans_days: env_i32_opt("WATCHER_RETENTION_SPANS_DAYS"), @@ -179,14 +179,14 @@ async fn main() -> anyhow::Result<()> { if let Some(rx) = selflog_rx { tokio::spawn(selflog::drain(pool.clone(), rx)); } - // Self-traces (JEF-462): drain the buffered self-spans (exported by the in-process + // Self-traces: drain the buffered self-spans (exported by the in-process // SpanExporter installed above) into the `spans` table via the same ingest path. // Spawned on the main runtime so its sqlx I/O runs on the pool's own reactors. if let Some(rx) = selftrace_rx { tokio::spawn(selftrace::drain(pool.clone(), rx)); } - // Origin-side Cloudflare Access JWT verification (JEF-473): when + // Origin-side Cloudflare Access JWT verification: when // WATCHER_ACCESS_TEAM_DOMAIN + WATCHER_ACCESS_AUD are set, the UI shell + /api // additionally re-verify the edge-issued Access token as defense-in-depth // (ADR 0013). Unset → not wired in, so local dev / non-Access deploys are @@ -201,9 +201,9 @@ async fn main() -> anyhow::Result<()> { ); } - // Read-only MCP server (JEF-471): mounted at /mcp by `app_with_access` only when + // Read-only MCP server (ADR 0018): mounted at /mcp by `app_with_access` only when // WATCHER_MCP_ENABLED is set (default OFF). Its Managed-OAuth assertion auth - // (JEF-493) requires WATCHER_ACCESS_TEAM_DOMAIN + WATCHER_MCP_ACCESS_AUD; with + // (ADR 0019) requires WATCHER_ACCESS_TEAM_DOMAIN + WATCHER_MCP_ACCESS_AUD; with // those unset the endpoint fails closed (is NOT served) rather than exposing read // access. if mcp::enabled() { @@ -225,7 +225,7 @@ async fn main() -> anyhow::Result<()> { let listener = tokio::net::TcpListener::bind(&http_bind).await?; tracing::info!("HTTP/OTLP + API on http://{http_bind}"); - // Online-DDL lane (ADR 0021, JEF-580): spawned only now that the listener is + // Online-DDL lane (ADR 0021): spawned only now that the listener is // bound and the pod is Ready, so an index build (which can take minutes on a // large table) never blocks boot or /healthz. Fire-and-forget: a failure is // logged, not fatal — reads fall back to whatever index already covers the diff --git a/server/src/mcp.rs b/server/src/mcp.rs index ee57923..6d3017d 100644 --- a/server/src/mcp.rs +++ b/server/src/mcp.rs @@ -1,4 +1,4 @@ -//! Read-only MCP server (JEF-471) mounted in-process on the axum app at `/mcp`. +//! Read-only MCP server (ADR 0018) mounted in-process on the axum app at `/mcp`. //! //! Exposes watcher's read API as Model Context Protocol tools over the official //! streamable-HTTP transport (`rmcp`), so an MCP client (MCP Inspector, Claude @@ -13,7 +13,7 @@ //! behind (Cloudflare Access, ADR 0013): an MCP client is not a browser and carries //! no Access cookie. Under Cloudflare Managed OAuth the edge resolves the client's //! opaque OAuth token and forwards a `Cf-Access-Jwt-Assertion`, which -//! [`crate::mcp_auth`] validates (JEF-493, its own AUD) — `app_with_access` wraps +//! [`crate::mcp_auth`] validates (ADR 0019, its own AUD) — `app_with_access` wraps //! this service in that guard and refuses to serve `/mcp` when the guard is //! unconfigured. @@ -36,7 +36,7 @@ use sqlx::PgPool; use crate::api; /// Env flag gating `/mcp`. Default OFF (opt-in) — unlike the self-telemetry -/// opt-outs — because enabling it exposes read access to anyone the auth (JEF-472) +/// opt-outs — because enabling it exposes read access to anyone the auth (ADR 0019) /// admits, so it stays an explicit operator decision. const ENABLE_FLAG: &str = "WATCHER_MCP_ENABLED"; @@ -300,7 +300,7 @@ impl WatcherMcp { name: a.name, service: a.service, hours: a.hours, - // Absolute from/to (JEF-433) are an HTTP-only affordance for now — the + // Absolute from/to are an HTTP-only affordance for now — the // MCP tool keeps its existing hours-only surface. from: None, to: None, @@ -351,7 +351,7 @@ pub fn service(pool: PgPool) -> StreamableHttpService Vec { /// Whether a live, valid index named `idx.name` carries a different `INCLUDE` /// column set than `idx.include_cols` desires — i.e. whether the definition has -/// drifted (e.g. JEF-591 narrowing the wide covering index migration 0017 +/// drifted (e.g. narrowing the wide covering index migration 0017 /// built). Order-independent: only the column *set* matters. async fn include_columns_drifted( conn: &mut PgConnection, @@ -170,7 +169,7 @@ async fn drop_index_concurrently(conn: &mut PgConnection, name: &str) -> anyhow: /// Reconciles one desired index against its current state on `conn`. Absent → /// build; valid and matching desired `INCLUDE` columns → no-op; valid but -/// drifted (a definition change since it was built, e.g. JEF-591's narrowing) +/// drifted (a definition change since it was built, e.g. the covering index's narrowing) /// → drop then rebuild; invalid (left by an interrupted build) → drop then /// rebuild. Runs entirely outside a transaction, as `CONCURRENTLY` requires. async fn reconcile_index(conn: &mut PgConnection, idx: &OnlineIndex) -> anyhow::Result<()> { @@ -576,7 +575,7 @@ mod tests { ); let oid_wide = relation_oid(&mut conn, name).await; - // Desired is now narrow (JEF-591-style): only column `a`. + // Desired is now narrow: only column `a`. let narrow = OnlineIndex { name, table, diff --git a/server/src/otlp.rs b/server/src/otlp.rs index 5e36c47..c511706 100644 --- a/server/src/otlp.rs +++ b/server/src/otlp.rs @@ -164,7 +164,7 @@ pub async fn store_traces(pool: &PgPool, req: ExportTraceServiceRequest) -> u64 pub async fn store_logs(pool: &PgPool, req: ExportLogsServiceRequest) -> u64 { // Decode the whole request into rows first, then write them in batched // statements (one per chunk) instead of one INSERT round-trip per record — - // the per-row loop was the source of the ~30s ingest p99 (JEF-495). + // the per-row loop was the source of the ~30s ingest p99. let mut rows: Vec = Vec::new(); for rl in &req.resource_logs { // Keep resource attributes (k8s.pod.name / node / container, …) so logs @@ -205,8 +205,8 @@ const _: () = assert!(FAILOVER_BACKOFF.len() == FAILOVER_MAX_ATTEMPTS as usize - /// failover produces — worth retrying on a *fresh* connection, unlike a bad row: /// /// * `25006` read_only_sql_transaction — the pooled connection's backend was -/// demoted to a read-only replica; the write hits a read-only node (the JEF-496 -/// symptom). A new connection re-resolves `-master` DNS to the new leader. +/// demoted to a read-only replica; the write hits a read-only node. A new +/// connection re-resolves `-master` DNS to the new leader. /// * `57P01` admin_shutdown — the old leader terminated the backend on demotion. /// /// Deliberately narrow: a constraint violation, encoding error, or any other @@ -265,7 +265,7 @@ async fn write_with_failover_retry( /// Write `rows` in chunks, one batched `INSERT` per chunk. Each write goes through /// [`write_with_failover_retry`], so a Patroni failover's read-only error is retried -/// on a fresh connection rather than dropped (JEF-496). On a *non-failover* chunk +/// on a fresh connection rather than dropped. On a *non-failover* chunk /// error, fall back to inserting each row of that chunk on its own so a single bad /// row can't drop the whole batch — every row that still fails is logged and counted /// in `DROP_INSERT`, preserving the per-row drop accounting the old loop had. If a @@ -522,7 +522,7 @@ struct NumRow { unit: Option, is_monotonic: Option, attrs: serde_json::Value, - /// One exemplar trace/span id (JEF-433), picked from the point's exemplars — + /// One exemplar trace/span id, picked from the point's exemplars — /// `None` for points that carry no sampled exemplar (the common case). exemplar_trace_id: Option, exemplar_span_id: Option, @@ -539,7 +539,7 @@ struct HistRow { count: i64, bounds: Vec, counts: Vec, - /// One exemplar trace/span id (JEF-433); see `NumRow`. + /// One exemplar trace/span id; see `NumRow`. exemplar_trace_id: Option, exemplar_span_id: Option, } @@ -639,7 +639,7 @@ fn num_row( }) } -/// Pick one exemplar to keep per data point (JEF-433): the first exemplar that +/// Pick one exemplar to keep per data point: the first exemplar that /// actually carries a trace id — an exemplar's span/trace ids are optional in the /// OTLP spec (absent when the measurement wasn't recorded inside a sampled trace), /// so a point can have exemplars with no usable id. `metrics` keeps at most one @@ -770,7 +770,7 @@ async fn flush_numbers(pool: &PgPool, rows: Vec) -> u64 { b.exemplar_trace_ids.push(r.exemplar_trace_id); b.exemplar_span_ids.push(r.exemplar_span_id); } - // The whole-batch write goes through failover retry (JEF-496): a Patroni failover's + // The whole-batch write goes through failover retry: a Patroni failover's // read-only error is retried on a fresh connection, not dropped. There's no per-row // fallback here (a metrics batch is one aggregating statement), so on a persistent // error — failover retries exhausted or any other DB error — the batch drops and @@ -816,7 +816,7 @@ async fn flush_histograms(pool: &PgPool, rows: Vec) -> u64 { }) .collect(), ); - // Whole-batch write through failover retry (JEF-496); see flush_numbers for the + // Whole-batch write through failover retry; see flush_numbers for the // drop/accounting rationale. The JSONB payload + bucket width travel as borrowed // `data` so the retry op captures nothing. let b = HistBatch { @@ -1228,7 +1228,7 @@ mod tests { assert_eq!(any_value_to_text(&n), "5"); } - // --- JEF-496: read-only-failover write retry ------------------------------- + // --- Read-only-failover write retry ----------------------------------------- // // These exercise `write_with_failover_retry` against a real Postgres (CI's // service container; skipped when DATABASE_URL is unset). They don't rely on diff --git a/server/src/retention.rs b/server/src/retention.rs index fae16ad..1505c9a 100644 --- a/server/src/retention.rs +++ b/server/src/retention.rs @@ -3,7 +3,7 @@ //! because `metric_series_rollups` (maintained on ingest) preserves their //! downsampled per-series history. //! -//! Spans, logs, and metric rollups can each be given their own window (JEF-434): +//! Spans, logs, and metric rollups can each be given their own window: //! [`Windows`] carries an optional per-table override, declared via //! `WATCHER_RETENTION_SPANS_DAYS` / `WATCHER_RETENTION_LOGS_DAYS` / //! `WATCHER_RETENTION_METRICS_DAYS`. A table with no override falls back to the @@ -11,7 +11,7 @@ //! today's single window, so this is a no-op for anyone who doesn't set the new //! vars. This is deliberately per-*table*, not per-service: a per-service delete //! over these tables would need `ctid`-batching like `prune_raw_metrics` below to -//! avoid the statement-timeout failure mode (JEF-425); that's a separate ticket. +//! avoid the statement-timeout failure mode; that's a separate follow-up. use sqlx::PgPool; use std::time::Duration; @@ -88,7 +88,7 @@ pub async fn prune_once( } // Record the successful sweep so self-telemetry can surface its recency and // /healthz can flag a stall (a silent retention stall is exactly what let the - // metrics table grow to tens of GB un-paged — JEF-425). + // metrics table grow to tens of GB un-paged). crate::selfmon::record_retention_success(total); Ok(total) } diff --git a/server/src/selflog.rs b/server/src/selflog.rs index 13dfa0f..dcc4c01 100644 --- a/server/src/selflog.rs +++ b/server/src/selflog.rs @@ -1,4 +1,4 @@ -//! watcher self-instrumentation of its *own logs* (JEF-452): a `tracing` Layer +//! watcher self-instrumentation of its *own logs* (ADR 0016): a `tracing` Layer //! that converts each event into an OTLP log record and hands it to the in-process //! ingest path ([`crate::otlp::store_logs`]) — the same table its UI reads — tagged //! `service.name=watcher`. diff --git a/server/src/selfmon.rs b/server/src/selfmon.rs index fd3c091..814e796 100644 --- a/server/src/selfmon.rs +++ b/server/src/selfmon.rs @@ -1,4 +1,4 @@ -//! watcher self-monitoring (JEF-425): operational gauges + counters about +//! watcher self-monitoring (ADR 0014): operational gauges + counters about //! watcher's own health, plus the deep `/healthz` computation. //! //! The ops metrics are handed straight to [`crate::otlp::store_metrics`] rather @@ -217,11 +217,11 @@ const TRACKED_TABLES: [&str; 6] = [ "alert_rules", ]; -/// The table the index-only-scan / visibility-map health canary (JEF-594) -/// watches. The JEF-591 covering index only produces index-only scans while -/// this high-churn table's visibility map stays current; if autovacuum falls -/// behind, scans silently degrade to heap fetches and the JEF-548-class -/// latency returns with no signal until dashboards get slow. +/// The table the index-only-scan / visibility-map health canary watches. The +/// covering index only produces index-only scans while this high-churn +/// table's visibility map stays current; if autovacuum falls behind, scans +/// silently degrade to heap fetches and the same class of latency regression +/// returns with no signal until dashboards get slow. const ROLLUP_TABLE: &str = "metric_series_rollups"; /// Build the `watcher_*` metric points from cheap catalog/aggregate queries plus @@ -361,7 +361,7 @@ async fn collect_metrics(pool: &PgPool) -> anyhow::Result> { )); } - // Index-only-scan / visibility-map health canary (JEF-594) — see + // Index-only-scan / visibility-map health canary — see // `ROLLUP_TABLE` doc comment. `pg_stat_user_tables` is always available (no // extension needed); the row is absent only before the catalog's stats have // ever been populated for the table, which shouldn't happen post-migration @@ -464,7 +464,7 @@ fn log_pg_visibility_missing_once() { "self-telemetry: pg_visibility extension not installed -- \ watcher.db.vm_all_visible_fraction will not be emitted; \ watcher.db.dead_tuple_ratio and watcher.db.last_autovacuum_age_seconds \ - still cover the JEF-594 canary" + still cover the index-only-scan health canary" ); } } diff --git a/server/src/selftrace.rs b/server/src/selftrace.rs index 60668c5..61b6b1c 100644 --- a/server/src/selftrace.rs +++ b/server/src/selftrace.rs @@ -1,4 +1,4 @@ -//! watcher self-instrumentation of its own *traces* (JEF-462): a custom +//! watcher self-instrumentation of its own *traces* (ADR 0017): a custom //! [`opentelemetry_sdk::trace::SpanExporter`] that maps exported `SpanData` into the //! in-process trace-ingest path ([`crate::otlp::store_traces`]) — the same table its //! UI reads — tagged `service.name=watcher`. @@ -8,7 +8,7 @@ //! Traces were the last one still on the fragile OTLP self-export (a batch //! `SpanExporter` POSTing to `localhost:4318`); that path had wedged into a //! shut-down state and never landed a single watcher span while flooding the `logs` -//! table with "Spans are being emitted even after Shutdown" warnings (JEF-462). +//! table with "Spans are being emitted even after Shutdown" warnings. //! Going in-process removes the network hop, the self-POST, and the batch-to-self //! that could shut down — the exporter just enqueues converted spans for a drain //! task that stores them on the main runtime. diff --git a/server/tests/smoke.rs b/server/tests/smoke.rs index f87fe83..72cf2ef 100644 --- a/server/tests/smoke.rs +++ b/server/tests/smoke.rs @@ -155,7 +155,7 @@ fn one_number( metric_request(name, service, data) } -/// One gauge point carrying a single OTLP exemplar (JEF-433), so ingest's +/// One gauge point carrying a single OTLP exemplar, so ingest's /// `first_exemplar` decode path has something to pick up. `trace_id`/`span_id` /// are raw bytes — hex-encode the return value to compare against the API. fn gauge_with_exemplar( @@ -365,7 +365,7 @@ async fn ingest_and_query_a_log() { scope_logs: vec![ScopeLogs { log_records: vec![LogRecord { // Real "now" rather than a fixed epoch value: /api/logs floors - // to a recent default window (JEF-546), so a fake 1970 + // to a recent default window, so a fake 1970 // timestamp would fall outside it and never come back. time_unix_nano: now_nanos(), severity_number: 9, // INFO @@ -418,7 +418,7 @@ async fn ingest_and_query_a_log() { #[tokio::test] #[serial] async fn ingest_a_log_batch_persists_every_row_in_one_call() { - // JEF-495: store_logs batches a whole request into chunked INSERTs. A single + // store_logs batches a whole request into chunked INSERTs. A single // multi-record request must land every row (identical column values) and not // touch DROP_INSERT. let Some(pool) = pool_or_skip().await else { @@ -429,7 +429,7 @@ async fn ingest_a_log_batch_persists_every_row_in_one_call() { const N: usize = 250; let drops_before = selfmon::DROP_INSERT.load(std::sync::atomic::Ordering::Relaxed); // Real "now" rather than a fixed epoch value: /api/logs floors to a recent - // default window (JEF-546), so fake 1970 timestamps would fall outside it + // default window, so fake 1970 timestamps would fall outside it // and never come back via the `/api/logs?service=batcher` check below. let base_nanos = now_nanos(); let records: Vec = (0..N) @@ -488,7 +488,7 @@ async fn ingest_a_log_batch_persists_every_row_in_one_call() { #[tokio::test] #[serial] async fn ingest_a_span_batch_persists_and_dedupes_in_one_call() { - // JEF-495: store_traces batches too. Distinct spans all land; an intra-batch + // store_traces batches too. Distinct spans all land; an intra-batch // duplicate (same trace_id/span_id) is collapsed by ON CONFLICT DO NOTHING, // exactly as the old per-row insert did. let Some(pool) = pool_or_skip().await else { @@ -678,7 +678,7 @@ async fn metric_series_honors_absolute_from_to_window() { assert_eq!(arr[0]["v"], 0.9); // Same window via facet and histogram — both must render the exact absolute - // range too, not just `series` (JEF-433). + // range too, not just `series`. ingest( &router, one_number("reqs", None, "api", None, 0.9, nanos_ago(5400)), @@ -742,8 +742,8 @@ async fn metric_series_honors_absolute_from_to_window() { /// The series endpoint's window is capped at the retention-derived ceiling /// (`resolve_window`'s `max_hours` for `/api/metrics/series` — `24 * 7` at the -/// default `WATCHER_RETENTION_DAYS`, JEF-593; previously a flat 90 days, -/// JEF-548). Originally the query whose wide-window rollup scan was traced +/// default `WATCHER_RETENTION_DAYS`; previously a flat 90 days). +/// Originally the query whose wide-window rollup scan was traced /// holding a pool connection for tens of seconds (the fix: a covering index on /// `metric_series_rollups`, migration 0017). This doesn't benchmark the index /// directly (no way to force a plan choice through the HTTP API), but it does @@ -786,7 +786,7 @@ async fn metric_series_wide_window_returns_bounded_correct_points() { assert_eq!(times, sorted, "points must be ordered t ASC"); } -/// JEF-561/JEF-593: at the (now retention-derived) 7-day ceiling the adaptive +/// At the (now retention-derived) 7-day ceiling the adaptive /// output bucket widens from the base 300s rollup bucket to /// `300 * ceil((168h in secs / 300) / 1000) = 900s` (see `output_bucket_secs`'s /// unit tests for that math) — a chart gets bounded output instead of one @@ -841,7 +841,7 @@ async fn metric_series_wide_window_reaggregates_merged_buckets_correctly() { assert_eq!(arr[1]["v"], 4.25); } -/// JEF-561: a narrow window (well under `TARGET_POINTS * base`) must keep the +/// A narrow window (well under `TARGET_POINTS * base`) must keep the /// base 5-min rollup resolution unchanged — two points a couple of buckets /// apart must stay distinct, not fold into a coarser bucket. #[tokio::test] @@ -1028,7 +1028,7 @@ async fn ui_fallback_does_not_shadow_api() { } } -// --- Self-monitoring + deep /healthz (JEF-425) ----------------------------- +// --- Self-monitoring + deep /healthz (ADR 0014) ----------------------------- #[tokio::test] #[serial] @@ -1162,7 +1162,7 @@ async fn rollup_vacuum_health_canary_emits_dead_tuple_ratio_and_optional_vm_frac "must not report an autovacuum age before autovacuum has ever run" ); - // pg_visibility is an optional contrib extension (JEF-594): the app never + // pg_visibility is an optional contrib extension: the app never // creates it itself (it's not "trusted", so it needs superuser -- that's a // cluster-ops concern, not read-only introspection), but exercise the // happy path when the test DB's role can install it, so the extraction @@ -1194,7 +1194,7 @@ async fn rollup_vacuum_health_canary_emits_dead_tuple_ratio_and_optional_vm_frac ); } -// --- Self-log instrumentation (JEF-452) ------------------------------------ +// --- Self-log instrumentation (ADR 0016) ------------------------------------ #[tokio::test] #[serial] @@ -1273,7 +1273,7 @@ async fn self_logs_correlate_with_current_span() { return; }; // With the otel layer in the stack, an event inside a span must carry that span's - // trace/span ids so self-logs link to self-traces (the span→logs drill, JEF-429). + // trace/span ids so self-logs link to self-traces (the span→logs drill). let (layer, mut rx) = selflog::channel_layer(); let provider = opentelemetry_sdk::trace::SdkTracerProvider::builder().build(); let otel_layer = tracing_opentelemetry::layer().with_tracer(provider.tracer("test")); @@ -1320,7 +1320,7 @@ async fn self_logs_correlate_with_current_span() { assert!(outside["span_id"].is_null()); } -// --- Self-trace instrumentation (JEF-462) ---------------------------------- +// --- Self-trace instrumentation (ADR 0017) ---------------------------------- #[tokio::test] #[serial] @@ -1353,7 +1353,7 @@ async fn self_traces_land_in_spans_and_appear_in_services() { tracing::info!("handling watcher request"); }); // Flush the batch processor so the ended span reaches the exporter's channel. A - // shut-down processor (the JEF-462 bug) would export nothing. + // shut-down processor (the original network-self-export bug) would export nothing. provider.force_flush().expect("force_flush"); // Drain into the DB while the subscriber is still active: storing self-spans runs @@ -1458,7 +1458,7 @@ async fn insert_rollup_at(pool: &sqlx::PgPool, name: &str, secs_ago: f64) { } /// Like `insert_rollup_at`, but with an explicit `count`/`sum` so a test can -/// pin the exact count-weighted average a re-aggregated (JEF-561) output +/// pin the exact count-weighted average a re-aggregated output /// bucket must produce. async fn insert_rollup_with(pool: &sqlx::PgPool, name: &str, secs_ago: f64, count: i64, sum: f64) { sqlx::query( @@ -1478,7 +1478,7 @@ async fn insert_rollup_with(pool: &sqlx::PgPool, name: &str, secs_ago: f64, coun /// A raw histogram rollup row (same `series_key` for every call with the same /// `name`, so multiple rows are one series) — lets a test pin exact -/// `bucket_counts` a re-aggregated (JEF-561) output bucket's `array_sum` must +/// `bucket_counts` a re-aggregated output bucket's `array_sum` must /// produce. async fn insert_hist_rollup_with( pool: &sqlx::PgPool, @@ -1888,7 +1888,7 @@ async fn metric_histogram_interpolates_percentiles() { assert_eq!(b["counts"], serde_json::json!([0, 100, 0, 0])); } -/// JEF-561: `metric_histogram` now pre-aggregates each series into the +/// `metric_histogram` pre-aggregates each series into the /// adaptive output bucket in SQL (`array_sum(bucket_counts)`, the same fold /// `insert_histograms` uses on ingest) before the Rust pass sums across /// series. `hours=100` puts the span at 360,000s, so the adaptive width there @@ -2221,7 +2221,7 @@ async fn retention_prunes_raw_metrics_before_rollups() { assert_eq!(count(&pool, "metric_series_rollups").await, 1); } -/// Per-table windows (JEF-434): spans get a short window, logs a long one, and +/// Per-table windows: spans get a short window, logs a long one, and /// metric rollups fall back to the global default (no override at all) — each /// table must prune to its own cutoff, not the global one. #[tokio::test] @@ -3106,7 +3106,7 @@ async fn service_red_aggregates() { assert!((s["p50_ms"].as_f64().unwrap() - 25.0).abs() < 1e-6); } -// JEF-532: an explicit `from` far beyond the max-lookback ceiling must not +// An explicit `from` far beyond the max-lookback ceiling must not // defeat it — the effective floor is clamped, not honored verbatim, so a // full scan of the retention-deep `spans` table can't be forced. #[tokio::test] @@ -3172,7 +3172,7 @@ async fn services_from_beyond_max_lookback_is_clamped() { ); } -// JEF-546: same clamp as JEF-532's query_traces, extended to /api/logs — an +// Same clamp as query_traces' above, extended to /api/logs — an // explicit `from` far beyond the max-lookback ceiling must not defeat it, and // with no `from` at all a full scan (e.g. an ILIKE search for a rare/absent // term) must not walk the whole retention window either. @@ -3239,7 +3239,7 @@ async fn logs_attribute_filter() { assert_eq!(all.as_array().unwrap().len(), 2); } -// --- Origin-side Cloudflare Access JWT verification (JEF-473) --------------- +// --- Origin-side Cloudflare Access JWT verification (ADR 0013) --------------- // // The middleware guards the UI shell + /api when Access is configured, and never // guards /v1 ingest or /healthz. These prove the route policy end-to-end using a @@ -3428,11 +3428,11 @@ async fn access_unconfigured_leaves_api_open() { ); } -// --- MCP server + auth (JEF-471 / JEF-493) --------------------------------- +// --- MCP server + auth (ADR 0018 / ADR 0019) -------------------------------- // // Under Cloudflare Managed OAuth the edge resolves the MCP client's opaque OAuth // token and forwards the origin the standard `Cf-Access-Jwt-Assertion` JWT — the -// SAME header `/api` validates (JEF-473), but minted for a DEDICATED Access app +// SAME header `/api` validates (ADR 0013), but minted for a DEDICATED Access app // (its own AUD, distinct from the browser app's) and validated by the shared // `access_jwt::Verifier`. These tests reuse the browser-auth test key/JWKS but sign // with the MCP AUD, and prove the 401/200 matrix (fail-closed) and the fail-closed @@ -3679,7 +3679,7 @@ async fn mcp_lists_tools_and_calls_read_queries() { server.abort(); } -// Regression for JEF-494: a faceted gauge whose latest meta row is a rollup with a +// Regression: a faceted gauge whose latest meta row is a rollup with a // NULL `kind` (metric_series_rollups.kind is nullable) must return 200, not 500. The // meta query used to decode `kind` as a bare String, so "unexpected null" surfaced as // an unlogged 500 — breaking the chart page for e.g. k8s.container.restarts. diff --git a/ui/eslint.config.js b/ui/eslint.config.js index 158d1c7..ed17e36 100644 --- a/ui/eslint.config.js +++ b/ui/eslint.config.js @@ -1,4 +1,4 @@ -// Accessibility floor (JEF-431). A focused gate: only the jsx-a11y ruleset runs +// Accessibility floor. A focused gate: only the jsx-a11y ruleset runs // here so the dense-table interaction model can't silently regress into // mouse-only rows or unlabeled charts. The type-check gate stays `tsc --noEmit`; // this adds the a11y lint alongside it (see the ui job in ci.yml). diff --git a/ui/src/api.ts b/ui/src/api.ts index 1e603e2..b136e4f 100644 --- a/ui/src/api.ts +++ b/ui/src/api.ts @@ -111,8 +111,8 @@ export interface SeriesPoint { } // `from`/`to` (RFC3339) render an exact absolute window instead of the -// hours-back-from-now default — e.g. "metrics around this trace" -// (JEF-433). Either may be omitted; an omitted end falls back to the +// hours-back-from-now default — e.g. "metrics around this trace". +// Either may be omitted; an omitted end falls back to the // hours-relative bound. export const getMetricSeries = (p: { name: string; @@ -152,7 +152,7 @@ export interface FacetResponse { export const getMetricFacet = (p: { name: string; hours?: number; from?: string; to?: string }) => get(`/api/metrics/facet?${qs(p)}`); -// Raw points that carry a sampled trace exemplar (JEF-433) — a chart overlays +// Raw points that carry a sampled trace exemplar — a chart overlays // these as markers linking to the exact trace. Only ever covers the raw-metric // retention window (a few hours); older points never have exemplars, by design // (rollups aggregate them away). Correlational, not causal. diff --git a/ui/src/components/Alerts.chartHref.test.ts b/ui/src/components/Alerts.chartHref.test.ts index 24a5eb4..db0837c 100644 --- a/ui/src/components/Alerts.chartHref.test.ts +++ b/ui/src/components/Alerts.chartHref.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from "vitest"; import type { AlertRule } from "../api"; import { chartHref } from "./Alerts"; -// The rules-table primary cell is a (JEF-443). These +// The rules-table primary cell is a . These // tests lock the exact deep-link target — same chart + threshold/firing overlay // params the whole-row navigation built before — so the row-interaction refactor // can't silently move where a rule points. diff --git a/ui/src/components/MetricChart.tsx b/ui/src/components/MetricChart.tsx index 65d7dfc..c9e3dd5 100644 --- a/ui/src/components/MetricChart.tsx +++ b/ui/src/components/MetricChart.tsx @@ -35,7 +35,7 @@ interface Series { points: SeriesPoint[]; } -// One raw point that carries a sampled trace exemplar (JEF-433), positioned by +// One raw point that carries a sampled trace exemplar, positioned by // time + value so `Chart` can plot it as a marker on the line it belongs to. interface ExemplarMark { t: string; @@ -304,7 +304,7 @@ function FacetView({ } | null>(null); const [error, setError] = useState(null); // Best-effort — exemplars only exist in the raw-metric window, so an empty - // result is the normal case, not a failure (JEF-433). + // result is the normal case, not a failure. const [exemplars, setExemplars] = useState([]); useEffect(() => { @@ -447,7 +447,7 @@ export default function MetricChart({ const rangeLabel = RANGES.find((r) => r.hours === hours)?.label ?? `${hours}h`; // One-action deep link to the trace list scoped to this chart's service + - // displayed window (JEF-433) — the window is the picker's hours-back-from-now + // displayed window — the window is the picker's hours-back-from-now // range, matching what's actually plotted. Correlational ("traces in this // window"), not a claim that any one of them caused what the chart shows. const tracesHref = useMemo(() => { diff --git a/ui/src/components/TraceList.tsx b/ui/src/components/TraceList.tsx index 7843c54..d0ed911 100644 --- a/ui/src/components/TraceList.tsx +++ b/ui/src/components/TraceList.tsx @@ -23,7 +23,7 @@ export default function TraceList({ to }: { to: (traceId: string) => string }) { const [errorsOnly, setErrorsOnly] = useState(false); const [minDuration, setMinDuration] = useState(""); // An absolute `?from=&to=` (e.g. a metric chart's "traces in this window" deep - // link, JEF-433) overrides the picker's relative range so a chart's exact + // link) overrides the picker's relative range so a chart's exact // moment is reachable even for a window the picker's presets can't express. const [urlParams] = useSearchParams(); const absFrom = urlParams.get("from"); diff --git a/ui/src/components/TraceWaterfall.logic.test.ts b/ui/src/components/TraceWaterfall.logic.test.ts index 85f0d95..a920f20 100644 --- a/ui/src/components/TraceWaterfall.logic.test.ts +++ b/ui/src/components/TraceWaterfall.logic.test.ts @@ -53,7 +53,7 @@ describe("barGeometry", () => { describe("traceDurationMs", () => { it("converts the trace's µs window to ms, matching what fmtDuration expects", () => { - // JEF-533: the header passed the µs delta straight into fmtDuration + // The header used to pass the µs delta straight into fmtDuration // (which expects ms), overstating a 480ms trace as "480.00s" — ~1000×. const total = 480_000; // 480ms in micros, same basis as `total` in the component const ms = traceDurationMs(total); diff --git a/ui/src/empty.ts b/ui/src/empty.ts index 611bd0b..12ae9b7 100644 --- a/ui/src/empty.ts +++ b/ui/src/empty.ts @@ -1,5 +1,5 @@ // First-run empty states name the OTLP ingest endpoint so a fresh install knows -// exactly where to point its exporter (JEF-431). Centralised so the port/path +// exactly where to point its exporter. Centralised so the port/path // wording lives in one place rather than copy-pasted per signal. export function firstRunHint( signal: "traces" | "logs" | "metrics", diff --git a/ui/src/links.test.ts b/ui/src/links.test.ts index 942d474..25c76db 100644 --- a/ui/src/links.test.ts +++ b/ui/src/links.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { logsHref, traceHref } from "./links"; -// JEF-534: trace_id/span_id are ingested OTLP data (attacker-shapeable) and were +// trace_id/span_id are ingested OTLP data (attacker-shapeable) and were // interpolated raw into these deep-links, unlike the neighboring `service` focus // which was already `encodeURIComponent`-ed. These tests pin the well-formed-id // shape (no behavior change vs. the pre-fix raw interpolation) and lock the diff --git a/ui/src/links.ts b/ui/src/links.ts index 773bad3..db62a5b 100644 --- a/ui/src/links.ts +++ b/ui/src/links.ts @@ -2,7 +2,7 @@ // metrics). Trace/span ids come from ingested OTLP data — attacker-shapeable — // so every id here is either `encodeURIComponent`-ed as a path segment or set // via URLSearchParams: a stray `&`/`#`/`?` must stay inside its own param, -// never land on an unexpected route or inject an extra one (JEF-534). +// never land on an unexpected route or inject an extra one. // `/traces/:traceId`, optionally scoped to a span and/or the global service // focus. Used by the trace list row link, a log row's drill-in, and a metric diff --git a/ui/src/metricLabels.test.ts b/ui/src/metricLabels.test.ts index 01e839e..ef0a52e 100644 --- a/ui/src/metricLabels.test.ts +++ b/ui/src/metricLabels.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "vitest"; import { facetLabels } from "./metricLabels"; -// JEF-494: node/host metrics carry both the reporting agent's identity and the +// Node/host metrics carry both the reporting agent's identity and the // node/container they describe. Series must be labeled by the node/container, not // the agent, and uid/id-style keys stay suppressed. describe("facetLabels", () => { diff --git a/ui/src/metricLabels.ts b/ui/src/metricLabels.ts index d965bb2..5b8936b 100644 --- a/ui/src/metricLabels.ts +++ b/ui/src/metricLabels.ts @@ -7,7 +7,7 @@ export const shortKey = (k: string) => k.replace(/^k8s\./, ""); // (e.g. system.cpu.load_average, k8s.container.restarts) carries both the collector // agent's identity AND the node/container it describes; label by the latter. Without // this ordering the first ≤3 varying keys could pick an agent key ahead of the node, -// mislabeling every series by the reporting agent instead of the node (JEF-494). +// mislabeling every series by the reporting agent instead of the node. const IDENTITY_PRIORITY = [ "k8s.node.name", "host.name", diff --git a/ui/src/routes.a11y.test.tsx b/ui/src/routes.a11y.test.tsx index 7d243e9..8032fa1 100644 --- a/ui/src/routes.a11y.test.tsx +++ b/ui/src/routes.a11y.test.tsx @@ -1,6 +1,6 @@ -// Runtime a11y route-smoke (JEF-442). +// Runtime a11y route-smoke. // -// JEF-431 gave us the STATIC a11y floor (eslint-plugin-jsx-a11y). This is the +// The eslint config gives us the STATIC a11y floor (eslint-plugin-jsx-a11y). This is the // RUNTIME counterpart: mount each top-level route with a mocked API so it renders // its loaded state, then run axe-core over the rendered tree. Static linting can't // see ARIA that only resolves against the live tree, focus/role structure of a @@ -17,7 +17,7 @@ // // The assertion is on axe violations by IMPACT, never on specific role/element // markup, so it stays green regardless of in-flight row-markup changes (e.g. -// JEF-443's Alerts row rework). +// the Alerts row rework). import { describe, expect, it, vi } from "vitest"; import { render, screen } from "@testing-library/react"; import { MemoryRouter } from "react-router"; @@ -275,9 +275,9 @@ describe("runtime axe route smoke", () => { it("service map has no serious/critical a11y violations", async () => { const { container } = render(renderAt("/map")); // The map is an labelled with the node/edge/call counts — - // a grouping, NOT role="img", so its focusable node-buttons (JEF-431) are + // a grouping, NOT role="img", so its focusable node-buttons are // legitimately interactive children. This resolves the pre-existing - // `nested-interactive` violation that role="img" caused (JEF-455), so the + // `nested-interactive` violation that role="img" caused, so the // rule is now enforced here too — no per-route waiver. await screen.findByRole("group", { name: /Service map/ }); await expectNoBlockingViolations(container); @@ -286,8 +286,8 @@ describe("runtime axe route smoke", () => { it("alerts has no serious/critical a11y violations", async () => { const { container } = render(renderAt("/alerts")); // Alerts renders two tables (rules + recent events); wait for them, then - // assert on axe impact — not on the row markup. JEF-443 reworks these rows in - // parallel, so this must pass whichever version of Alerts.tsx is on main. + // assert on axe impact — not on the row markup, so this passes regardless + // of markup changes to Alerts.tsx. await screen.findAllByRole("table"); await expectNoBlockingViolations(container); }); diff --git a/ui/src/styles.css b/ui/src/styles.css index b6b1045..b202002 100644 --- a/ui/src/styles.css +++ b/ui/src/styles.css @@ -339,7 +339,7 @@ tr.clickable:hover td { } /* Error spans carry the red into the label too, not just the bar — legible - at a glance without hunting across the row (JEF-432). */ + at a glance without hunting across the row. */ .bar-label.err { color: var(--error); } @@ -494,7 +494,7 @@ a.xlink:hover, font-size: 10px; } -/* Exemplar markers (JEF-433): a hollow ring, ink on hover/focus — data-colored +/* Exemplar markers: a hollow ring, ink on hover/focus — data-colored like everything else in the chart, not a decorative accent. */ .exemplar-dot { fill: var(--bg); @@ -609,7 +609,7 @@ button.range.active { padding: 0.2rem 0; } -/* JEF-430 — alerts view: firing badge, clickable rules, breach strip. */ +/* Alerts view: firing badge, clickable rules, breach strip. */ /* Nav firing count: colored text (not a pill), per ADR 0009. */ .firing-count { @@ -624,7 +624,7 @@ button.range.active { } /* Rules deep-link to their metric chart via a primary-cell - (JEF-443, matching TraceList/Services/MetricList); the row hover-scent comes from + (matching TraceList/Services/MetricList); the row hover-scent comes from the shared tr.clickable rule. */ /* Per-rule breach small-multiple: red ticks on a hairline baseline. */ @@ -644,7 +644,7 @@ button.range.active { white-space: nowrap; border: 0; } -/* ── JEF-431: accessibility floor ────────────────────────────────────────── +/* ── Accessibility floor ──────────────────────────────────────────────────── Keyboard-operable rows/nodes, one ink focus ring, a polite status line. This block is grouped + appended so it 3-way-merges cleanly alongside other work. */