Support calculated (ephemeral) measures via a new expression measure compute - #9855
Conversation
…compute Adds an expression compute to MetricsViewAggregationMeasure that derives ad-hoc measures from existing metrics view measures via a whitelisted arithmetic expression, compiled server-side as a derived measure so field access policies apply through the referenced measures. Also extends MetricsViewTimeSeries with expression measures, includes expression refs in query field analysis for alert/report security inference, and accepts calculated_measures renderer props in canvas component validation.
| message MetricsViewTimeSeriesRequest { | ||
| string instance_id = 1; | ||
| string metrics_view_name = 2 [(validate.rules).string.min_len = 1]; | ||
| repeated string measure_names = 3 [(validate.rules).repeated.min_items = 1]; | ||
| // Names of measures in the metrics view to include. | ||
| // At least one of measure_names or measures must be provided. | ||
| repeated string measure_names = 3; | ||
| // Optional additional measures, e.g. ephemeral measures with an `expression` compute. | ||
| // Only the `expression` compute is supported for time series. | ||
| repeated MetricsViewAggregationMeasure measures = 17; |
There was a problem hiding this comment.
I actually thought the UI had completely moved over to MetricsViewAggregtion. If not, maybe consider if it would be simpler to finish the migration than to introduce computed measures into the legacy timeseries API.
There was a problem hiding this comment.
IIRC there were performance issues when we moved off it. Keeping it for now to unblock this PR; noted to move off separately.
| // Calculated (ephemeral) measures for the explore, serialized in the URL | ||
| // param format (frontend-only; persisted in URL state). | ||
| optional string calculated_measures = 40; |
There was a problem hiding this comment.
Subjective, but consider calling them ephemeral_measures in code instead. The word "calculated" doesn't really convey the difference since both ephemeral and predefined measures are "calculated" from underlying numbers.
There was a problem hiding this comment.
Renamed to ephemeral_measures throughout. (Looker and Omni both say "custom fields" / "ad hoc" here and reserve "calculations" for the post-query kind, which this isn't. derived collides with the existing type: derived YAML measure.)
| // They guard against abusive input since expressions may be supplied by non-admin users at query time. | ||
| const ( | ||
| maxMeasureExpressionLength = 1024 | ||
| maxMeasureExpressionDepth = 64 |
There was a problem hiding this comment.
Not sure, but this perhaps seems high to me? Would not expect many levels of nesting in these expressions.
There was a problem hiding this comment.
Lowered to 32. It counts AST levels rather than parens, so a + b + c costs one per term — 32 keeps flat sums working while still bounding recursion. Comment added.
| case m.Compute.Expression != nil: | ||
| return "" // skip; the referenced measures are added separately in AnalyzeQueryFields |
There was a problem hiding this comment.
Seems a little hacky – maybe just rename getMeasureName to getMeasureNames and return a list of names instead? Then I think all the use cases would fit in the one function.
There was a problem hiding this comment.
Done — returns []string, expression case returns the refs directly, special-case block in AnalyzeQueryFields gone. Also stops adding "" for count computes.
| MeasureNames []string `json:"measure_names,omitempty"` | ||
| // Measures are additional measures, e.g. ephemeral measures with an `expression` compute. | ||
| // Only the `expression` compute is supported for time series. | ||
| Measures []*runtimev1.MetricsViewAggregationMeasure `json:"measures,omitempty"` |
There was a problem hiding this comment.
Maybe call the field EphemeralMeasures to distinguish it from MeasureNames (which are not ephemeral). Otherwise they appear to overlap.
There was a problem hiding this comment.
Renamed, and the proto field measures → ephemeral_measures with it, since measures next to measure_names kept the same ambiguity. New in this PR, so nothing to stay compatible with.
- Rename `calculated_measures` to `ephemeral_measures` across the proto, canvas renderer property, Go identifiers, error messages and tests. - Rename `MetricsViewTimeSeriesRequest.measures` to `ephemeral_measures` and the corresponding Go field to `EphemeralMeasures`, so it no longer reads as an overlap with `measure_names`. - Lower `maxMeasureExpressionDepth` from 64 to 32. - Replace `getMeasureName` with `getMeasureNames`, folding the expression refs into the switch instead of special-casing them in `AnalyzeQueryFields`. Claude-Session: https://claude.ai/code/session_013gavAQusi8yn3d1wwfe7SJ
…ures` typing Adding the `expression` compute to `V1MetricsViewAggregationMeasure` made it structurally incompatible with `MetricsViewSpecMeasure`, which has a `string` `expression`. `LeaderboardControls.svelte` relied on that compatibility when passing spec measures through `filterOutSomeAdvancedAggregationMeasures`, so make the function generic over the measure type: it only reads the fields that identify the source spec measure, and callers now get back what they passed in. Also merge the `test_driver` import into the third-party group to satisfy gci. Claude-Session: https://claude.ai/code/session_01RNCJtvYoFBMhHLkqQTf1YQ
begelundmuller
left a comment
There was a problem hiding this comment.
Approved, but not the CI failures
- Render `round` as `roundDecimal` on Pinot, where `round` is a time-bucketing function and would silently return wrong values. Adds a per-dialect function name override table; validation stays dialect-free. - Reject `ephemeral_measures` entries with no compute in the time series query, so the field can no longer act as an alias for `measure_names`. - Reject an ephemeral measure named after the metrics view's time dimension at parse time in canvas, mirroring `checkNameForComputedField`. - Allow expression measures on the rollup path: they are pure SQL wrappers over referenced base measures, so they are eligible when every ref is in the rollup. - Reject block comments in measure expressions, and track single quotes so that string literals are reported as literals rather than as comments. - Handle expression measures in `filterOutSomeAdvancedAggregationMeasures`, which would otherwise drop them for having no entry in the metrics view spec. Claude-Session: https://claude.ai/code/session_01UhUoimUr7wTeAaesSnLGEH
Resolved two conflicts with main: - BaseCanvasComponent.ts: both branches widened `component` to a Svelte 4 | Svelte 5 union; kept main's wording and ordering. - runtime/canvas/component.go: main's calculated-measure support (#9855) added an `ephemeralNames` parameter to validateOptionalMeasureField, so validateMap now resolves ephemeral measure names and accepts them for `color.measure` and `size_measure.field`, matching the other renderer validators. Claude-Session: https://claude.ai/code/session_01WvUwKbBhkbebqv6vkp3G6U
Platform support for calculated (ephemeral) measures: ad-hoc measures derived from existing metrics view measures via an arithmetic expression (e.g.
revenue - cost), computed server-side without editing metrics view YAML. Frontend surfaces (pivot, explore, canvas) build on this in follow-up PRs.expressioncompute onMetricsViewAggregationMeasure({expression, display_name}). It compiles to aMEASURE_TYPE_DERIVEDmeasure withReferencedMeasures, so field access policies are enforced through the referenced measures.metricsview.ParseMeasureExpression: a strict whitelist parser (TiDB-based, same technique as metrics SQL filters) allowing arithmetic (+ - * / %), numeric literals, parentheses, unary minus, and a small function allowlist (abs,round,floor,ceil,sqrt,ln,exp,power,coalesce,nullif,greatest,least). SQL is always re-rendered from the parsed tree with dialect escaping and safe division, never spliced from user input.MetricsViewTimeSeriesgains ameasuresfield acceptingexpressioncomputes so time series can render calculated measures.AnalyzeQueryFieldsincludes expression refs, keeping alert/report security inference (InferRequiredSecurityRules) correct for queries with expression measures.calculated_measuresrenderer prop onkpi,kpi_grid,table,pivot,leaderboardand all chart types, validating each expression and its refs at reconcile time.ExplorePreset.calculated_measuresis a frontend-only preset string (likepivot_formatting) used by the explore URL state in follow-ups.Part of PLAT-135
Checklist: