Skip to content

sort grafana tooltip lists according to values#2193

Merged
MichaelMraka merged 1 commit into
RedHatInsights:masterfrom
MichaelMraka:pr1
May 11, 2026
Merged

sort grafana tooltip lists according to values#2193
MichaelMraka merged 1 commit into
RedHatInsights:masterfrom
MichaelMraka:pr1

Conversation

@MichaelMraka
Copy link
Copy Markdown
Collaborator

@MichaelMraka MichaelMraka commented May 11, 2026

Secure Coding Practices Checklist GitHub Link

Secure Coding Checklist

  • Input Validation
  • Output Encoding
  • Authentication and Password Management
  • Session Management
  • Access Control
  • Cryptographic Practices
  • Error Handling and Logging
  • Data Protection
  • Communication Security
  • System Configuration
  • Database Security
  • File Management
  • Memory Management
  • General Coding Practices

Summary by Sourcery

Update the patchman-engine Grafana dashboard configuration to improve tooltip ordering and default time range.

Enhancements:

  • Sort multi-value Grafana tooltips in descending order across relevant panels to surface highest values first.
  • Adjust a panel’s reduction calculation to use the last non-null value instead of the mean for more accurate current-state display.
  • Tweak panel IDs, positions, and dashboard metadata (ID, version, default time range) to align with the updated dashboard layout and behavior.

@MichaelMraka MichaelMraka requested a review from a team as a code owner May 11, 2026 11:48
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented May 11, 2026

Reviewer's Guide

Updates 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

Change Details Files
Enable value-based descending sorting for multi-series tooltips across the dashboard.
  • Set tooltip.sort from "none" to "desc" for many time-series panels that use multi-mode tooltips.
  • Change one panel’s tooltip sort from "asc" to "desc" to make it consistent with others.
dashboards/app-sre/grafana-dashboard-insights-patchman-engine-general.configmap.yaml
Adjust metric aggregation and dashboard metadata for the patchman-engine Grafana dashboard.
  • Change a panel reduceOptions.calcs value from "mean" to "lastNotNull" to better reflect the latest data point in aggregations.
  • Update dashboard id and version fields to new values, likely reflecting a re-export from Grafana.
  • Change the default dashboard time range from the last 3 hours (now-3h) to the last 30 days (now-30d).
  • Update several panel ids and gridPos.y coordinates, reflecting layout shifts or panel reordering while keeping widths/heights unchanged.
dashboards/app-sre/grafana-dashboard-insights-patchman-engine-general.configmap.yaml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 2 issues, and left some high level feedback:

  • 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.
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>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.21%. Comparing base (3148cbc) to head (70483d8).

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           
Flag Coverage Δ
unittests 59.21% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@MichaelMraka MichaelMraka merged commit c611e21 into RedHatInsights:master May 11, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants