Skip to content

Commit

Permalink
Change conflicting meter names in ServerMetrics (#5804)
Browse files Browse the repository at this point in the history
Motivation:

`MetricCollectingService` uses `.active.requests` to track the number of the current requests. If `armeria.server` is used as the prefix of the meter name, the name conflicts with "armeria.server.active.requests" in `ServerMetrics`.

`ServerMetrics.activeRequests()` counts all requests including `TransientService` which is excluded by `MetricCollectingService` with no `TransientServiceOption`. So, semantically, it would be better to use `.all` to the meter name to indicate that it counts all requests.

Modifications:

- Rename "armeria.server.active.requests" in `ServerMetrics` to "armeria.server.all.requests{state=active}".
- Rename "armeria.server.pending.requests" in `ServerMetrics` to "armeria.server.all.requests{state=pending}".

Result:

The meter names in `ServerMetrics` no longer conflict with `MetricCollectingService`.
  • Loading branch information
ikhoon authored Jul 10, 2024
1 parent 72ebfe1 commit a38bcf9
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 9 deletions.
21 changes: 13 additions & 8 deletions core/src/main/java/com/linecorp/armeria/server/ServerMetrics.java
Original file line number Diff line number Diff line change
Expand Up @@ -157,17 +157,22 @@ void decreaseActiveConnections() {
public void bindTo(MeterRegistry meterRegistry) {
meterRegistry.gauge("armeria.server.connections", activeConnections);
// pending requests
meterRegistry.gauge("armeria.server.pending.requests",
ImmutableList.of(Tag.of("protocol", "http1")), pendingHttp1Requests);
meterRegistry.gauge("armeria.server.pending.requests",
ImmutableList.of(Tag.of("protocol", "http2")), pendingHttp2Requests);
final String allRequestsMeterName = "armeria.server.all.requests";
meterRegistry.gauge(allRequestsMeterName,
ImmutableList.of(Tag.of("protocol", "http1"), Tag.of("state", "pending")),
pendingHttp1Requests);
meterRegistry.gauge(allRequestsMeterName,
ImmutableList.of(Tag.of("protocol", "http2"), Tag.of("state", "pending")),
pendingHttp2Requests);
// Active requests
meterRegistry.gauge("armeria.server.active.requests", ImmutableList.of(Tag.of("protocol", "http1")),
meterRegistry.gauge(allRequestsMeterName,
ImmutableList.of(Tag.of("protocol", "http1"), Tag.of("state", "active")),
activeHttp1Requests);
meterRegistry.gauge("armeria.server.active.requests", ImmutableList.of(Tag.of("protocol", "http2")),
meterRegistry.gauge(allRequestsMeterName,
ImmutableList.of(Tag.of("protocol", "http2"), Tag.of("state", "active")),
activeHttp2Requests);
meterRegistry.gauge("armeria.server.active.requests",
ImmutableList.of(Tag.of("protocol", "http1.websocket")),
meterRegistry.gauge(allRequestsMeterName,
ImmutableList.of(Tag.of("protocol", "http1.websocket"), Tag.of("state", "active")),
activeHttp1WebSocketRequests);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@
import static org.awaitility.Awaitility.await;

import java.time.Duration;
import java.util.Map;
import java.util.concurrent.CompletableFuture;

import org.assertj.core.api.Condition;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;

import com.linecorp.armeria.client.BlockingWebClient;
import com.linecorp.armeria.client.ClientFactory;
import com.linecorp.armeria.client.ClientOptions;
import com.linecorp.armeria.client.WebClient;
Expand All @@ -37,14 +40,31 @@
import com.linecorp.armeria.common.HttpResponse;
import com.linecorp.armeria.common.HttpStatus;
import com.linecorp.armeria.common.SessionProtocol;
import com.linecorp.armeria.common.metric.MeterIdPrefixFunction;
import com.linecorp.armeria.common.metric.MoreMeters;
import com.linecorp.armeria.common.prometheus.PrometheusMeterRegistries;
import com.linecorp.armeria.server.metric.MetricCollectingService;
import com.linecorp.armeria.testing.junit5.server.ServerExtension;

import io.micrometer.prometheusmetrics.PrometheusMeterRegistry;

class ServerMetricsTest {

@RegisterExtension
private static final ServerExtension server = new ServerExtension() {
final ServerExtension server = new ServerExtension() {
@Override
protected boolean runForEachTest() {
return true;
}

@Override
protected void configure(ServerBuilder sb) throws Exception {
final PrometheusMeterRegistry prometheusMeterRegistry = PrometheusMeterRegistries.newRegistry();
sb.meterRegistry(prometheusMeterRegistry);
// Use 'armeria.server' to make sure that the metric names are not conflicted with `ServerMetrics`.
sb.decorator(MetricCollectingService.newDecorator(
MeterIdPrefixFunction.ofDefault("armeria.server")));

sb.requestTimeoutMillis(0)
.requestAutoAbortDelayMillis(0)
.service("/ok/http", new HttpService() {
Expand Down Expand Up @@ -257,4 +277,29 @@ void checkWhenRequestTimeout(SessionProtocol sessionProtocol, long expectedPendi
await().until(() -> serverMetrics.activeConnections() == 0);
}
}

@CsvSource({ "H1C", "H2C" })
@ParameterizedTest
void meterNames(SessionProtocol protocol) {
final BlockingWebClient client = BlockingWebClient.of(server.uri(protocol));
assertThat(client.get("/ok/http").status()).isEqualTo(HttpStatus.OK);

await().untilAsserted(() -> {
final Map<String, Double> meters = MoreMeters.measureAll(server.server().meterRegistry());
// armeria.server.active.requests#value is measured by MetricCollectingService
assertThat(meters).hasKeySatisfying(new Condition<String>("armeria.server.active.requests#value") {
@Override
public boolean matches(String key) {
return key.startsWith("armeria.server.active.requests#value{hostname.pattern=");
}
});

final String protocolName = protocol == SessionProtocol.H1C ? "http1" : "http2";
// armeria.server.active.requests.all#value is measured by ServerMetrics
assertThat(meters).containsKey("armeria.server.all.requests#value{protocol=" + protocolName +
",state=active}");
assertThat(meters).containsKey("armeria.server.all.requests#value{protocol=" + protocolName +
",state=pending}");
});
}
}

0 comments on commit a38bcf9

Please sign in to comment.