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

Fix UTF-8 not allowed in Equal field for inhibition rules #4177

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

grobinson-grafana
Copy link
Contributor

@grobinson-grafana grobinson-grafana commented Dec 20, 2024

This commit fixes a bug where UTF-8 characters are not allowed in the Equal field for inhibition rules, even when UTF-8 strict mode is enabled.

This bug occurred because we forgot to override the validation in model.LabelName. I have copied the same logic used for GroupBy with GroupByStr, adding EqualStr.

We would like to upgrade prometheus/common in future and use the validation there instead, but it presents challenges with downstream projects like Mimir and Cortex where, at present, UTF-8 can be enabled and disabled in separate components at the same time, which is not supported in prometheus/common.

@grobinson-grafana grobinson-grafana force-pushed the grobinson/fix-inhibit-rules-equal-utf8 branch 6 times, most recently from 83e8f18 to ed0e832 Compare December 20, 2024 20:10
This commit fixes a bug where UTF-8 characters are not allowed
in the Equal field for inhibition rules, even when UTF-8 strict
mode is enabled.

This bug occurred because we forgot to override the validation
in model.LabelName. I have copied the same logic used for
GroupBy with GroupByStr, adding EqualStr.

We would like to upgrade prometheus/common in future and use
the validation there instead, but it presents challenges with
downstream projects like Mimir and Cortex where, at present,
UTF-8 can be enabled and disabled in separate components at the
same time, which is not supported in prometheus/common.

Signed-off-by: George Robinson <[email protected]>
@grobinson-grafana grobinson-grafana force-pushed the grobinson/fix-inhibit-rules-equal-utf8 branch from ed0e832 to ee55078 Compare December 20, 2024 20:22
@grobinson-grafana
Copy link
Contributor Author

I did a full test of this, including the inhibiting behavior, and can confirm that it works. The first test was without an emoji in equal as a control test:

inhibit_rules:
    - equal:
        - baz
      source_matchers:
        - "foo\U0001F642=bar"
      target_matchers:
        - "bar\U0001F642=baz"

and we can see that the alert was inhibited:

[
  {
    "annotations": {},
    "endsAt": "2024-12-21T02:23:33.061Z",
    "fingerprint": "3381011f87e9020c",
    "receivers": [
      {
        "name": "default"
      }
    ],
    "startsAt": "2024-12-21T02:18:33.061Z",
    "status": {
      "inhibitedBy": [
        "c72bcc07a25e8ca7"
      ],
      "mutedBy": null,
      "silencedBy": [],
      "state": "suppressed"
    },
    "updatedAt": "2024-12-21T02:18:33.061Z",
    "labels": {
      "bar🙂": "baz",
      "baz": "corge"
    }
  },
  {
    "annotations": {},
    "endsAt": "2024-12-21T02:23:27.690Z",
    "fingerprint": "c72bcc07a25e8ca7",
    "receivers": [
      {
        "name": "default"
      }
    ],
    "startsAt": "2024-12-21T02:18:27.690Z",
    "status": {
      "inhibitedBy": [],
      "mutedBy": null,
      "silencedBy": [],
      "state": "active"
    },
    "updatedAt": "2024-12-21T02:18:27.690Z",
    "labels": {
      "baz": "corge",
      "foo🙂": "bar"
    }
  }
]

The second test used the same matchers, but equal contains an emoji:

inhibit_rules:
    - equal:
        - "baz\U0001F642"
      source_matchers:
        - "foo\U0001F642=bar"
      target_matchers:
        - "bar\U0001F642=baz"
[
  {
    "annotations": {},
    "endsAt": "2024-12-21T02:25:27.553Z",
    "fingerprint": "50e470c7e420595a",
    "receivers": [
      {
        "name": "default"
      }
    ],
    "startsAt": "2024-12-21T02:20:27.553Z",
    "status": {
      "inhibitedBy": [
        "81366be48b8b7751"
      ],
      "mutedBy": null,
      "silencedBy": [],
      "state": "suppressed"
    },
    "updatedAt": "2024-12-21T02:20:27.553Z",
    "labels": {
      "bar🙂": "baz",
      "baz🙂": "corge"
    }
  },
  {
    "annotations": {},
    "endsAt": "2024-12-21T02:25:21.385Z",
    "fingerprint": "81366be48b8b7751",
    "receivers": [
      {
        "name": "default"
      }
    ],
    "startsAt": "2024-12-21T02:20:21.385Z",
    "status": {
      "inhibitedBy": [],
      "mutedBy": null,
      "silencedBy": [],
      "state": "active"
    },
    "updatedAt": "2024-12-21T02:20:21.385Z",
    "labels": {
      "baz🙂": "corge",
      "foo🙂": "bar"
    }
  }
]

