Proposes ADR-0006: telemetry and observability for the package - #24
Merged
Conversation
ADR-0006, at status proposed, answers ADR-0002's open question 3: it fixes the package's telemetry surface before any of it is written, as the repository's ADR-first rule requires. Ten event names under [:encryptor, ...], defined once in an Encryptor.Telemetry module, split into four point events and three operation spans plus a provider span. Measurements are numbers and metadata is a closed allow-list - the inversion of st-ADR-0040, which can afford to let structs ride in metadata because its worst case is a verbose span rather than a data key in a log line. The record states what is never emitted and why each item: plaintext, any key material, an encryption context value, the raw :key selector, and the cache partition id, which ADR-0001 decision 7 correctly calls non-secret but which is an unkeyed hash of the selector and so is confirmable by guess-and-confirm. No event carries a per-tenant dimension. ADR-0001 decision 10's oracle collapse holds in telemetry too, and harder, because an event reaches every attached handler rather than the caller who made the call. Written against the code on main rather than the implementation graph: four of the ten events can be emitted today, and the record says which six specify paths that do not exist yet. It also declines to promise a cache hit rate, because LocalCache's table is private and the decision is three frames deep in the engine. Seven open questions, including whether the provider span counts calls or cold misses - a seam ADR-0001 decision 2 and ADR-0002 decision 2 describe differently, and one the encrypt path owns. Refs: enc-wpy
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
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.
What this is
ADR-0006, at status proposed, answering ADR-0002's open question 3, which
declined to decide telemetry and asked for "a record of its own covering the
whole package". Design only: nothing is implemented, no dependency is added,
and acceptance is the operator's call as it was for the founding five.
The shape of the decision
Ten event names under
[:encryptor, ...], defined once in anEncryptor.Telemetrymodule with anevents/0that is the single definitionsite:
[:encryptor, :vault, :started | :stopped | :start_refused]and[:encryptor, :cache, :recycled]encrypt,decrypt,rekey{:key_unavailable, _}intothe metric the bead asks for
Naming rule:
:start/:stopname a span pair and nothing else; every pointevent is past tense.
The part that matters
Measurements are numbers and metadata is a closed allow-list. This is the
one place the record departs from the family precedent rather than inheriting
it. st-ADR-0040 lets the raw effect struct and datamodel values ride in
metadata for in-VM consumers and draws the redaction line at the bridge, which
is right there, where the worst case is a verbose span. It is not available
here, where the worst case is a data key in a log line.
So the record states what is never emitted, and why each one:
sizeis the only thing said about it, on ADR-0004decision 12's argument that the ciphertext already discloses the length
Configis ever ametadata value, so there is no field for it to ride in
caller already holds)
:keyselector - ADR-0004's acceptance amendment 1 exists becausepublishing the raw tenant identifier voided ADR-0003 decision 5's keying,
and a metrics vendor is a worse place for it than a header
get wrong: ADR-0001 decision 7 truthfully says it is not secret, and it is
also an unkeyed SHA-256 of the selector, so it is confirmable by
guess-and-confirm - ADR-0004 open question 1's own argument, applied to the
other unkeyed derivation in the package
Encryptor.Error's:engineterm, because a handler is not a log lineShortest form of the rule, and the one for the moduledoc: no event carries a
per-tenant dimension, keyed or unkeyed.
ADR-0001 decision 10's oracle collapse holds in telemetry too, and harder: an
event reaches every attached handler whether or not anyone asked, so a
decrypt-side event that distinguished a context mismatch from a wrong key
would be a worse oracle than an error return. The two failures ADR-0002
decision 6 carved out -
:unknown_keyand:key_unavailable- stay distincthere for the same reason they do there.
Written against the code on main, not the plan
Four of the ten events can be emitted against
92886bctoday; the record saysplainly that the other six specify paths that do not exist yet (
encrypt/2,decrypt/2andrekey/2are undefined, and nothing inlib/calls aprovider callback). Specifying ahead is deliberate - without the record, the
encrypt-path bead either emits nothing or invents a vocabulary.
It also declines to promise a cache hit rate.
LocalCache's ETS table is:privateand the hit-or-miss decision is three frames deep in the engine,which is the same fact ADR-0001 decision 6 gave as the reason the recycler
exists at all. That becomes a third upstream ask rather than a wrapper CMM
whose only job is to count.
Open questions worth a reader's attention
says the materials cache "collapses provider round trips to one per
partition per
max_age"; ADR-0001 decision 2 says encrypt and decrypt"build the engine's keyring, CMM, and
Clientstructs per call", and akeyring needs descriptors, which needs the provider. Those are not
obviously compatible. The seam belongs to the encrypt path, so decision 8
is deliberately neutral and stays correct either way - but it is a real
tension between two accepted records and it is flagged, not resolved here.
sizeshould exist at all (open question 5 in the record) is adisclosure judgement rather than a mechanism question, and it is the
operator's: a length distribution over a column of short encrypted values
is not nothing.
Seven in the record in total; the rest are narrower (the upstream hit-rate
ask, an optional dependency, an opt-in keyed dimension, whether
:start_refusedis reachable in the case it is for, and whether anyone writesa bridge).
Gate
Docs only -
gate.rbreportsapplicable: false,carve_out_reason: "no changes under lib/, test/, config/, mix.exs, mix.lock - nothing for the gate to measure". No Elixir code changed, no dependency added, no changelogfragment (the fragments README excludes ADRs).
Refs: enc-wpy