Add eventexecutor workers metric for netty by ttaehee · Pull Request #6612 · micrometer-metrics/micrometer

ttaehee · 2025-08-10T17:25:11Z

Problem

It is difficult to monitor the state of Netty event loop workers in real-time during production, and having this visibility would be helpful for debugging and root cause analysis of network issues.
For example, in our Netty-based WebSocket + STOMP + RabbitMQ environment, we encountered ConnectTimeoutException errors under load.
Being able to immediately check the status of workers would help us identify and resolve these problems more easily.

Solution

Add the eventexecutor.workers metric to expose the number of Netty event loop workers in real-time.

Rationale

This metric allows operators to quickly monitor the count and status of workers, enabling faster diagnosis and response to issues.
Ultimately, it improves system stability and operational efficiency.

Local Verification

I've confirmed locally that the new metric is registered correctly after applying the changes.
As you can see below, netty.eventexecutor.workers is successfully created with the expected value.

Resolves #6375

- it determines the worker count by checking if the provided `Iterable` is an `instanceof MultithreadEventLoopGroup`, allowing it to safely call the `executorCount()` method for an accurate value - resolve test deprecations Signed-off-by: ttaehee <ttaehee1105@gmail.com>

shakuzen · 2025-08-20T02:46:06Z

@divijvaidya would this implementation work for what you had in mind when opening #6375?

shakuzen

Looks good to me. Thank you for a very complete pull request. I'll wait a bit to see if we can get feedback from the original author of the issue requesting this metric.

divijvaidya · 2025-08-20T17:57:32Z


    @Override
    public void bindTo(MeterRegistry registry) {
+        if (this.eventExecutors instanceof MultithreadEventLoopGroup) {


Is MultithreadEventLoopGroup the right class for all sorts of event loop groups? I am curious to understand why we didn't choose EventLoopGroup here.

Hi,
That's a great question. I chose MultithreadEventLoopGroup for two main reasons:

It aligns with the metric's purpose. The eventexecutor.workers metric is designed to measure multi-threaded worker pools. For single-threaded event loops, the value is always 1 and thus less meaningful for monitoring. Targeting MultithreadEventLoopGroup is the most logical approach as it’s the base for all widely-used, multi-threaded implementations.

Practicality: We need the executorCount() method to get the worker count. MultithreadEventLoopGroup provides this, but the general EventLoopGroup interface does not.

This approach ensures we're measuring what's actually meaningful while keeping the implementation straightforward. Thanks :)

For single-threaded event loops, the value is always 1 and thus less meaningful for monitoring.

You are assuming that the user of the metric knows whether the underlying implementation is single threaded or multi-threaded. Micrometer library can be used by all sorts of use cases, also in situations perhaps where the user of the metric may not know the implementation.

For the sake of ensuring that this metric doesn't require a conditional clause based on concrete implementation of event loop, I think there is value in representing single threaded loop here as well. From implementation perspective, you can choose to copy https://github.com/line/armeria/blob/024f86dedccc7897a6206770dd95852ca2ed66cc/core/src/main/java/com/linecorp/armeria/common/metric/EventLoopMetrics.java#L88 or have a special case for single event loop here.

Hi @divijvaidya

Thanks for the great feedback!
I've updated the PR to make the metric more robust and expanded the tests to cover all the cases you mentioned.

The implementation now supports all EventLoopGroup types by adding a generic fallback, while still keeping the fast-path for MultithreadEventLoopGroup.

The new tests verify this works for subclasses and other Iterable types as well.

Let me know what you think.

divijvaidya · 2025-08-23T17:15:39Z

    }

