Skip to content

Add configurable Max query time range limit#9386

Merged
nishantmonu51 merged 9 commits into
mainfrom
Nishant-Bangarwa/dashboard-max-time-range
May 21, 2026
Merged

Add configurable Max query time range limit#9386
nishantmonu51 merged 9 commits into
mainfrom
Nishant-Bangarwa/dashboard-max-time-range

Conversation

@nishantmonu51
Copy link
Copy Markdown
Collaborator

@nishantmonu51 nishantmonu51 commented May 5, 2026

Today a user can ask a metrics view to aggregate data over an arbitrarily wide time range.
For datasets with high cardinality or fine-grained time dimensions this is expensive and can starve the OLAP engine.
This PR introduces a per-metrics-view safety cap: a maximum time span that any single interactive query can cover (e.g., "no query may span more than 90 days").
It is configurable max time range limit on the metrics view.

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!

@nishantmonu51 nishantmonu51 requested review from a team as code owners May 5, 2026 12:01
@nishantmonu51 nishantmonu51 requested a review from AdityaHegde May 5, 2026 12:08
@nishantmonu51
Copy link
Copy Markdown
Collaborator Author

@AdityaHegde : can you help review and take t his further ?

Comment thread runtime/metricsview/executor/executor_enforce_query_limits.go Outdated
Comment thread runtime/metricsview/executor/executor_enforce_query_limits.go Outdated
// isoDurationToDays approximates an ISO 8601 duration (e.g. "P90D", "P3M") in days,
// matching the executor's StandardDuration.EstimateNative() heuristic (1 month ≈ 30 days, 1 year ≈ 365 days).
// Returns NaN if the input cannot be parsed.
export function isoDurationToDays(isoDuration: string): number {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldnt redo what rilltime already does for us. Lets use that instead. Note that this will add an additional query. We can instead return it in MetricsViewTimeRange/MetricsViewTimeRanges if max_query_time_range is defined on metrics view.

With this change most components should take luxon.Interval and use that to calculate caps.

@AdityaHegde AdityaHegde self-requested a review May 6, 2026 10:06
Comment thread runtime/metricsview/executor/executor_enforce_query_limits.go Outdated

// rangeWithinCap returns true unless the range can be statically sized to more than maxRange.
// rill-time expressions and unparseable inputs pass through; the backend has the final say.
function rangeWithinCap(range: string, maxRange: Duration): boolean {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need this? If we use API to resolve ranges then we can also support rilltime syntax.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can take this a future enhancement. Getting the API response in can be tricky.

Comment thread runtime/server/queries_metrics.go Outdated
return nil, ErrForbidden
}

mv, _, err := resolveMVAndSecurity(ctx, s.runtime, req.InstanceId, req.MetricsViewName)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can just do lookupMetricsView(ctx, s.runtime, req.InstanceId, req.MetricsViewName) instead of this as only mv spec is needed, security is not used.

@nishantmonu51 nishantmonu51 added the blocker A release blocker issue that should be resolved before a new release label May 18, 2026
nishantmonu51 and others added 2 commits May 20, 2026 17:48
…board-max-time-range

# Conflicts:
#	proto/gen/rill/runtime/v1/resources.pb.go
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@nishantmonu51 nishantmonu51 merged commit 169e257 into main May 21, 2026
19 of 21 checks passed
@nishantmonu51 nishantmonu51 deleted the Nishant-Bangarwa/dashboard-max-time-range branch May 21, 2026 13:24
nishantmonu51 added a commit that referenced this pull request May 22, 2026
* Add configurable Max query time range limit

* Fix prettier

* Handle review comments

* Fix CI

* add file

* Fix lint

* Handle review comments

* Regenerate proto files after merging main

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocker A release blocker issue that should be resolved before a new release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants