feat: allow nested details fields in pagerduty

[siavashs]

This change allows nested key/value pair in pageduty configuration.

Events API V1 supports an object in details field:
https://developer.pagerduty.com/docs/send-v1-event#parameters

Events API V2 supports an object in payload.custom_details field:
https://developer.pagerduty.com/docs/send-alert-event#parameters

The configuration and message/payload types where changed from map[string]string to map[string]any.

The default template is updated to use the new toJson function.

Fixes #2477
Fixes #3218

Signed-off-by: Siavash Safi siavash@cloudflare.com

[grobinson-grafana]

I don't think it is intentional that the default template is valid YAML, it's just meant to be plain text. I'd even argue it's meant to be Markdown. If I understand the use case, you would like to be able to add structured objects (with nesting) to the custom_details field?

[siavashs]

I don't think it is intentional that the default template is valid YAML, it's just meant to be plain text. I'd even argue it's meant to be Markdown. If I understand the use case, you would like to be able to add structured objects (with nesting) to the custom_details field?

Correct, our teams prefer that so they can add logic on PagerDuty side depending on the field values.
I tried to keep the logic generic so it would resolve the 2 referenced issues as well.

[grobinson-grafana]

Have you considered changing Details in the configuration file from map[string]string to map[string]interface{}?

// PagerdutyConfig configures notifications via PagerDuty.
type PagerdutyConfig struct {
...
	Details        map[string]interface{} `yaml:"details,omitempty" json:"details,omitempty"`
}

Would this avoid having to embed YAML inside existing YAML? I haven't checked if this will affect existing configurations, but it would be good to understand if this is a better option.

[siavashs]

Have you considered changing Details in the configuration file from map[string]string to map[string]interface{}?

That was the initial approach I though about. It will create a free style format and would require checking the interfaces one by one as they can be either maps or templated strings. I'm open to experiment with it.

[Daniel-Vaz]

Curious to know... Is this going forward ?
Seems a real blessing having PD Custom Details properly rendered from the Alert Payload.

[siavashs]

I'll update this PR with the suggested map[string]interface{} to support nested fields.

[siavashs]

Thinking about this more, using a template like alerts: {{ .Alerts }} will translate into alert key with value of string as templates are only rendered into strings.

Parsing template results into maps or slices is possible but could be error-prone and we'd need a format for it.
Alternative option could be toggles to include specific details in the payload:

details: ...

details_include:
  alerts: true
  ...

[siavashs]

I updated the PR to use the new toJson template function.
Also we can now support this in both Events API v1 and v2.

[NargiT]

@siavashs you are a true hero

[abhijith-db]

@siavashs For v1 integration, this seems to have changed the urls in label/annotations to not render as links in PD. PD used to make them clickable hyperlinks when firing value was string, but now it's all jsonified and render as plain text.

[siavashs]

@abhijith-db this sounds like a UI issue on PD side.
As a workaround you can always use a "non-json" template on Alertmanager side and it should restore the old format.

[abhijith-db]

@siavashs Yeah that's a workaround. Only problem is there's no global PD default template to configure. Each receiver has to use the non-json template to linkify urls in annotations

[siavashs]

I see, we'll add a global config for PagerDuty.

[abhijith-db]

@siavashs Also the deep copy with template was affecting this too. Had to fork and fix on our end to exclude string type. As:

		if strVal, ok := v.(string); ok {
			// For string values, evaluate the template directly without
			// YAML-parsing the result. This preserves plain text output
			// (e.g. from pagerduty.default.instances) as strings.
			rendered[k], err = tmplTextFunc(strVal)
		} else {
			rendered[k], err = template.DeepCopyWithTemplate(v, tmplTextFunc)
		}

Failing testcase that would have passed before this change:

		{
			name: "default text template",
			args: args{
				details: map[string]any{
					"firing":       `{{ template "pagerduty.default.instances" .Alerts.Firing }}`,
					"resolved":     `{{ template "pagerduty.default.instances" .Alerts.Resolved }}`,
					"num_firing":   `{{ .Alerts.Firing | len }}`,
					"num_resolved": `{{ .Alerts.Resolved | len }}`,
				},
				data: &template.Data{
					Alerts: template.Alerts{
						{
							Status: "firing",
							Labels: template.KV{
								"alertname": "TestAlert",
							},
							Annotations: template.KV{
								"description": "Something is wrong",
							},
							GeneratorURL: "http://localhost:9090/graph",
						},
					},
				},
			},
			want: map[string]any{
				"firing":       "Labels:\n - alertname = TestAlert\nAnnotations:\n - description = Something is wrong\nSource: http://localhost:9090/graph\n",
				"resolved":     "",
				"num_firing":   "1",
				"num_resolved": "0",
			},
			wantErr: false,
		},

It's a regression only because PD would render addresses in annotations etc as links which broke after this.

[Rohlik]

This PR should be marked as potentially breaking changes in the changelog 😞.
In our case, it completely broke up our incident routing on the PagerDuty side via the Event Orchestration feature, where we have routes configured like:

event.custom_details matches part 'owner = security' and event.custom_details matches part 'cluster = worker01'

So, we were heavily reliant on the legacy non-JSON format, and we found that the hard way, unfortunately 😢.