+    @Test
+    void shouldHaveWorkersMetric() throws Exception {


please add test for other sub-classes of MultiThreadIoEventLoopGroup for example, https://netty.io/4.1/api/io/netty/channel/epoll/EpollEventLoopGroup.html

The test should validate that the code works for subclasses as well as direct implementations

divijvaidya · 2025-08-23T17:36:31Z


    @Override
    public void bindTo(MeterRegistry registry) {
+        if (this.eventExecutors instanceof MultithreadEventLoopGroup) {


For single-threaded event loops, the value is always 1 and thus less meaningful for monitoring.

You are assuming that the user of the metric knows whether the underlying implementation is single threaded or multi-threaded. Micrometer library can be used by all sorts of use cases, also in situations perhaps where the user of the metric may not know the implementation.

For the sake of ensuring that this metric doesn't require a conditional clause based on concrete implementation of event loop, I think there is value in representing single threaded loop here as well. From implementation perspective, you can choose to copy https://github.com/line/armeria/blob/024f86dedccc7897a6206770dd95852ca2ed66cc/core/src/main/java/com/linecorp/armeria/common/metric/EventLoopMetrics.java#L88 or have a special case for single event loop here.

- Based on feedback, the 'eventexecutor.workers' metric now supports all EventLoopGroup types, not just MultithreadEventLoopGroup. - It retains the fast-path for the common case and adds a generic fallback that iterates over the group for an accurate count. - Tests are expanded to verify all code paths, including subclasses and the new fallback. Signed-off-by: ttaehee <ttaehee1105@gmail.com>

divijvaidya

Thank you for making the change. Left one final comment

divijvaidya · 2025-08-25T06:25:20Z

+        int count = 0;
+        for (EventExecutor ignored : this.eventExecutors) {
+            count++;
+        }
+        return count;


There is no test which tests this part of the code. can you please add one

Actually, there was already a test covering the fallback-path: shouldHaveWorkersMetricForNonMultithreadEventLoopGroup.

That said, I agree the intent wasn’t very clear.
To clarify, I replaced it with a simpler mock-based test: shouldCountWorkersForGenericIterableViaFallbackPath.

Thanks for flagging this!

- Replace shouldHaveWorkersMetricForNonMultithreadEventLoopGroup with a simpler, mock-based test (shouldCountWorkersForGenericIterableViaFallbackPath) to make the fallback path intent explicit Signed-off-by: ttaehee <ttaehee1105@gmail.com>

divijvaidya

LGTM! Left a minor comment.

divijvaidya · 2025-08-26T07:14:33Z

                .tags(Tags.of("name", name))
                .gauge()
-                .value()).isNaN());
+                .value()).isEqualTo(0.0));


Let's move this into a separate PR since it is unrelated.

My apologies for the extra notification! I accidentally re-requested the review.

You're right to point out the test change. It was a necessary update, as my changes caused the original test to fail.
This simply aligns it with the new expected behavior.

I see what's happening now, I think. However, this change to the test makes the test meaningless. The point of the test is to ensure that the metrics do not prevent the underlying object (executor) from being garbage collected. It's doing that by asserting that eventually after garbage collection is initiated, the underlying object will be garbage collected, and therefore the metric will return NaN.

See #6641. If the changes here are preventing it from being garbage collected, this should be changed so we aren't reintroducing that bug. Perhaps getWorkerCount can take Iterable<EventExecutor> as a parameter and the Gauge registration can pass it.

@shakuzen You're right about the strong reference issue—thanks for pointing that out.
I've switched to using a WeakReference to resolve it, and I’ve reverted the test to assert isNaN() accordingly.
Let me know if this approach looks good to you.

I've updated the code in a new commit with what I suggested. By default, we keep weak references of objects passed to Gauge builders as the backing object. That helps eliminate some boilerplate code that would otherwise be common to gauge functions in a lot of places.

Great to see this merged!
Thank you both, @divijvaidya and @shakuzen, for all the help and guidance on this.

- Use WeakReference in worker count gauge to allow executor garbage collection. Signed-off-by: ttaehee <ttaehee1105@gmail.com>

shakuzen

Thank you everyone who worked on this. This is a great example of open source working well. We ended up with something improved with each person's input, and now all users will be able to benefit from that.

injae-kim · 2025-08-29T00:43:25Z

Nice work @ttaehee 👍

jonatan-ivanov · 2025-09-03T21:40:07Z

+            return ((MultithreadEventLoopGroup) executors).executorCount();
+        }
+
+        int count = 0;


I know I'm late to the party but would it make sense to do this before iterating over the executors?

if (executors instanceof Collection) { return ((Collection<EventExecutor>) executors).size(); }

I mean is there a chance that executors will be a Collection or there is no implementation in Netty that would conform this?

Hi @jonatan-ivanov
Thanks for the great feedback!
I think it might be more efficient to first check if executors is a Collection, which would avoid an unnecessary loop and also serve as a defensive check for any potential custom implementations.
Should I go ahead and open a new PR for this?

Yes, feel free to send a new PR.

Signed-off-by: Johnny Lim <izeye@naver.com>

ttaehee force-pushed the add_eventexecutor_workers_metric_for_netty_6375 branch 2 times, most recently from 79c1fc3 to 9ea1193 Compare August 10, 2025 17:32

ttaehee mentioned this pull request Aug 10, 2025

Add eventexecutor.workers metrics for Netty #6375

Closed

ttaehee force-pushed the add_eventexecutor_workers_metric_for_netty_6375 branch 2 times, most recently from 02c63cc to 8e7bbd6 Compare August 20, 2025 01:01

ttaehee force-pushed the add_eventexecutor_workers_metric_for_netty_6375 branch from 8e7bbd6 to eb7f487 Compare August 20, 2025 01:07

shakuzen approved these changes Aug 20, 2025

View reviewed changes

shakuzen linked an issue Aug 20, 2025 that may be closed by this pull request

Add eventexecutor.workers metrics for Netty #6375

Closed

divijvaidya reviewed Aug 20, 2025

View reviewed changes

divijvaidya reviewed Aug 23, 2025

View reviewed changes

ttaehee force-pushed the add_eventexecutor_workers_metric_for_netty_6375 branch 2 times, most recently from 42060ec to 6799a8c Compare August 23, 2025 19:26

ttaehee force-pushed the add_eventexecutor_workers_metric_for_netty_6375 branch from 6799a8c to d3c3a3d Compare August 23, 2025 19:34

divijvaidya reviewed Aug 25, 2025

View reviewed changes

divijvaidya approved these changes Aug 26, 2025

View reviewed changes

ttaehee requested a review from divijvaidya August 26, 2025 08:14

ttaehee force-pushed the add_eventexecutor_workers_metric_for_netty_6375 branch from eaa366c to 6c0d4a2 Compare August 27, 2025 09:27

fix: prevent strong reference in worker count gauge

896822d

- Use WeakReference in worker count gauge to allow executor garbage collection. Signed-off-by: ttaehee <ttaehee1105@gmail.com>

ttaehee force-pushed the add_eventexecutor_workers_metric_for_netty_6375 branch from 6c0d4a2 to 896822d Compare August 27, 2025 09:56

Polish gauge registration

a7b7ee8

shakuzen approved these changes Aug 28, 2025

View reviewed changes

Polish description

18d7873

shakuzen enabled auto-merge (squash) August 28, 2025 09:16

shakuzen merged commit f2ea418 into micrometer-metrics:main Aug 29, 2025
11 checks passed

jonatan-ivanov reviewed Sep 3, 2025

View reviewed changes

izeye mentioned this pull request Dec 2, 2025

Polish gh-6612 #6932

Merged

izeye added a commit to izeye/micrometer that referenced this pull request Dec 2, 2025

Polish micrometer-metricsgh-6612

dbf9b3f

Signed-off-by: Johnny Lim <izeye@naver.com>

jonatan-ivanov pushed a commit that referenced this pull request Dec 2, 2025

Polish gh-6612 (#6932)

d0b368d

Signed-off-by: Johnny Lim <izeye@naver.com>

shakuzen mentioned this pull request Feb 9, 2026

Downgrade Netty from 4.2.10 to 4.1.131 #7177

Merged

Conversation

ttaehee commented Aug 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Rationale

Local Verification

Uh oh!

shakuzen commented Aug 20, 2025

Uh oh!

shakuzen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

divijvaidya left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

divijvaidya left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shakuzen left a comment

Choose a reason for hiding this comment

Uh oh!

injae-kim commented Aug 29, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ttaehee commented Aug 10, 2025 •

edited

Loading