prometheus / jmx_exporter

A process for exposing JMX Beans via HTTP for Prometheus consumption
Apache License 2.0
3.06k stars 1.2k forks source link

Javaagent can no longer be loaded with /dev/null #883

Closed FireBurn closed 6 months ago

FireBurn commented 1 year ago

We used to use:

-javaagent:jmx_prometheus_javaagent.jar=<host>:<port>:/dev/null

but that no longer works since https support was added, is it possible to use a sensible default if no file is specified or to allow /dev/null again?

https://github.com/prometheus/jmx_exporter/blob/89275ac5fed732b943c248867db578b6f12bb756/jmx_prometheus_common/src/main/java/io/prometheus/jmx/common/http/HTTPServerFactory.java#L137

dhoard commented 1 year ago

Can you elaborate on why you are using /dev/null as a configuration practice?

I don't see that as a common use case. My concern with defaults is that "sensible defaults" are based on perspective.

Considering the code using the exporter has to be installed/configured, adding a generic /etc/exporter.yaml as part of that process (or in a base Docker image) seems like a reasonable step for users.

FireBurn commented 1 year ago

We only passed in a yaml file where we needed to manipulate the output or delay startup

Can I just check, was passing /dev/null before the same as providing:

rules:
- pattern: ".*"
dhoard commented 1 year ago

I feel that the negatives outweigh the positives for such a niche configuration scenario.

Bug

The fact that /dev/null worked in the past feels like a bug. The documentation and code before HTTPS/basic authentication support was added both references the requirement for an exporter YAML file.

https://github.com/prometheus/jmx_exporter/tree/release-0.17.2

https://github.com/prometheus/jmx_exporter/blob/4bbb8ed6a25b5ce1e05ba9e0d10c884d0ca3f25a/jmx_prometheus_javaagent_common/src/main/java/io/prometheus/jmx/JavaAgent.java#L21-L38

Platform-specific code

Coding to specifically check for /dev/null introduces Linux-specific code that isn't easily/if at all possible to be implemented on Windows.

Defaults

Defaults will cause issues when the configuration filename is invalid.

Example:

Someone configures /etc/exporter.yaml, but the actual filename is /etc/exporer.yaml (typo). The exporter would run with the defaults, resulting in configuration/debugging time... "why is the exporter not working?"

FireBurn commented 1 year ago

We're moving to use a file, just need you to confirm what passing /dev/null was equivalent to, then I'm happy to close this

dhoard commented 1 year ago

Looking at the code, it should be the equivalent of...

rules:
- pattern: ".*"

Code

If no rules are defined, which would be the scenario for /dev/null (empty file), then the code creates a single Rule object. Note: the default Rulle object value type is undefined (catch-all)

https://github.com/prometheus/jmx_exporter/blob/4bbb8ed6a25b5ce1e05ba9e0d10c884d0ca3f25a/collector/src/main/java/io/prometheus/jmx/JmxCollector.java#L270

... ultimately calling default export...

https://github.com/prometheus/jmx_exporter/blob/4bbb8ed6a25b5ce1e05ba9e0d10c884d0ca3f25a/collector/src/main/java/io/prometheus/jmx/JmxCollector.java#L396