sort grafana tooltip lists according to values#2193
Merged
Conversation
Reviewer's GuideUpdates the patchman-engine Grafana dashboard configuration to sort multi-value tooltips in descending order by value, adjust one panel’s reduction calculation, and realign panel IDs, positions, and dashboard metadata (id, time range, version). File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The default time range change from
now-3htonow-30dis a substantial behavior shift for this dashboard; consider whether this will negatively affect query performance or usability for common workflows and, if needed, align it with other dashboards’ defaults. - Switching the reduce function from
meantolastNotNullchanges the semantics of the displayed metric; please double‑check that all panels using this now truly represent a point‑in‑time value rather than an aggregated one and that tooltip sorting bydescstill matches the intended interpretation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The default time range change from `now-3h` to `now-30d` is a substantial behavior shift for this dashboard; consider whether this will negatively affect query performance or usability for common workflows and, if needed, align it with other dashboards’ defaults.
- Switching the reduce function from `mean` to `lastNotNull` changes the semantics of the displayed metric; please double‑check that all panels using this now truly represent a point‑in‑time value rather than an aggregated one and that tooltip sorting by `desc` still matches the intended interpretation.
## Individual Comments
### Comment 1
<location path="dashboards/app-sre/grafana-dashboard-insights-patchman-engine-general.configmap.yaml" line_range="713" />
<code_context>
"reduceOptions": {
"calcs": [
- "mean"
+ "lastNotNull"
],
"fields": "",
</code_context>
<issue_to_address>
**issue (bug_risk):** Switching the reduction from `mean` to `lastNotNull` may change the semantic meaning of this panel more than intended.
`lastNotNull` will show only the most recent non-null sample instead of an average over the selected range. Combined with the new 30-day window, this is a substantial behavior change and may affect how alerting/triage interprets this panel. Please confirm that a “current reading” is desired here rather than a time-window summary for this metric.
</issue_to_address>
### Comment 2
<location path="dashboards/app-sre/grafana-dashboard-insights-patchman-engine-general.configmap.yaml" line_range="7762" />
<code_context>
},
"time": {
- "from": "now-3h",
+ "from": "now-30d",
"to": "now"
},
</code_context>
<issue_to_address>
**suggestion (performance):** Expanding the default time range to 30 days could impact both usability and query performance.
This wider window can slow many panels (especially for high-cardinality/high-frequency metrics) and makes trends harder to read, while also shifting expectations for users who rely on short-term operational views. Consider keeping a shorter default with 30d as a quick preset, or verify that the underlying queries and data sources are tuned to handle 30d by default.
Suggested implementation:
```
"time": {
"from": "now-3h",
"to": "now"
},
```
To fully apply your suggestion, you should also:
1. Verify whether this dashboard JSON already defines a `timepicker` block (typically at the root level of the dashboard object).
2. Ensure that the `timepicker.time_options` array includes `"30d"` so that users can easily select a 30-day range without it being the default, for example:
```json
"timepicker": {
"time_options": ["5m", "15m", "1h", "6h", "12h", "24h", "7d", "30d"],
"refresh_intervals": ["5s", "10s", "30s", "1m", "5m", "15m", "30m", "1h"]
}
```
3. If a `timepicker` section already exists, just append `"30d"` to the existing `time_options` array instead of replacing it.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2193 +/- ##
=======================================
Coverage 59.21% 59.21%
=======================================
Files 136 136
Lines 8781 8781
=======================================
Hits 5200 5200
Misses 3035 3035
Partials 546 546
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
TenSt
approved these changes
May 11, 2026
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
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.
Secure Coding Practices Checklist GitHub Link
Secure Coding Checklist
Summary by Sourcery
Update the patchman-engine Grafana dashboard configuration to improve tooltip ordering and default time range.
Enhancements: