Make it easier to detect blocked connections

[jjank]

Is your feature request related to a problem? Please describe.

TL;DR: Detecting blocked connections can be quite tricky when you don't have access to the broker

• In the project I'm currently on we are using the rabbitmq-jms client (not my choice but we cannot change this in the forseeable future)
• For reasons beyond our control the RabbitMQ broker blocked our clients again and again (Ops is working to resolve this properly)
• We found it quite hard to get to the bottom of this, specifically:
  • We found no (supported?) way to add a BlockedListener using the rabbitmq-jms apis (as far as I remember access to com.rabbitmq.client.Connection is hidden behind the ConnectionFactory but that's a deliberate decision I suppose and a different discussion)
  • To the best of our research no default (e.g. logging) happens upon blocking
  • MetricsCollector does not record blocked connections

Describe the solution you'd like

1. Track/Record blocked connections in MetricsCollector - I've drafted a proposal and would be happy to file a PR for this to get your opinion
2. Optionally: Consider adding a default BlockedListener to AMQConnection that at the very least logs a warning upon blocking

Describe alternatives you've considered

Additional context

[acogoluegnes]

I think this belongs to the JMS client library: the AMQP 091 client library provides methods to deal with BlockedListeners.

The JMS client provides a callback for the AMQP ConnectionFactory (setAmqpConnectionFactoryPostProcessor), do you think a similar callback for AMQP connections would do the trick?

[jjank]

Note to self: I should not have mentioned the JMS aspect 😬

The JMS client provides a callback for the AMQP ConnectionFactory (setAmqpConnectionFactoryPostProcessor), ...

I just checked that and: com.rabbitmq.client.ConnectionFactory does not expose methods to set a BlockedListener(the only listeners one can set are #setTrafficListener and #setErrorOnWriteListener unless I miss something)

EDIT: Re-reading your comment I think this was the point your were trying to make...It's not possible at the moment without a customization callback for the actual com.rabbitmq.client.Connection

But regardless of what the JMS client library can or cannot do:

What are your thoughts about tracking the number of blocked connections in the metrics? This would benefit both the 'vanilla' client library and JMS clients. I'd be happy to push my prototype as a PR

[acogoluegnes]

Here are my thoughts about collecting blocked connections in a metric:

• it means adding a new method to the MetricsCollector. It's a breaking change in theory, but we can use a default method in the implementation.
• all the existing MetricsCollector should (must) be updated.
• the metric may not be always accurate (a blocked connection may get closed without the unblock callback). We may be able to keep the blocked state with a flag to avoid this though.
• there is no strong demand for this (first time since we introduced metrics collection in 2016).
• anyone interested in this can use a BlockedListener with their own metric.

I find it overly complicated over a simple connection callback in the JMS client.

@michaelklishin Your thoughts on this?

[jjank]

it means adding a new method to the MetricsCollector. It's a breaking change in theory, but we can use a default method in the implementation.

Really? Isn't com.rabbitmq.client.MetricsCollector#newConnection enough?

from my experiments in com.rabbitmq.client.impl.AbstractMetricsCollector

@Override
    public void newConnection(final Connection connection) {
        try {
            if(connection.getId() == null) {
                connection.setId(UUID.randomUUID().toString());
            }
            incrementConnectionCount(connection);
            connectionState.put(connection.getId(), new ConnectionState(connection));
            connection.addShutdownListener(cause -> closeConnection(connection));
            // => adding the listener to the new connection
            connection.addBlockedListener(this::incrementBlockedConnectionCount, this::decrementBlockedConnectionCount);
        } catch(Exception e) {
            LOGGER.info("Error while computing metrics in newConnection: " + e.getMessage());
        }
    }

But you are right:

It's surely possible to subclass e.g. MicrometerMetricsCollector, override newConnection and add the blocked listener there.

[acogoluegnes]

You are right, that's a good way to a metric like this one.

Still, I am not convinced this is a metric worth having in the core of the library (we never had any request for it in 8 years, you are the first one to ask).

If you want to have a metric for blocked connections in your application, you just need to create the metric in your Micrometer registry and update it in a BlockedListener. I think the library provides the appropriate extension point and extensibility to reach this goal, without having to modify it.

Note to self: I should not have mentioned the JMS aspect

On the contrary, I think it's the correct way to approach the problem. The AMQP client library has everything you need, but the JMS client library misses a small bridge, this is why I suggested this connection post-processing hook.

By hardcoding the blocked connection metric in the AMQP library, you greatly limit the scope of the feature. What if another user of the JMS client wants to find out about blocked connection but they do not use metrics collection? With the connection creation callback they could provide their own blocked listener, and even plug onto other extension points.

[jjank]

Thanks for your detailed response!

I'll give the subclassing and metric creation a shot 👍

With the connection creation callback they could provide their own blocked listener, and even plug onto other extension points.

Would you like me to create an (issue/discussion) in the JMS client repo regarding this? Again I'd be more than happy to give it a try with a PR

[acogoluegnes]

I don't think you need to subclass anything: create a metric from the Micrometer registry you pass into the metrics collector, keep a reference of this metric and use it in the blocked listener you add to your connections.

Sure, let's move this to the JMS client repository, you can create a PR directly. Thanks!