feat: measure annotations backend #7673

AdityaHegde · 2025-07-25T11:31:35Z

Adds annotation support in metrics view.

Adds annotations key in metrics view yaml.

annotations:
- name: annotation # optional - defaults to model
  model: annotation_source # required
  measures: * # optional - defaults to all. supports selector syntax similar to explore config
- table: annotation_source # alternative way to specify a table directly
  connector: clickhouse # connector for the table
  database: annotation # database for the table

Adds annotation resolver that queries the underlying table. Takes metrics view, annotation, time start/end, optionally a time grain.
MetricsViewAnnotations api for UI to query the annotation. Takes the same args as annotation

Ref PRD: https://www.notion.so/rilldata/Measure-Annotations-9242c8d90da648b88c3b789ae3c67983?d=23bba33c8f5780c6974a001c74bc9f67#23aba33c8f5780a38834c6cb6ee39e4d

Closes PLAT-114

Checklist:

Covered by tests
Ran it and it works as intended
Reviewed the diff before requesting a review
Checked for unhandled edge cases
Linked the issues it closes
Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
Intend to cherry-pick into the release branch
I'm proud of this work!

begelundmuller · 2025-07-31T13:30:01Z

proto/rill/runtime/v1/queries.proto

+message MetricsViewAnnotationsRequest {
+  string instance_id = 1;
+  string metrics_view_name = 2 [(validate.rules).string.min_len = 1];
+  string annotation_name = 3 [(validate.rules).string.min_len = 1];


Did you consider passing a list of measure names, and then returning all relevant annotations for those measures? Not sure if that's desirable, just thought that would more map to the use case.

Mapping one backend query to a API request made sense to me. I feel like something like this would be achieved with query batching if and when we integrate it in UI. Open to other opinions.

Hmm, so the way I see it, the choice of having one or many underlying models is a bit of an implementation detail. We have also discussed supporting inline annotations in the future, so then there may be many more.

Since annotations are small data (we should ensure there aren't millions of annotations), I just wonder if it would make more sense to return them in one go instead of requiring to iterate over each annotation name in the metrics view spec.

For example, if you put yourself in the place of an LLM using MCP or a customer integrating with our APIs, wouldn't they just expect to make one call (maybe with some filters) and get a list of all the annotations to render in the current context?

Ok, one sense then. We dont really need annotations metadata outside of getting annotation name per measure.

begelundmuller · 2025-07-31T13:30:18Z

proto/rill/runtime/v1/queries.proto

+  // Not optional, not null
+  StructType schema = 1;
+  // Not optional, not null
+  repeated google.protobuf.Struct data = 2;


Could it be a fixed schema for annotations?

Updated to use fixed schema. Unknown fields will be put into additional_fields

proto/rill/runtime/v1/resources.proto

runtime/metricsview/executor.go

runtime/parser/parse_metrics_view.go

runtime/resolvers/annotations.go

begelundmuller · 2025-07-31T15:22:24Z

runtime/server/queries_metrics.go

+	data := make([]*structpb.Struct, 0)
+	for {
+		row, err := res.Next()
+		if err != nil {
+			if errors.Is(err, io.EOF) {
+				break
+			}
+			return nil, err
+		}
+		rowStruct, err := pbutil.ToStruct(row, res.Schema())
+		if err != nil {
+			return nil, status.Error(codes.Internal, err.Error())
+		}
+		data = append(data, rowStruct)
+	}


Mentioned this earlier, but would it make sense to parse into a more strictly typed response proto here? Or do you prefer it like this?

begelundmuller · 2025-08-05T08:11:58Z

proto/rill/runtime/v1/queries.proto

+message MetricsViewAnnotationsResponse {
+  message Annotation {
+    // Time when the annotation applies. Maps to `time` column from the table.
+    google.protobuf.Timestamp time = 1;
+    // Optional. Time when the annotation ends. Only present if the underlying table has the `time_end` column.
+    optional google.protobuf.Timestamp time_end = 2;
+    // User defined description of the annotation applies. Maps to `description` column from the table.
+    string description = 3;
+    // Minimum grain this annotation is displayed for. Maps to `grain` column from the table.
+    optional string grain = 4;
+    // Any other fields are captured here. Will be used in predicates in the future.
+    google.protobuf.Struct additional_fields = 5;
+  }
+
+  message MeasureAnnotation {
+    // Not optional, not null
+    string name = 1;
+    // Not optional, not null
+    StructType schema = 2;
+    // Not optional, not null
+    repeated Annotation data = 3;
+  }
+
+  message Measure {
+    // Not optional, not null
+    string name = 1;
+    // Not optional, not null
+    repeated MeasureAnnotation annotations = 2;
+  }
+
+  repeated Measure measures = 1;
+}


What do you think about something like:

Suggested change

message MetricsViewAnnotationsResponse {

message Annotation {

// Time when the annotation applies. Maps to `time` column from the table.

google.protobuf.Timestamp time = 1;

// Optional. Time when the annotation ends. Only present if the underlying table has the `time_end` column.

optional google.protobuf.Timestamp time_end = 2;

// User defined description of the annotation applies. Maps to `description` column from the table.

string description = 3;

// Minimum grain this annotation is displayed for. Maps to `grain` column from the table.

optional string grain = 4;

// Any other fields are captured here. Will be used in predicates in the future.

google.protobuf.Struct additional_fields = 5;

}

message MeasureAnnotation {

// Not optional, not null

string name = 1;

// Not optional, not null

StructType schema = 2;

// Not optional, not null

repeated Annotation data = 3;

}

message Measure {

// Not optional, not null

string name = 1;

// Not optional, not null

repeated MeasureAnnotation annotations = 2;

}

repeated Measure measures = 1;

}

message MetricsViewAnnotationsResponse {

message Annotation {

// Description of the annotation. Maps to `description` column from the table.

string description = 3;

// Time when the annotation applies. Maps to `time` column from the table.

google.protobuf.Timestamp time = 1;

// Optional. Time when the annotation ends. Only present if the underlying table has the `time_end` column.

optional google.protobuf.Timestamp time_end = 2;

// Minimum grain this annotation is displayed for. Maps to `grain` column from the table.

optional string grain = 4;

// Any other fields are captured here. Will be used in predicates in the future.

google.protobuf.Struct additional_fields = 5;

// List of measure names that this annotation applies to. If empty, no restrictions apply.

repeated string only_measures = 6;

}

repeated Annotation rows = 1;

}

Ya works, updated.

begelundmuller · 2025-08-05T13:23:11Z

runtime/server/queries_metrics.go

+	observability.AddRequestAttributes(ctx,
+		attribute.String("args.instance_id", req.InstanceId),
+		attribute.String("args.metric_view", req.MetricsViewName),
+		attribute.String("args.annotation", strings.Join(req.Measures, ",")),


Key doesn't match values here

begelundmuller · 2025-08-05T13:27:37Z

runtime/server/queries_metrics.go

+	ts, err := queries.ResolveTimestampResult(ctx, s.runtime, req.InstanceId, req.MetricsViewName, mv.ValidSpec.TimeDimension, auth.GetClaims(ctx).SecurityClaims(), int(req.Priority))
+	if err != nil {
+		return nil, err
+	}
+
+	// If there are no requested measures, return all measures of the metrics view.
+	reqMeasures := req.Measures
+	if len(reqMeasures) == 0 {
+		for _, mes := range mv.ValidSpec.Measures {
+			reqMeasures = append(reqMeasures, mes.Name)
+		}
+	}


The logic of resolving which annotations to fetch for which measures should ideally happen inside the resolver or metricsview package. I.e. the server function should just be a thin wrapper. This enables easier re-use for invocations from other places (e.g. MCP tools).

Can it pass the measures in the metricsview.AnnotationsQuery and delegate all that handling to the metrics view package? Since the goal of the metricsview package is that it contains the logic related to how to consume metrics view specs.

begelundmuller · 2025-08-05T13:28:42Z

runtime/resolvers/annotations.go

+type annotationsResolverProps struct {
+	MetricsView string `mapstructure:"metrics_view"`
+	Annotation  string `mapstructure:"annotation"`
+}
+
+type annotationsResolverArgs struct {
+	Priority   int                           `mapstructure:"priority"`
+	TimeRange  *metricsview.TimeRange        `mapstructure:"time_range"`
+	TimeGrain  runtimev1.TimeGrain           `mapstructure:"time_grain"`
+	TimeZone   string                        `mapstructure:"time_zone"`
+	Limit      *int64                        `mapstructure:"limit"`
+	Offset     *int64                        `mapstructure:"offset"`
+	Timestamps *metricsview.TimestampsResult `mapstructure:"timestamps"`
+}


Could it take a metricsview.AnnotationsQuery as the properties? Similar to the metrics resolver:

rill/runtime/resolvers/metrics.go

Lines 38 to 42 in 9eab659

func newMetrics(ctx context.Context, opts *runtime.ResolverOptions) (runtime.Resolver, error) {

qry := &metricsview.Query{}

if err := mapstructureutil.WeakDecode(opts.Properties, qry); err != nil {

return nil, err

}

begelundmuller · 2025-08-05T13:33:15Z

runtime/resolvers/annotations.go

+	TimeZone   string                        `mapstructure:"time_zone"`
+	Limit      *int64                        `mapstructure:"limit"`
+	Offset     *int64                        `mapstructure:"offset"`
+	Timestamps *metricsview.TimestampsResult `mapstructure:"timestamps"`


Can it handle timestamps the same way as the metricsResolver? I.e. look them up inside ResolveInteractive instead of requiring them to be passed in.

The idea of the resolvers is that they provide a key-value based interface that can be queried/cached directly from many places, such as data: sections in alerts/reports/custom APIs, from LLM tool calls, from reconcilers, API handlers, etc.

So internal/computed values should ideally be resolved inside the resolver, not passed in the input.

* Annotations UI: Initial commit * Add range highlight * Truncate times * Add see more feature * Imrpove hover behaviour * UXQA * Fix hover on scroll hiding popover * Location adjustment * PR comments

runtime/metricsview/executor.go

begelundmuller · 2025-08-05T16:51:16Z

runtime/metricsview/executor.go

+	if qry.Limit != nil {
+		b.WriteString(" LIMIT ?")
+		args = append(args, *qry.Limit)
+	}


So basically the limit gets applied to each annotation call instead of the combined query? That could lead to issues if multiple annotation models have many rows.

But it's a bit tricky to fix (probably requires a combined SQL query or a pagination token with more state). I'm okay with keeping this for now and fixing when/if this becomes an issue.

For the time being we wont have too much of data. Since we query for the selected time range in dashboard this should fine for now.

runtime/parser/schema/project.schema.yaml

begelundmuller · 2025-08-05T16:58:34Z

runtime/resolvers/annotations.go

+	tsRes, err := resolveTimestampResult(ctx, r.runtime, r.instanceID, r.query.MetricsView, r.mv.TimeDimension, r.claims, r.query.Priority)
+	if err != nil {
+		return nil, err
+	}
+
+	if r.query.TimeRange == nil || r.query.TimeRange.IsZero() {
+		r.query.TimeRange = &metricsview.TimeRange{
+			Start: tsRes.Min,
+			End:   tsRes.Max,
+		}
+	}
+
+	err = r.executor.BindAnnotationsQuery(ctx, r.query, tsRes)
+	if err != nil {
+		return nil, err
+	}


If the time range already has a fixed start and end (which I think it often will since the frontend resolves time ranges before making other calls), then it doesn't need to get the timestamps dynamically, right? So should this be moved into a nested case somehow?

Looking in the metricsResolver, seems like a similar issue may be there. Not sure if I'm missing something.

Ya we can skip it here. But since dashboard always calls MetricsViewTimeRange this wont give use too much.

Wont change metricsResolver in this PR, dont know if it could lead to issues.

runtime/server/queries_metrics.go

AdityaHegde added 3 commits July 24, 2025 20:06

Add parsing and validation of annotations

e2dda2d

Add API to get annotations

ad6d77a

Merge branch 'main' into feat/annotations-backend

321d2d4

AdityaHegde force-pushed the feat/annotations-backend branch from 4180600 to 9cc2105 Compare July 28, 2025 10:08

AdityaHegde marked this pull request as ready for review July 28, 2025 10:20

AdityaHegde requested a review from begelundmuller July 28, 2025 10:20

Add tests

969b8a7

AdityaHegde force-pushed the feat/annotations-backend branch 3 times, most recently from 0698896 to 50a3f3d Compare July 29, 2025 16:13

Handle TODOs

c9625f1

AdityaHegde force-pushed the feat/annotations-backend branch from 50a3f3d to c9625f1 Compare July 29, 2025 16:14

AdityaHegde mentioned this pull request Jul 31, 2025

feat: measure annotations #7689

Merged

12 tasks

begelundmuller requested changes Jul 31, 2025

View reviewed changes

AdityaHegde force-pushed the feat/annotations-backend branch from 7f91bb0 to 8d2e820 Compare August 1, 2025 06:53

PR comments

e665669

AdityaHegde force-pushed the feat/annotations-backend branch from 8d2e820 to e665669 Compare August 1, 2025 10:54

Fix lint

2835a85

AdityaHegde requested a review from begelundmuller August 1, 2025 14:01

Unify API to have a single one for metrics view

48903da

AdityaHegde force-pushed the feat/annotations-backend branch from d9139c9 to 48903da Compare August 5, 2025 07:58

begelundmuller reviewed Aug 5, 2025

View reviewed changes

Update response to be LLM friendly

dc70931

AdityaHegde force-pushed the feat/annotations-backend branch from 623486a to dc70931 Compare August 5, 2025 09:03

begelundmuller requested changes Aug 5, 2025

View reviewed changes

AdityaHegde added 2 commits August 5, 2025 21:01

feat: measure annotations (#7689)

cab483b

* Annotations UI: Initial commit * Add range highlight * Truncate times * Add see more feature * Imrpove hover behaviour * UXQA * Fix hover on scroll hiding popover * Location adjustment * PR comments

Merge branch 'main' into feat/annotations-backend

3b33a80

AdityaHegde force-pushed the feat/annotations-backend branch from 1beccd6 to 2c616c6 Compare August 5, 2025 16:07

Move resolutions to metrics package

dbab18e

AdityaHegde force-pushed the feat/annotations-backend branch from 2c616c6 to dbab18e Compare August 5, 2025 16:13

begelundmuller approved these changes Aug 5, 2025

View reviewed changes

AdityaHegde force-pushed the feat/annotations-backend branch from f7dcfa1 to 05ef3c6 Compare August 5, 2025 17:40

PR comments

e41c48f

AdityaHegde force-pushed the feat/annotations-backend branch from 05ef3c6 to e41c48f Compare August 5, 2025 17:51

AdityaHegde merged commit 61e8d02 into main Aug 5, 2025
14 checks passed

AdityaHegde deleted the feat/annotations-backend branch August 5, 2025 18:06

	func newMetrics(ctx context.Context, opts *runtime.ResolverOptions) (runtime.Resolver, error) {
	qry := &metricsview.Query{}
	if err := mapstructureutil.WeakDecode(opts.Properties, qry); err != nil {
	return nil, err
	}

feat: measure annotations backend #7673

feat: measure annotations backend #7673

Uh oh!

Conversation

AdityaHegde commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AdityaHegde Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AdityaHegde commented Jul 25, 2025 •

edited

Loading

AdityaHegde Aug 5, 2025 •

edited

Loading