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

Update Mimir Prometheus to baf7468 #8029

Merged
merged 4 commits into from
May 3, 2024
Merged

Conversation

gotjosh
Copy link
Contributor

@gotjosh gotjosh commented May 2, 2024

What this PR does

The whole list of changes is included as part of Mimir Prometheus PR in grafana/mimir-prometheus#622, grafana/mimir-prometheus#623 and grafana/mimir-prometheus#625

I don't think there's any change that's worth mentioning, given that they aren't user-facing but do let me know if you think there's something worth adding to the changelog.

As part of the second mimir-prometheus PR there is a change that I have noted in the changelog. Matrix results were returning null instead of [] in some cases.

I have managed to reproduce this by running:

# Main branch
$ ~> curl "http://localhost:61793/prometheus/api/v1/query_range?query=bottomk(2,%20notExists)&start=1714745905&end=1714745910&step=30s" \
     -H 'X-Scope-OrgID: tenant-1' | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    65  100    65    0     0  24780      0 --:--:-- --:--:-- --:--:-- 32500
{
  "status": "success",
  "data": {
    "resultType": "matrix",
    "result": null
  }
}


# This PR

$ ~> curl "http://localhost:61235/prometheus/api/v1/query_range?query=bottomk(2,%20notExists)&start=1714745710&end=1714745715&step=30s" \
     -H 'X-Scope-OrgID: tenant-1' | jq
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100    63  100    63    0     0   6039      0 --:--:-- --:--:-- --:--:--  6300
{
  "status": "success",
  "data": {
    "resultType": "matrix",
    "result": []
  }
}

Against a querier in both main and this branch.

Which issue(s) this PR fixes or relates to

Related to https://github.com/grafana/mimir-squad/issues/1845

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

The whole list of changes is included as part of Mimir Prometheus PR in grafana/mimir-prometheus#622

I don't think there's any change that's worth mentioning, given that they aren't user-facing but do let me know if you think there's something worth adding to the changelog.

Signed-off-by: gotjosh <josue.abreu@gmail.com>
@gotjosh gotjosh requested review from grafanabot and a team as code owners May 2, 2024 15:11
Copy link
Collaborator

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

LGTM. I've reviewed mimir-prometheus changes, and checked protobuf release note (I can't see anything suspicious there).

@gotjosh
Copy link
Contributor Author

gotjosh commented May 2, 2024

I'm investigating the CI failure as is very relevant to the CI changes - looks like it's failing to execute the restoration because it does not return any samples when querying the ALERTS_FOR_STATE metric.

Signed-off-by: gotjosh <josue.abreu@gmail.com>
@gotjosh gotjosh changed the title Update Mimir Prometheus to ee1b0be Update Mimir Prometheus to ~~ee1b0be~~ 352c7ff May 3, 2024
@gotjosh gotjosh changed the title Update Mimir Prometheus to ~~ee1b0be~~ 352c7ff Update Mimir Prometheus to ~~ee1b0be~~ 352c7ff May 3, 2024
@gotjosh gotjosh changed the title Update Mimir Prometheus to ~~ee1b0be~~ 352c7ff Update Mimir Prometheus to 352c7ff May 3, 2024
Signed-off-by: gotjosh <josue.abreu@gmail.com>
* [BUGFIX] querier: Don't cache context.Canceled errors for bucket index. #7620
* [BUGFIX] Querier: Don't cache context.Canceled errors for bucket index. #7620
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is a typo all the others are capitalised.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's OK editing it, but we're not very strict with the CHANGELOG. Anyway, it's good you improved it.

@gotjosh
Copy link
Contributor Author

gotjosh commented May 3, 2024

The CI failures are real; they relate to prometheus/prometheus#14047, which is going to be shortly merged.

What are my options here? I could technically fix it in mimir-Prometheus very quickly and re-import or just wait?

I'd like this PR to be merged today so that this gets rolled out next week 😭

@pstibrany
Copy link
Member

I'd like this PR to be merged today so that this gets rolled out next week 😭

It looks like that PR will indeed be merged shortly. I'd suggest to wait for it and if it's not merged by monday, do the fix in mimir-prometheus. We can update our weekly release after it's been cut.

@ArthurSens
Copy link
Member

Looks like that PR is blocking your work, I'm just going to add the missing test case to the contributor's PR. Do you want to add a review over there? :)

Signed-off-by: gotjosh <josue.abreu@gmail.com>
@gotjosh gotjosh changed the title Update Mimir Prometheus to 352c7ff Update Mimir Prometheus to baf7468 May 3, 2024
@gotjosh gotjosh merged commit 17bee0d into main May 3, 2024
29 checks passed
@gotjosh gotjosh deleted the gotjosh/update-mimir-prom-85e3c43 branch May 3, 2024 19:04
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.

5 participants