Add support for remote jmx rules file.

[luigidemasi]

Add support to specify remote jmx rules files for otel.jmx.config property, for example:

java -javaagent:opentelemetry-javaagent.jar -Dotel.jmx.config=https://raw.githubusercontent.com/luigidemasi/opentelemetry-java-instrumentation/refs/heads/camel-update/instrumentation/jmx-metrics/javaagent/src/main/resources/jmx/rules/camel.yaml,/path/to/my/customFile.yaml ..... myApp.jar

[trask]

cc @PeterF778 @SylvainJuge @robsunday @breedx-splk

[SylvainJuge]

Adding this is a great addition as it helps to have one extra config file to manage, at least while we still have to rely on a separate YAML definition file for the JMX configuration (maybe later when the declarative configuration will be ready the whole agent/sdk configuration will fit in a single file and/or can be changed at runtime).

It would be nice to add some documentation in https://github.com/open-telemetry/opentelemetry-java-instrumentation/tree/main/instrumentation/jmx-metrics/javaagent#configuration-files

One potential issue here is that the agent listener is synchronous as far as I remember, thus if the URL is slow to respond then it might delay the whole agent startup. I remember that we had similar concerns with the resource attributes providers, but here given it's optional maybe documenting this is enough.

[laurit]

Are there any security implications to parsing the configuration from a remote source?

[SylvainJuge]

Are there any security implications to parsing the configuration from a remote source?

I definitely see a few ones:

• if the remote host is compromised, we can't have any guarantees on the YAML content
• if there is a vulnerability in the yaml parser, this adds a way to propagate to the monitored app.

For example, if there is any remote code execution (RCE) in the yaml parsing library, then compromise from the remote host means RCE on the whole application. If the vulnerability is just a "bug" like an infinite loop when using fancy YAML features like references, then it could still be used to keep the CPU busy and have side-effects on the app.

However:

• the YAML format is more descriptive than imperative, and it's not usually parsed by doing an equivalent of a JS eval(...), so it's quite safe by nature. However I would not bet strongly on that assumption.

While I definitely see value in adding out-of-the box support for this as a convenience, it could also be replaced by a curl + writing to local file before the application startup as an ad-hoc alternative implementation (still not safer security-wise, but at least we could assume the efforts to implement this would make it a more intentional and deliberate choice).

[luigidemasi]

Hi @SylvainJuge

I definitely see a few ones:

if the remote host is compromised, we can't have any guarantees on the YAML content

For a mission critical app, you wan't use a public and/or untrusted URL for rules, and if someone compromises the private remote host, probably you are in a bigger trouble...

if there is a vulnerability in the yaml parser, this adds a way to propagate to the monitored app.

Also, to be vulnerable to the (un)famous "Unsafe deserialization" issue, you need to have at least a gadget class in your classpath and give the possibility to deserialize arbitrary class
(and from a security scanner point of view, you are still vulnerable, even if you are deserializing a local file)

https://snyk.io/blog/unsafe-deserialization-snakeyaml-java-cve-2022-1471/

While I definitely see value in adding out-of-the box support for this as a convenience, it could also be replaced by a curl + writing to local file before the application startup as an ad-hoc alternative implementation (still not safer security-wise, but at least we could assume the efforts to implement this would make it a more intentional and deliberate choice).

This is just moving the problem somewhere else, not solving it.

[trask]

at least while we still have to rely on a separate YAML definition file for the JMX configuration

hi @luigidemasi! once https://github.com/open-telemetry/opentelemetry-configuration is stable, we are going to move the JMX configuration inside of that file, at which point we wouldn't support reading that config file from a remote location without broader OpenTelemetry community consensus (beyond just Java), since it will be part of the specification.

as @SylvainJuge mentions, there is work in progress to support remote configuration in the OpenTelemetry ecosystem under https://github.com/open-telemetry/opamp-spec, with some initial work in Java at https://github.com/open-telemetry/opentelemetry-java-contrib/pulls.

I'm wondering if it would be better to solve this use case for now outside of OpenTelemetry (e.g. curl) until we work out (and implement) the broader remote configuration story in OpenTelemetry.

[luigidemasi]

hi @luigidemasi! once https://github.com/open-telemetry/opentelemetry-configuration is stable, we are going to move the JMX configuration inside of that file, at which point we wouldn't support reading that config file from a remote location without broader OpenTelemetry community consensus (beyond just Java), since it will be part of the specification.

Hi @trask, do you have a rough estimate on when it will happen?

[trask]

Hi @trask, do you have a rough estimate on when it will happen?

Hard to say. Experimental yaml configuration is already available, but no one has hooked the JMX rules into it yet. Sometime after that happens I believe we would deprecate otel.jmx.config.

[SylvainJuge]

Hi @trask, do you have a rough estimate on when it will happen?

Hard to say. Experimental yaml configuration is already available, but no one has hooked the JMX rules into it yet. Sometime after that happens I believe we would deprecate otel.jmx.config.

In order to have an almost identical use-case as the one this PR aims to solve, two things would be required:

• allow to define the JMX configuration directly in the yaml configuration, which is something that will be required anyway, but depends on the (yet experimental) yaml configuration.
• have the ability to fetch configuration from a URL.

However, the declarative configuration with JMX metrics removes the need to deal with an extra configuration file, thus having the ability to download it from an URL is probably not needed anymore, or would be implemented directly in the SDK where the declarative configuration is handled.

In addition to that, the main blocker for doing this now is that declarative configuration does not allow to customize configuration (opentelemetry-java#6575), which is if I understand it correctly the way JMX metrics configuration would be integrated.