chore: add a panel with sent notifications rate#2415
Conversation
Reviewer's GuideAdds a new Grafana timeseries panel showing the rate of sent notifications, using Prometheus metrics and consistent styling with existing panels. 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 1 issue, and left some high level feedback:
- Consider whether summing
ve_notificator_sent_notifs_cnt_totalacross all labels is desirable or if you should group by a key like service or tenant to preserve visibility into which source is driving notification volume. - The axis label
notifs/secand unitshortare slightly inconsistent; you may want to use a rate-based unit (e.g.,opsor a custom unit) or adjust the axis label to match the configured unit for clarity.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider whether summing `ve_notificator_sent_notifs_cnt_total` across all labels is desirable or if you should group by a key like service or tenant to preserve visibility into which source is driving notification volume.
- The axis label `notifs/sec` and unit `short` are slightly inconsistent; you may want to use a rate-based unit (e.g., `ops` or a custom unit) or adjust the axis label to match the configured unit for clarity.
## Individual Comments
### Comment 1
<location path="monitoring/grafana/dashboards/grafana-dashboard-clouddot-insights-vulnerability-engine.configmap.yml" line_range="7597-7550" />
<code_context>
+ "links": [],
+ "mappings": [],
+ "min": 0,
+ "thresholds": {
+ "mode": "absolute",
+ "steps": [
+ {
+ "color": "green"
+ },
</code_context>
<issue_to_address>
**suggestion:** Revisit the threshold values to ensure they reflect meaningful limits for notifications per second.
Currently the thresholds use a single cutoff at `80` with just `green` and `red`, which may not map to meaningful states for notifications/sec (e.g., normal, warning, overload). Consider basing the values on known capacity or SLOs and possibly adding an intermediate `yellow` threshold so that color changes represent actionable conditions rather than arbitrary numbers.
Suggested implementation:
```
"links": [],
"mappings": [],
"min": 0,
"thresholds": {
"mode": "absolute",
"steps": [
{
"color": "green"
},
{
"color": "yellow",
"value": 60
},
{
"color": "red",
"value": 90
}
]
},
```
1. If there are other panels tracking notifications per second in this dashboard (or related dashboards), consider aligning their thresholds to the same ranges so that "green/yellow/red" semantics are consistent.
2. If you have specific SLOs or capacity figures, you may want to tweak the `60` and `90` values to better match those numbers (e.g. setting `yellow` near the sustained-capacity limit and `red` near overload).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| "title": "Manager API Latency - 3scale", | ||
| "type": "heatmap" | ||
| }, | ||
| { |
There was a problem hiding this comment.
suggestion: Revisit the threshold values to ensure they reflect meaningful limits for notifications per second.
Currently the thresholds use a single cutoff at 80 with just green and red, which may not map to meaningful states for notifications/sec (e.g., normal, warning, overload). Consider basing the values on known capacity or SLOs and possibly adding an intermediate yellow threshold so that color changes represent actionable conditions rather than arbitrary numbers.
Suggested implementation:
"links": [],
"mappings": [],
"min": 0,
"thresholds": {
"mode": "absolute",
"steps": [
{
"color": "green"
},
{
"color": "yellow",
"value": 60
},
{
"color": "red",
"value": 90
}
]
},
- If there are other panels tracking notifications per second in this dashboard (or related dashboards), consider aligning their thresholds to the same ranges so that "green/yellow/red" semantics are consistent.
- If you have specific SLOs or capacity figures, you may want to tweak the
60and90values to better match those numbers (e.g. settingyellownear the sustained-capacity limit andrednear overload).
Secure Coding Practices Checklist GitHub Link
Secure Coding Checklist
Summary by Sourcery
Enhancements: