Add custom alert URL annotation#7
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesAlert formatting now supports an Alert formatting
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
![indent-zero[bot]](https://avatars.githubusercontent.com/in/1426746?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils.js (1)
517-524: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve custom alert links in grouped output.
formatAlertsnow omitsalert_urlfrom annotation details, but never adds those URLs tourls. Grouped alerts therefore lose the requested custom link and may still show an unrelated generated Grafana query link. Aggregate uniquealert_urlvalues and exclude overridden alerts from generator-URL aggregation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils.js` around lines 517 - 524, Update formatAlerts so grouped output aggregates unique custom alert_url annotation values into urls. Exclude alerts with an explicit alert_url from generated Grafana query URL aggregation, while preserving generated URLs for alerts without overrides and the existing annotation omission behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils.js`:
- Line 336: Update the GRAFANA_URL condition in the surrounding annotation-link
logic to use the file’s established truthy presence check instead of comparing
against an empty string. Preserve the existing alert_url guard and link-building
behavior when GRAFANA_URL is configured.
---
Outside diff comments:
In `@src/utils.js`:
- Around line 517-524: Update formatAlerts so grouped output aggregates unique
custom alert_url annotation values into urls. Exclude alerts with an explicit
alert_url from generated Grafana query URL aggregation, while preserving
generated URLs for alerts without overrides and the existing annotation omission
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 03bce045-1bd3-4b85-8b76-09f00d14f1a2
📒 Files selected for processing (1)
src/utils.js
📜 Review details
🔇 Additional comments (1)
src/utils.js (1)
213-217: LGTM!Also applies to: 333-335, 396-398, 407-409, 429-443, 454-458, 486-488, 583-588, 624-628, 636-640, 657-674
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/utils.js`:
- Around line 609-617: Update the grouped alert-link generation around the
alertURLs collection and existing generatorURL loop to exclude alerts whose
annotations.alert_url is present from Grafana grouping. Preserve the custom
alert_url links, while ensuring overridden alerts do not also produce default
“Alert query” links when Grafana is configured.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6430fee-e12c-4456-b698-c85f33f6fc68
📒 Files selected for processing (1)
src/utils.js
📜 Review details
🔇 Additional comments (2)
src/utils.js (2)
333-335: Use a truthy check forGRAFANA_URL(duplicate of the prior review).
process.env.GRAFANA_URL != ""is true when the variable is unset, so this branch can construct anundefined/explore...link. Use the established truthy check instead.
216-217: LGTM!Also applies to: 395-395, 406-406, 428-440, 456-456, 485-487, 519-519, 586-586, 634-635, 645-647, 667-683
| const alertURLs = new Set(data.alerts | ||
| .map(alert => alert.annotations.alert_url) | ||
| .filter(Boolean)); | ||
| let alertURLNum = 1; | ||
| for (const alertURL of alertURLs) { | ||
| const name = alertURLs.size > 1 ? `Alert link ${alertURLNum}` : "Alert link"; | ||
| urls.push(`<a href="${alertURL}">📈 ${name}</a>`); | ||
| alertURLNum += 1; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Honor alert_url as an override in grouped mode.
The per-alert path suppresses Grafana URL generation when alert_url exists, but this new grouped link block does not exclude those alerts from the existing generatorURL loop. With Grafana configured, grouped alerts can therefore show both the custom external link and an incorrect/default-datasource “Alert query” link. Filter overridden alerts out of the Grafana groups as well.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils.js` around lines 609 - 617, Update the grouped alert-link
generation around the alertURLs collection and existing generatorURL loop to
exclude alerts whose annotations.alert_url is present from Grafana grouping.
Preserve the custom alert_url links, while ensuring overridden alerts do not
also produce default “Alert query” links when Grafana is configured.
radon-at-beeper
left a comment
radon-at-beeper
left a comment
There was a problem hiding this comment.
So this makes it so that you can set an alert_url annotation, and if that is present, it will be displayed in place of the Grafana link in the Matrix message? Sounds good.
2 participants
alert_urlannotation for grafana alerts outside of the default datasource, or alerts eventually generated outside of grafana