feat: add $__range variable for dashboard queries
#8917
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Failed to generate code suggestions for PR |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Greptile Overview
Greptile Summary
added three new dashboard query variables ($__range, $__range_s, $__range_ms) to represent the total time window selection, following the existing pattern used for $__interval and $__rate_interval
Critical Issue:
- incorrect unit conversion: divides by 1000000 (microseconds→seconds) when timestamps are in milliseconds (needs division by 1000)
- this will cause all
$__rangevalues to be 1000x smaller than expected (e.g., 7 days becomes ~10 minutes)
Other observations:
- variable replacement logic follows existing patterns correctly
- tests verify variable replacement works but don't validate calculated values
- fallback to "1s" minimum for PromQL compatibility is reasonable
Confidence Score: 0/5
- this PR has a critical unit conversion bug that breaks the core functionality
- the incorrect division by 1000000 instead of 1000 causes all
$__rangecalculations to be off by 1000x, making the feature completely non-functional (7-day range would appear as ~10 minutes) - web/src/composables/dashboard/usePanelDataLoader.ts requires immediate attention to fix unit conversion bug on line 1912
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| web/src/composables/dashboard/usePanelDataLoader.ts | 1/5 | added $__range, $__range_s, and $__range_ms variables with incorrect unit conversion (divides by 1000000 instead of 1000) |
| web/src/composables/dashboard/usePanelDataLoader.spec.ts | 4/5 | added tests for $__range variable replacement in queries, but tests don't validate actual calculated values |
Sequence Diagram
sequenceDiagram
participant User
participant Dashboard
participant usePanelDataLoader
participant replaceQueryValue
participant API
User->>Dashboard: Select time range
Dashboard->>usePanelDataLoader: loadData()
usePanelDataLoader->>usePanelDataLoader: Calculate startISOTimestamp/endISOTimestamp (milliseconds)
usePanelDataLoader->>replaceQueryValue: replaceQueryValue(query, startISOTimestamp, endISOTimestamp)
replaceQueryValue->>replaceQueryValue: Calculate __range_seconds from timestamps
replaceQueryValue->>replaceQueryValue: Format __range using formatRateInterval()
replaceQueryValue->>replaceQueryValue: Create $__range, $__range_s, $__range_ms variables
replaceQueryValue->>replaceQueryValue: Replace variables in query string
replaceQueryValue-->>usePanelDataLoader: Modified query with variables replaced
usePanelDataLoader->>API: Execute query with replaced variables
API-->>usePanelDataLoader: Query results
usePanelDataLoader-->>Dashboard: Update panel data
2 files reviewed, 1 comment
Comment on lines
+1909
to
+1912
| // calculate range in seconds (total time range of the dashboard) | ||
| // Note: startISOTimestamp and endISOTimestamp are in microseconds (from API) | ||
| const __range_micros = endISOTimestamp - startISOTimestamp; | ||
| const __range_seconds = __range_micros / 1000000; // Convert microseconds to seconds |
Contributor
There was a problem hiding this comment.
logic: comment says "microseconds" but startISOTimestamp and endISOTimestamp are in milliseconds (from .getTime() on line 1288-1289). division should be by 1000, not 1000000
Suggested change
| // calculate range in seconds (total time range of the dashboard) | |
| // Note: startISOTimestamp and endISOTimestamp are in microseconds (from API) | |
| const __range_micros = endISOTimestamp - startISOTimestamp; | |
| const __range_seconds = __range_micros / 1000000; // Convert microseconds to seconds | |
| // calculate range in seconds (total time range of the dashboard) | |
| // Note: startISOTimestamp and endISOTimestamp are in milliseconds (from Date.getTime()) | |
| const __range_millis = endISOTimestamp - startISOTimestamp; | |
| const __range_seconds = __range_millis / 1000; // Convert milliseconds to seconds |
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/composables/dashboard/usePanelDataLoader.ts
Line: 1909:1912
Comment:
**logic:** comment says "microseconds" but `startISOTimestamp` and `endISOTimestamp` are in milliseconds (from `.getTime()` on line 1288-1289). division should be by 1000, not 1000000
```suggestion
// calculate range in seconds (total time range of the dashboard)
// Note: startISOTimestamp and endISOTimestamp are in milliseconds (from Date.getTime())
const __range_millis = endISOTimestamp - startISOTimestamp;
const __range_seconds = __range_millis / 1000; // Convert milliseconds to seconds
```
How can I resolve this? If you propose a fix, please make it concise.
|
| Status | Total | Passed | Failed | Skipped | Flaky | Pass Rate | Duration |
|---|---|---|---|---|---|---|---|
| All tests passed | 366 | 346 | 0 | 19 | 1 | 95% | 4m 38s |
ByteBaker
force-pushed
the
fix/promql-range-variable
branch
from
17b6cd7 to
52a8f1b
Compare
|
| Status | Total | Passed | Failed | Skipped | Flaky | Pass Rate | Duration |
|---|---|---|---|---|---|---|---|
| All tests passed | 366 | 342 | 0 | 19 | 5 | 93% | 4m 43s |
ktx-abhay
approved these changes
Oct 30, 2025
ByteBaker
force-pushed
the
fix/promql-range-variable
branch
from
52a8f1b to
9ef60cb
Compare
|
| Status | Total | Passed | Failed | Skipped | Flaky | Pass Rate | Duration |
|---|---|---|---|---|---|---|---|
| All tests passed | 366 | 344 | 0 | 19 | 3 | 94% | 4m 44s |
Add support for $__range variable that represents the dashboard's total time window selection. This complements existing variables like $__interval and $__rate_interval. Variables added: - $__range: formatted time range (e.g., "1h", "24h", "7d") - $__range_s: time range in seconds - $__range_ms: time range in milliseconds The variable dynamically updates when users change the dashboard time range and works with both PromQL and SQL queries. Fixes #8914
ByteBaker
force-pushed
the
fix/promql-range-variable
branch
from
9ef60cb to
6fa55cd
Compare
|
| Status | Total | Passed | Failed | Skipped | Flaky | Pass Rate | Duration |
|---|---|---|---|---|---|---|---|
| All tests passed | 366 | 342 | 0 | 19 | 5 | 93% | 4m 39s |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add support for $__range variable that represents the dashboard's total time window selection. This complements existing variables like
$__intervaland$__rate_interval.Variables added:
$__range: formatted time range (e.g., "1h", "24h", "7d")$__range_s: time range in seconds$__range_ms: time range in millisecondsThe variable dynamically updates when users change the dashboard time range and works with both PromQL and SQL queries.
Fixes #8914