jmx-scraper implement stable service.instance.id #2270
Conversation
|
|
||
| class PropertiesCustomizerTest { | ||
|
|
||
| private static final String DUMMY_URL = "service:jmx:rmi:///jndi/rmi://host:999/jmxrmi"; |
There was a problem hiding this comment.
[for reviewer] we now have to test with an URL that can be parsed because the parsing is triggered, while it used to not be parsed previously.
breedx-splk
left a comment
breedx-splk
left a comment
There was a problem hiding this comment.
Looks good. Had a couple comments, but this is nice work. Thanks.
jmx-scraper/src/integrationTest/java/io/opentelemetry/contrib/jmxscraper/JmxConnectionTest.java
Outdated
Show resolved
Hide resolved
jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/config/PropertiesCustomizer.java
Show resolved
Hide resolved
| ObjectName objectName = new ObjectName("java.lang:type=Runtime"); | ||
| for (String attribute : Arrays.asList("StartTime", "Name")) { | ||
| Object value = connection.getAttribute(objectName, attribute); | ||
| if (id.length() > 0) { | ||
| id.append(" "); | ||
| } | ||
| id.append(value); | ||
| } |
There was a problem hiding this comment.
The description in #2206 includes the Pid but it's missing here. Is that intentional? Maybe the start time is enough....but I like the idea of including the pid, because time might introduce an edge case.
There was a problem hiding this comment.
Including the Pid is the first thing that failed when testing with Java8, so it must have been added in a later JVM version. From what I've seen so far it's implicitly included in the Name attribute which is usually in the 1234@hostname format, where 1234 is the PID.
…ontrib into stable-service-id
jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java
Outdated
Show resolved
Hide resolved
jmx-scraper/src/main/java/io/opentelemetry/contrib/jmxscraper/JmxScraper.java
Outdated
Show resolved
Hide resolved
|
@breedx-splk can you merge it ? |
3 participants
Fixes #2206
JMX scraper initialization have to be modified as
This is using an UUID v3 which relies on MD5, however this should be fine because it's only used to fingerprint JVM identity, apart from potentially confusing duplicated metrics there is no potential security risk involved.
Testing of this feature is done through what the tool reports on the standard output, like the "connection test" feature. Having proper end-to-end testing would require to validate the data sent to the otlp collector, and there is currently no such test, which means it would likely require a larger effort to be implemented. I think this shortcut is however fine given the feature only removes the natural randomness of
service.instance.idresource attribute in the context of metrics captured through JMX.