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

Rules HTTP API weirdness: nulls, different values for "same" field, lastEvaluation set to 0000 #2938

Closed
GiedriusS opened this issue Jul 24, 2020 · 1 comment · Fixed by #2957

Comments

@GiedriusS
Copy link
Member

GiedriusS commented Jul 24, 2020

On top of:

b2f1485 from #2865

With:

TZ=UTC PROMETHEUS_EXECUTABLE=~/dev/prometheus/prometheus ./scripts/quickstart.sh

Going to localhost:19999/api/v1/rules?type=alert gives:

{"status":"success","data":{"groups":[{"name":"example","file":"data/rules.yml","rules":null,"interval":30,"evaluationTime":0,"lastEvaluation":"0001-01-01T00:00:00Z","partial_response_strategy":"WARN","partialResponseStrategy":"ABORT"}]}}

Three things seem off to me:

  • rules is null even though the branch contains pkg/rules/rulespb: fix JSON marshaling for nil slices #2888 which supposedly fixes this problem
  • Two fields partial_response_strategy and partialResponseStrategy that should show the same value but they are different in this case 🤔
  • lastEvaluation is always set to 0001-01-01T00:00:00Z
@GiedriusS GiedriusS changed the title Rules HTTP API weirdness: nulls, different values for "same" field Rules HTTP API weirdness: nulls, different values for "same" field, lastEvaluation set to 0000 Jul 24, 2020
@yeya24
Copy link
Contributor

yeya24 commented Jul 28, 2020

@GiedriusS Is this fixed by #2925 already? 😄

Updated: please ignore me, #2925 only fixes lastEvaluation is always set to 0001-01-01T00:00:00Z

GiedriusS added a commit to GiedriusS/thanos that referenced this issue Jul 29, 2020
Fixes thanos-io#2938 and is a follow up
to thanos-io#2925.

Changes:
* Copying all fields properly to the group-level as well
* Set TZ to UTC explicitly in other `time.Time` fields to avoid a panic
* Make sure that `RuleGroup` always has an empty array in the `rules`
field because that's customary and that is what the new React UI
expects.

Manual testing for now.

Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>
bwplotka pushed a commit that referenced this issue Aug 11, 2020
* rules: fix silly mistakes in the rules API

Fixes #2938 and is a follow up
to #2925.

Changes:
* Copying all fields properly to the group-level as well
* Set TZ to UTC explicitly in other `time.Time` fields to avoid a panic
* Make sure that `RuleGroup` always has an empty array in the `rules`
field because that's customary and that is what the new React UI
expects.

Manual testing for now.

Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>

* rules: remove DeprecatedPartialResponseStrategy

It has been marked deprecated for more than 2 months. Remove it finally.

Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>

* e2e: rules_api: add checks for non-zero values

Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>

* test: e2e: fix rule test

Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>

* test: rule: fix govet error

Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>

* Update CHANGELOG.md

Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>

Co-authored-by: Lucas Servén Marín <lserven@gmail.com>

* Update CHANGELOG.md

Signed-off-by: Giedrius Statkevičius <giedriuswork@gmail.com>

Co-authored-by: Lucas Servén Marín <lserven@gmail.com>

Co-authored-by: Lucas Servén Marín <lserven@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants