Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Added list_jvms option to list available JVMs (that this process can … #100

Closed
wants to merge 6 commits into from
Closed

Added list_jvms option to list available JVMs (that this process can … #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4ddf9fa
Added list_jvms option to list available JVMs (that this process can …
cslee00 May 14, 2016
4a83168
Adjustements based on pull request feedback
cslee00 Jun 4, 2016
100c26f
Fix crash on OpenJDK
cslee00 Jun 4, 2016
6715343
Fix crash on OpenJDK
cslee00 Jun 4, 2016
0b44718
Fix crash on OpenJDK
cslee00 Jun 4, 2016
60f3744
Fix crash on OpenJDK
cslee00 Jun 4, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/main/java/org/datadog/jmxfetch/App.java
View file
Open in desktop
Expand Up @@ -25,6 +25,7 @@
import com.beust.jcommander.JCommander;
import com.beust.jcommander.ParameterException;


@SuppressWarnings("unchecked")
public class App {
private final static Logger LOGGER = Logger.getLogger(App.class.getName());
Expand Down Expand Up @@ -81,6 +82,15 @@ public static void main(String[] args) {
System.exit(1);
}

if( config.getAction().equals( AppConfig.ACTION_LIST_JVMS )) {
List<com.sun.tools.attach.VirtualMachineDescriptor> descriptors = com.sun.tools.attach.VirtualMachine.list();
System.out.println("List of JVMs for user " + System.getProperty("user.name") );
for( com.sun.tools.attach.VirtualMachineDescriptor descriptor : descriptors ) {
System.out.println( "\tJVM id " + descriptor.id() + ": '" + descriptor.displayName() + "'" );
}
System.exit(0);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This block can potentially lead to java.lang.NoClassDefFoundError when no tools_jar_path is defined in the YAML configuration file.

root@xxxx:~# sudo /etc/init.d/datadog-agent jmx list_jvms
2016-06-17 15:46:39,166 | INFO | dd.collector | jmxfetch(jmxfetch.py:244) | Starting jmxfetch:
2016-06-17 15:46:39,167 | INFO | dd.collector | jmxfetch(jmxfetch.py:292) | Running java -Xms50m -Xmx200m -classpath /opt/datadog-agent/agent/checks/libs/jmxfetch-0.11.0-jar-with-dependencies.jar 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 console --status_location /opt/datadog-agent/run/jmx_status.yaml list_jvms
Exception in thread "main" java.lang.NoClassDefFoundError: com/sun/tools/attach/VirtualMachine
    at org.datadog.jmxfetch.App.main(App.java:86)
Caused by: java.lang.ClassNotFoundException: com.sun.tools.attach.VirtualMachine
    at java.net.URLClassLoader$1.run(URLClassLoader.java:366)
    at java.net.URLClassLoader$1.run(URLClassLoader.java:355)
    at java.security.AccessController.doPrivileged(Native Method)
    at java.net.URLClassLoader.findClass(URLClassLoader.java:354)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:425)
    at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:308)
    at java.lang.ClassLoader.loadClass(ClassLoader.java:358)
    ... 1 more

Can we catch it ad prompt a more comprehensive error message when this happens ?


// Set up the shutdown hook to properly close resources
attachShutdownHook();

Expand Down Expand Up @@ -320,8 +330,10 @@ public void init(boolean forceNewConnection) {
clearInstances(instances);
clearInstances(brokenInstances);


Reporter reporter = appConfig.getReporter();


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this whole block should be moved to the main() method instead, i.e. here for instance.

Iterator<Entry<String, YamlParser>> it = configs.entrySet().iterator();
while (it.hasNext()) {
Map.Entry<String, YamlParser> entry = it.next();
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/org/datadog/jmxfetch/AppConfig.java
View file
Open in desktop
Expand Up @@ -18,14 +18,15 @@
@Parameters(separators = "=")
class AppConfig {
public static final String ACTION_COLLECT = "collect";
public static final String ACTION_LIST_JVMS = "list_jvms";
public static final String ACTION_LIST_EVERYTHING = "list_everything";
public static final String ACTION_LIST_COLLECTED = "list_collected_attributes";
public static final String ACTION_LIST_MATCHING = "list_matching_attributes";
public static final String ACTION_LIST_NOT_MATCHING = "list_not_matching_attributes";
public static final String ACTION_LIST_LIMITED = "list_limited_attributes";
public static final String ACTION_HELP = "help";
public static final HashSet<String> ACTIONS = new HashSet<String>(Arrays.asList(ACTION_COLLECT, ACTION_LIST_EVERYTHING,
ACTION_LIST_COLLECTED, ACTION_LIST_MATCHING, ACTION_LIST_NOT_MATCHING, ACTION_LIST_LIMITED, ACTION_HELP));
ACTION_LIST_COLLECTED, ACTION_LIST_MATCHING, ACTION_LIST_NOT_MATCHING, ACTION_LIST_LIMITED, ACTION_HELP, ACTION_LIST_JVMS));

@Parameter(names = {"--help", "-h"},
description = "Display this help page",
Expand Down Expand Up @@ -81,7 +82,7 @@ class AppConfig {

@Parameter(description = "Action to take, should be in [help, collect, " +
"list_everything, list_collected_attributes, list_matching_attributes, " +
"list_not_matching_attributes, list_limited_attributes]",
"list_not_matching_attributes, list_limited_attributes, list_jvms]",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as below 😄 : https://github.com/DataDog/jmxfetch/blob/master/src/test/java/org/datadog/jmxfetch/TestParsingJCommander.java#L330-L333

required = true)
private List<String> action = null;

Expand Down
10 changes: 8 additions & 2 deletions src/main/java/org/datadog/jmxfetch/AttachApiConnection.java
View file
Open in desktop
Expand Up @@ -2,8 +2,10 @@

import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;

import javax.management.remote.JMXServiceURL;

Expand All @@ -29,13 +31,15 @@ private JMXServiceURL getAddress(LinkedHashMap<String, Object> connectionParams)
throw new IOException("Unnable to attach to process regex: "+ processRegex, e);
}
return address;

}

private String getJMXUrlForProcessRegex(String processRegex) throws com.sun.tools.attach.AttachNotSupportedException, IOException {
List<String> jvms = new ArrayList<String>();
for (com.sun.tools.attach.VirtualMachineDescriptor vmd : com.sun.tools.attach.VirtualMachine.list()) {
if (vmd.displayName().matches(processRegex)) {
com.sun.tools.attach.VirtualMachine vm = com.sun.tools.attach.VirtualMachine.attach(vmd);
LOGGER.info("Matched JVM '" + vmd.displayName() + "' against regex '" + processRegex + "'");
String connectorAddress = vm.getAgentProperties().getProperty(CONNECTOR_ADDRESS);
//If jmx agent is not running in VM, load it and return the connector url
if (connectorAddress == null) {
Expand All @@ -48,8 +52,10 @@ private String getJMXUrlForProcessRegex(String processRegex) throws com.sun.tool

return connectorAddress;
}
jvms.add( vmd.displayName() );
}
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 );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this test requires the same update https://github.com/DataDog/jmxfetch/blob/master/src/test/java/org/datadog/jmxfetch/TestServiceChecks.java#L116

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

private void loadJMXAgent(com.sun.tools.attach.VirtualMachine vm) throws IOException {
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/datadog/jmxfetch/TestParsingJCommander.java
View file
Open in desktop
Expand Up @@ -312,7 +312,7 @@ public void testParsingAction() {
} catch (ParameterException pe) {
String expectedMessage = "Main parameters are required (\"Action to take, should be in [help, collect, " +
"list_everything, list_collected_attributes, list_matching_attributes, " +
"list_not_matching_attributes, list_limited_attributes]\")";
"list_not_matching_attributes, list_limited_attributes, list_jvms]\")";
assertEquals(expectedMessage, pe.getMessage());
}

Expand All @@ -329,7 +329,7 @@ public void testParsingAction() {
} catch (ParameterException pe) {
String expectedMessage = "Main parameters are required (\"Action to take, should be in [help, collect, " +
"list_everything, list_collected_attributes, list_matching_attributes, " +
"list_not_matching_attributes, list_limited_attributes]\")";
"list_not_matching_attributes, list_limited_attributes, list_jvms]\")";
assertEquals(expectedMessage, pe.getMessage());
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/datadog/jmxfetch/TestServiceChecks.java
View file
Open in desktop
Expand Up @@ -113,7 +113,7 @@ public void testServiceCheckCRITICAL() throws Exception {

assertEquals(Reporter.formatServiceCheckPrefix("non_running_process"), scName);
assertEquals(Status.STATUS_ERROR, scStatus);
assertEquals("Cannot connect to instance process_regex: .*non_running_process_test.* Cannot find JVM matching regex: .*non_running_process_test.*", scMessage);
assertTrue( scMessage, scMessage.startsWith("Cannot connect to instance process_regex: .*non_running_process_test.* Cannot find JVM matching regex: '.*non_running_process_test.*'; available JVMs (for this user account): "));
assertEquals(scTags.length, 3);
assertTrue(Arrays.asList(scTags).contains("instance:jmx_test_instance"));

Expand Down