Propagate labels to Opsgenie details #2276

ricoberger · 2020-06-04T07:29:03Z

Introduce two new config options for the Opsgenie receiver:

all_labels_as_details: If true, all labels are propagated to the Opsgenie details (default is false).
labels_as_details: An optional list of labels, which should be propagated to the Opsgenie details.

If the all_labels_as_details and labels_as_details value isn't set, the existing details option is used.

Closes #682.

Signed-off-by: ricoberger <[email protected]>

simonpasquier · 2020-06-04T10:55:24Z

Thanks for looking into this.

Instead of having 2 more parameters, maybe we can interpret labels_as_details: ['...'] as "include all common labels in the details" (this would be similar to what has been done for group_by).

As it stands, labels_as_details and details are exclusive so it would be good IMO to fail the configuration parsing if the user configures both at the same time. Or maybe the resulting map should be a merge of both with details taking precedence over labels_as_details?

brian-brazil · 2020-06-04T11:00:46Z

this would be similar to what has been done for group_by

Grouping labels are not the same as common labels. The naming of these proposed config fields is confusing.

Signed-off-by: ricoberger <[email protected]>

ricoberger · 2020-06-04T13:50:29Z

Hi @simonpasquier and @brian-brazil, I made the suggested changes, so that there is only a details_labels field. The handling is similar to the group_by handling.

If the details_labels and details field is provided the loading of the configuration will fail now.

brian-brazil

This is still misleading. Common is not all, in fact all is not possible.

config/notifiers.go

ricoberger · 2020-06-04T14:31:04Z

This is still misleading. Common is not all, in fact all is not possible.

So you want to change the option to details_common_labels or sth. similar?

notify/opsgenie/opsgenie.go

Signed-off-by: ricoberger <[email protected]>

bwplotka

Amazing contribution and sorry for the delay in review.

I think this is almost ready to go. I have some idea that might make this even more flexible and simple to implement. Let me know what you think @ricoberger 🤗

I have a tiny proposal: What about having this option CommonLabelsAsDetails and if true it will populate details from common labels, and then anything in Details will be added additionally? It will be bit more flexible for user and also quite simpler to implement and ensure. (:

config/notifiers.go

notify/opsgenie/opsgenie.go

config/notifiers.go

Signed-off-by: ricoberger <[email protected]>

ricoberger · 2020-06-09T05:45:39Z

Hi @bwplotka, good point. I adjusted the code accordingly.

bwplotka

This looks perfect to me, thanks! 👍

cc @simonpasquier

bwplotka · 2020-06-09T05:52:14Z

notify/opsgenie/opsgenie.go

@@ -120,7 +120,14 @@ func (n *Notifier) createRequest(ctx context.Context, as ...*types.Alert) (*http

 	tmpl := notify.TmplText(n.tmpl, data, &err)

-	details := make(map[string]string, len(n.conf.Details))
+	details := make(map[string]string)
+


to be very strict this new line could be omitted. It's best to only put new line to separate logically different sections, whereas the map is related to below if. Not a blocker (:

bwplotka · 2020-06-09T05:52:40Z

Also do you think we can add a quick unit test @ricoberger ? (:

Signed-off-by: ricoberger <[email protected]>

ricoberger · 2020-06-09T07:02:34Z

Also do you think we can add a quick unit test @ricoberger ? (:

Sure. I added some test cases to the existing Opsgenie test.

brian-brazil · 2020-06-09T08:30:14Z

then anything in Details will be added additionally?

I had had a similar thought. Would there be harm in always doing this, not needing a config option?

Also, if there is harm then is common labels safe as it's unpredictable what labels will actually be present?

bwplotka · 2020-06-09T08:50:32Z

Yea, good question. I think this is breaking compatibility, but with very tiny bad effect, just additional details. It's up to maintainers to decide, cc @simonpasquier

Since we are 0.x I think it's ok to just remove bool option and do it ;p

Sorry @ricoberger for so many direction changes. Open source is a collaborative work!

brian-brazil · 2020-06-09T08:55:58Z

I think this is breaking compatibility but with very tiny bad effect, just additional details. It's up to maintainers to decide

Are you using OpsGenie so you can say exactly what might break? If that is the case then common labels is incorrect, and group labels should be used instead - though if it's group labels then you can do it with existing templating as you know the labels a-priori.

ricoberger · 2020-06-09T09:04:38Z

Sorry @ricoberger for so many direction changes. Open source is a collaborative work!

No problem 🙂

I think this is breaking compatibility but with very tiny bad effect, just additional details. It's up to maintainers to decide

Are you using OpsGenie so you can say exactly what might break? If that is the case then common labels is incorrect, and group labels should be used instead - though if it's group labels then you can do it with existing templating as you know the labels a-priori.

I think this shouldn't break anything in Opsgenie. Existing details would be the same, so that created routing rules shouldn't be effected.

Should I adjust the PR accordingly?

brian-brazil · 2020-06-09T09:13:29Z

I think this shouldn't break anything in Opsgenie.

In that case making this hardcoded would seem the best approach.

Signed-off-by: ricoberger <[email protected]>

ricoberger · 2020-06-09T11:52:43Z

I removed the config option and adjusted the tests.

bwplotka

LGTM!

Thanks 👍

brian-brazil · 2020-06-09T13:39:10Z

👍

The docs over in the docs repo will also need an update.

osterman · 2020-07-13T16:55:05Z

This is wondeful! Any chance a new release will be cut @bwplotka?

simonpasquier · 2020-07-24T14:23:36Z

@osterman last release was mid-June. I'd expect we can have another one beginning of September.

Links2004 · 2020-11-10T08:57:55Z

Any timeline for the next release, this feature would help us a lot.

srueg · 2021-01-12T21:23:00Z

Really looking forward to this, thanks! 🎉
Any news on a release?

kedare · 2021-04-20T09:10:22Z

Looks like the option is now gone or has never been released ?
I don't see it in the current documentation :
https://prometheus.io/docs/alerting/latest/configuration/#opsgenie_config

ricoberger · 2021-04-20T11:17:09Z

I think this was never released, because last release is from last summer.

FlxPeters · 2021-05-05T07:04:39Z

That should be included in 0.22.0, right? It's not included in the release notes.

Signed-off-by: Julien Pivotto <[email protected]>

roidelapluie · 2021-05-05T09:05:32Z

Thank you, I have missed it!

ties · 2021-05-06T07:26:35Z

For any other reader: In 4b59db0 it was changed to default to always passing all labels to OpsGenie.

(I did not find any reference to labels_as_details/all_labels_as_details in the current master branch)

Add #2276 to release notes

darkl0rd · 2021-07-03T21:42:35Z

Is there a way to override the behavior? We also raise alerts for our support department, some of the labels we add should ideally not be visible to them? I’ve tried setting details explicitly, but it seems all labels are always added, no matter what I define?

ricoberger · 2021-07-03T22:10:36Z

Hi @darkl0rd, no the behavior can not be changed, but it should be possible to overwrite the values for the labels which shouldn't be visible. When you set a label in the details section with an empty value, the original value for the label value shouldn't be visible in Opsgenie:

alertmanager/notify/opsgenie/opsgenie.go

Line 129 in 44f8adc

for k, v := range n.conf.Details {

darkl0rd · 2021-07-03T23:07:26Z

Thanks for the quick and detailed response. This should do the trick.

darkl0rd · 2021-07-04T16:19:24Z

Thanks for the quick and detailed response. This should do the trick.

Overriding with an empty value does indeed clear out the label. But the key remains visible in OpsGenie. Would it be an idea to adjust the code to only publish those keys which actually have a value set?

darkl0rd · 2021-07-06T10:07:08Z

Honestly, after playing around with it some more - I'd really like to have a way to get rid of the ExtraProperties all together. As labels differ per alert type, it's a hell of a job to determine which ones I need to blank out where. I'd much rather just explicitly define which ones I do want to show or even just disable the entire "details" section when the alert is sent to our first line support department.

v0.22.2 * tag 'v0.22.2': (181 commits) Release 0.22.2 Include pending silences for future muting decisions Release 0.22.1 Default the isEqual flag to true in alertmanager Add test to expose issue prometheus#2426 Release alertmanager 0.22 Relase 0.22.0-rc.2 API: Only pass cluster peer if empty fixed small typo Release 0.22.0-rc.1 Fix panic when HA is disabled Update matcher examples Add prometheus#2276 to release notes Release 0.22.0-rc.0 Lift moratorium on AlertManager receivers with long-term maintenance plan (prometheus#2561) Add OAuth 2.0 Config (prometheus#2560) Add missing EOL Fix flapping acceptance test Unlock at specific points instead of deferring Dispatch: Make sure mutex gets unlocked on call to Stop ...

Propagate labels to Opsgenie details

117c8ba

Signed-off-by: ricoberger <[email protected]>

simonpasquier requested a review from bwplotka June 4, 2020 10:57

simonpasquier added the kind/enhancement label Jun 4, 2020

Adjust Opsgenie config for labels propagation

dcccf54

Signed-off-by: ricoberger <[email protected]>

brian-brazil reviewed Jun 4, 2020

View reviewed changes

config/notifiers.go Outdated Show resolved Hide resolved

brian-brazil reviewed Jun 4, 2020

View reviewed changes

config/notifiers.go Outdated Show resolved Hide resolved

brian-brazil reviewed Jun 4, 2020

View reviewed changes

notify/opsgenie/opsgenie.go Outdated Show resolved Hide resolved

Provide option to use common labels for OpsGenie details

8248c50

Signed-off-by: ricoberger <[email protected]>

bwplotka reviewed Jun 9, 2020

View reviewed changes

config/notifiers.go Outdated Show resolved Hide resolved

config/notifiers.go Outdated Show resolved Hide resolved

notify/opsgenie/opsgenie.go Outdated Show resolved Hide resolved

config/notifiers.go Outdated Show resolved Hide resolved

Populate details from common labels and details

9a87f5c

Signed-off-by: ricoberger <[email protected]>

bwplotka approved these changes Jun 9, 2020

View reviewed changes

Add tests for Opsgenie details

3cff6cb

Signed-off-by: ricoberger <[email protected]>

Always pass all labels to Opsgenie

4b59db0

Signed-off-by: ricoberger <[email protected]>

bwplotka approved these changes Jun 9, 2020

View reviewed changes

bwplotka merged commit 6d77929 into prometheus:master Jun 9, 2020

ricoberger mentioned this pull request Jun 10, 2020

Add note about details handling for Opsgenie prometheus/docs#1663

Closed

srueg mentioned this pull request Feb 24, 2021

Add how-to for sending alerts to OpsGenie appuio/component-openshift4-monitoring#17

Merged

3 tasks

roidelapluie added a commit to roidelapluie/alertmanager that referenced this pull request May 5, 2021

Add prometheus#2276 to relase notes

8cec3a5

Signed-off-by: Julien Pivotto <[email protected]>

roidelapluie added a commit to roidelapluie/alertmanager that referenced this pull request May 5, 2021

Add prometheus#2276 to relase notes

c74204c

Signed-off-by: Julien Pivotto <[email protected]>

roidelapluie added a commit to roidelapluie/alertmanager that referenced this pull request May 5, 2021

Add prometheus#2276 to release notes

edc148b

Signed-off-by: Julien Pivotto <[email protected]>

roidelapluie added a commit that referenced this pull request May 6, 2021

Merge pull request #2568 from roidelapluie/new-release-0.22-cl

fc9f728

Add #2276 to release notes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Propagate labels to Opsgenie details #2276

Propagate labels to Opsgenie details #2276

ricoberger commented Jun 4, 2020

simonpasquier commented Jun 4, 2020

brian-brazil commented Jun 4, 2020

ricoberger commented Jun 4, 2020

brian-brazil left a comment

ricoberger commented Jun 4, 2020

bwplotka left a comment •

edited

Loading

ricoberger commented Jun 9, 2020

bwplotka left a comment

bwplotka Jun 9, 2020

bwplotka commented Jun 9, 2020

ricoberger commented Jun 9, 2020

brian-brazil commented Jun 9, 2020

bwplotka commented Jun 9, 2020

brian-brazil commented Jun 9, 2020

ricoberger commented Jun 9, 2020

brian-brazil commented Jun 9, 2020

ricoberger commented Jun 9, 2020

bwplotka left a comment

brian-brazil commented Jun 9, 2020

osterman commented Jul 13, 2020

simonpasquier commented Jul 24, 2020

Links2004 commented Nov 10, 2020

srueg commented Jan 12, 2021

kedare commented Apr 20, 2021

ricoberger commented Apr 20, 2021

FlxPeters commented May 5, 2021 •

edited

Loading

roidelapluie commented May 5, 2021

ties commented May 6, 2021 •

edited

Loading

darkl0rd commented Jul 3, 2021

ricoberger commented Jul 3, 2021

darkl0rd commented Jul 3, 2021

darkl0rd commented Jul 4, 2021

darkl0rd commented Jul 6, 2021

Propagate labels to Opsgenie details #2276

Propagate labels to Opsgenie details #2276

Conversation

ricoberger commented Jun 4, 2020

simonpasquier commented Jun 4, 2020

brian-brazil commented Jun 4, 2020

ricoberger commented Jun 4, 2020

brian-brazil left a comment

Choose a reason for hiding this comment

ricoberger commented Jun 4, 2020

bwplotka left a comment • edited Loading

Choose a reason for hiding this comment

ricoberger commented Jun 9, 2020

bwplotka left a comment

Choose a reason for hiding this comment

bwplotka Jun 9, 2020

Choose a reason for hiding this comment

bwplotka commented Jun 9, 2020

ricoberger commented Jun 9, 2020

brian-brazil commented Jun 9, 2020

bwplotka commented Jun 9, 2020

brian-brazil commented Jun 9, 2020

ricoberger commented Jun 9, 2020

brian-brazil commented Jun 9, 2020

ricoberger commented Jun 9, 2020

bwplotka left a comment

Choose a reason for hiding this comment

brian-brazil commented Jun 9, 2020

osterman commented Jul 13, 2020

simonpasquier commented Jul 24, 2020

Links2004 commented Nov 10, 2020

srueg commented Jan 12, 2021

kedare commented Apr 20, 2021

ricoberger commented Apr 20, 2021

FlxPeters commented May 5, 2021 • edited Loading

roidelapluie commented May 5, 2021

ties commented May 6, 2021 • edited Loading

darkl0rd commented Jul 3, 2021

ricoberger commented Jul 3, 2021

darkl0rd commented Jul 3, 2021

darkl0rd commented Jul 4, 2021

darkl0rd commented Jul 6, 2021

bwplotka left a comment •

edited

Loading

FlxPeters commented May 5, 2021 •

edited

Loading

ties commented May 6, 2021 •

edited

Loading