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

ALERT labels do not overwrite. #1678

Closed
beorn7 opened this Issue May 27, 2016 · 10 comments

Comments

Projects
None yet
5 participants
@beorn7
Copy link
Member

beorn7 commented May 27, 2016

https://prometheus.io/docs/alerting/rules/ reads (about LABELS in ALERT rules): "Any existing conflicting labels will be overwritten."

That doesn't seem to be true. Existing labels are left intact, and the newly added label is ignored in case of a conflict.

Either we need to fix the docs, or the code.

What's the intended behavior? @brian-brazil @fabxc

@fabxc

This comment has been minimized.

Copy link
Member

fabxc commented May 27, 2016

Overwritten should probably be used as the labels from the rule are more likely to be important in routing.
We can go the exported_ route to resolve conflicts with overwriting again.

@beorn7

This comment has been minimized.

Copy link
Member Author

beorn7 commented May 27, 2016

OK, important is to bring the behavior in line with the documentation then. I'm not sure if we really need the prefixing. It's only for an alerting rule, so we are not really throwing data away. It's all under control of the user.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented May 27, 2016

If the rule labels have precedence, what happens when overriding a label results in duplicate alerts? Should they just be deduped then?

@matthiasr

This comment has been minimized.

Copy link
Contributor

matthiasr commented May 27, 2016

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented May 28, 2016

Prefixing isn't required (can be done by an additional label if needed), and with the default receiver templates would just cause noise for those using it.

@beorn7

This comment has been minimized.

Copy link
Member Author

beorn7 commented May 28, 2016

If the label override creates duplicate alerts (== time series), it should be handled in the same way as we handle duplicate time series created by rules in general (via label replacement or plainly by duplicate rules). At the moment, we just silently feed the results into the same time series. That's not ideal, but if we want to change the behavior, we should do it consistently and not create a one-off solution just for LABELS in an ALERT rule.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Jul 8, 2016

The is a bug in currentAlerts, the overwrite is going in the wrong direction so it'd ending up as a default instead.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Jul 8, 2016

I have a fix, unittests will take some time though.

brian-brazil added a commit that referenced this issue Jul 12, 2016

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Jul 12, 2016

Fix is at https://github.com/prometheus/prometheus/compare/am-label but github won't let me create a PR.

@lock

This comment has been minimized.

Copy link

lock bot commented Mar 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.