#368 feat(audit): add OpenTelemetry AuditSink (closes #245)

[Anonymous]

Originally created by: Maqbool61

Summary

Implements the OpenTelemetry AuditSink described in issue [#245]. Ships redacted decision records to any OTLP/HTTP-compatible collector (Grafana Alloy, OpenTelemetry Collector, Honeycomb, Datadog Agent, etc.) using the same isolation discipline as the webhook sink (#233).

Changes

• src/doberman/storage/otel_sink.py — OtelAuditSink implementation
• tests/unit/test_otel_sink.py — 27 unit tests covering all required invariants
• docs/audit_otel.md — collector setup docs with a working docker example

Behaviour

• emit() is enqueue-only on the decision path — returns in O(1), no network I/O, no blocking, no exception propagation
• A background daemon thread drains the queue and POSTs via OTLP/HTTP (/v1/logs, JSON)
• Queue overflow drops oldest and increments drop_count — never grows unbounded
• Config-gated by .doberman/audit_otel.yaml (endpoint, optional auth_env, timeout_s, queue_max). Absent or malformed config → sink is completely inert, zero network I/O
• Auth token read from a named env-var at send time — never stored in YAML, never logged
• Exports only the allowlisted fields (timestamp, verdict, tool, reason_codes, explanation, session_id) — filtered both in emit() and again in the payload builder as defence-in-depth. Adds no fields of its own
• Stdlib only (urllib.request, json, threading, queue) — no opentelemetry-sdk dependency required

Tests prove

• emit() never blocks or raises into the decision path (worker paused, timed)
• Queue overflow drops-and-counts rather than growing unbounded
• Non-allowlisted fields are stripped; sink adds no fields of its own
• Absent config → is_active == False, zero threads started, urlopen never called
• Synthetic secret (AKIAIOSFODNN7SYNTHETIC) never appears in any exported payload
• Auth token sourced from env-var; absent var sends no header; token never appears in logs
• OTLP payload is valid JSON with correct resourceLogs / scopeLogs / logRecords shape

Security checklist

• [x] Fails closed on error / uncertainty — worker errors are logged at DEBUG and discarded, never propagated
• [x] No secret, full file, or unredacted prompt logged or committed
• [x] Raise-only — sink adds no loosening of any policy or guardrail
• [x] Auth token never stored in config, never logged, read from env-var at send time only

Closes [#245]

[Anonymous]

Originally posted by: Maqbool61

Fixing those failing tests you will see more commits from my side.

[Anonymous]

Originally posted by: Maqbool61

All three points addressed. close() now uses _state_lock/_active/_stop_event mirroring WebhookAuditSink exactly, with six lifecycle tests. OtelAuditSink is wired into emit_to_sinks() via the same module-level cache pattern as the webhook sink, with one test proving a record reaches it through the real path. Endpoint validation now uses _is_loopback() and rejects non-http(s) schemes, with nine tests covering all cases.

[Anonymous]

Originally posted by: Maqbool61

All points from the review addressed.

Allowlist / fixtures

_ALLOWED_FIELDS is now derived from build_record() in storage/log.py:
ts, action_type, risk, final_verdict, decided_layer, reason_codes, auth_result, session_id.
The four stale names (timestamp, verdict, tool, explanation) are gone.

The _record() test helper is replaced by _build_record() which mirrors the exact shape
build_record() emits, including all non-exportable fields (action_id, agent_role,
target_path_class, auth_required, elevation_id, entity_id). The suite now validates
the sink against the real producer, not against itself.

Two guard tests make this durable:

• test_allowed_fields_are_subset_of_build_record_keys — every name in _ALLOWED_FIELDS must
  exist in a real _build_record() output. A future rename in log.py breaks this test instead
  of silently emptying the export.
• test_old_field_names_are_gone — asserts the stale names are not in _ALLOWED_FIELDS.

On the judgment call: included decided_layer and auth_result — both are in build_record(),
both are redaction-safe.

Docs / loopback

docs/audit_otel.md example updated from localhost:4318 to otel-collector.internal:4318.
Added one paragraph explaining why loopback is rejected: an accidental loopback config silently
discards every export; a non-loopback host makes the misconfiguration visible.

[Anonymous]

Originally posted by: fu351

Merged — this round landed it. Deriving _ALLOWED_FIELDS from what build_record() actually emits, mirroring the webhook sink's _state_lock/_active lifecycle instead of inventing a second pattern, and proving the wiring through the real emit_to_sinks() path were exactly the right three moves. The stale-names canary test (timestamp/verdict/tool/explanation must never reappear) is a nice touch — that is the bug class this review was about, now permanently fenced.

Full disclosure: I pushed one commit (a499ebf) to your branch before merging, all remnants of the field rename. The OTLP payload builder still read safe.get("timestamp"), which no longer exists, so timeUnixNano silently used wall-clock time instead of the record's ts. The module docstring and the docs table still listed the old field names. A few WebhookAuditSink docstrings had been dropped along the way, which I restored. I also pointed the allowlist subset test at the real storage.log.build_record() — not just this file's mirror of it — so a future rename in log.py fails the suite instead of silently emptying the export. That is the same failure mode you just fixed, now closed for good.

If you want the next one: [#192] (level-5) — define the shared JSON-output contract across the CLI's JSON commands, in docs + tests. After what you just went through with field names, you are exactly the right person to pin that contract down. Yours if you would like it.

[Anonymous]

Ticket changed by: fu351

• status: open --> closed