Declare named and filtered thresholds in the v1 report payload - #1019
Conversation
|
| Project | Bencher |
| Branch | u/ep/parameters-api/thresholds-payload |
| Testbed | intel-v1 |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result microseconds (碌s) (Result 螖%) | Upper Boundary microseconds (碌s) (Limit %) |
|---|---|---|---|
| Adapter::Json | 馃搱 view plot 馃毞 view threshold | 5.25 碌s(+4.64%)Baseline: 5.02 碌s | 6.10 碌s (86.12%) |
| Adapter::Magic (JSON) | 馃搱 view plot 馃毞 view threshold | 5.06 碌s(+4.45%)Baseline: 4.85 碌s | 5.77 碌s (87.68%) |
| Adapter::Magic (Rust) | 馃搱 view plot 馃毞 view threshold | 27.12 碌s(+0.77%)Baseline: 26.91 碌s | 30.13 碌s (90.01%) |
| Adapter::Rust | 馃搱 view plot 馃毞 view threshold | 4.81 碌s(+12.73%)Baseline: 4.27 碌s | 6.59 碌s (73.02%) |
| Adapter::RustBench | 馃搱 view plot 馃毞 view threshold | 4.82 碌s(+13.00%)Baseline: 4.26 碌s | 6.58 碌s (73.20%) |
4a5328c to
2ad64f8
Compare
2ad64f8 to
75903f9
Compare
75903f9 to
296d96c
Compare
296d96c to
f813b3f
Compare
Review after the rebase onto develDevel retired the BMF project version gate: a project's Major: the inherited project version picks the thresholds shape and the reach of
|
f813b3f to
297a2e7
Compare
297a2e7 to
5156f04
Compare
206d8a4 to
e57aa05
Compare
1a5e8cb to
cd769aa
Compare
cd769aa to
c09dc4f
Compare
c09dc4f to
9dcb998
Compare
A report has always carried its thresholds as a map of measure to model, and a map key names a measure and nothing else. Every threshold a report could declare was therefore the bare one: the conventional `value` name of every variant. A pipeline that wanted `p99` watched, or wanted only some variants watched, had to reach for the thresholds endpoint and then keep it in step with the run by hand. At BMF version 1 `thresholds.models` is a list. An entry is `parameters`, `measure`, `metric`, and `model`: the dimensions a threshold hangs off that the report does not already state, in their canonical order, and the model to check with. `measure` is the same name, slug, or UUID the map key is today, and a name or slug the project has never seen creates the measure. `metric` is required, and `value` names the conventional metric. An absent `parameters` checks every variant, so an entry naming only a measure, `value`, and a model is the map pair written out longhand. One measure may carry several entries, and each one creates or updates the threshold with those dimensions under the report's branch and testbed, through the same null collapse and canonical filter storage every other writer goes through. A threshold the report declares checks the very report that declared it. Nothing moved to make that true: thresholds are resolved before results are parsed, which is where they already were. It is worth saying out loud because it is what makes the list worth having, and it is pinned by a test whose history is five unchecked reports and whose alerts all belong to thresholds that did not exist when the request arrived. The report's BMF version, the one it declares or else the project's default, says which shape to expect, and the shape is checked rather than guessed at. A list at version 0 and a map at version 1 are both a 400 naming that version and the shape it calls for, a list whose entry is malformed is refused first by that entry's field error, and the check runs before anything is created for the report. `reset` means one thing at both versions: it takes the model away from every threshold on the report's branch and testbed that the payload does not name, including a payload that names none at all. A version 0 reset therefore reaches the named and filtered thresholds its map has no way to spell, where before it reached only the bare ones. One set of dimensions declared twice in one version 1 list is one threshold: the position is where it was first written and the model is what it was last told, resolved without an error. Two spellings of one filter address one threshold, because a filter canonicalizes before it is compared. A version 0 map merges the same way, so a measure keyed by both its name and its UUID is one threshold rather than a failed insert, and which of its two models wins is not defined. The creates a payload plans count against the threshold creation ceiling after they merge, in one check whichever shape they arrived in. Two shapes behind one key is a place where error quality quietly dies. The obvious spelling, an untagged enum, buffers the input, tries each variant, and on failure says only that nothing matched, so a misspelled model test in a version 0 map would come back as "data did not match any variant" rather than as the field and the variants it could have been. The shape is known from the first token, so it is decided by looking rather than by trying: every error a malformed version 0 map got before this layer it gets after it, byte for byte, and a malformed version 1 entry is named by its position and its field. Two messages move: a `models` that is neither shape now names both shapes it expects, and a list sent at version 0 is refused by the shape check, or by its malformed entry's field error, instead of by serde's `expected a map`. The CLI still declares only bare thresholds, which is what the version 0 map spells, and the payload it sends is byte identical. `--thresholds-reset` follows `reset`, so it now strips named and filtered thresholds too, and its help no longer carves them out. Naming a metric or a variant from the command line is a separate piece of work.
9dcb998 to
c7b7de8
Compare
Built on #1075, which counts a report's new thresholds against the creation ceiling. The entries this layer adds are more creates in that same count.
The list shape
A report has always carried its thresholds as a map of measure to model, and a map key names a measure and nothing else. Every threshold a report could declare was therefore the bare one: the conventional
valuename of every variant.At BMF version 1
thresholds.modelsis a list:{ "bmf_version": 1, "thresholds": { "models": [ { "measure": "latency", "metric": "p99", "model": { "test": "t_test", "upper_boundary": 0.98 } }, { "parameters": [{ "size_mb": 16 }], "measure": "latency", "metric": "value", "model": { "test": "percentage", "upper_boundary": 0.25 } } ] } }An entry is
parameters,measure,metric,model: the dimensions a threshold hangs off that the report does not already state, in their canonical order, and the model to check with.measureis the same name, slug, or UUID the map key is today, and a name or slug the project has never seen creates the measure.metricis required, andvaluenames the conventional metric; thevaluedefault is only for version 0's map. An absentparameterschecks every variant, so an entry naming a measure,value, and a model is the map pair written out longhand. One measure may carry several entries, and each creates or updates the threshold with those dimensions under the report's branch and testbed, through the same null collapse and canonical filter storage every other writer goes through.A threshold the report declares checks the very report that declared it. Nothing moved to make that true: thresholds are resolved before results are parsed, which is where they already were. It is pinned by a test whose history is five unchecked reports and whose alerts all belong to thresholds that did not exist when the request arrived.
One shape per version
The report's BMF version, the one it declares or else the project's default, says which shape to expect, and only that shape is accepted. A list at version 0 and a map at version 1 are both a 400 naming that version and the shape it calls for, an empty map at version 1 included, while a list whose entry is malformed, an entry with no
metricincluded, is refused first by that entry's field error. The check runs before anything is created for the report. A project whosebmf_versionis 1 takes its thresholds as a list, and a test pins that a report declaring no version takes its shape from the project.One set of dimensions is one threshold
One set of dimensions declared twice in one version 1 list is one threshold: the position is where it was first written and the model is what it was last told, resolved without an error. Two spellings of one filter address one threshold, because a filter canonicalizes before it is compared. A version 0 map merges through the same path, so a measure keyed by both its name and its UUID is one threshold rather than a failed insert; which of its two models wins is not defined. The creates a payload plans count against the threshold creation ceiling after they merge, in one check whichever shape they arrived in.
Reset
resetmeans one thing at both versions: it takes the model away from every threshold on the report's branch and testbed that the payload does not name, including a payload that names none at all. A version 0 reset therefore reaches the named and filtered thresholds its map has no way to spell, where before it reached only the bare ones.resetstays on the report's own branch and testbed.Adjudicated calls worth a second opinion
modelrather than flattening it.JsonNewThresholdflattens itsModel, which is the sibling precedent. The entry is the map pair unrolled into fields, and the map's value is the model, somodelis a key here. It also keeps the entry legible besidemetricandparameters.serde(untagged). An untagged enum buffers the input and, on failure, discards every inner error for one opaque sentence. The two shapes are distinguished by the first JSON token, so the visitor dispatches a map to the map's own deserializer and a list to the list's, and their field-level errors survive verbatim, path prefixes included. Two messages change. Amodelsvalue that is neither shape is now told the field expects a map of measure to threshold model (BMF version 0) or a list of threshold entries (BMF version 1); the old text named only the map, and a test pins the new sentence. A list sent at version 0, which serde used to refuse asinvalid type: sequence, expected a map, now parses and is refused by the shape check, or by its entry's field error when an entry is malformed.oneOf, not a derivedanyOf. A derived untagged enum emitsanyOf, and the client generator turns ananyOfof two subschemas into one struct of flattened optional members. A list cannot be flattened into a struct.oneOfis also what the shapes are: mutually exclusive. The generated client gets a clean untagged enum withMapandListarms, named from the schema titles.models: models.map(JsonReportThresholdModels::Map)), and the payload it sends is byte identical. The CLI declares bare thresholds, which is what the version 0 map spells, so it does not yet send thresholds to a project whosebmf_versionis 1.--thresholds-resetfollowsreset, and its help no longer carves out named and filtered thresholds. Naming a metric or a variant from the command line is a separate piece of work.Version 0 behavior
Version 0 compatibility was capture-proven at
2927c700: the map ingest, map update, and map-with-reset responses, and the malformed-map 400 bodies with their field paths, were captured on the base branch and on that head and diffed empty, apart from the two messages above. Since then two version 0 behaviors changed on purpose, each pinned by a test:resetreaches named and filtered thresholds, and a map that names one measure twice merges instead of failing on the unique index.Gates
Locally, at
749c1a55on devel at #1077's merge:cargo fmt -- --check,cargo clippy --no-deps --all-targets --all-features -- -Dwarnings,cargo nextest run --all-features --profile ci -p bencher_schema -p api_projects -p bencher_json -p bencher_adapter -p api_run -p api_runners -p bencher_cli -p bencher_comment, doc tests,cargo check --no-default-features, andcargo gen-typeswith only the intendedopenapi.jsonchange. Rebased since onto #1083's merge as9dcb9989, where fmt and clippy pass. No migration in this layer.