jmxtrans / jmxtrans-agent

Java Agent based JMX metrics exporter.
MIT License
179 stars 110 forks source link

Add option for waiting for an MBeanServer as opposed to flat delay #119

Closed alechenninger closed 6 years ago

alechenninger commented 6 years ago

This should be a little more stable and faster than a flat delay.

Let me know what you think and I can try and add a test for it.

Tested with EAP7:

2017-12-02 19:30:44.76 INFO [main] org.jmxtrans.agent.JmxTransAgent - jmxtrans agent initialization delayed waiting for MBeanServer
19:30:45,056 INFO  [org.jboss.modules] (main) JBoss Modules version 1.6.0.Final-redhat-1
2017-12-02 19:30:45.768 INFO [jmxtrans-agent-delayed-starter-waitForMBeanServer] org.jmxtrans.agent.JmxTransAgent - Starting 'JMX metrics exporter agent: 1.2.7-SNAPSHOT' with configuration '/Users/ahenning/Code/jmxtrans-agent/sample-config.xml'...
2017-12-02 19:30:45.77 INFO [jmxtrans-agent-delayed-starter-waitForMBeanServer] org.jmxtrans.agent.JmxTransAgent - PropertiesLoader: Empty Properties Loader
19:30:55,322 INFO  [org.jboss.msc] (main) JBoss MSC version 1.2.7.SP1-redhat-1
19:30:55,433 INFO  [org.jboss.as] (MSC service thread 1-7) WFLYSRV0049: JBoss EAP 7.1.0.GA (WildFly Core 3.0.10.Final-redhat-1) starting
alechenninger commented 6 years ago

@cyrille-leclerc @kerlandsson Any thoughts on this? Thank you!

cyrille-leclerc commented 6 years ago

Thanks for your proposal @alechenninger, it's great to see RedHat employees helping us.

Your idea is to have another "wait" strategy which would be to wait for "one" mbean server to be created with a timeout security, correct? I have no doubt that you know better than us how we should integrate with JBoss AS.

Could you please update the documentation on README.md? Maybe introducing a section called "Delayed Startup for JBoss AS / Wildfly (version >=1.2.8)" and refactoring the documentation section "Delayed startup (version >= 1.2.1)" removing from it the references to JBoss AS and Wildfly as your solution works better.

alechenninger commented 6 years ago

Thanks for taking a look @cyrille-leclerc ! Full transparency: while I work at Red Hat, I don't work on JBoss. That said, I do have a lot of experience with it :-).

I updated the pull request. It looks like the root cause of these problems is this in the jboss-modules code:

        final String mbeanServerBuilderName = getServiceName(bootClassLoader, "javax.management.MBeanServerBuilder");
        if (mbeanServerBuilderName != null) {
            System.setProperty("javax.management.builder.initial", mbeanServerBuilderName);
            ManagementFactory.getPlatformMBeanServer();

If anything initializes the PlatformMBeanServer before jboss-modules has a chance to set "javax.management.builder.initial", then the PlatformMBeanServer JBoss expects to be used, won't be. JBoss uses a PluggableMBeanServerBuilder that adds some additional functionality.

I'll try and add a test for good measure. Thanks!

alechenninger commented 6 years ago

Looks like there aren't any tests for JmxTransAgent class yet... not sure if I have time to be the first :-). Let me know what you think! If this could be released, we can stop using my fork :-). Thanks!