diff --git a/docs/docs/developers/build/metrics-view/annotations.md b/docs/docs/developers/build/metrics-view/annotations.md index 87fa10c21ee..64c59ca230c 100644 --- a/docs/docs/developers/build/metrics-view/annotations.md +++ b/docs/docs/developers/build/metrics-view/annotations.md @@ -39,7 +39,7 @@ annotations: Annotations appear as markers or ranges on the time series charts in your dashboard. - **Point Annotations**: Events with a single `time` timestamp appear as point markers. -- **Range Annotations**: Events with both `time` and `time_end` timestamps appear as shaded regions spanning the duration. +- **Range Annotations**: Events with both `time` and `time_end` timestamps appear as shaded regions spanning the duration. A range annotation is shown whenever it overlaps the selected time range, even if it starts before or ends after it. A row with an empty `time_end` is shown as a point annotation. - **Hover Details**: Hovering over an annotation marker reveals the text from the `description` column. :::info diff --git a/runtime/metricsview/executor/executor.go b/runtime/metricsview/executor/executor.go index 2538cd1504f..c48a644294e 100644 --- a/runtime/metricsview/executor/executor.go +++ b/runtime/metricsview/executor/executor.go @@ -951,14 +951,18 @@ END) as __rill_time_grain`) b.WriteString(" FROM ") b.WriteString(dialect.EscapeTable(annotation.Database, annotation.DatabaseSchema, annotation.Table)) + // Select the annotations that overlap the time range. + // A point annotation overlaps when its time falls in the range. + // A range annotation overlaps when it ends after the range starts and starts before the range ends. + // A null time_end denotes a point annotation. b.WriteString(" WHERE ") - - b.WriteString("time >= ? AND time < ?") - args := []any{start, end} - + var args []any if annotation.HasTimeEnd { - b.WriteString(" AND time_end >= ? AND time_end < ?") - args = append(args, start, end) + b.WriteString("(time >= ? OR time_end > ?) AND time < ?") + args = []any{start, start, end} + } else { + b.WriteString("time >= ? AND time < ?") + args = []any{start, end} } if annotation.HasDuration && qry.TimeGrain != metricsview.TimeGrainUnspecified { diff --git a/runtime/resolvers/testdata/metrics_annotations.yaml b/runtime/resolvers/testdata/metrics_annotations.yaml index 06d0fa5a1f4..56e4fa4022d 100644 --- a/runtime/resolvers/testdata/metrics_annotations.yaml +++ b/runtime/resolvers/testdata/metrics_annotations.yaml @@ -103,6 +103,10 @@ tests: time_end: "2024-02-20T00:00:00Z" description: "2nd event" for_measures: ['sum'] + - time: "2024-03-27T00:00:00Z" + time_end: "2024-04-11T00:00:00Z" + description: "3rd event" + for_measures: ['sum'] - name: query_grain_duckdb_annotation resolver: metrics_annotations properties: diff --git a/runtime/resolvers/testdata/metrics_annotations_time_end.yaml b/runtime/resolvers/testdata/metrics_annotations_time_end.yaml new file mode 100644 index 00000000000..217d661d2a9 --- /dev/null +++ b/runtime/resolvers/testdata/metrics_annotations_time_end.yaml @@ -0,0 +1,96 @@ +project_files: + duckdb_data.yaml: + type: model + connector: duckdb + sql: | + select '2024-01-01T00:00:00Z'::TIMESTAMP as time, 1 as val union all + select '2024-02-01T00:00:00Z'::TIMESTAMP as time, 2 as val union all + select '2024-03-01T00:00:00Z'::TIMESTAMP as time, 3 as val + time_end_duckdb_annotation.yaml: + type: model + connector: duckdb + sql: | + select '2024-01-05T00:00:00Z'::TIMESTAMP as time, NULL::TIMESTAMP as time_end, 'point before' as description + union all + select '2024-01-20T00:00:00Z'::TIMESTAMP as time, '2024-02-05T00:00:00Z'::TIMESTAMP as time_end, 'range crossing start' as description + union all + select '2024-02-10T00:00:00Z'::TIMESTAMP as time, NULL::TIMESTAMP as time_end, 'point inside' as description + union all + select '2024-02-16T00:00:00Z'::TIMESTAMP as time, '2024-02-20T00:00:00Z'::TIMESTAMP as time_end, 'range inside' as description + union all + select '2024-02-25T00:00:00Z'::TIMESTAMP as time, '2024-03-10T00:00:00Z'::TIMESTAMP as time_end, 'range crossing end' as description + union all + select '2024-03-05T00:00:00Z'::TIMESTAMP as time, NULL::TIMESTAMP as time_end, 'point after' as description + union all + select '2024-03-06T00:00:00Z'::TIMESTAMP as time, '2024-03-08T00:00:00Z'::TIMESTAMP as time_end, 'range after' as description + duckdb_metrics.yaml: + type: metrics_view + model: duckdb_data + timeseries: time + measures: + - name: sum + expression: sum(val) + annotations: + - model: time_end_duckdb_annotation +tests: + # Point annotations have a null time_end and are selected when their time falls in the range. + # Range annotations are selected when they overlap the range, even when only partially. + - name: annotations_overlapping_the_time_range + resolver: metrics_annotations + properties: + metrics_view: duckdb_metrics + time_range: + start: 2024-02-01T00:00:00Z + end: 2024-03-01T00:00:00Z + result: + - time: "2024-01-20T00:00:00Z" + time_end: "2024-02-05T00:00:00Z" + description: "range crossing start" + for_measures: ['sum'] + - time: "2024-02-10T00:00:00Z" + time_end: null + description: "point inside" + for_measures: ['sum'] + - time: "2024-02-16T00:00:00Z" + time_end: "2024-02-20T00:00:00Z" + description: "range inside" + for_measures: ['sum'] + - time: "2024-02-25T00:00:00Z" + time_end: "2024-03-10T00:00:00Z" + description: "range crossing end" + for_measures: ['sum'] + # A range that ends exactly when the time range starts does not overlap it. + - name: range_ending_at_the_start_is_excluded + resolver: metrics_annotations + properties: + metrics_view: duckdb_metrics + time_range: + start: 2024-02-05T00:00:00Z + end: 2024-02-09T00:00:00Z + result: [] + # Without a time range, the metrics view's full time range is used. + - name: annotations_without_time_range + resolver: metrics_annotations + properties: + metrics_view: duckdb_metrics + result: + - time: "2024-01-05T00:00:00Z" + time_end: null + description: "point before" + for_measures: ['sum'] + - time: "2024-01-20T00:00:00Z" + time_end: "2024-02-05T00:00:00Z" + description: "range crossing start" + for_measures: ['sum'] + - time: "2024-02-10T00:00:00Z" + time_end: null + description: "point inside" + for_measures: ['sum'] + - time: "2024-02-16T00:00:00Z" + time_end: "2024-02-20T00:00:00Z" + description: "range inside" + for_measures: ['sum'] + - time: "2024-02-25T00:00:00Z" + time_end: "2024-03-10T00:00:00Z" + description: "range crossing end" + for_measures: ['sum'] diff --git a/runtime/server/queries_metrics.go b/runtime/server/queries_metrics.go index d8662d28391..2438d3d5448 100644 --- a/runtime/server/queries_metrics.go +++ b/runtime/server/queries_metrics.go @@ -630,7 +630,7 @@ func (s *Server) MetricsViewAnnotations(ctx context.Context, req *runtimev1.Metr var offset *int64 if req.Offset != 0 { - limit = &req.Offset + offset = &req.Offset } qry := metricsview.AnnotationsQuery{ diff --git a/web-common/src/features/dashboards/time-series/measure-chart/annotation-utils.spec.ts b/web-common/src/features/dashboards/time-series/measure-chart/annotation-utils.spec.ts new file mode 100644 index 00000000000..7971f913a17 --- /dev/null +++ b/web-common/src/features/dashboards/time-series/measure-chart/annotation-utils.spec.ts @@ -0,0 +1,132 @@ +import { V1TimeGrain } from "@rilldata/web-common/runtime-client"; +import { scaleLinear } from "d3-scale"; +import { DateTime } from "luxon"; +import { describe, expect, it } from "vitest"; +import { + type Annotation, + AnnotationWidth, + groupAnnotations, +} from "./annotation-utils"; +import type { ChartConfig, ChartScales, TimeSeriesPoint } from "./types"; + +// Four daily data points from Feb 1 to Feb 4, 2024, plotted from x=0 to x=300. +const data: TimeSeriesPoint[] = [1, 2, 3, 4].map((day) => ({ + ts: DateTime.utc(2024, 2, day), + value: day, +})); + +const plotBounds = { + left: 0, + right: 300, + top: 0, + bottom: 100, + width: 300, + height: 100, +}; +const config: ChartConfig = { + width: 300, + height: 100, + margin: { top: 0, right: 0, bottom: 0, left: 0 }, + plotBounds, +}; +const scales: ChartScales = { + x: scaleLinear() + .domain([0, data.length - 1]) + .range([0, 300]), + y: scaleLinear().domain([0, 4]).range([100, 0]), +}; + +function annotation(time: string, timeEnd?: string): Annotation { + return { + time, + timeEnd, + description: time, + startTime: DateTime.fromISO(time, { zone: "UTC" }), + endTime: timeEnd ? DateTime.fromISO(timeEnd, { zone: "UTC" }) : undefined, + formattedTimeOrRange: "", + }; +} + +function group(annotations: Annotation[]) { + return groupAnnotations( + annotations, + scales, + data, + config, + V1TimeGrain.TIME_GRAIN_DAY, + ); +} + +describe("groupAnnotations", () => { + it("positions a point annotation at its grain bucket", () => { + const groups = group([annotation("2024-02-03T06:00:00Z")]); + + expect(groups).toHaveLength(1); + expect(groups[0].index).toBe(2); + expect(groups[0].left).toBe(200); + expect(groups[0].right).toBe(200 + AnnotationWidth); + expect(groups[0].hasRange).toBe(false); + }); + + it("groups annotations that fall in the same bucket", () => { + const groups = group([ + annotation("2024-02-02T01:00:00Z"), + annotation("2024-02-02T23:00:00Z"), + annotation("2024-02-04T00:00:00Z"), + ]); + + expect(groups.map((g) => g.index)).toEqual([1, 3]); + expect(groups[0].items).toHaveLength(2); + }); + + it("spans a range annotation from its start to its end bucket", () => { + const groups = group([ + annotation("2024-02-02T00:00:00Z", "2024-02-03T00:00:00Z"), + ]); + + expect(groups).toHaveLength(1); + expect(groups[0].left).toBe(100); + expect(groups[0].right).toBe(200); + expect(groups[0].hasRange).toBe(true); + }); + + it("anchors a range that starts before the data to the first data point", () => { + const groups = group([ + annotation("2024-01-30T00:00:00Z", "2024-02-03T00:00:00Z"), + ]); + + expect(groups).toHaveLength(1); + expect(groups[0].index).toBe(0); + expect(groups[0].left).toBe(0); + expect(groups[0].right).toBe(200); + expect(groups[0].hasRange).toBe(true); + }); + + it("drops a range that ends exactly where the data starts", () => { + const groups = group([ + annotation("2024-01-30T00:00:00Z", "2024-02-01T00:00:00Z"), + ]); + + expect(groups).toEqual([]); + }); + + it("extends a range that ends after the data to the last data point", () => { + const groups = group([ + annotation("2024-02-03T00:00:00Z", "2024-02-10T00:00:00Z"), + ]); + + expect(groups).toHaveLength(1); + expect(groups[0].left).toBe(200); + expect(groups[0].right).toBe(300); + }); + + it("drops annotations that do not overlap the data", () => { + const groups = group([ + annotation("2024-01-30T00:00:00Z"), + annotation("2024-01-20T00:00:00Z", "2024-01-25T00:00:00Z"), + annotation("2024-02-05T00:00:00Z"), + ]); + + expect(groups).toEqual([]); + }); +}); diff --git a/web-common/src/features/dashboards/time-series/measure-chart/annotation-utils.ts b/web-common/src/features/dashboards/time-series/measure-chart/annotation-utils.ts index bab02699e9a..007c8d58b00 100644 --- a/web-common/src/features/dashboards/time-series/measure-chart/annotation-utils.ts +++ b/web-common/src/features/dashboards/time-series/measure-chart/annotation-utils.ts @@ -45,6 +45,10 @@ export function groupAnnotations( const diamondY = config.plotBounds.top + config.plotBounds.height - AnnotationHeight; + // Data time range — annotations outside this are not visible + const dataStartMs = data[0].ts.toMillis(); + const dataEndMs = data[data.length - 1].ts.toMillis(); + // Bucket annotations by their grain-truncated start time. // Store the truncated millis so the visibility check and positioning // use the bucket boundary (not the raw annotation time). @@ -54,7 +58,22 @@ export function groupAnnotations( >(); for (const a of annotations) { - const bucketStart = a.startTime.startOf(unit); + let bucketStart = a.startTime.startOf(unit); + + // A range annotation that starts before the visible data but ends inside it + // is anchored to the first data point so the visible part of its range is shown. + // The end check mirrors the query's `time_end > start` filter. + // It is still needed here because the annotations and the time series are fetched + // independently and keep their previous data while refetching, + // so a stale range from the previous time range must not be anchored to the new data. + if ( + a.endTime && + bucketStart.toMillis() < dataStartMs && + a.endTime.toMillis() > dataStartMs + ) { + bucketStart = data[0].ts; + } + const key = bucketStart.toISO() ?? a.startTime.toISO() ?? @@ -76,10 +95,6 @@ export function groupAnnotations( // Convert buckets to groups with pixel positions const groups: AnnotationGroup[] = []; - // Data time range — annotations outside this are not visible - const dataStartMs = data[0].ts.toMillis(); - const dataEndMs = data[data.length - 1].ts.toMillis(); - for (const [bucketKey, bucket] of buckets) { // Use the grain-truncated bucket time for visibility and positioning. // This ensures e.g. an hour annotation at 06:00 snaps to its day bucket