Closed
Description
Code of Conduct
- I agree to follow this project's Code of Conduct
Search before asking
- I have searched in the issues and found no similar issues.
What would you like to be improved?
Current monitoring configuration in the Helm chart seems to be inconsistent and not convenient to use.
Inconsistency
Property monitoring.prometheus.enabled
is used for kyuubi.metrics.enabled
which seems to be confusing, because metrics can be enabled without PROMETHEUS
reporter. Also using monitoring.prometheus.enabled
in PrometheusRule
, ServiceMonitor
and PodMonitor
conditions is redundant, because having PROMETHEUS
in metricsReporters
is enough for the check.
Bug
The following conditions check if metricsReporters
equal to PROMETHEUS
:
However,
metricsReporters
is used to initialize kyuubi.metrics.reporters
which is a comma-separated list for all metrics reporters. I.e. metricsReporters
can be also PROMETHEUS,CONSOLE
, JMX,PROMETHEUS,CONSOLE
etc.The implementation should consider such cases in the conditions and check if
PROMETHEUS
included into the comma-separated list.
The command renders PrometheusRule
:
helm template kyuubi charts/kyuubi --set prometheusRule.enabled=true --set metricsReporters="PROMETHEUS" --show-only templates/kyuubi-alert.yaml
These commands also should render PrometheusRule
:
helm template kyuubi charts/kyuubi --set prometheusRule.enabled=true --set metricsReporters="JMX\,PROMETHEUS" --show-only templates/kyuubi-alert.yaml
helm template kyuubi charts/kyuubi --set prometheusRule.enabled=true --set metricsReporters="PROMETHEUS\,CONSOLE" --show-only templates/kyuubi-alert.yaml
helm template kyuubi charts/kyuubi --set prometheusRule.enabled=true --set metricsReporters="JMX\,PROMETHEUS\,CONSOLE" --show-only templates/kyuubi-alert.yaml
How should we improve?
Fix the bug and change values structure, for instance to the following:
metrics:
# Use for kyuubi.metrics.enabled value
enabled: true
# Use for kyuubi.metrics.reporters value
reporters: PROMETHEUS
# Use for kyuubi.metrics.prometheus.port value and for the chart templates
prometheusPort: 10019
# Move inside "metrics" tree to keep monitoring related configs together
podMonitor:
enabled: false
...
# Move inside "metrics" tree to keep monitoring related configs together
serviceMonitor:
enabled: false
...
# Move inside "metrics" tree to keep monitoring related configs together
prometheusRule:
enabled: false
...
Are you willing to submit PR?
- Yes. I would be willing to submit a PR with guidance from the Kyuubi community to improve.
- No. I cannot submit a PR at this time.
Metadata
Metadata
Assignees
Labels
No labels