Spring Boot 1.5.4 JMS Provider: Active MQ

My application uses org.apache.activemq.ActiveMQConnectionFactory#setClientID(String) and this causes JmsHealthIndicator to fail with:

2017-06-22 15:09:13,895 - [          ] - | WARN| - [uate.health.JmsHealthIndicator] - #:           - USER:N/A - Health check failed
javax.jms.InvalidClientIDException: Broker: localhost - Client: myClientId already connected from vm://localhost#6
    at org.apache.activemq.broker.region.RegionBroker.addConnection(RegionBroker.java:256)
    at org.apache.activemq.broker.jmx.ManagedRegionBroker.addConnection(ManagedRegionBroker.java:227)
    at org.apache.activemq.broker.BrokerFilter.addConnection(BrokerFilter.java:98)
    at org.apache.activemq.advisory.AdvisoryBroker.addConnection(AdvisoryBroker.java:116)
    at org.apache.activemq.broker.BrokerFilter.addConnection(BrokerFilter.java:98)
    at org.apache.activemq.broker.BrokerFilter.addConnection(BrokerFilter.java:98)
    at org.apache.activemq.broker.MutableBrokerFilter.addConnection(MutableBrokerFilter.java:103)
    at org.apache.activemq.broker.TransportConnection.processAddConnection(TransportConnection.java:852)

I.e. the health indicator cannot make a second connection with the configured client id.

The improvement I imagine is to try/catch the call to Connection#start() and ignore javax.jms.InvalidClientIDException. Something like:

protected void doHealthCheck(Builder builder) throws Exception {
        Connection connection = this.connectionFactory.createConnection();

        try {
            try {
               connection.start();
            } catch (javax.jms.InvalidClientIDException itsOK) {}
            builder.up().withDetail("provider", connection.getMetaData().getJMSProviderName());
        } finally {
            connection.close();
        }

    }

Comment From: snicoll

@martin-g that doesn't sound right to me. It looks like the ConnectionFactory you've created is only able to start one connection to the broker. That makes it very hard for us to check if the broker is alive since we have to open another connection.

Comment From: martin-g

alive currently means that the connection is successfully started. InvalidClientIDException means that connecting is OK but there is another client with this id already. Isn't that enough to say that the health check passed ?

Comment From: wilkinsona

Isn't that enough to say that the health check passed ?

I don't think so. What happens if another client somewhere else is using the same ID so the whole app is unable to connect to the broker?

Comment From: martin-g

In that case it seems I wrongly understand the purpose of the health check. I thought it is checking whether the broker is up and reachable, not that the created connection is usable.

I'll roll my own jmsHealthIndicator that uses ActiveMQConnectionFactory's #clone() and then set a new clientId (or even null).

Comment From: snicoll

I thought it is checking whether the broker is up and reachable, not that the created connection is usable.

Simply because creating the connection is not enough to figure out the broker is up. See also #6818

Comment From: martin-g

Here is my impl in case someone else needs it:

package com.bmg.spring.context;

import org.apache.activemq.ActiveMQConnectionFactory;
import org.springframework.boot.actuate.health.AbstractHealthIndicator;
import org.springframework.boot.actuate.health.Health;

import javax.jms.Connection;
import javax.jms.ConnectionFactory;
import java.util.List;

/**
 * A custom JMS health indicator that disables temporary the clientId
 * to be able to test the connection to the JMS broker
 */
public class JmsHealthIndicator extends AbstractHealthIndicator {

    private final ConnectionFactory connectionFactory;

    public JmsHealthIndicator(ConnectionFactory connectionFactory) {
        this.connectionFactory = connectionFactory;
    }

    @Override
    protected void doHealthCheck(Health.Builder builder) throws Exception {
        ActiveMQConnectionFactory activeMQConnectionFactory = (ActiveMQConnectionFactory) connectionFactory;
        final String clientID = activeMQConnectionFactory.getClientID();

        activeMQConnectionFactory.setClientID(null);
        Connection connection = activeMQConnectionFactory.createConnection();
        try {
            connection.start();
            builder.up().withDetail("provider", connection.getMetaData().getJMSProviderName());
        } finally {
            activeMQConnectionFactory.setClientID(clientID);
            connection.close();
        }
    }
}

In my application we have only MessageListener that is constructed at the start of the app so it is no problem to unset temporarily the clientId this way.

Comment From: Sti2nd

So the conclusion to this Exception that I have got on startup is that Spring framework is doing it correctly and org.apache.activemq.ActiveMQConnectionFactory is wrongly "only able to start one connection to the broker"?

So this is a bug with the Active MQ library(?)

Comment From: wilkinsona

@Sti2nd, the problem discussed in this issue isn't related to start up. It's about the JmsHealthIndicator which will only be called when a request is made to /actuator/health. If you are seeing an InvalidClientIDException at startup, you have a different problem.

If you have any further questions, please follow up on Stack Overflow. As mentioned in the guidelines for contributing, we prefer to use GitHub issues only for bugs and enhancements.

Comment From: Sti2nd

Thanks for the reply @wilkinsona.

This is on startup, and the problem was the JmsHealthIndicator. I am using IntelliJ, so with your latest comment I belive it might be calling /actuator/health automatically on startup 😊

Overriding JmsHealthIndicator fixed my problem. Thanks @martin-g!

Edit: I still consider it to be a bug here. Calling the health endpoint shouldn't lead to an error.