diff --git a/docs/modules/ROOT/partials/_metrics.adoc b/docs/modules/ROOT/partials/_metrics.adoc index 10d76eee4..9178361c4 100644 --- a/docs/modules/ROOT/partials/_metrics.adoc +++ b/docs/modules/ROOT/partials/_metrics.adoc @@ -11,9 +11,9 @@ Observation created when sending a request through the gateway. ____ -**Metric name** `http.client.requests` (defined by convention class `org.springframework.cloud.gateway.filter.headers.observation.DefaultGatewayObservationConvention`). **Type** `timer`. +**Metric name** `spring.cloud.gateway.http.client.requests` (defined by convention class `org.springframework.cloud.gateway.filter.headers.observation.DefaultGatewayObservationConvention`). **Type** `timer`. -**Metric name** `http.client.requests.active` (defined by convention class `org.springframework.cloud.gateway.filter.headers.observation.DefaultGatewayObservationConvention`). **Type** `long task timer`. +**Metric name** `spring.cloud.gateway.http.client.requests.active` (defined by convention class `org.springframework.cloud.gateway.filter.headers.observation.DefaultGatewayObservationConvention`). **Type** `long task timer`. IMPORTANT: KeyValues that are added after starting the Observation might be missing from the *.active metrics. diff --git a/docs/modules/ROOT/partials/_spans.adoc b/docs/modules/ROOT/partials/_spans.adoc index 41a4883de..6441eabd2 100644 --- a/docs/modules/ROOT/partials/_spans.adoc +++ b/docs/modules/ROOT/partials/_spans.adoc @@ -8,7 +8,7 @@ Below you can find a list of all spans declared by this project. > Observation created when sending a request through the gateway. -**Span name** `http.client.requests` (defined by convention class `org.springframework.cloud.gateway.filter.headers.observation.DefaultGatewayObservationConvention`). +**Span name** `spring.cloud.gateway.http.client.requests` (defined by convention class `org.springframework.cloud.gateway.filter.headers.observation.DefaultGatewayObservationConvention`). Fully qualified name of the enclosing class `org.springframework.cloud.gateway.filter.headers.observation.GatewayDocumentedObservation`. diff --git a/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/headers/observation/DefaultGatewayObservationConvention.java b/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/headers/observation/DefaultGatewayObservationConvention.java index 54b27e424..31f885005 100644 --- a/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/headers/observation/DefaultGatewayObservationConvention.java +++ b/spring-cloud-gateway-server-webflux/src/main/java/org/springframework/cloud/gateway/filter/headers/observation/DefaultGatewayObservationConvention.java @@ -70,9 +70,18 @@ public KeyValues getHighCardinalityKeyValues(GatewayContext context) { return KeyValues.of(URI.withValue(context.getRequest().getURI().toString())); } + /** + * The {@code spring.cloud.gateway.} prefix avoids a metric name collision with Spring + * Web's {@code DefaultClientRequestObservationConvention}, which uses + * {@code http.client.requests} for {@code WebClient} requests with a different + * low-cardinality label set. Without it, Prometheus refuses to register one of the + * two meters and a metric series is dropped. + * @see gh-3153 + */ @Override public String getName() { - return "http.client.requests"; + return "spring.cloud.gateway.http.client.requests"; } @Override diff --git a/spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/DefaultGatewayObservationConventionTests.java b/spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/DefaultGatewayObservationConventionTests.java new file mode 100644 index 000000000..cb2154ef5 --- /dev/null +++ b/spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/DefaultGatewayObservationConventionTests.java @@ -0,0 +1,72 @@ +/* + * Copyright 2013-present the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.gateway.filter.headers.observation; + +import io.micrometer.common.KeyValue; +import io.micrometer.common.KeyValues; +import org.junit.jupiter.api.Test; + +import org.springframework.cloud.gateway.route.Route; +import org.springframework.cloud.gateway.support.ServerWebExchangeUtils; +import org.springframework.http.HttpHeaders; +import org.springframework.mock.http.server.reactive.MockServerHttpRequest; +import org.springframework.mock.web.server.MockServerWebExchange; + +import static org.assertj.core.api.Assertions.assertThat; + +class DefaultGatewayObservationConventionTests { + + @Test + void getName_returnsSpringCloudGatewayPrefixedName() { + assertThat(DefaultGatewayObservationConvention.INSTANCE.getName()) + .isEqualTo("spring.cloud.gateway.http.client.requests"); + } + + @Test + void getName_doesNotCollideWithSpringWebClientMetric() { + assertThat(DefaultGatewayObservationConvention.INSTANCE.getName()).isNotEqualTo("http.client.requests"); + } + + @Test + void getContextualName_returnsHttpMethodPrefixedName() { + MockServerHttpRequest request = MockServerHttpRequest.get("http://localhost:8080/foo").build(); + MockServerWebExchange exchange = MockServerWebExchange.from(request); + GatewayContext context = new GatewayContext(new HttpHeaders(), exchange.getRequest(), exchange); + + assertThat(DefaultGatewayObservationConvention.INSTANCE.getContextualName(context)).isEqualTo("HTTP GET"); + } + + @Test + void getLowCardinalityKeyValues_pinsGatewayRouteKeys() { + MockServerHttpRequest request = MockServerHttpRequest.get("http://localhost:8080/foo").build(); + Route route = Route.async() + .id("test-route") + .uri("http://localhost:8080/") + .order(1) + .predicate(exchange -> true) + .build(); + MockServerWebExchange exchange = MockServerWebExchange.from(request); + exchange.getAttributes().put(ServerWebExchangeUtils.GATEWAY_ROUTE_ATTR, route); + GatewayContext context = new GatewayContext(new HttpHeaders(), exchange.getRequest(), exchange); + + KeyValues keyValues = DefaultGatewayObservationConvention.INSTANCE.getLowCardinalityKeyValues(context); + + assertThat(keyValues.stream().map(KeyValue::getKey)).containsExactlyInAnyOrder("http.method", + "http.status_code", "spring.cloud.gateway.route.id", "spring.cloud.gateway.route.uri"); + } + +} diff --git a/spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/ObservedHttpHeadersFilterTests.java b/spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/ObservedHttpHeadersFilterTests.java index 89054f36e..7623ce942 100644 --- a/spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/ObservedHttpHeadersFilterTests.java +++ b/spring-cloud-gateway-server-webflux/src/test/java/org/springframework/cloud/gateway/filter/headers/observation/ObservedHttpHeadersFilterTests.java @@ -99,11 +99,12 @@ public SampleTestRunnerConsumer yourCode() throws Exception { .hasTag("spring.cloud.gateway.route.uri", "http://localhost:8080/") .hasTag("spring.cloud.gateway.route.id", "foo")); MeterRegistryAssert.then(meterRegistry) - .hasTimerWithNameAndTags("http.client.requests", + .hasTimerWithNameAndTags("spring.cloud.gateway.http.client.requests", Tags.of("spring.cloud.gateway.route.id", "foo", "error", "none", "http.method", "GET", "http.status_code", "200", "spring.cloud.gateway.route.uri", "http://localhost:8080/")) - .hasMeterWithNameAndTags("http.client.requests.active", Tags.of("spring.cloud.gateway.route.id", "foo", - "http.method", "GET", "spring.cloud.gateway.route.uri", "http://localhost:8080/")); + .hasMeterWithNameAndTags("spring.cloud.gateway.http.client.requests.active", + Tags.of("spring.cloud.gateway.route.id", "foo", "http.method", "GET", + "spring.cloud.gateway.route.uri", "http://localhost:8080/")); }; }