Files
DodoSSH/docs/adr/0006-observability-stack.md
jaap-jan ce43f397a6 Add ADRs 0001-0006 and README (M0)
Records the decisions the milestone plan already made, with their costs stated rather
than only their benefits:

- 0001 e2ee-trust-model: key hierarchy, the AAD-to-row binding that stops the server
  moving ciphertext between rows, and the four-layer public-key trust story. States
  plainly that revocation is not retroactive, that Connect cannot be a security
  boundary, and that the IdP becomes a key-distribution trust root.
- 0002 minimal-apis: feature modules with explicit registration; capability negotiation
  instead of Asp.Versioning, since client and server upgrade independently when
  self-hosted.
- 0003 sync-protocol: single write path, revision cursors, and the bigserial
  pre-commit sequence gap that silently corrupts sync — plus the per-vault advisory
  lock that fixes it and the test that must prove it.
- 0004 relay-authorization: relay forwards bytes rather than terminating SSH, so
  zero-knowledge survives; server-resolved target IPs in the ticket to defeat DNS
  rebinding; why host addresses must be plaintext when relay is enabled.
- 0005 no-application-layer: why the usual Application/mediator layer earns nothing
  here, with the trigger that would make us revisit it.
- 0006 observability-stack: OTel plus built-in ILogger; liveness excludes dependencies
  so a database blip cannot restart the container and kill live SSH sessions.

Also adds a README covering layout, build, enforced conventions and milestones.
2026-07-28 12:28:44 +02:00

72 lines
4.4 KiB
Markdown

# ADR 0006 — OpenTelemetry with built-in ILogger, not Serilog
- Status: accepted
- Date: 2026-07-28
## Context
The server is self-hosted by people who did not write it and who will debug it themselves. It
also handles data it cannot read, which changes what observability can and should capture.
## Decision
**Logging:** the built-in `ILogger` with source-generated `[LoggerMessage]` partial methods.
No Serilog. `AddJsonConsole` outside Development; self-hosters capture container stdout.
**Telemetry:** OpenTelemetry via `OpenTelemetry.Extensions.Hosting` with ASP.NET Core, HttpClient
and runtime instrumentation, Npgsql's own `ActivitySource`, and custom sources `DodoSSH.Api`,
`DodoSSH.Sync`, `DodoSSH.Relay`. OTLP exporter honouring the standard `OTEL_EXPORTER_OTLP_ENDPOINT`
and `OTEL_SERVICE_NAME` variables, because self-hosters expect them. Sampling
`ParentBased(TraceIdRatioBased(0.1))` by default, with tail sampling for errors done in the
collector. Optional Prometheus scrape on a separate port, off by default.
**Redaction is a hard requirement, not a nicety.** `Microsoft.Extensions.Compliance.Redaction`
with `[PrivateData]` on DTO properties, plus an `ActivityProcessor` that strips `url.query` — the
relay ticket fallback lives there ([ADR 0004](0004-relay-authorization.md)) — and drops
`Authorization` and `Idempotency-Key`. Sync request bodies are never logged: they are ciphertext,
but size and shape still leak.
**Health:** `/healthz/live` checks the process only. `/healthz/ready` additionally checks
PostgreSQL, OIDC discovery and JWKS reachability, the Data Protection key ring, and that no
migrations are pending.
## Consequences
- One telemetry pipeline instead of two configuration systems. OTel logs are first-class in
.NET 10, so Serilog's usual payoff — sinks and enrichers — is redundant once a collector is in
the compose file, and dual configuration is a real support burden ("why is my log level being
ignored?"). If someone wants file logs, that is the collector's file exporter, not a second
logging framework.
- `[LoggerMessage]` is allocation-free and produces structured events by construction. CA1848 is
a warning during early development and is raised to error when the logging pass lands in M4.
- **Liveness deliberately excludes dependencies.** A transient PostgreSQL outage must not cause
the orchestrator to restart the container, because that would kill every live relay session for
a fault that has nothing to do with them. Readiness only removes the instance from load
balancing, which is the correct response.
- **Audit is a separate concern from logs.** `audit_event` is a partitioned, append-only table
(`REVOKE UPDATE, DELETE` from the application role) with a per-day hash chain. It records who,
when, which item id, which operation, outcome and source — plus non-secret shape data such as
`fieldsChanged` and `payloadBytes`. Its `detail` jsonb must contain ids, counts and *field
names* only, enforced by a serializer whitelist and a test.
- Given no plaintext is visible, audit is nonetheless strong on the thing that matters: every
ciphertext fetch is recorded, and there is no server-side path that reads a secret without a
client fetch. Bulk fetch of an entire vault is a high-signal exfiltration indicator and should
alert.
- Honest limit on tamper evidence: a compromised server can rewrite the chain from any point
forward unless heads are anchored externally. Publishing the daily head to clients, which cache
it, makes truncation detectable — the same trick as the key log in
[ADR 0001](0001-e2ee-trust-model.md).
- Human-readable audit is a **client** concern: rows carry `subject_id`, and the client, which
holds the keys, resolves ids to names.
- Behind a reverse proxy, `UseForwardedHeaders` needs `KnownProxies`/`KnownNetworks` configured.
**Log a startup warning if forwarded headers are enabled with no known proxies**, because
self-hosters will get this wrong and it silently turns per-user rate limiting into per-proxy.
### Rejected
- **Serilog.** Excellent library; the second configuration system is the problem, not the code.
- **Application Insights or another vendor SDK.** Wrong for a self-hosted product. OTLP lets the
operator point at whatever they already run.
- **Logging request bodies for sync.** Ciphertext, so it looks harmless, and it leaks item sizes
and access patterns while ballooning log volume.