-
Notifications
You must be signed in to change notification settings - Fork 581
Upgrade Prometheus to 59d02b5ef003f125286a62aa6daf4c8b7583042f #16
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
Conversation
Signed-off-by: Ganesh Vernekar <[email protected]>
b9708f9
to
5b3a2a5
Compare
Signed-off-by: Ganesh Vernekar <[email protected]>
f3ca6ad
to
b2ea888
Compare
enableExemplars := false | ||
if i.cfg.BlocksStorageConfig.TSDB.MaxExemplars > 0 { | ||
enableExemplars = true | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additionally, we could always keep it enabled and resize the exemplar storage when needed (this upgrades brings in the Resize function on exemplar storage).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, it's fine to keep it disabled if MaxExemplars = 0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes LGTM but I would remove the CHANGELOG entry (or just add a [BUGFIX]
entry for the specific fix we're vendording in). I think you also need to run go mod tidy
.
CHANGELOG.md
Outdated
@@ -5,6 +5,7 @@ | |||
* [CHANGE] Removed `configdb` support from Ruler and Alertmanager backend storages. #15 | |||
* [CHANGE] Changed `-ruler.storage.type` default value from `configdb` to `local`. #15 | |||
* [CHANGE] Changed `-alertmanager.storage.type` default value from `configdb` to `local`. #15 | |||
* [ENHANCEMENT] Prometheus upgraded. #4 #16 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See this:
#4 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made it a BUGFIX since this is a important bugfix for the bug that we are hitting. Does the wording look fine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, thanks!
Signed-off-by: Ganesh Vernekar <[email protected]>
What this PR does:
Upgrades Prometheus to prometheus/prometheus@59d02b5 to get prometheus/prometheus#9081 in
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]