From 7ea938b2cff1ccc6cfcdb1307e52674f370267ac Mon Sep 17 00:00:00 2001 From: Mattias-Sehlstedt <60173714+Mattias-Sehlstedt@users.noreply.github.com> Date: Wed, 18 Dec 2024 16:44:18 +0100 Subject: [PATCH] Change the description for the uri client request observation metric to describe what parts that are removed from the template Signed-off-by: Mattias-Sehlstedt <60173714+Mattias-Sehlstedt@users.noreply.github.com> --- .../modules/ROOT/pages/integration/observability.adoc | 6 +++--- .../ClientHttpObservationDocumentation.java | 2 +- ...efaultClientRequestObservationConventionTests.java | 11 +++++++++++ .../client/ClientHttpObservationDocumentation.java | 2 +- ...efaultClientRequestObservationConventionTests.java | 7 +++++++ 5 files changed, 23 insertions(+), 5 deletions(-) diff --git a/framework-docs/modules/ROOT/pages/integration/observability.adoc b/framework-docs/modules/ROOT/pages/integration/observability.adoc index 85af67d5d052..ac99a6870eb9 100644 --- a/framework-docs/modules/ROOT/pages/integration/observability.adoc +++ b/framework-docs/modules/ROOT/pages/integration/observability.adoc @@ -285,7 +285,7 @@ Instrumentation uses the `org.springframework.http.client.observation.ClientRequ |=== |Name | Description |`method` _(required)_|Name of the HTTP request method or `"none"` if not a well-known method. -|`uri` _(required)_|URI template used for HTTP request, or `"none"` if none was provided. Only the path part of the URI is considered. +|`uri` _(required)_|URI template used for HTTP request, or `"none"` if none was provided. The protocol, host and port part of the URI are not considered. |`client.name` _(required)_|Client name derived from the request URI host. |`status` _(required)_|HTTP response raw status code, or `"IO_ERROR"` in case of `IOException`, or `"CLIENT_ERROR"` if no response was received. |`outcome` _(required)_|Outcome of the HTTP client exchange. @@ -313,7 +313,7 @@ Instrumentation uses the `org.springframework.http.client.observation.ClientRequ |=== |Name | Description |`method` _(required)_|Name of the HTTP request method or `"none"` if the request could not be created. -|`uri` _(required)_|URI template used for HTTP request, or `"none"` if none was provided. Only the path part of the URI is considered. +|`uri` _(required)_|URI template used for HTTP request, or `"none"` if none was provided. The protocol, host and port part of the URI are not considered. |`client.name` _(required)_|Client name derived from the request URI host. |`status` _(required)_|HTTP response raw status code, or `"IO_ERROR"` in case of `IOException`, or `"CLIENT_ERROR"` if no response was received. |`outcome` _(required)_|Outcome of the HTTP client exchange. @@ -342,7 +342,7 @@ Instrumentation uses the `org.springframework.web.reactive.function.client.Clien |=== |Name | Description |`method` _(required)_|Name of the HTTP request method or `"none"` if not a well-known method. -|`uri` _(required)_|URI template used for HTTP request, or `"none"` if none was provided. Only the path part of the URI is considered. +|`uri` _(required)_|URI template used for HTTP request, or `"none"` if none was provided. The protocol, host and port part of the URI are not considered. |`client.name` _(required)_|Client name derived from the request URI host. |`status` _(required)_|HTTP response raw status code, or `"IO_ERROR"` in case of `IOException`, or `"CLIENT_ERROR"` if no response was received. |`outcome` _(required)_|Outcome of the HTTP client exchange. diff --git a/spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservationDocumentation.java b/spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservationDocumentation.java index 3c256ccb9ebe..cc14d6b90004 100644 --- a/spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservationDocumentation.java +++ b/spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservationDocumentation.java @@ -74,7 +74,7 @@ public String asString() { /** * URI template used for HTTP request, or {@value KeyValue#NONE_VALUE} if * none was provided. - *

Only the path part of the URI is considered. + *

The protocol, host and port part of the URI are not considered. */ URI { @Override diff --git a/spring-web/src/test/java/org/springframework/http/client/observation/DefaultClientRequestObservationConventionTests.java b/spring-web/src/test/java/org/springframework/http/client/observation/DefaultClientRequestObservationConventionTests.java index dea5002df035..984f7b26ee00 100644 --- a/spring-web/src/test/java/org/springframework/http/client/observation/DefaultClientRequestObservationConventionTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/observation/DefaultClientRequestObservationConventionTests.java @@ -88,6 +88,17 @@ void addsKeyValuesForRequestWithUriTemplateWithHost() { assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("http.url", "https://example.org/resource/42")); } + @Test + void addsKeyValuesForRequestWithUriTemplateWithHostAndQuery() { + ClientRequestObservationContext context = createContext( + new MockClientHttpRequest(HttpMethod.GET, "https://example.org/resource/{id}?queryKey={queryValue}", 42, "Query"), response); + context.setUriTemplate("https://example.org/resource/{id}?queryKey={queryValue}"); + assertThat(this.observationConvention.getLowCardinalityKeyValues(context)) + .contains(KeyValue.of("exception", "none"), KeyValue.of("method", "GET"), KeyValue.of("uri", "/resource/{id}?queryKey={queryValue}"), + KeyValue.of("status", "200"), KeyValue.of("client.name", "example.org"), KeyValue.of("outcome", "SUCCESS")); + assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("http.url", "https://example.org/resource/42?queryKey=Query")); + } + @Test void addsKeyValuesForRequestWithUriTemplateWithoutPath() { ClientRequestObservationContext context = createContext( diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientHttpObservationDocumentation.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientHttpObservationDocumentation.java index c452a078b8b3..5f02518338c4 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientHttpObservationDocumentation.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientHttpObservationDocumentation.java @@ -71,7 +71,7 @@ public String asString() { /** * URI template used for HTTP request, or {@value KeyValue#NONE_VALUE} if * none was provided. - *

Only the path part of the URI is considered. + *

The protocol, host and port part of the URI are not considered. */ URI { @Override diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConventionTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConventionTests.java index 1e2c08fb70f6..01dc86c0d9d0 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConventionTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConventionTests.java @@ -119,6 +119,13 @@ void shouldOnlyConsiderPathForUriKeyValue() { assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("uri", "/resource/{id}")); } + @Test + void shouldKeepQueryParameterForUriKeyValue() { + ClientRequestObservationContext context = createContext(ClientRequest.create(HttpMethod.GET, URI.create("https://example.org/resource/42?queryKey=Query"))); + context.setUriTemplate("https://example.org/resource/{id}?queryKey={queryValue}"); + assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("uri", "/resource/{id}?queryKey={queryValue}")); + } + private ClientRequestObservationContext createContext(ClientRequest.Builder request) { ClientRequestObservationContext context = new ClientRequestObservationContext(request); context.setResponse(ClientResponse.create(HttpStatus.OK).build());