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

Query: Fix rendered JSON state value for rules and alerts should be in lowercase #2834

Merged
merged 1 commit into from Jul 3, 2020

Conversation

s-urbaniak
Copy link
Contributor

@s-urbaniak s-urbaniak commented Jul 2, 2020

Currently, alert state is rendered as upper case.
In Prometheus it is lower case.
This fixes it.

Signed-off-by: Sergiusz Urbaniak sergiusz.urbaniak@gmail.com

Fixes #2830

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

  • the web /apiv1/rules endpoint renders alert states now in lowercase to retain compatibility with the Prometheus API.

Verification

  • Changed unit tests
  • Verified manually in a local setup:
$ curl -s localhost:32775/api/v1/rules | jq .
{
  "status": "success",
  "data": {
    "groups": [
      {
        "name": "example_abort",
        "file": "/shared/rules/rules-0.yaml",
        "rules": [
          {
            "state": "inactive",
            "name": "TestAlert_AbortOnPartialResponse",
            "query": "absent(some_metric)",
            "duration": 0,
            "labels": {
              "replica": "rule1",
              "severity": "page"
            },
            "annotations": {
              "summary": "I always complain, but I don't allow partial response in query."
            },
            "alerts": [],
            "health": "err",
            "lastError": "no query API server reachable",
            "evaluationTime": 0.00042013,
            "lastEvaluation": "2020-07-02T11:36:10.20089108Z",
            "type": "alerting"
          }
        ],
        "interval": 3,
        "evaluationTime": 0,
        "lastEvaluation": "0001-01-01T00:00:00Z",
        "partial_response_strategy": "WARN",
        "partialResponseStrategy": "ABORT"
      },
      {
        "name": "example_warn",
        "file": "/shared/rules/rules-1.yaml",
        "rules": [
          {
            "state": "inactive",
            "name": "TestAlert_WarnOnPartialResponse",
            "query": "absent(some_metric)",
            "duration": 0,
            "labels": {
              "replica": "rule1",
              "severity": "page"
            },
            "annotations": {
              "summary": "I always complain and allow partial response in query."
            },
            "alerts": [],
            "health": "err",
            "lastError": "no query API server reachable",
            "evaluationTime": 0.000391637,
            "lastEvaluation": "2020-07-02T11:36:11.856795352Z",
            "type": "alerting"
          }
        ],
        "interval": 3,
        "evaluationTime": 0,
        "lastEvaluation": "0001-01-01T00:00:00Z",
        "partial_response_strategy": "WARN",
        "partialResponseStrategy": "WARN"
      }
    ]
  }
}

@s-urbaniak s-urbaniak changed the title pkg/rules/rulespb.AlertState: marshal JSON as lowercase Rules: Fix state value for rules and alerts should be in lowercase Jul 2, 2020
@s-urbaniak s-urbaniak changed the title Rules: Fix state value for rules and alerts should be in lowercase Query: Fix state value for rules and alerts should be in lowercase Jul 2, 2020
@s-urbaniak s-urbaniak changed the title Query: Fix state value for rules and alerts should be in lowercase Query: Fix rendered JSON state value for rules and alerts should be in lowercase Jul 2, 2020
@brancz
Copy link
Member

brancz commented Jul 2, 2020

There appear to be a couple more occurrences in tests that need to be fixed. Otherwise lgtm

Currently, alert state is rendered as upper case.
In Prometheus it is lower case.
This fixes it.

Signed-off-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com>
@s-urbaniak
Copy link
Contributor Author

@brancz all green, PTAL :-)

Copy link
Member

@brancz brancz left a comment

Choose a reason for hiding this comment

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

Thank you!

@brancz brancz merged commit e95b518 into thanos-io:master Jul 3, 2020
@s-urbaniak s-urbaniak deleted the alertstate_case branch July 3, 2020 09:47
paulfantom pushed a commit to paulfantom/thanos that referenced this pull request Jul 8, 2020
Currently, alert state is rendered as upper case.
In Prometheus it is lower case.
This fixes it.

Signed-off-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com>
paulfantom pushed a commit to paulfantom/thanos that referenced this pull request Jul 9, 2020
Currently, alert state is rendered as upper case.
In Prometheus it is lower case.
This fixes it.

Signed-off-by: Sergiusz Urbaniak <sergiusz.urbaniak@gmail.com>
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.

state value for rules and alerts should be in lowercase
2 participants