Public Access
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.
72 lines
4.4 KiB
Markdown
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.
|