Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Make client.name low cardinality keyvalue for client observations

Browse Source 
Prior to this commit, the `"client.name"` key value for the
`"http.client.requests"` client HTTP observations would be considered as
high cardinality, as the URI host is technically unbounded.
In practice, the number of hosts used by a client in a given application
can be considered as low cardinality. This commit moves this keyvalue to
low cardinality so that it's present for both metrics and traces.

Closes gh-29839
This commit is contained in:
Brian Clozel
2023-01-25 20:51:15 +01:00
parent da088e58c3
commit 767f59a3ae
7 changed files with 82 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
spring-web/src/test/java/org/springframework/http/client/observation/DefaultClientRequestObservationConventionTests.java
View File

@@ -69,9 +69,8 @@ class DefaultClientRequestObservationConventionTests {
context.setUriTemplate("/resource/{id}");
assertThat(this.observationConvention.getLowCardinalityKeyValues(context))
.contains(KeyValue.of("exception", "none"), KeyValue.of("method", "GET"), KeyValue.of("uri", "/resource/{id}"),
KeyValue.of("status", "200"), KeyValue.of("outcome", "SUCCESS"));
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(2)
.contains(KeyValue.of("client.name", "none"), KeyValue.of("http.url", "/resource/42"));
KeyValue.of("status", "200"), KeyValue.of("client.name", "none"), KeyValue.of("outcome", "SUCCESS"));
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("http.url", "/resource/42"));
}
@Test
@@ -79,9 +78,8 @@ class DefaultClientRequestObservationConventionTests {
ClientRequestObservationContext context = createContext(
new MockClientHttpRequest(HttpMethod.GET, "/resource/42"), new MockClientHttpResponse());
assertThat(this.observationConvention.getLowCardinalityKeyValues(context))
.contains(KeyValue.of("method", "GET"), KeyValue.of("uri", "none"));
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(2)
.contains(KeyValue.of("http.url", "/resource/42"));
.contains(KeyValue.of("method", "GET"), KeyValue.of("client.name", "none"), KeyValue.of("uri", "none"));
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("http.url", "/resource/42"));
}
@Test
@@ -89,7 +87,7 @@ class DefaultClientRequestObservationConventionTests {
ClientRequestObservationContext context = createContext(
new MockClientHttpRequest(HttpMethod.GET, "https://localhost:8080/resource/42"),
new MockClientHttpResponse());
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("client.name", "localhost"));
assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("client.name", "localhost"));
}
@Test