Skip to content

Conversation

@siavashs
Copy link
Contributor

@siavashs siavashs commented Aug 5, 2024

This change allows nested key/value pair in pageduty configuration when using Event API V2 integration.

The configuration and payload types where changed from map[string]string to map[string]interface{}.
The details map is walked in stages to render all template strings.

Fixes #2477
Fixes #3218

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

@siavashs siavashs force-pushed the fix_pagerduty_custom_details branch from e838ea9 to 47a3b07 Compare August 5, 2024 14:30
@siavashs siavashs marked this pull request as draft August 5, 2024 14:34
@siavashs siavashs force-pushed the fix_pagerduty_custom_details branch 3 times, most recently from 4398f8e to 4200259 Compare August 5, 2024 16:06
@siavashs siavashs marked this pull request as ready for review August 5, 2024 16:07
@grobinson-grafana
Copy link
Collaborator

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
Copy link
Contributor Author

siavashs commented Aug 6, 2024

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
Copy link
Collaborator

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
Copy link
Contributor Author

siavashs commented Sep 5, 2024

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
Copy link

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

@siavashs
Copy link
Contributor Author

siavashs commented Apr 1, 2025

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

@siavashs siavashs force-pushed the fix_pagerduty_custom_details branch from 4200259 to 20fa318 Compare April 1, 2025 12:11
@siavashs siavashs changed the title fix: pageduty custom_details in payload feat: allow nested details fields in pagerduty Apr 1, 2025

Verified

This commit was signed with the committer’s verified signature.
siavashs Siavash Safi
This change allows nested key/value pair in pageduty configuration when
using `Event API V2` integration.

The configuration and payload types where changed from
`map[string]string` to `map[string]interface{}`.
The details map is walked in stages to render all template strings.

Fixes prometheus#2477
Fixes prometheus#3218

Signed-off-by: Siavash Safi <siavash@cloudflare.com>
@siavashs siavashs force-pushed the fix_pagerduty_custom_details branch from 20fa318 to 6fc0d06 Compare April 1, 2025 12:16
@siavashs
Copy link
Contributor Author

siavashs commented Apr 8, 2025

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 siavashs marked this pull request as draft October 20, 2025 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants