DataDog / jmxfetch

Export JMX metrics
Other
97 stars 69 forks source link

collision in App#brokenInstanceMap causes jmx config to be ignored. #253

Open jcrobak opened 5 years ago

jcrobak commented 5 years ago

We have two jmx config files that match (via autodiscovery) a single JVM. During startup, we sometime see errors in the datadog agent log like (note that there are two different checks, check1 and check2):

2019-10-22 15:05:31 UTC | CORE | INFO | (pkg/jmxfetch/jmxfetch.go:216 in func1) | 2019-10-22 15:05:31,684 | INFO | App | Could not initialize instance: check1-x.x.x.x-yyyy: java.util.concurrent.ExecutionException: java.io.IOException: java.io.IOException: Failed to retrieve RMIServer stub: javax.naming.ServiceUnavailableException [Root exception is java.rmi.ConnectException: Connection refused to host: x.x.x.x; nested exception is:
2019-10-22 15:05:31 UTC | CORE | INFO | (pkg/jmxfetch/jmxfetch.go:216 in func1) | 2019-10-22 15:05:31,684 | INFO | App | Could not initialize instance: check2-x.x.x.x-yyyy: java.util.concurrent.ExecutionException: java.io.IOException: java.io.IOException: Failed to retrieve RMIServer stub: javax.naming.ServiceUnavailableException [Root exception is java.rmi.ConnectException: Connection refused to host: x.x.x.x; nested exception is: 

Looking at App.java, two lines after that message, the instance is put in brokenInstanceMap:

https://github.com/DataDog/jmxfetch/blob/f59dacea44d70f6b19a76a4360d752ef4c27b965/src/main/java/org/datadog/jmxfetch/App.java#L1032-L1036

In our case, instance.toString() resolves to ${host}:${port}, which is not unique across the two checks:

https://github.com/DataDog/jmxfetch/blob/f59dacea44d70f6b19a76a4360d752ef4c27b965/src/main/java/org/datadog/jmxfetch/Instance.java#L395

We tried to set Instance.getName() to something unique (see below), but that didn't solve the problem because Instance.toString() doesn't take into account the name...

instances:
  - host: '%%host%%'
    port: '9999'
    name: 'check1-%%host%%-9999'

A workaround is to combine the two JMX check configs into a single file.

remeh commented 4 years ago

Hey @jcrobak , thanks for having opened an issue! We're looking into this and I'll keep you informed.

remeh commented 4 years ago

With #255, I've merged the change to the Instance.toString() method in order to use the configured instance name if one has been provided. I've also released JMXFetch 0.33.1 to ship this change.

It will be available starting with the Datadog Agent 6.16.0 (PR: https://github.com/DataDog/datadog-agent/pull/4530) which should be released soon.

If you want to test it sooner, you can:

Please let me know when you'll have the chance to test this!