Skip to content

fix(grafana): always set optional alert fields to avoid render_context errors#5658

Open
hakatt wants to merge 5 commits intokeephq:mainfrom
hakatt:fix/grafana-provider-always-set-optional-fields
Open

fix(grafana): always set optional alert fields to avoid render_context errors#5658
hakatt wants to merge 5 commits intokeephq:mainfrom
hakatt:fix/grafana-provider-always-set-optional-fields

Conversation

@hakatt
Copy link
Contributor

@hakatt hakatt commented Feb 20, 2026

Summary

Fixes #5070 and the general class of "Could not find key 'alert.X'" errors that occur when a workflow template references an alert field that is absent from the context.

Root cause

render_context() in iohandler.py renders all provider with: parameters using Chevron/Mustache with safe=True. When Chevron encounters {{ alert.someField }} and that field doesn't exist in the alert context dict, it writes "Could not find key alert.someField" to stderr — which _render() detects and raises a RenderException.

This affects schema fields and extra fields differently:

  • Schema fields on AlertDto (e.g. url, description) always exist in the context because they have None defaults — Chevron renders None as "", no error.
  • Extra fields (via extra = Extra.allow) that are never set are genuinely absent from the context dict, which triggers the error.

For the Grafana provider, several useful fields (panelUrl, dashboardUrl, silenceURL, valueString, value, datasource) fall into this second category — they're only populated for certain alert types (webhook vs. datasource alerts) and were previously set to None or not set at all when absent.

Fix

Always set these optional extra fields as "" when absent, in both the webhook alert path (_format_alert) and the datasource alert path (_get_alerts_datasource). Workflow templates can then safely reference them and use or-chains for fallbacks:

# This now works even when panelUrl is absent
'link': '{{ alert.panelUrl }}' or '{{ alert.dashboardUrl }}' or 'N/A'

The same pattern should be applied to other providers where fields are conditionally present. The underlying issue is that render_context() uses safe=True uniformly — a more general fix could be to allow providers or individual parameters to opt out of safe rendering, but that is a larger change.

Test plan

  • Grafana webhook alert (has panelUrl, dashboardUrl, silenceURL, valueString) — optional fields populated correctly
  • Grafana datasource alert (has datasource, value) — webhook-only fields default to ""
  • Workflow template referencing absent Grafana fields no longer raises RenderException
  • Existing Grafana provider tests pass

🤖 Generated with Claude Code

…t errors

Fields like panelUrl, dashboardUrl, silenceURL, valueString, value and
datasource are only present for certain Grafana alert types (webhook vs
datasource). When absent they are missing entirely from the AlertDto
context, causing render_context() to raise a RenderException with
"Could not find key" because it renders all provider with: parameters
using safe=True.

Schema fields defined on AlertDto default to None (Chevron renders None
as "") so they are fine. Extra fields that are never set are genuinely
absent from the context dict, which is what triggers the error.

Fix: always set these optional extra fields (as "" when absent) in both
the webhook alert path and the datasource alert path of the Grafana
provider. Workflow templates can then safely reference them and use
Python or-chains for fallbacks: '{{ alert.panelUrl }}' or 'N/A'.

Fixes keephq#5070

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Feb 20, 2026

@nflx is attempting to deploy a commit to the KeepHQ Team on Vercel.

A member of the Team first needs to authorize it.

@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Feb 20, 2026
@CLAassistant
Copy link

CLAassistant commented Feb 20, 2026

CLA assistant check
All committers have signed the CLA.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 20, 2026

Target branch is not in the allowed branches list.

@hakatt
Copy link
Contributor Author

hakatt commented Feb 24, 2026

Blocked by #5676 — the frontend-tests check is failing on all PRs due to an out-of-sync package-lock.json (missing @types/react@18.3.28). Once that merges, CI should go green here too.

Copy link
Member

@shahargl shahargl left a comment

Choose a reason for hiding this comment

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

lgtm

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Feb 25, 2026
@shahargl
Copy link
Member

shahargl commented Mar 1, 2026

@hakatt can you fix "test docs" so it will pass?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer Provider Providers related issues size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: Support conditional namespace display in Teams Adaptive Cards when label may be missing

4 participants