-
Notifications
You must be signed in to change notification settings - Fork 162
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
Remove confusing job!="" filteringSelector when no selector is required. #1370
Conversation
97d2fe7
to
b8ffd20
Compare
6b10482
to
39af36a
Compare
To pass tests, we need to merge #1372 separately, then rebase here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from the comments on the dependency PR this looks good :)
@@ -13,6 +13,11 @@ local utils = import '../utils.libsonnet'; | |||
prometheusDatasourceName=if enableLokiLogs then 'prometheus_datasource' else 'datasource', | |||
prometheusDatasourceLabel=if enableLokiLogs then 'Prometheus data source' else 'Data source', | |||
): { | |||
// strip trailing or starting comma if present: | |||
// while trailing comman is accepted in PromQL expressions, starting comma is not. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo fix suggestion in dependency PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be fixed by merge.
since filteringSelector is now optional (can be set to ""),
remove confusing job!="" filteringSelector when no selector is required,
from some mixins/libs:
This should help with mixins rendered on
monitoring-mixin website:
https://monitoring.mixins.dev/jvm/