Add metrics for active connections on the client side

[ikhoon]

Motivation:

There are no metrics for active connections on the client side out of the box. The number has to be computed by metric queries such as PromQL by subtracting closed from opened.
Otherwise, a custom metric should be implemented by users.

Modifications:

• Record armeria.client.connections#value{state="active"} in MetricCollectingConnectionPoolListener to get the active connections for a remote peer.

Result:

• You can now monitor the number of active connections with armeria.client.connections#value{state="active"}
• Closes Add the gauge meter for the number of active client connections #4277

[github-actions]

🔍 Build Scan® (commit: c2588b7)

Job name	Status	Build Scan®
build-windows-latest-jdk-19	✅	https://ge.armeria.dev/s/iywxccyomumok
build-self-hosted-jdk-8	✅	https://ge.armeria.dev/s/xzvrrvd6lbd4s
build-self-hosted-jdk-19-snapshot-blockhound	✅	https://ge.armeria.dev/s/uw37q3zhv5vi6
build-self-hosted-jdk-17-min-java-17-coverage	✅	https://ge.armeria.dev/s/qslzhdtjzgka4
build-self-hosted-jdk-17-min-java-11	✅	https://ge.armeria.dev/s/n4bcbovdb4mqg
build-self-hosted-jdk-17-leak	✅	https://ge.armeria.dev/s/w4vrt4f33inhi
build-self-hosted-jdk-11	✅	https://ge.armeria.dev/s/zvzfrba2onpai
build-macos-12-jdk-19	✅	https://ge.armeria.dev/s/lznnwezxjc2yi

[jrhee17]

Thanks @ikhoon 🙇 👍 🙇

[jrhee17]

Question) Will the AssertionError be caught anywhere?

Suggested change

	assert meters.activeConnections() >= 0 : "active connections should not be negative" + meters;
	checkState(meters.activeConnections() >= 0, : "active connections should not be negative" + meters);

[ikhoon]

The AssertionError should be logged by:

armeria/core/src/main/java/com/linecorp/armeria/client/HttpChannelPool.java

Lines 516 to 523 in 9823b46

	try {
	listener.connectionClosed(protocol, remoteAddr, localAddr, channel);
	} catch (Exception e) {
	if (logger.isWarnEnabled()) {
	logger.warn("{} Exception handling {}.connectionClosed()",
	channel, listener.getClass().getName(), e);
	}
	}

I believe assert would be proper because if the value becomes negative, it is a bug but not a bad state.

[jrhee17]

I see, I thought Error didn't inherit from Exception. No problem if it is somehow logged and propagated to the user

[ikhoon]

Good point. I don't see we need to log only Exceptions. Let me change it to Throwable.

[codecov]

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (fd31c66) 0.00% compared to head (c2588b7) 73.95%.
Report is 14 commits behind head on main.

Files	Patch %	Lines
...linecorp/armeria/client/ConnectionPoolMetrics.java	96.00%	0 Missing and 2 partials ⚠️
...a/com/linecorp/armeria/client/HttpChannelPool.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #5288       +/-   ##
===========================================
+ Coverage        0   73.95%   +73.95%     
- Complexity      0    20104    +20104     
===========================================
  Files           0     1730     +1730     
  Lines           0    74178    +74178     
  Branches        0     9464     +9464     
===========================================
+ Hits            0    54855    +54855     
- Misses          0    14849    +14849     
- Partials        0     4474     +4474     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[minwoox]

Left small suggestions.
Thanks!

[ikhoon]

Then, probably better to use Caffeine? If so, we could also limit the size or use weak key and value.

I didn't think Caffeine was a good idea because that may produce incorrect metric information. Weak key seems worth a try.