Mixin: support multiple querier scaled objects #3962
Conversation
We're experimenting with multiple querier scaled objects, querier-burst and querier-burst-backup. This adapts the mixin to support them on the dashboards. Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
colega
force-pushed
the
support-multiple-querier-scaled-objects
branch
from
6357778
to
66a2037
Compare
Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
| 'sum(kube_horizontalpodautoscaler_spec_min_replicas{%s, horizontalpodautoscaler=~"%s"})' % [$.namespaceMatcher(), $._config.autoscaling.querier.hpa_name], | ||
| 'sum(kube_horizontalpodautoscaler_spec_max_replicas{%s, horizontalpodautoscaler=~"%s"})' % [$.namespaceMatcher(), $._config.autoscaling.querier.hpa_name], |
There was a problem hiding this comment.
It looks weird summing min/max. I understand what you're doing here, but the original idea of displaying min/max was that the current number of replicas will be between that band. That's no more true. What if we do min(kube_horizontalpodautoscaler_spec_min_replicas) and max(kube_horizontalpodautoscaler_spec_max_replicas) instead? Would be slightly more accurate.
If you accept this suggestion, then shouldn't be a stack anymore.
There was a problem hiding this comment.
One of the lines of "current" could be under "min", but the topmost would be between min and max.
I think that having min(min) would be also misleading, as that would make that number show half of the real min.
Maybe I should just add another row of panels for querier-burst and querier-burst-backup and make that feature explicitly enabled?
There was a problem hiding this comment.
Discussed offline. I'm up to remove min and keep max as max(max).
There was a problem hiding this comment.
I've left each one of the max values, as they might be different, and doing the max(max) could hide some of the autoscaler being at max value.
|
I'll hold on merging this because the hpa replicas metric never goes to 0, and I need to find a workaround for that. |
Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
|
@pracucci I've updated the PR and updated the screenshot in the description. I removed the min replicas completely, as agreed, and rendered the "scaling metric / target" as "desired replicas". I also added a warning about the "Current" replicas reporting 1 when it's actually 0. |
| $.filterKedaMetricByHPA('keda_metrics_adapter_scaler_metrics_value', $._config.autoscaling.querier.hpa_name), | ||
| 'kube_horizontalpodautoscaler_spec_target_metric{%s, horizontalpodautoscaler="%s"}' % [$.namespaceMatcher(), $._config.autoscaling.querier.hpa_name], | ||
| ||| | ||
| keda_metrics_adapter_scaler_metrics_value |
There was a problem hiding this comment.
I think you should wrap it with ceil() to have a more accurate measurement of the desired replicas.
There was a problem hiding this comment.
I would prefer leaving it as decimal, this way it's clear that it's just the number coming from the metric, not the actual "desired replicas" that HPA has already decided.
Signed-off-by: Oleg Zaytsev <mail@olegzaytsev.com>
What this PR does
We're experimenting with multiple querier scaled objects, querier-burst and querier-burst-backup. This adapts the mixin to support them on the dashboards.
This is how it looks with
hpa_name: 'keda-hpa-querier(-burst(-backup)?)?':Which issue(s) this PR fixes or relates to
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]