Skip to content

Conversation

@loserwang1024
Copy link
Contributor

@loserwang1024 loserwang1024 commented Oct 30, 2025

Purpose

Linked issue: close #1895
Before:
image

After:
image

Brief change log

Tests

API and Format

Documentation

@loserwang1024 loserwang1024 requested review from swuferhong and wuchong and removed request for wuchong October 30, 2025 03:54
Copy link
Member

@wuchong wuchong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appended a commit to improve the implementation a bit, please take a look @loserwang1024

  1. Make ConnectionMetricGroup not extend AbstractMetricGroup to simplify the logic of ConnectionMetricGroup
  2. Rename ConnectionMetricGroup to ConnectionMetrics as it is not a MetricGroup now.
  3. Remove the unused metric names in MetricNames.
  4. Unregister itself from ClientMetricGroup when calling ConnectionMetrics
  5. Change the Set<ConnectionMetricGroup> in ClientMetricGroup into a concurrent HashMap for thread-safe as connections will be created in multi-threads. And we should deduplicate the connections by serverId to avoid memory leak.

@loserwang1024
Copy link
Contributor Author

I appended a commit to improve the implementation a bit, please take a look @loserwang1024

LGTM!

@wuchong wuchong merged commit b94b2b6 into apache:main Nov 5, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Aggregate Client connection metrics.

2 participants