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

Consider supporting the use of annotation values in annotations #1375

Closed
grobie opened this Issue Feb 5, 2016 · 11 comments

Comments

Projects
None yet
3 participants
@grobie
Copy link
Member

grobie commented Feb 5, 2016

erroneously opened as prometheus/alertmanager#243 first

There are alerts where the current value is not useful, for example predict_linear(foo_current, 3600) > foo_max. It's possible to run another expression during evaluation {{ printf 'foo_current{instance="%s"}' $labels.instance | query }}. Using that in a description makes the description quickly unreadable.

Allowing evaluated annotations to be used in other annotations could help here

...
ANNOTATIONS {
  current = "{{ printf 'foo_current{instance="%s"}' $labels.instance | query }}"
  description = "The server has {{$annotations.current}} foos and will reach the maximum in less than 1h"
}

p.s. real world expressions will be a lot longer and thus harder to read.

@grobie grobie added question labels Feb 5, 2016

@grobie

This comment has been minimized.

Copy link
Member Author

grobie commented Feb 5, 2016

@brian-brazil wrote

I don't think this is something we should implement, as labels and annotations are unordered it'd require us to parse them and determine an ordering. Particularly with alerts where evaluation should be kept really low for reliability, we shouldn't be encouraging complex analysis and the sort of complexity templating that requires abstractions like this to manage.

@grobie

This comment has been minimized.

Copy link
Member Author

grobie commented Feb 5, 2016

@fabxc wrote

I agree that it needs ordering in the implementation. But it certainly
doesn't involve complex analysis or a remotely relevant performance impact.

@grobie

This comment has been minimized.

Copy link
Member Author

grobie commented Feb 5, 2016

@brian-brazil When you say evaluation complexity, do you mean the evaluation of additional expressions in general? If yes, what ways do you see to write alerts using predict_linear which give an overview about the current situation.

Otherwise, I don't see why an implementation would necessarily require any analysis. Annotations are written in an order which can't change, if someone wants to use such templating obviously only previously defined annotations could be used.

@matthiasr

This comment has been minimized.

Copy link
Contributor

matthiasr commented Feb 5, 2016

👍 this would be very useful. I think using the ordering in the rules file is the right thing to do here. That there currently "is none" doesn't mean that it needs to stay that way.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Feb 5, 2016

When you say evaluation complexity, do you mean the evaluation of additional expressions in general?

I mean that the feature you're proposing enables and encourages alert templates with a high degree of complexity. Particularly given what you want is trivially possible with go templates already.

What you're asking for here is very close to configuration templates (it's simple text substitution macros), and I'm strongly against those generally and particularly in rule evaluation where latency matters a lot more than for console templates.

If you want to to simple text substitution, there's nothing stopping you from using your configuration management to do so or using go template variables.

I feel that if your alert templates are so complex that you feel the need to add this sort of feature to the core of Prometheus, you should strongly consider ways to simplify them.

@grobie

This comment has been minimized.

Copy link
Member Author

grobie commented Feb 5, 2016

I'm not sure how you picture the implementation, but I'm certain making the list of already evaluated annotations available as additional template input would have a neglectable performance impact.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Feb 5, 2016

The problem isn't the implementation, it's what users would do with it.

We also have the rule that if what you want can be implemented in configuration management, then you should do it there rather than adding features to Prometheus.

@grobie

This comment has been minimized.

Copy link
Member Author

grobie commented Feb 5, 2016

I can't use configuration management here to maintain readable rule files.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Feb 5, 2016

Configuration management includes templating. We've rejected many feature requests for prometheus on the basis that configuration management templating can adequately solve the problem.

There's two solutions proposed so far that require no Prometheus changes, and the implications of the change itself seem quite dangerous to me, so I think implementing this doesn't make any sense.

@grobie

This comment has been minimized.

Copy link
Member Author

grobie commented Feb 15, 2016

Configuration management does not help to produce readable rule files. It's possible to improve the readability of the template file in version control, but the actual config on each host will still be hard to read. Same applies to golang's templating language (while go1.6 will bring some improvements here).

For the sake of keeping the number of open issues maintainable, I'll close this one until we receive more use cases or requests.

@grobie grobie closed this Feb 15, 2016

@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.