[AMQ-9769] Add name property to connector interface and MBean.

[thezbyg]

No description provided.

[jbonofre]

I'm a bit confused here. Is it con.getCliientId() or connection name MBean constructed.
Can you please just add a comment here to explain the use of clientId (instead of constructing with the name, and more) ?

[mattrpav]

Heads up-- iirc, there are inconsistencies in connection name/id/clientId.

Some protocols only allow a single clientId-per-broker (mqtt) and some can in certain scenarios (jms). We need to be super sure about which is used to avoid a scenario where the web ui can have two entries for the same id in a list view.

[jbonofre]

That's my point here. I'm a bit concerned by this connection name here.

[thezbyg]

@jbonofre
Connection MBeans with connectionViewType=clientId type use connection clientId value for connectionName parameter, so connectionName MBean parameter value and clientId connection property value should be equivalent.

This is the code responsible for connectionName value in connection MBean:

activemq/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/ManagedTransportConnection.java

Line 84 in 51553dd

byClientIdName = createObjectName("clientId", clientId);

activemq/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/ManagedTransportConnection.java

Line 117 in 51553dd

return BrokerMBeanSupport.createConnectionViewByType(connectorName, type, value);

activemq/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/BrokerMBeanSupport.java

Line 181 in 51553dd

objectNameStr += ",connectionName="+ JMXSupport.encodeObjectNamePart(name);

@mattrpav
If JMS allows multiple connections with same clientId value, then it should have already caused problems by attempting to create multiple MBeans with the same name. If this was not an issue before my changes, then I will have to investigate more, because I thought that clientId must always be unique and can not be shared between connections.

[jbonofre]

@thezbyg thanks for your feedback. It looks good. Let me do a new pass, but I think it's good to merge. @mattrpav thoughts ?

[jbonofre]

LGTM, imho, it would be great to add a little comment to frame use of con.getClientId().

[mattrpav]

Requested change:

1. Please add getConnectionId() to ConnectionViewMBean

The connectionId is the unique value within the broker for the connection.

Side note: We should be registering JMX using that unique value instead of the clientId since clientId can be reused across connections and quickly swapped out if link stealing is enabled, but we won't dial up that change now.

[thezbyg]

Done, but now ConnectionView returns the same value for both getClientId and getConnectionId:

public String getClientId() {
    return connection.getConnectionId();
}
public String getConnectionId() {
    return connection.getConnectionId();
}

And connection.getConnectionId() executes the following code in TransportConnection class:

    public String getConnectionId() {
        List<TransportConnectionState> connectionStates = listConnectionStates();
        for (TransportConnectionState cs : connectionStates) {
            if (cs.getInfo().getClientId() != null) {
                return cs.getInfo().getClientId();
            }
            return cs.getInfo().getConnectionId().toString();
        }
        return null;
    }

So it may return either clientId or connectionId.

I propose to remove connection handling changes from this pull request and keep only changes to connector MBean and connector output in web UI.