Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alerting: Encode query model map to string in rule export to avoid html escape sequences #87663

Merged
merged 3 commits into from May 14, 2024

Conversation

rwwiv
Copy link
Contributor

@rwwiv rwwiv commented May 10, 2024

What is this feature?

This PR fixes a bug where the alert query model could contain unicode escape sequences for certain characters in the HCL export format.

For example, math expressions could be represented as $A \u003e 1 instead of $A > 1.

Why do we need this feature?

These characters cause issues when attempting to provision exported rules via Terraform.

Who is this feature for?

Users of alerting provisioning rules via Terraform.

Which issue(s) does this PR fix?:

Fixes #

Special notes for your reviewer:

Please check that:

  • It works as expected from a user's perspective.
  • If this is a pre-GA feature, it is behind a feature toggle.
  • The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

@rwwiv rwwiv requested a review from a team as a code owner May 10, 2024 19:04
@rwwiv rwwiv requested review from JacobsonMT, yuri-tceretian and grobinson-grafana and removed request for a team May 10, 2024 19:04
@grafana-delivery-bot grafana-delivery-bot bot added this to the 11.1.x milestone May 10, 2024
@rwwiv rwwiv added type/bug area/alerting Grafana Alerting no-changelog Skip including change in changelog/release notes labels May 10, 2024
@@ -17,7 +17,7 @@ resource "grafana_rule_group" "rule_group_0000" {
}

datasource_uid = "000000002"
model = "{\n \"expr\": \"http_request_duration_microseconds_count\",\n \"hide\": false,\n \"interval\": \"\",\n \"intervalMs\": 1000,\n \"legendFormat\": \"\",\n \"maxDataPoints\": 100,\n \"refId\": \"query\"\n }"
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it's possible to keep the result the same. Changes in export could cause diff downstream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Considering this also removes the noise coming from the extra whitespace and newlines, I'm of the opinion that it's not worth spending time working to keep the current output. WDYT?

That being said if we feel like consistency is more important I would have to imagine there's a way to maintain the formatting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was determined this was a quirk of the test. I've pushed up a change that compacts the json to make the behavior clearer, but in any case rules previously exported like this should not contain those whitespace characters.

@rwwiv rwwiv merged commit 563fcb8 into main May 14, 2024
12 checks passed
@rwwiv rwwiv deleted the rwwiv/unicode-gt-lt-fix branch May 14, 2024 13:29
@ephemeral-instances-bot
Copy link

Error building instance: Contact #proj-ephemeral-hg-instances if it is not a compile error. Logs

Error message

handling pull request closed event: deleting instance by slug: unexpected response status: status=502 responseBody=

<title>502 Server Error</title>

Error: Server Error

The server encountered a temporary error and could not complete your request.

Please try again in 30 seconds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/alerting Grafana Alerting area/backend no-changelog Skip including change in changelog/release notes type/bug
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants