Added list_jvms option to list available JVMs (that this process can … #100
Conversation
|
That's a great addon, thanks a lot @cslee00. I am adding the PR to the next milestone: it'll get reviewed soon, and merged with Datadog Agent |
| for (com.sun.tools.attach.VirtualMachineDescriptor vmd : com.sun.tools.attach.VirtualMachine.list()) { | ||
| jvms.add( vmd.displayName() ); | ||
| } | ||
| throw new IOException("Cannot find JVM matching regex: '" + processRegex + "'; available JVMs (for this user account): " + jvms ); |
There was a problem hiding this comment.
Looks like this test requires the same update https://github.com/DataDog/jmxfetch/blob/master/src/test/java/org/datadog/jmxfetch/TestServiceChecks.java#L116
|
I just added a few nitpicks here and there. It looks great overall, I am looking forward to see it merged. Thanks again @cslee00. |
|
Thanks, great feedback, incorporated those changes. Build is failing on openjdk6/7 (buffer overflow), unable to determine how that is related to these changes. |
| } | ||
| throw new IOException("Cannot find JVM matching regex: " + processRegex); | ||
|
|
||
| throw new IOException("Cannot find JVM matching regex: '" + processRegex + "'; available JVMs (for this user account): " + jvms ); |
There was a problem hiding this comment.
This log line is directly forwarded to the dd-agent info command, i.e.
jmx
---
- instance #foobar [ERROR]: "Cannot connect to instance process_regex: test Cannot find JVM matching regex: 'test'; available JVMs (for this user account): [org.datadog.jmxfetch.App --check jmx.yaml --check_period 15000 --conf_directory /etc/dd-agent/conf.d --log_level INFO --log_location /var/log/datadog/jmxfetch.log --reporter statsd:localhost:8125 --status_location /opt/datadog-agent/run/jmx_status.yaml collect]" collected 0 metrics
- Collected 0 metrics, 0 events & 0 service checks
I think it's difficult to understand what it means when being agnostic to the code. What do you think about keeping the old message ? Alternatively the message could invite the user to run the list_jvms command to list all available JVMs.
|
I restarted the tests and it's now passing 💚 . I also added a few extra nitpicks, I'd appreciate having your thoughts on it :) |
Add list_jvms to show JVMs available for use by process_name_regex. Improve diagnostic message when no matching JVM found.
Also requires change to jmxfetch.py script to allow this option.