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

Use DeletePartialMatch() #2660

Merged
merged 1 commit into from Aug 22, 2022

Conversation

LeviHarrison
Copy link
Contributor

What this PR does

Switches from Mimir's own DeleteMatchingLabels() to client_golang's new DeletePartialMatch().

Which issue(s) this PR fixes or relates to

Fixes #1764

Checklist

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

@pracucci pracucci self-requested a review August 8, 2022 12:53
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.

Thanks for working on this! This PR brings in quite a lot of changes to vendored libraries. Some notes:

  • github.com/prometheus/client_golang: I checked the changelog and LGTM. I've also reviewed the PR introducing the DeletePartialMatch() function and LGTM. The biggest changes are related to the Go collector and come from this PR.
  • github.com/prometheus/common: change to OAuth2 roundtripper, LGTM
  • github.com/prometheus/procfs: I haven't checked changes, shouldn't really affect Mimir
  • google.golang.org/protobuf: checked the changelog, LGTM

Given we're not in a hurry, and given the Prometheus go client 1.13.0 has been released 3 days, we may wait a bit before merging or wait until another maintainer does a review on the changes we're vendoring.

@pracucci
Copy link
Collaborator

pracucci commented Aug 19, 2022

@LeviHarrison We merged some of these vendor updates as part of #2758. Can you rebase, please? Then I will merge this PR too. Thanks!

Signed-off-by: Levi Harrison <git@leviharrison.dev>
@LeviHarrison
Copy link
Contributor Author

Rebased 👍🏻

@pracucci
Copy link
Collaborator

Thanks!

@pracucci pracucci merged commit 9af647e into grafana:main Aug 22, 2022
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.

Use the new Prometheus client's DeletePartialMatch()
2 participants