feat(intent_algebra): thread the actual item filter value into Frequency (Phase 3a) - #415
Merged
Merged
Conversation
Phase 3a of the named-key PointCount gap (workload-driven half only -- the PromQL-string bare-count() recognition half needs a new metric->item_label lookup mechanism threaded through parse_query_expr_canonical's 18 call sites and is deferred separately). frequency(accuracy, item: Option<(String, String)>) now embeds item_label/item_value into the Extension payload when a specific item is being filtered on -- ControlPlaneCostModel::readout_extension (added in the Frequency-realization PR) already reads these same keys back out, so this is the missing half of that wiring, not new payload shape. default_frequency() stays 0-arg (passes None), so the two PromQL-driven lower.rs call sites are unaffected -- only the 4 other direct frequency() callers (3 tests, 1 real) needed updating for the new parameter. bind_workload_typed gains a sibling, bind_workload_typed_with_item_filter, rather than adding an item_label field to QueryWorkload (which would break its 30+ struct-literal construction sites with no Default impl). Wired at the one real call site that already has both pieces needed in scope without any new lookup: emit::collect_metric_to_family's loop already walks WorkloadEntry (item_label) and QueryWorkload (label_filters) together. Co-Authored-By: Claude Sonnet 5 <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
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
Phase 3 (of 5) of the named-key
SketchQuery::PointCountgap — workload-driven half only. Based directly onmain(#162/#413/#414 already merged).Split from the original Phase 3 scope after investigation: the PromQL-string-driven half (recognizing a bare
count(metric{item="x"})and resolvingitemagainst a metric's configured item-label at parse time) needs a new metric→item_label lookup threaded throughparse_query_expr_canonical(18 call sites) andQueryWorkload(31 struct-literal sites, noDefault) — a genuinely larger, separate task, still deferred. This PR does the achievable, low-risk half.frequency(accuracy, item: Option<(String, String)>)now embedsitem_label/item_valueinto theExtensionpayload when a specific item is being filtered on.ControlPlaneCostModel::readout_extension(from feat(sketch_algebra): realize Frequency (AggIntent::Extension) as a real Cms sketch #413) already reads these exact keys back out — this closes the other half of that wiring, not a new payload shape.default_frequency()stays 0-arg (passesNone), so the two PromQL-drivenlower.rscall sites (agg_func_to_intents) are untouched — only the 4 other directfrequency()callers (3 tests + 1 real) needed updating for the new parameter.bind_workload_typedgains a sibling,bind_workload_typed_with_item_filter, rather than adding anitem_labelfield toQueryWorkloaditself (which would break all 30+ struct-literal construction sites, noDefaultimpl to fall back on).emit::collect_metric_to_family's loop already walksWorkloadEntry(hasitem_label) andQueryWorkload(haslabel_filters) together.Test plan
cargo test -p control_plane --lib— 766 passed, 1 failed (the same single pre-existing, unrelated failure present on unmodifiedmain).bind_workload_typed_with_item_filter_threads_the_actual_value— proves both directions: no filter still reads out as the bare bucket total (PointCount{key: SampleValue, value: None}, unchanged), and a real(label, value)filter reads out asPointCount{key: Named(label), value: Some(value)}.cargo build --workspace— clean.cargo clippy -p control_plane --lib --tests— verified every warning falls outside this diff's touched line ranges (all pre-existing).rustfmtscoped to touched files only.Remaining deferred: the PromQL-string bare-
count()recognition half, and the overarching goal (wiringSummaryExecutorinto the live serving path, retiringsketch_reducer.rs).🤖 Generated with Claude Code