Skip to content

feat: allow nested details fields in pagerduty #3944

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

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 [email protected]

@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
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 <[email protected]>
@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
  ...

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