Jmx Exports for Workers

trinodb / charts

Apache License 2.0

151 stars 173 forks source link

Jmx Exports for Workers #224

Closed gauravmohta closed 1 month ago

gauravmohta commented 2 months ago

The current helm chart has jmx exporter available only for coordinator and the worker pods do not have jmx exports. Could you please add the jmx exports for workers as well?

sdaberdaku commented 1 month ago

If I remember correctly, there are some JVM metrics that are available only by running the jmx exporter on workers, so I believe this feature would be useful. @nineinchnick what do you think?

I can provide a PR with this feature.

A couple of questions:

Should we have separate JMX configurations for the coordinator and workers?
Should we include scrape annotations or PodMonitors?

nineinchnick commented 1 month ago

We might not need separate JMX configuration for the coordinator and workers. I don't get the second question. We already integrate with service monitors for the Prometheus operator, let's continue using that.

sdaberdaku commented 1 month ago

@nineinchnick ~The service monitor will get the metrics from the service, which will act as "load balancer" for the workers. I think we need to scrape the Pods individually. Does this make sense? Every time Prometheus scrapes the service, it might get readings from different workers, I think we might want the metrics of all workers.~

So this is not actually true: https://github.com/prometheus-operator/prometheus-operator/issues/3119#issuecomment-631409690

nineinchnick commented 1 month ago

Sure, we'll need a test for this setup anyway.

sdaberdaku commented 1 month ago

I think we should have separate JMX configurations for coordinator and workers for the following reasons:

One might want to enable JMX Export only on the coordinator and ignore the worker specific metrics;
In the worker JMX configuration we might want to set includeObjectNames or excludeObjectNames or rules differently from the coordinator (maybe on the worker we only want the JVM metrics and nothing more, while on the coordinator we want the specific Trino metrics).

sdaberdaku commented 1 month ago

After some reading I realized that my previous comment regarding the Service Monitor scraping only one Pod was wrong. https://github.com/prometheus-operator/prometheus-operator/issues/3119

It turns out that a ServiceMonitor will "scrape all pods behind the service, because the Service maintains an Endpoints object. That's where Prometheus discovers the targets from."

However, since it does not make sense to have a Service for workers, I would go on and add a PodMonitor only for worker pods.

I also extended the existing jmx tests, I will shortly submit a new PR for this feature.