chore(planning): adopt decision records; record metrics-recorder non-unification (review #4)#112
Merged
Merged
Conversation
…corder non-unification Add the lightweight decision-record convention from modern-di: a decision.md template, index.py support (lists decisions newest-first), and README/CLAUDE.md docs. Record candidate #4 from the 2026-06-23 architecture review (unify the two metrics recorders behind a shared event->metric table) as rejected — the per-event bodies are irreducibly backend-specific, so a shared table is a shallow abstraction with negative locality — with a revisit trigger. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adopt the lightweight decision-record convention from
modern-di(planning/decisions/), and use it to record candidate #4 from the 2026-06-23 architecture review as rejected.Candidate #4 proposed unifying the two
MetricsRecorderadapters (PrometheusRecorder,OpenTelemetryRecorder) behind a shared data-driven event→metric table. On inspection that doesn't earn its keep, so rather than build a shallow abstraction we record the decision (with a revisit trigger) so a future review doesn't re-propose it.Why #4 was rejected
The two
__call__switches share only the event-name dispatch ladder; the per-event bodies are irreducibly backend-specific:dispatched→ Prometheus does 3 ops (counter + size histogram + in-process gauge.inc); OpenTelemetry is a no-op.acked/nacked_*→ Prometheus uses separate labeled counters + gauge.dec; OTel foldsreason/exceptioninto attributes on one shared histogram.lease_lost→ Prometheus reusesprocessed_total{status=error}; OTel doesn't.fetched→ Prometheus has anon_emptylabel; OTel has no such attribute.A
(metric kind, name, label set)table can't express this (different instrument counts per backend; label-vs-attribute placement differs; cross-event reuse). Deletion test: a shared table only removes the trivial dispatch ladder and pushes the bodies back where they are — a shallow abstraction that worsens locality. Full rationale + revisit trigger in the decision record.What this adds (convention)
planning/_templates/decision.md— ported from modern-di.planning/index.py— listsplanning/decisions/*.mdnewest-first under a## Decisionssection (just index).planning/README.md+CLAUDE.md— document the convention.planning/decisions/2026-06-23-metrics-recorders-not-unified.md— the decision.No package code changed.
just lint-ciclean.🤖 Generated with Claude Code