Skip to content

Enable support for custom callbacks as part of maintenance#2689

Merged
codesome merged 6 commits intoprometheus:mainfrom
gotjosh:allow-custom-functions-for-maintenance
Sep 6, 2021
Merged

Enable support for custom callbacks as part of maintenance#2689
codesome merged 6 commits intoprometheus:mainfrom
gotjosh:allow-custom-functions-for-maintenance

Conversation

@gotjosh
Copy link
Member

@gotjosh gotjosh commented Sep 1, 2021

This enables support for custom Maintenance callbacks as part of the periodic maintenance of silences and notification logs.
Effectively a no-op for the Alertmanager but allows a downstream implementation to inject custom logic as part of it.

  • Does this requires a changelog? Given it's a no-op for anything alertmanager related.

This enables support for custom Maintenance callbacks as part of the periodic maintenance of silences and notification logs.
Effectively a no-op for the Alertmanager but allows downstream implementation to inject custom logic as part of it.

Signed-off-by: gotjosh <josue.abreu@gmail.com>
Signed-off-by: gotjosh <josue.abreu@gmail.com>
@gotjosh gotjosh force-pushed the allow-custom-functions-for-maintenance branch from 6dc913f to 641eb2d Compare September 1, 2021 17:13
@roidelapluie
Copy link
Member

roidelapluie commented Sep 1, 2021

=== RUN   TestWithMaintenance_SupportsCustomCallback
==================
WARNING: DATA RACE

Signed-off-by: gotjosh <josue.abreu@gmail.com>
@gotjosh gotjosh force-pushed the allow-custom-functions-for-maintenance branch from 5819fc5 to 2db75f7 Compare September 2, 2021 08:41
Copy link
Member

@gouthamve gouthamve left a comment

Choose a reason for hiding this comment

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

Looks good, but omg the code is hard to grok due to the small variable names. Can we make that better and document in the function comment that this new parameter is optional and advanced usage?

Signed-off-by: gotjosh <josue.abreu@gmail.com>
Signed-off-by: gotjosh <josue.abreu@gmail.com>
@gotjosh gotjosh force-pushed the allow-custom-functions-for-maintenance branch from 4f1a7e4 to cdfbc2b Compare September 2, 2021 11:46
Copy link
Member

@gouthamve gouthamve left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@codesome codesome left a comment

Choose a reason for hiding this comment

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

LGTM with a nit :)

Signed-off-by: gotjosh <josue.abreu@gmail.com>
@codesome codesome merged commit 8da5175 into prometheus:main Sep 6, 2021
@gotjosh gotjosh deleted the allow-custom-functions-for-maintenance branch September 6, 2021 13:22
juliusv added a commit that referenced this pull request Sep 13, 2021
#2689 introduced a
regression where the default maintenance function would no longer be
called even if no override was specified. The Alertmanager now crashes
on any silence maintenance run without this fix.

Signed-off-by: Julius Volz <julius.volz@gmail.com>
@juliusv
Copy link
Member

juliusv commented Sep 13, 2021

This introduced a crash bug for the default silence maintenance function, see fix in #2701.

codesome pushed a commit that referenced this pull request Sep 13, 2021
#2689 introduced a
regression where the default maintenance function would no longer be
called even if no override was specified. The Alertmanager now crashes
on any silence maintenance run without this fix.

Signed-off-by: Julius Volz <julius.volz@gmail.com>
@pracucci pracucci mentioned this pull request Sep 15, 2021
3 tasks
nekketsuuu pushed a commit to nekketsuuu/alertmanager that referenced this pull request Oct 1, 2021
…s#2689)

* Enable support for custom callbacks as part of maintenance

This enables support for custom Maintenance callbacks as part of the periodic maintenance of silences and notification logs.
Effectively a no-op for the Alertmanager but allows downstream implementation to inject custom logic as part of it.

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* Add tests

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* Fix tests and remove whitespace

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* Address review comments

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* run go fmt

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* Fix import ordering

Signed-off-by: gotjosh <josue.abreu@gmail.com>
nekketsuuu pushed a commit to nekketsuuu/alertmanager that referenced this pull request Oct 1, 2021
prometheus#2689 introduced a
regression where the default maintenance function would no longer be
called even if no override was specified. The Alertmanager now crashes
on any silence maintenance run without this fix.

Signed-off-by: Julius Volz <julius.volz@gmail.com>
markoposavec pushed a commit to markoposavec/alertmanager that referenced this pull request Nov 4, 2021
…s#2689)

* Enable support for custom callbacks as part of maintenance

This enables support for custom Maintenance callbacks as part of the periodic maintenance of silences and notification logs.
Effectively a no-op for the Alertmanager but allows downstream implementation to inject custom logic as part of it.

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* Add tests

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* Fix tests and remove whitespace

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* Address review comments

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* run go fmt

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* Fix import ordering

Signed-off-by: gotjosh <josue.abreu@gmail.com>
markoposavec pushed a commit to markoposavec/alertmanager that referenced this pull request Nov 4, 2021
prometheus#2689 introduced a
regression where the default maintenance function would no longer be
called even if no override was specified. The Alertmanager now crashes
on any silence maintenance run without this fix.

Signed-off-by: Julius Volz <julius.volz@gmail.com>
markoposavec pushed a commit to markoposavec/alertmanager that referenced this pull request Nov 4, 2021
…s#2689)

* Enable support for custom callbacks as part of maintenance

This enables support for custom Maintenance callbacks as part of the periodic maintenance of silences and notification logs.
Effectively a no-op for the Alertmanager but allows downstream implementation to inject custom logic as part of it.

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* Add tests

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* Fix tests and remove whitespace

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* Address review comments

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* run go fmt

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* Fix import ordering

Signed-off-by: gotjosh <josue.abreu@gmail.com>
Signed-off-by: Marko Posavec <Marko.Posavec@infobip.com>
markoposavec pushed a commit to markoposavec/alertmanager that referenced this pull request Nov 4, 2021
prometheus#2689 introduced a
regression where the default maintenance function would no longer be
called even if no override was specified. The Alertmanager now crashes
on any silence maintenance run without this fix.

Signed-off-by: Julius Volz <julius.volz@gmail.com>
Signed-off-by: Marko Posavec <Marko.Posavec@infobip.com>
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