Migrate JMX module to declarative config API#15733
Conversation
| // Handle jmx prefix: java.jmx.* → otel.jmx.* | ||
| if (translated.startsWith("jmx.")) { | ||
| return "otel." + translated; | ||
| } |
There was a problem hiding this comment.
I think better to limit this mapping to the current set of system properties so added them instead to SPECIAL_MAPPINGS
| if (discoveryDelay != null) { | ||
| return discoveryDelay; | ||
| private static Duration beanDiscoveryDelay( | ||
| ExtendedDeclarativeConfigProperties config, ConfigProperties configProperties) { |
There was a problem hiding this comment.
will remove ConfigProperties later since it will be a breaking change as it's used to access otel.metric.export.interval below which isn't available via declarative config API
| SPECIAL_MAPPINGS.put("java.jmx.discovery.delay", "otel.jmx.discovery.delay"); | ||
| SPECIAL_MAPPINGS.put("java.jmx.target_system", "otel.jmx.target.system"); |
There was a problem hiding this comment.
I missed this PR, but I think here we could probably have choosen target.system instead of target_system as with JMX Scraper we also have target.source, is there a particular reason to prefer one or the other ?
I've opened #15856 as an attempt to fix this with a target object then system sub-attribute, but maybe it's intentional and we prefer to keep it the way it is to prevent too much structure in config, I'm fine with any implications it could have on jmx-scraper. At the same time, the JMX scraper target.source option might go away in some distant future, but we are not there yet.
Also, with discovery.delay there is no other discovery.* configuration option, so for this one it could have made sense to use discovery_delay, unless we prefer to preserve the existing dotted structure or we know that other discovery-related configuration options will be needed later.
There was a problem hiding this comment.
I don't think there was a particular reason - feel free to change both
3 participants
No description provided.