@grobinson-grafana
Copy link
Contributor Author

I also tested with two alerts that matched the target matchers, however, with the expectation that just one of those alerts is inhibited as it has a matching equal label. The first test was without an emoji as a control test:

inhibit_rules:
    - equal:
        - baz
      source_matchers:
        - "foo\U0001F642=bar"
      target_matchers:
        - "bar\U0001F642=baz"
[
  {
    "annotations": {},
    "endsAt": "2024-12-21T02:32:55.511Z",
    "fingerprint": "3381011f87e9020c",
    "receivers": [
      {
        "name": "default"
      }
    ],
    "startsAt": "2024-12-21T02:27:55.511Z",
    "status": {
      "inhibitedBy": [
        "c72bcc07a25e8ca7"
      ],
      "mutedBy": null,
      "silencedBy": [],
      "state": "suppressed"
    },
    "updatedAt": "2024-12-21T02:27:55.511Z",
    "labels": {
      "bar🙂": "baz",
      "baz": "corge"
    }
  },
  {
    "annotations": {},
    "endsAt": "2024-12-21T02:33:00.138Z",
    "fingerprint": "ae726b055d42000d",
    "receivers": [
      {
        "name": "default"
      }
    ],
    "startsAt": "2024-12-21T02:28:00.138Z",
    "status": {
      "inhibitedBy": [],
      "mutedBy": null,
      "silencedBy": [],
      "state": "active"
    },
    "updatedAt": "2024-12-21T02:28:00.138Z",
    "labels": {
      "bar🙂": "baz",
      "baz": "jorge"
    }
  },
  {
    "annotations": {},
    "endsAt": "2024-12-21T02:32:50.699Z",
    "fingerprint": "c72bcc07a25e8ca7",
    "receivers": [
      {
        "name": "default"
      }
    ],
    "startsAt": "2024-12-21T02:27:50.699Z",
    "status": {
      "inhibitedBy": [],
      "mutedBy": null,
      "silencedBy": [],
      "state": "active"
    },
    "updatedAt": "2024-12-21T02:27:50.699Z",
    "labels": {
      "baz": "corge",
      "foo🙂": "bar"
    }
  }
]

The second test uses the same matchers, but the equal label has an emoji:

inhibit_rules:
    - equal:
        - "baz\U0001F642"
      source_matchers:
        - "foo\U0001F642=bar"
      target_matchers:
        - "bar\U0001F642=baz"
[
  {
    "annotations": {},
    "endsAt": "2024-12-21T02:35:58.223Z",
    "fingerprint": "50e470c7e420595a",
    "receivers": [
      {
        "name": "default"
      }
    ],
    "startsAt": "2024-12-21T02:30:58.223Z",
    "status": {
      "inhibitedBy": [
        "81366be48b8b7751"
      ],
      "mutedBy": null,
      "silencedBy": [],
      "state": "suppressed"
    },
    "updatedAt": "2024-12-21T02:30:58.223Z",
    "labels": {
      "bar🙂": "baz",
      "baz🙂": "corge"
    }
  },
  {
    "annotations": {},
    "endsAt": "2024-12-21T02:36:01.436Z",
    "fingerprint": "5e88f11e42a70d0b",
    "receivers": [
      {
        "name": "default"
      }
    ],
    "startsAt": "2024-12-21T02:31:01.436Z",
    "status": {
      "inhibitedBy": [],
      "mutedBy": null,
      "silencedBy": [],
      "state": "active"
    },
    "updatedAt": "2024-12-21T02:31:01.436Z",
    "labels": {
      "bar🙂": "baz",
      "baz🙂": "jorge"
    }
  },
  {
    "annotations": {},
    "endsAt": "2024-12-21T02:35:54.303Z",
    "fingerprint": "81366be48b8b7751",
    "receivers": [
      {
        "name": "default"
      }
    ],
    "startsAt": "2024-12-21T02:30:54.303Z",
    "status": {
      "inhibitedBy": [],
      "mutedBy": null,
      "silencedBy": [],
      "state": "active"
    },
    "updatedAt": "2024-12-21T02:30:54.303Z",
    "labels": {
      "baz🙂": "corge",
      "foo🙂": "bar"
    }
  }
]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant