Improve ConnectionStatistics to report per-protocol information

[sbordet]

Currently, ConnectionStatistics listens to the "connection closed" event, and when it happens it retrieves the connection information from the Connection object passed in the event.

Would be great if this information could be split per-protocol.

This would involve:

1. a new Connection.getProtocol() method so that it would be possible to know what protocol was that connection speaking
2. a different implementation of ConnectionStatistics to store per-protocol information (as well as the totals)
3. Possibly a ConnectionStatisticsMBean that displays the per-protocol data in a JMX friendly way. JMX's CompositeType represents a struct; in this case it can have these fields: [protocol, bytesIn, bytesOut, messagesIn, messagesOut]. JMX's CompositeData is an instance of the struct with the actual values for those fields. JMX's TabularData is a Map where you can map a String to a CompositeData (I think - this class is so obscure I completely forgotten it was so obscure). Point being that either via an array of CompositeData, or via TabularData we can return the connection information split by protocol.

[stale]

This issue has been automatically marked as stale because it has been a full year without activity. It will be closed if no further activity occurs. Thank you for your contributions.

[lachlan-roberts]

@sbordet we now have IncludeExcludeConnectionStatistics which can can only record stats for specific types of connection. This can be used to get stats for a specific protocol, for the websocket protocol we get stats by including org.eclipse.jetty.websocket.common.io.AbstractWebSocketConnection.

However this would be a much cleaner solution to have all the information in ConnectionStatistics instead of having multiple IncludeExcludeConnectionStatistics for different protocols.

[github-actions]

This issue has been automatically marked as stale because it has been a
full year without activity. It will be closed if no further activity occurs.
Thank you for your contributions.

[arsenalzp]

Hello,
Is this issue still valid?

[sbordet]

@arsenalzp yes, it is still valid.

[arsenalzp]

I take it, if it's possible!

[sbordet]

Please do.

[arsenalzp]

I've just found per-protocol ConnectionStatistics reports have already been implemented:

public void onClosed(Connection connection)
{
        if (!isStarted())
            return;
        onTotalClosed(connection);
        onConnectionClosed(connection);
}

However, protocol name is taken from class name of the connection instance:

protected void onConnectionClosed(Connection connection)
{
        Stats stats = _statsMap.get(connection.getClass().getName());
        if (stats != null)
            onClosed(stats, connection);
}

So, I propose:

1. add Connection.getProtocol() method to get protocol name, for example for HttpConnection it is getHttpVersion().asString();

2. look at all connection implementations to check was getProtocol method implemented and correct information is returned;

3. rewrite ConnectionStatistics to use Connection.getProtocol() instead of Connection.getClass().getName();

4. try to implement ConnectionStatisticsMBean for JMX.

[sbordet]

@arsenalzp I forgot that this was done already 🤦🏼‍♂️

I think we can just close this issue.

I don't see much value adding Connection.getProtocol().

However, the ConnectionStatisticsMBean can definitely be improved.
@arsenalzp would you like to look at improving ConnectionStatisticsMBean by using JMX open types?
See the first comment: ConnectionStatistics.Stats should be converted to a TabularData.
As usual, comment here for directions/help, and I'll help you out.
Thanks!

[arsenalzp]

@arsenalzp I forgot that this was done already 🤦🏼‍♂️

I think we can just close this issue.

I don't see much value adding Connection.getProtocol().

However, the ConnectionStatisticsMBean can definitely be improved. @arsenalzp would you like to look at improving ConnectionStatisticsMBean by using JMX open types? See the first comment: ConnectionStatistics.Stats should be converted to a TabularData. As usual, comment here for directions/help, and I'll help you out. Thanks!

Yes,
I cat try to improve it!