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.

Sign up for GitHub

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

Jump to bottom

Add the option to update message and description when sending alerts to opsgenie #2519

Merged
merged 9 commits into from
Sep 8, 2021
Merged

Add the option to update message and description when sending alerts to opsgenie #2519

merged 9 commits into from
Sep 8, 2021

Conversation

tomasfreund
Copy link
Contributor

@tomasfreund tomasfreund mentioned this pull request Apr 7, 2021
Closed
HamidEDD
HamidEDD reviewed May 17, 2021
View reviewed changes
Copy link

@HamidEDD HamidEDD left a comment

Choose a reason for hiding this comment

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

LGTM

@dgsb
Copy link

dgsb commented Jun 1, 2021 •
edited
Loading

Hello, any plan to merge that soon ?

roidelapluie
roidelapluie reviewed Jul 1, 2021
View reviewed changes
Copy link
Member

@roidelapluie roidelapluie left a comment

Choose a reason for hiding this comment

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

Thanks! I have added a few comments/questions to start with.

docs/configuration.md Outdated Show resolved Hide resolved
docs/configuration.md Outdated Show resolved Hide resolved
docs/configuration.md Outdated Show resolved Hide resolved
notify/opsgenie/opsgenie.go Show resolved Hide resolved
alistarle
alistarle reviewed Aug 7, 2021
View reviewed changes
docs/configuration.md Outdated Show resolved Hide resolved
tomasfreund and others added 9 commits August 10, 2021 19:05
@tomasfreund
add the option to update message and description when sending alerts …
79dfb86 
…to opsgenie

Signed-off-by: Tomáš Freund <tomas.freund@datamole.cz>
@tomasfreund
fix opsgenie retry handling logic error
adb6955 
Signed-off-by: Tomáš Freund <tomas.freund@datamole.cz>
@tomasfreund
fix response drain order
09e755e 
Signed-off-by: Tomáš Freund <tomas.freund@datamole.cz>
@roidelapluie @tomasfreund
Apply suggestions from code review
e9f1f19 
Co-authored-by: Julien Pivotto <roidelapluie@gmail.com>
Signed-off-by: Tomáš Freund <tomas.freund@datamole.cz>
@alistarle @tomasfreund
Update docs/configuration.md
cafd84b 
Co-authored-by: Victor Coutellier <victor.coutellier@gmail.com>
Signed-off-by: Tomáš Freund <tomas.freund@datamole.cz>
@tomasfreund
change update_message and update_description options to a single upda…
6f07195 
…te_alerts option

Signed-off-by: Tomáš Freund <tomas.freund@datamole.cz>
@tomasfreund
add update_alerts option docs
ecf87d1 
Signed-off-by: Tomáš Freund <tomas.freund@datamole.cz>
@tomasfreund
fix imports
92e7c4b 
Signed-off-by: Tomáš Freund <tomas.freund@datamole.cz>
@tomasfreund
fix imports order
10e6e04 
Signed-off-by: Tomáš Freund <tomas.freund@datamole.cz>
@tomasfreund
Copy link
Contributor Author

This PR should be ready to merge if nobody has any other comments. We've been running alertmanager with these changes in production and haven't noticed any issues. The alerts get updated as expected.

@HamidEDD
Copy link

HamidEDD commented Sep 3, 2021

it has also been deployed in our production for about 2 months, work as expected and no issue to report !

@rbizos
Copy link

rbizos commented Sep 8, 2021 •
edited
Loading

Hello 👋 this feature was requested by our users, it would be great to see it merged 😄

@roidelapluie roidelapluie merged commit 4401141 into prometheus:main Sep 8, 2021
@roidelapluie
Copy link
Member

Thanks, LGTM!

@pracucci pracucci mentioned this pull request Sep 15, 2021
Merged
3 tasks
mahumpula added a commit to mahumpula/prometheus-operator that referenced this pull request Dec 25, 2021
@mahumpula
alertmanagerconfig/opsgenie: allow specify updateAlert
c06bbca 
See prometheus/alertmanager#2519
mahumpula added a commit to mahumpula/prometheus-operator that referenced this pull request Dec 25, 2021
@mahumpula
alertmanagerconfig/opsgenie: allow specify updateAlert
67c94f2 
AlertManager Opsgenie receiver supports update_alert option, which is by
default turned off. Allow controlling the settings via
AlertmanagerConfig.

See prometheus/alertmanager#2519
mahumpula added a commit to mahumpula/prometheus-operator that referenced this pull request Dec 25, 2021
@mahumpula
alertmanagerconfig/opsgenie: allow specify updateAlerts option
b73d884 
AlertManager Opsgenie receiver supports update_alerts option, which is by
default turned off. Allow controlling the settings via
AlertmanagerConfig.

See prometheus/alertmanager#2519
mahumpula added a commit to mahumpula/prometheus-operator that referenced this pull request Dec 25, 2021
@mahumpula
alertmanagerconfig/opsgenie: allow specify updateAlerts option
36b0e94 
AlertManager Opsgenie receiver supports update_alerts option, which is by
default turned off. Allow controlling the option via
AlertmanagerConfig.

See prometheus/alertmanager#2519
mahumpula added a commit to mahumpula/prometheus-operator that referenced this pull request Dec 25, 2021
@mahumpula
Allow overriding update_alerts for Opsgenie receiver
68e85fa 
AlertManager Opsgenie receiver supports update_alerts option, which is by
default turned off. Allow controlling the option via
AlertmanagerConfig.

See prometheus/alertmanager#2519
@GustavJaner
Copy link

@tomasfreund thanks for this PR! However when I'm adding update_alerts: true to an existing working alertmanager config, the operator gives an error on the alertmanager config

  • prometheus operator version: v0.56.0
  • prometheus alertmanager version: v0.24.0

Any idea?
operator logs

rancher-monitoring-operator-6c58844c48-jvngr rancher-monitoring level=error ts=2022-05-06T08:09:49.626875745Z caller=klog.go:116 component=k8s_client_runtime func=ErrorDepth msg="Sync \"cattle-monitoring-system/rancher-monitoring-alertmanager\" failed: provision alertmanager configuration: failed to initialize from secret: yaml: unmarshal errors:\n  line 64: field update_alerts not found in type config.plain\n```

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alistarle alistarle alistarle left review comments

@HamidEDD HamidEDD HamidEDD left review comments

@roidelapluie roidelapluie Awaiting requested review from roidelapluie

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@tomasfreund @dgsb @HamidEDD @rbizos @roidelapluie @GustavJaner @alistarle