warn on rollup/base query coverage mismatch - #9729
Conversation
nishantmonu51
added
Type:Feature
AdityaHegde
left a comment
AdityaHegde
left a comment
There was a problem hiding this comment.
Timeseries chart icon is squished. How about showing it in top left?
Big number icon is not aligned. Missing items-center somewhere.
Note that we are not showing any info in other screens like pivot and time dimension detail. Not sure if the customer asking for this use those. cc @nishantmonu51
| // The widget is missing data from max(requested start, widest coverage start) up to | ||
| // its own coverage start. Only warn when the gap exceeds one grain bucket of the | ||
| // widest table (grain-truncation tolerance; 0 for the base table). | ||
| const availableStartMs = Math.max(requestedStartMs, widestMin); |
There was a problem hiding this comment.
What case do we need to check with widest table vs just the requested range? We can simplify a lot if we only check with requested range.
| "dashboard_output_excludes": "Output excludes selected values", | ||
| "dashboard_output_includes": "Output includes selected values", | ||
| "dashboard_partial_data": "Partial data", | ||
| "dashboard_partial_data_detail": "This view only includes data from {servedStart} because the fields it uses have limited history. Other parts of this dashboard include data from {availableStart}.", |
There was a problem hiding this comment.
Other parts... is already part of time range controls, feels too much info for the tooltip, epecially since the user will be aware of selected time range.
| /** Rollup table name → time grain, for grain-truncation tolerance. */ | ||
| readonly rollupGrains: Readable<Map<string, V1TimeGrain>>; | ||
| /** Serving tables of all widgets currently on the dashboard. */ | ||
| readonly tablesInUse: Readable<Set<string>>; |
There was a problem hiding this comment.
Why is this needed? Shouldnt each widget be aware of just the start/end of selected time range, table queried and its available start/end?
|
|
||
| export interface CoverageWarning { | ||
| /** Timestamp (ms) where the widget's serving table's data actually starts. */ | ||
| servedStartMs: number; |
There was a problem hiding this comment.
Are we not planning to support when end doesnt match?
| }, | ||
| ); | ||
|
|
||
| $: chartServingTable = servingTableOf($timeSeriesQuery.data); |
There was a problem hiding this comment.
How about simplifying components with a utility,
// RollupCoverageStore.ts
this.computeCoverageWarning = $derived([this.coverage, this.rollupGrains, this.tablesInUse], () => {
return (data: { servingTable?: string } | undefined, requestedStartMs: number) => {
// does calc similar to existing servingTableOf + computeCoverageWarning
}
});
// MeasureChart.svelte
const computeCoverageWarning = rollupCoverage?.computeCoverageWarning ?? readable(undefined)
$: primaryCoverageWarning = $computeCoverageWarning?.($timeSeriesQuery.data, requestedStartMs(timeStart))
$: comparisonCoverageWarning = ...
$: coverageWarning = primaryCoverageWarning ?? comparisonCoverageWarning
| import { DateTime } from "luxon"; | ||
| import type { CoverageWarning } from "./rollup-coverage"; | ||
|
|
||
| export let warning: CoverageWarning; |
There was a problem hiding this comment.
New component should be in svelte5.
|
@nishantmonu51 is of opinion that we should not do this, we should assume rollups are always subsets of base table. We should document this and still if customer does this then not then we should not handle the error cases and its their responsibility. |
| // Rollup table the query was routed to. Empty when the query was served from the metrics view's base table. | ||
| // Matches the keys of rollup_time_ranges in MetricsViewTimeRangeResponse and MetricsViewTimeRangesResponse. | ||
| string serving_table = 4; |
There was a problem hiding this comment.
It maybe feels a little dirty to use table names here since a) usually we advise using models, not tables, b) it ignores database/schema name so could theoretically have collisions, c) leaks underlying table info into metrics queries.
It's not a huge problem, so you can keep if you think it makes sense, but otherwise consider instead either a) using index in spec.rollups, or b) fully qualified table names.
4 participants
This is to solve cases when dimensions are served from different tables example one from a rollup another from base because may be that dimension was not present in the rollup or some other criteria. So add a warning in such cases, we return the serving table along with response, UI already know the rollup and base coverage so it can do mismatch check with time coverage of serving table.
Backend changes
Expose which physical table served a metrics query, plus per-rollup time coverage, so the frontend can detect when widgets on one dashboard are served from tables with different retention:
serving_tableadded to MetricsViewAggregationResponse, MetricsViewTimeSeriesResponse, and MetricsViewComparisonResponse: the rollup table the query was routed to, empty when served from the base table.rollup_time_ranges(map<table, TimeRangeSummary>) added to MetricsViewTimeRange and MetricsViewTimeRanges responses.Frontend warning logic
Each dashboard gets a RollupCoverageStore (on StateManagers) holding per-table time coverage from MetricsViewTimeRange (timeRangeSummary + new rollupTimeRanges) and a registry of which table served each mounted widget (from the new servingTable field on query responses; leaderboards, big numbers, charts, and the dimension table register).
A widget shows the warning if its serving table's coverage starts later than both the requested range start (primary or comparison, whichever is earlier) and the widest coverage among all tables currently in use on the dashboard:
served.min > max(requestedStart, widestInUse.min) + oneGrainBucket(widestTable)
The grain-bucket tolerance suppresses false positives from rollup min timestamps being truncated to bucket starts. Consequences: only widgets actually missing requested data warn; if all widgets route to the same table (e.g. a filter forces everything to base), the dashboard is internally consistent and nothing warns; ranges fully covered by the serving table never warn regardless of table mismatch.
Checklist: