Emit Netty event loop metrics provided by MicroMeter

[divijvaidya]

Motivation:

Use Micrometer out-of-the-box provided netty metrics.

Modifications:

• Micrometer's MeterBinder for Netty event loops is registered when CommonPools is created.
• Added a unit test to verify registration of the above MeterBinder
• Added a mockito inline dependency for mocking final, static etc.

Result:

• Closes Replace custom netty event loop metrics with micrometer provided metrics #6274
• User should see new metrics with name netty.eventexecutor.tasks.pending

[CLAassistant]

All committers have signed the CLA.

[divijvaidya]

I am figuring out the ICLA thing with my company. Will report back as soon as I have signed the ICLA.

[github-actions]

🔍 Build Scan® (commit: 0bf2e08)

Job name	Status	Build Scan®
build-ubicloud-standard-16-jdk-8	✅	https://ge.armeria.dev/s/qlxpcnwgx6nby
build-ubicloud-standard-16-jdk-21-snapshot-blockhound	❌ (failure)	https://ge.armeria.dev/s/ouypwfyuvrpli
build-ubicloud-standard-16-jdk-17-min-java-17-coverage	✅	https://ge.armeria.dev/s/kvt65eh27cpee
build-ubicloud-standard-16-jdk-17-min-java-11	✅	https://ge.armeria.dev/s/pgajljwpwmr6k
build-ubicloud-standard-16-jdk-17-leak	✅	https://ge.armeria.dev/s/24vn6liv4kgay
build-ubicloud-standard-16-jdk-11	✅	https://ge.armeria.dev/s/ldmhntja25ljy
build-macos-latest-jdk-21	❌ (failure)	https://ge.armeria.dev/s/3nkgedcrjmwwu

[minwoox]

Basically, looks good. Left a few suggestions. 👍

[minwoox]

Let's add @Deprecated to the class. 😉

[minwoox]

Woud you also deprecate MoreMeterBinders.eventLoopMetrics() that takes name and meterIdPrefix?

[divijvaidya]

I did not deprecate the whole class because it emits a metric (num event loop workers) which is not available in Micrometer right now. We will continue using it until we are able to add that metric in Micrometer.

[jrhee17]

I did not deprecate the whole class because it emits a metric (num event loop workers) which is not available in Micrometer right now

What do you think of first adding the metric to Micrometer first before proceeding with this PR?
I think the direction of this PR will be clearer if we can consider NettyEventExecutorMetrics as a replacement of MoreMeterBinders.eventLoopMetrics.

Also, it would be nice if the meter name prefix can also be specified like ExecutorServiceMetrics does - especially given that thread names are not necessarily unique. It can also allow users to group armeria related metrics easily depending on the prefix.

[minwoox]

What do you think of first adding the metric to Micrometer first before proceeding with this PR?
I think the direction of this PR will be clearer if we can consider NettyEventExecutorMetrics as a replacement of MoreMeterBinders.eventLoopMetrics.

I agree with you. Initially, I thought users could simply count the number of event loops using a count query with the name tag,
but since there's some additional calculation involved, we probably do need to add the metric as well.

Also, it would be nice if the meter name prefix can also be specified like ExecutorServiceMetrics does - especially given that thread names are not necessarily unique. It can also allow users to group armeria related metrics easily depending on the prefix.

I thought the current approach was fine because I personally recommend using only a single event loop group.
However, there are cases where users can't unify event loop groups — for example, when using Armeria with Lettuce.
So, let's ask the Micrometer team if it's possible to customize the meter name

[divijvaidya]

ok, I will get that done.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.55%. Comparing base (8150425) to head (0bf2e08).
⚠️ Report is 148 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #6290      +/-   ##
============================================
+ Coverage     74.46%   74.55%   +0.08%     
- Complexity    22234    22423     +189     
============================================
  Files          1963     1987      +24     
  Lines         82437    82998     +561     
  Branches      10764    10779      +15     
============================================
+ Hits          61385    61877     +492     
- Misses        15918    15956      +38     
- Partials       5134     5165      +31     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jrhee17]

Given that 1) Flags.meterRegistry is a static variable and 2) the jvm is forked for every gradle module I can imagine this test affecting other tests within the core module.

What do you think of adding a second parameter MeterRegistry for testing purposes? (i.e. using new SimpleMeterRegistry())

[jrhee17]

I did not deprecate the whole class because it emits a metric (num event loop workers) which is not available in Micrometer right now

What do you think of first adding the metric to Micrometer first before proceeding with this PR?
I think the direction of this PR will be clearer if we can consider NettyEventExecutorMetrics as a replacement of MoreMeterBinders.eventLoopMetrics.

Also, it would be nice if the meter name prefix can also be specified like ExecutorServiceMetrics does - especially given that thread names are not necessarily unique. It can also allow users to group armeria related metrics easily depending on the prefix.