Skip to content

mixin: Reintroduce thanos_objstore_bucket_operation_failures_total alert#3567

Merged
yeya24 merged 1 commit intothanos-io:masterfrom
MalloZup:renable-alert
Dec 17, 2020
Merged

mixin: Reintroduce thanos_objstore_bucket_operation_failures_total alert#3567
yeya24 merged 1 commit intothanos-io:masterfrom
MalloZup:renable-alert

Conversation

@MalloZup
Copy link
Copy Markdown
Contributor

@MalloZup MalloZup commented Dec 10, 2020 •
edited
Loading

Description:

Metric was removed on PR 2002 accidentally. fix #3005

  • I added CHANGELOG entry for this change.

Changes

  • reintroduce alert thanos_objstore_bucket_operation_failures_total which was deleted by mistake

Verification

tested with make example-rules-lint

@MalloZup MalloZup changed the title mixin: Reintroduce thanos_objstore_bucket_operation_failures_total alert [WIP:] mixin: Reintroduce thanos_objstore_bucket_operation_failures_total alert Dec 10, 2020
@MalloZup MalloZup changed the title [WIP:] mixin: Reintroduce thanos_objstore_bucket_operation_failures_total alert [WIP]: mixin: Reintroduce thanos_objstore_bucket_operation_failures_total alert Dec 10, 2020
@yeya24
Copy link
Copy Markdown
Contributor

yeya24 commented Dec 10, 2020

Hi, please add this alert to the jsonnet file https://github.com/thanos-io/thanos/blob/master/mixin/alerts/sidecar.libsonnet.
The markdown files can be auto-generated I think.

@MalloZup MalloZup force-pushed the renable-alert branch 2 times, most recently from babac04 to 23a0e51 Compare December 10, 2020 21:41
@MalloZup MalloZup changed the title [WIP]: mixin: Reintroduce thanos_objstore_bucket_operation_failures_total alert mixin: Reintroduce thanos_objstore_bucket_operation_failures_total alert Dec 10, 2020
@MalloZup
Copy link
Copy Markdown
Contributor Author

MalloZup commented Dec 10, 2020 •
edited
Loading

@yeya24 thx. I added the jsonnet and generated the markdown via makefile targets. Let me know if something else is missing.

@MalloZup
Copy link
Copy Markdown
Contributor Author

MalloZup commented Dec 11, 2020

I see unit-test are failing I will take look on this.

@MalloZup MalloZup force-pushed the renable-alert branch 3 times, most recently from 12e71a6 to eba61a6 Compare December 11, 2020 13:55
@yeya24
Copy link
Copy Markdown
Contributor

yeya24 commented Dec 11, 2020

I see unit-test are failing I will take look on this.

That's not related to this pr so all good.

MalloZup
MalloZup commented Dec 11, 2020
View reviewed changes
@yeya24
Copy link
Copy Markdown
Contributor

yeya24 commented Dec 16, 2020

CC @kakkoyun or @metalmatze for a review ^^

kakkoyun
kakkoyun requested changes Dec 16, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

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

Looking good. Thanks for your contribution. Only one comment.

@MalloZup MalloZup requested review from kakkoyun and yeya24 December 16, 2020 08:59
@MalloZup
Copy link
Copy Markdown
Contributor Author

MalloZup commented Dec 16, 2020 •
edited
Loading

@yeya24 @kakkoyun thank you for your reviews. I have fixed your kind comments.

Let me know if other things are missing. 🙏

After this I wanted also to look to a next issue to contribute on 🐕 🐕‍🦺

yeya24
yeya24 requested changes Dec 16, 2020
View reviewed changes
@MalloZup
Reintroduce thanos_objstore_bucket_operation_failures_total alert
d00e8d2 
Metric was removed on PR 2002 by mistake

Signed-off-by: dmaiocchi <dmaiocchi@suse.com>
@MalloZup MalloZup requested a review from yeya24 December 17, 2020 11:10
kakkoyun
kakkoyun approved these changes Dec 17, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 Thanks for the contribution.

@kakkoyun
Copy link
Copy Markdown
Member

kakkoyun commented Dec 17, 2020

@yeya24 Feel free to merge it if it's also looking good to you.

@MalloZup
Copy link
Copy Markdown
Contributor Author

MalloZup commented Dec 17, 2020 •
edited
Loading

thx guys for patience and help for getting through this. 🌻
I have learned the mixin concept of prometheus/grafana, which I wasn't quite aware, so I did definitely discovered something new by this PR 🐩

And also jsonnet, was not familiar with it. I knew dhall and toml but not yet jsonnet. 😁 That is definitely something I enjoy when picking up new issue on unfamiliar environment .. Thx for reviews again 🎸

yeya24
yeya24 approved these changes Dec 17, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

LGTM

@yeya24 yeya24 merged commit 5c12d46 into thanos-io:master Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kakkoyun kakkoyun kakkoyun approved these changes

@yeya24 yeya24 yeya24 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Alert missing for bucket operation failures on the sidecar

3 participants

@MalloZup @yeya24 @kakkoyun