improvement(grafana): align tools and block with Grafana API spec#4574
Conversation
… spec Validates and corrects the Grafana integration against the official API docs: fixes wire-format field naming for provisioned alert rules (missing_series_evals_to_resolve, keepFiringFor, orgID), adds X-Disable-Provenance support, expands alert-rule params (isPaused, notificationSettings, record, annotations, labels), corrects defaults (execErrState=Error, dashboard overwrite=false), and centralizes alert-rule output mapping in a shared utils module. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryMedium Risk Overview Alert rules: expands create/update inputs (recording rules support, custom UID, pause/keep-firing/missing-series/notification settings, provenance control via Dashboards/annotations/data sources/folders/contact points: adds UID-based dashboard filtering + pagination, makes dashboard updates default to not overwriting on version conflict, allows org-wide annotations + additional annotation filters/fields, improves data source ID-vs-UID routing and output fields, adds nested folder support ( Reviewed by Cursor Bugbot for commit e997595. Configure here. |
Greptile SummaryThis PR aligns the Grafana tool suite with the provisioning API spec: it fixes wire-format field names (
Confidence Score: 5/5The changes are additive and fix real API compatibility bugs; no regressions were found in the changed paths. All critical wire-format fixes are correct, error surfacing is consistent, and the centralized mapAlertRule utility correctly handles both API response casings. The two findings are minor quality observations that do not affect correctness of the existing paths. utils.ts: the notification_settings output key naming inconsistency is worth resolving before it becomes a convention across downstream workflow steps. Important Files Changed
Sequence DiagramsequenceDiagram
participant Block as Grafana Block
participant Tool as Tool (create/update/get/list)
participant Utils as utils.ts
participant API as Grafana Provisioning API
Block->>Tool: params (folderUid, keepFiringFor, notificationSettings, disableProvenance)
Tool->>API: POST/PUT /api/v1/provisioning/alert-rules
API-->>Tool: ProvisionedAlertRule JSON (keep_firing_for, orgID, notification_settings)
Tool->>Utils: mapAlertRule(data)
Utils-->>Tool: normalized output (keepFiringFor, orgID fallbacks, dual-casing reads)
Tool-->>Block: success and output
Note over Tool,API: update_alert_rule GET existing rule first then merge and PUT
Reviews (4): Last reviewed commit: "fix(grafana): surface invalid JSON for a..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…ields Grafana's ProvisionedAlertRule schema (verified against upstream Go source and swagger spec) uses keep_firing_for (snake_case) and missingSeriesEvalsToResolve (camelCase) — the opposite of what prior audit rounds assumed. POST/PUT bodies now send the correct field names; mapAlertRule reads the correct primary names with the old casings kept as fallbacks. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Drop hardcoded orgID: 1 fallback; only send orgID when organizationId is provided, so token-scoped org context drives rule placement. - Surface invalid JSON for notificationSettings/record on alert rule create/update instead of silently dropping the input. - Fix execErrState description in update_alert_rule to include Error. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
@greptile |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit da17194. Configure here.
…rt rules
Match the behavior of other JSON params (data, notificationSettings, record):
return a descriptive error instead of silently falling back to {} (create)
or keeping the existing value (update).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
@greptile |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e997595. Configure here.
|
@greptile |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e997595. Configure here.
Move ALERT_RULE_OUTPUT_FIELDS from utils.ts to types.ts and rename to SCREAMING_SNAKE_CASE so scripts/generate-docs.ts (which only resolves const references from types.ts matching [A-Z][A-Z_0-9]+) can inline the per-field rows into the generated alert-rule output tables. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 participant
Summary
missing_series_evals_to_resolve,keepFiringFor,orgID) and reads with fallbacks so output is robust to both casingsuid,isPaused,keepFiringFor,missingSeriesEvalsToResolve,notificationSettings,record,annotations,labels,disableProvenance) and supportsX-Disable-ProvenanceheaderexecErrState→Error, dashboardoverwritedefaults tofalse, drops hardcodedfor: 5mtools/grafana/utils.ts; tightensget_data_sourceUID heuristic; drops phantomslug/sortMetafrom list_dashboards; adds legacydashboardIdparam to list_annotations; PATCH-friendly optionaltextfor update_annotationTest plan
disableProvenanceand verify rule is editable via UIoverwriteand confirm version-conflict behaviordashboardUidand (legacy)dashboardIdtext