Loki mixin: make labelsSelector in loki chunks dashboards configurable #5536
Conversation
jiachengxu
changed the title
jiachengxu
changed the title
There was a problem hiding this comment.
Thanks for this @jiachengxu 👍
Can you try out my suggestion below please?
| @@ -16,14 +16,14 @@ local utils = import 'mixin-utils/utils.libsonnet'; | |||
| $.row('Active Series / Chunks') | |||
| .addPanel( | |||
| $.panel('Series') + | |||
| $.queryPanel('sum(loki_ingester_memory_chunks{%s})' % labelsSelector, 'series'), | |||
| $.queryPanel('sum(loki_ingester_memory_chunks{%s})' % dashboards['loki-chunks.json'].labelsSelector, 'series'), | |||
There was a problem hiding this comment.
I think these can all reference self
There was a problem hiding this comment.
Thank you @dannykopping for the review.
I just tried:
$mixtool lint dashboards.libsonnet
RUNTIME ERROR: Field does not exist: labelsSelector
dashboards/loki-chunks.libsonnet:19:83-102
<std>:237:21-22 thunk from <function <anonymous>>
<std>:776:20-24 thunk from <function <anonymous>>
<std>:32:25-26 thunk from <function <anonymous>>
<std>:32:16-27 function <anonymous>
<std>:776:8-25 function <anonymous>
<std>:237:7-23 function <anonymous>
dashboards/loki-chunks.libsonnet:19:42-102
/Users/jiachengxu/projects/github.com/grafana/loki/production/loki-mixin/vendor/grafana-builder/grafana.libsonnet:234:19-26 thunk from <object <anonymous>>
/Users/jiachengxu/projects/github.com/grafana/loki/production/loki-mixin/vendor/grafana-builder/grafana.libsonnet:234:10-27 object <anonymous>
...
/Users/jiachengxu/projects/github.com/grafana/loki/production/loki-mixin/vendor/grafana-builder/grafana.libsonnet:255:17-24 object <anonymous>
/Users/jiachengxu/projects/github.com/grafana/loki/production/loki-mixin/vendor/grafana-builder/grafana.libsonnet:(246:15)-(256:6)
/Users/jiachengxu/projects/github.com/grafana/loki/production/loki-mixin/vendor/grafana-builder/grafana.libsonnet:(246:15)-(256:6) +:
Field "targets"
Array element 0
Field "panels"
Array element 0
Field "rows"
Field "loki-chunks.json"
During manifestation
I think the reason is that the labelsSelector doesn't belong to the current object(https://github.com/grafana/loki/pull/5536/files#diff-d15d89e43869bd43b09a788d084921eff62a763bbb4c7df33c95cb919a7bd644R7-R10)
There was a problem hiding this comment.
OK, I think it'd be best to make it a local then; I'm not sure what value there is in having this as a field
There was a problem hiding this comment.
but this PR is trying to switch from local to a field, to make it configurable for different use cases. local basically cannot be overridden if you want to use a different labelsSelector.
I saw similar use cases in other dashboards: https://github.com/grafana/loki/blob/main/production/loki-mixin/dashboards/loki-operational.libsonnet#L35 for example, in this dashboard, user can override matchers for difference use cases
There was a problem hiding this comment.
localbasically cannot be overridden if you want to use a different labelsSelector
Ah, right. Got it.
I think we need to take a more deliberate approach with the labelsSelector, then. This implementation feels a little hacky.
For now, I think let's merge it and we should try split this file up and provide a cleaner way to override these labelsSelectors in the future.
Thanks 👍
Signed-off-by: Jiacheng Xu <jiachengxu@users.noreply.github.com>
Head branch was pushed to by a user without write access
|
@dannykopping Thanks for your help! Would you mind approve it again? I just squashed the commits but looks like it can not be auto-merged |
For future reference, there's no need to squash your commits because we squash-merge anyway 👍 |
What this PR does / why we need it:
This PR makes
labelsSelectorin loki chunks dashboards configurable so that it can be configured for different Loki installations.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Checklist
CHANGELOG.mdabout the changes.