Skip to content

Emit Netty event loop metrics provided by MicroMeter #6290

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
divijvaidya wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Emit Netty event loop metrics provided by MicroMeter #6290

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0beccc8
Emit event loop metrics provided by MicroMeter
divijvaidya Jun 24, 2025
24bc1f7
Simpify test and remove dependency on mockito inline
divijvaidya Jun 27, 2025
d3f8740
add new guideline for header
divijvaidya Jun 30, 2025
0bf2e08
Revert "add new guideline for header"
divijvaidya Jun 30, 2025
7506635
Merge branch 'line:main' into main
divijvaidya Sep 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ configure(projectsWithFlags('java')) {
testRuntimeOnly libs.checkerframework // Required by guava-testlib
testImplementation libs.assertj
testImplementation libs.mockito
testImplementation libs.mockito.inline
testImplementation libs.apache.httpclient5
testImplementation libs.hamcrest
testImplementation libs.hamcrest.library
Expand Down
21 changes: 16 additions & 5 deletions core/src/main/java/com/linecorp/armeria/common/CommonPools.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.annotations.VisibleForTesting;

import com.linecorp.armeria.client.ClientFactoryBuilder;
import com.linecorp.armeria.common.metric.MeterIdPrefix;
import com.linecorp.armeria.common.metric.MoreMeterBinders;
import com.linecorp.armeria.common.util.BlockingTaskExecutor;
import com.linecorp.armeria.common.util.EventLoopGroups;
import com.linecorp.armeria.server.ServerBuilder;

import io.micrometer.core.instrument.binder.netty4.NettyEventExecutorMetrics;
import io.netty.channel.EventLoopGroup;

/**
Expand All @@ -45,11 +48,7 @@ public final class CommonPools {
EventLoopGroups.newEventLoopGroup(Flags.numCommonWorkers(), "armeria-common-worker", true);

static {
// Bind EventLoopMetrics for the common worker group.
MoreMeterBinders
.eventLoopMetrics(WORKER_GROUP, new MeterIdPrefix("armeria.netty.common"))
.bindTo(Flags.meterRegistry());

bindEventLoopMetricsForWorkerGroup(WORKER_GROUP);
try {
final Class<?> aClass = Class.forName("reactor.core.scheduler.Schedulers",
true, CommonPools.class.getClassLoader());
Expand All @@ -65,6 +64,18 @@ public final class CommonPools {
}
}

@VisibleForTesting
static void bindEventLoopMetricsForWorkerGroup(EventLoopGroup evGroup) {
// Bind EventLoopMetrics for the common worker group.
MoreMeterBinders
.eventLoopMetrics(evGroup, new MeterIdPrefix("armeria.netty.common"))
.bindTo(Flags.meterRegistry());
// Bind the common event loop metrics emitted automatically by micrometer.
// These metrics intentionally duplicate those emitted by EventLoopMetrics to preserve
// backward compatibility.
new NettyEventExecutorMetrics(evGroup).bindTo(Flags.meterRegistry());
}

/**
* Returns the default common blocking task {@link BlockingTaskExecutor} which is used for
* potentially long-running tasks which may block I/O threads.
Expand Down
1 change: 1 addition & 0 deletions core/src/main/java/com/linecorp/armeria/common/metric/EventLoopMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ void remove(EventLoopGroup eventLoopGroup) {
return result;
}

@Deprecated // use the same metric with micrometer namespace instead
Copy link
Contributor

@minwoox minwoox Jun 25, 2025

Choose a reason for hiding this comment

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

Let's add @Deprecated to the class. 😉

Copy link
Contributor

@minwoox minwoox Jun 25, 2025

Choose a reason for hiding this comment

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

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

Copy link
Author

@divijvaidya divijvaidya Jun 27, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@jrhee17 jrhee17 Jul 2, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@minwoox minwoox Jul 8, 2025

Choose a reason for hiding this comment

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

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

Copy link
Author

@divijvaidya divijvaidya Jul 30, 2025

Choose a reason for hiding this comment

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

ok, I will get that done.

double pendingTasks() {
int result = 0;
for (EventLoopGroup group : registry) {
Expand Down
35 changes: 35 additions & 0 deletions core/src/test/java/com/linecorp/armeria/common/CommonPoolsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package com.linecorp.armeria.common;

import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.verify;

import org.junit.jupiter.api.Test;
import org.mockito.MockedConstruction;
import org.mockito.Mockito;

import com.linecorp.armeria.common.util.EventLoopGroups;

import io.micrometer.core.instrument.binder.netty4.NettyEventExecutorMetrics;
import io.netty.channel.EventLoopGroup;

public class CommonPoolsTest {
@Test
public void testEventLoopMetricsBinding() throws Exception {
final EventLoopGroup testGroup = EventLoopGroups.newEventLoopGroup(1);

try {
try (MockedConstruction<NettyEventExecutorMetrics> microMeterEventloopMetrics =
Mockito.mockConstruction(NettyEventExecutorMetrics.class)) {
CommonPools.bindEventLoopMetricsForWorkerGroup(testGroup);

assertThat(microMeterEventloopMetrics.constructed().isEmpty()).isFalse();

final NettyEventExecutorMetrics instance = microMeterEventloopMetrics.constructed().get(0);
verify(instance).bindTo(any());
}
} finally {
testGroup.shutdownNow();
}
}
}
3 changes: 3 additions & 0 deletions dependencies.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,9 @@ version.ref = "mockito"
[libraries.mockito-junit-jupiter]
module = "org.mockito:mockito-junit-jupiter"
version.ref = "mockito"
[libraries.mockito-inline]
module = "org.mockito:mockito-inline"
version.ref = "mockito"

[libraries.monix-reactive_v212]
module = "io.monix:monix-reactive_2.12"
Expand Down
Loading