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.

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

Truncating fields that are sent to slack #2774

Merged
merged 1 commit into from
Dec 21, 2021

Conversation

prashbnair
Copy link
Contributor

Signed-off-by: Prashant Balachandran [email protected]

Fixed issue #2769

@roidelapluie
Copy link
Member

roidelapluie commented Nov 26, 2021

Thanks for your contribution. I think we should limit the length of the fields we send to slack, like we do for pagerduty (IIRC), instead of limiting the number of alerts.

@prashbnair
Copy link
Contributor Author

Removed the max alerts config parameter and truncated the field that is the longest as per https://github.com/prometheus/alertmanager/blob/main/template/default.tmpl#L15

@prashbnair prashbnair changed the title adding max_alerts parameter to slack webhook config Truncating fields that are sent to slack Dec 3, 2021
Comment on lines 96 to 100
key, err := notify.ExtractGroupKey(ctx)

if err != nil {
return false, err
}
Copy link
Member

Choose a reason for hiding this comment

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

i would move this block inside the if truncated { ... } block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

if err != nil {
return false, err
}
level.Debug(n.logger).Log("msg", "Truncated text", "text", title, "key", key)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
level.Debug(n.logger).Log("msg", "Truncated text", "text", title, "key", key)
level.Debug(n.logger).Log("msg", "Truncated title", "text", title, "key", key)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

correcting the logic to trucate fields instead of dropping alerts in the slack integration

Signed-off-by: Prashant Balachandran <[email protected]>
Copy link
Member

@simonpasquier simonpasquier left a comment

Choose a reason for hiding this comment

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

lgtm. Thanks!

@simonpasquier simonpasquier merged commit dcaa3a0 into prometheus:main Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants