Skip to content

Commit 767f59a

Browse files
bclozelbclozel
committed
Make client.name low cardinality keyvalue for client observations
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
1 parent da088e5 commit 767f59a

File tree

7 files changed

+82
-54
lines changed

7 files changed

+82
-54
lines changed

‎framework-docs/src/docs/asciidoc/integration/observability.adoc

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -131,18 +131,18 @@ Instrumentation is using the `org.springframework.http.client.observation.Client
131131
[cols="a,a"]
132132
|===
133133
|Name | Description
134-
|`exception` _(required)_|Name of the exception thrown during the exchange, or `"none"` if no exception happened.
135134
|`method` _(required)_|Name of HTTP request method or `"none"` if the request could not be created.
136-
|`outcome` _(required)_|Outcome of the HTTP client exchange.
137-
|`status` _(required)_|HTTP response raw status code, or `"IO_ERROR"` in case of `IOException`, or `"CLIENT_ERROR"` if no response was received.
138135
|`uri` _(required)_|URI template used for HTTP request, or `"none"` if none was provided.
136+
|`client.name` _(required)_|Client name derived from the request URI host.
137+
|`status` _(required)_|HTTP response raw status code, or `"IO_ERROR"` in case of `IOException`, or `"CLIENT_ERROR"` if no response was received.
138+
|`outcome` _(required)_|Outcome of the HTTP client exchange.
139+
|`exception` _(required)_|Name of the exception thrown during the exchange, or `"none"` if no exception happened.
139140
|===
140141

141142
.High cardinality Keys
142143
[cols="a,a"]
143144
|===
144145
|Name | Description
145-
|`client.name` _(required)_|Client name derived from the request URI host.
146146
|`http.url` _(required)_|HTTP request URI.
147147
|===
148148

@@ -157,18 +157,18 @@ Instrumentation is using the `org.springframework.web.reactive.function.client.C
157157
[cols="a,a"]
158158
|===
159159
|Name | Description
160-
|`exception` _(required)_|Name of the exception thrown during the exchange, or `"none"` if no exception happened.
161160
|`method` _(required)_|Name of HTTP request method or `"none"` if the request could not be created.
162-
|`outcome` _(required)_|Outcome of the HTTP client exchange.
163-
|`status` _(required)_|HTTP response raw status code, or `"IO_ERROR"` in case of `IOException`, or `"CLIENT_ERROR"` if no response was received.
164161
|`uri` _(required)_|URI template used for HTTP request, or `"none"` if none was provided.
162+
|`client.name` _(required)_|Client name derived from the request URI host.
163+
|`status` _(required)_|HTTP response raw status code, or `"IO_ERROR"` in case of `IOException`, or `"CLIENT_ERROR"` if no response was received.
164+
|`outcome` _(required)_|Outcome of the HTTP client exchange.
165+
|`exception` _(required)_|Name of the exception thrown during the exchange, or `"none"` if no exception happened.
165166
|===
166167

167168
.High cardinality Keys
168169
[cols="a,a"]
169170
|===
170171
|Name | Description
171-
|`client.name` _(required)_|Client name derived from the request URI host.
172172
|`http.url` _(required)_|HTTP request URI.
173173
|===
174174

‎spring-web/src/main/java/org/springframework/http/client/observation/ClientHttpObservationDocumentation.java

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -50,7 +50,7 @@ public KeyName[] getLowCardinalityKeyNames() {
5050

5151
@Override
5252
public KeyName[] getHighCardinalityKeyNames() {
53-
return HighCardinalityKeyNames.values();
53+
return new KeyName[] {HighCardinalityKeyNames.HTTP_URL};
5454
}
5555

5656
};
@@ -89,6 +89,17 @@ public String asString() {
8989
}
9090
},
9191

92+
93+
/**
94+
* Client name derived from the request URI host.
95+
*/
96+
CLIENT_NAME {
97+
@Override
98+
public String asString() {
99+
return "client.name";
100+
}
101+
},
102+
92103
/**
93104
* Name of the exception thrown during the exchange, or {@value KeyValue#NONE_VALUE} if no exception happened.
94105
*/
@@ -126,7 +137,10 @@ public String asString() {
126137

127138
/**
128139
* Client name derived from the request URI host.
140+
* @deprecated in favor of {@link LowCardinalityKeyNames#CLIENT_NAME}.
141+
* This will be available both as a low and high cardinality key value.
129142
*/
143+
@Deprecated(since = "6.0.5", forRemoval = true)
130144
CLIENT_NAME {
131145
@Override
132146
public String asString() {

‎spring-web/src/main/java/org/springframework/http/client/observation/DefaultClientRequestObservationConvention.java

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -51,12 +51,12 @@ public class DefaultClientRequestObservationConvention implements ClientRequestO
5151

5252
private static final KeyValue HTTP_OUTCOME_UNKNOWN = KeyValue.of(LowCardinalityKeyNames.OUTCOME, "UNKNOWN");
5353

54+
private static final KeyValue CLIENT_NAME_NONE = KeyValue.of(LowCardinalityKeyNames.CLIENT_NAME, KeyValue.NONE_VALUE);
55+
5456
private static final KeyValue EXCEPTION_NONE = KeyValue.of(LowCardinalityKeyNames.EXCEPTION, KeyValue.NONE_VALUE);
5557

5658
private static final KeyValue HTTP_URL_NONE = KeyValue.of(HighCardinalityKeyNames.HTTP_URL, KeyValue.NONE_VALUE);
5759

58-
private static final KeyValue CLIENT_NAME_NONE = KeyValue.of(HighCardinalityKeyNames.CLIENT_NAME, KeyValue.NONE_VALUE);
59-
6060

6161
private final String name;
6262

@@ -87,7 +87,7 @@ public String getContextualName(ClientRequestObservationContext context) {
8787

8888
@Override
8989
public KeyValues getLowCardinalityKeyValues(ClientRequestObservationContext context) {
90-
return KeyValues.of(uri(context), method(context), status(context), exception(context), outcome(context));
90+
return KeyValues.of(uri(context), method(context), status(context), clientName(context), exception(context), outcome(context));
9191
}
9292

9393
protected KeyValue uri(ClientRequestObservationContext context) {
@@ -119,6 +119,13 @@ protected KeyValue status(ClientRequestObservationContext context) {
119119
}
120120
}
121121

122+
protected KeyValue clientName(ClientRequestObservationContext context) {
123+
if (context.getCarrier() != null && context.getCarrier().getURI().getHost() != null) {
124+
return KeyValue.of(LowCardinalityKeyNames.CLIENT_NAME, context.getCarrier().getURI().getHost());
125+
}
126+
return CLIENT_NAME_NONE;
127+
}
128+
122129
protected KeyValue exception(ClientRequestObservationContext context) {
123130
Throwable error = context.getError();
124131
if (error != null) {
@@ -129,7 +136,7 @@ protected KeyValue exception(ClientRequestObservationContext context) {
129136
return EXCEPTION_NONE;
130137
}
131138

132-
protected static KeyValue outcome(ClientRequestObservationContext context) {
139+
protected KeyValue outcome(ClientRequestObservationContext context) {
133140
if (context.getResponse() != null) {
134141
try {
135142
return HttpOutcome.forStatus(context.getResponse().getStatusCode());
@@ -143,7 +150,7 @@ protected static KeyValue outcome(ClientRequestObservationContext context) {
143150

144151
@Override
145152
public KeyValues getHighCardinalityKeyValues(ClientRequestObservationContext context) {
146-
return KeyValues.of(requestUri(context), clientName(context));
153+
return KeyValues.of(requestUri(context));
147154
}
148155

149156
protected KeyValue requestUri(ClientRequestObservationContext context) {
@@ -153,12 +160,6 @@ protected KeyValue requestUri(ClientRequestObservationContext context) {
153160
return HTTP_URL_NONE;
154161
}
155162

156-
protected KeyValue clientName(ClientRequestObservationContext context) {
157-
if (context.getCarrier() != null && context.getCarrier().getURI().getHost() != null) {
158-
return KeyValue.of(HighCardinalityKeyNames.CLIENT_NAME, context.getCarrier().getURI().getHost());
159-
}
160-
return CLIENT_NAME_NONE;
161-
}
162163

163164
static class HttpOutcome {
164165

‎spring-web/src/test/java/org/springframework/http/client/observation/DefaultClientRequestObservationConventionTests.java

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,27 +69,25 @@ void addsKeyValuesForRequestWithUriTemplate() {
6969
context.setUriTemplate("/resource/{id}");
7070
assertThat(this.observationConvention.getLowCardinalityKeyValues(context))
7171
.contains(KeyValue.of("exception", "none"), KeyValue.of("method", "GET"), KeyValue.of("uri", "/resource/{id}"),
72-
KeyValue.of("status", "200"), KeyValue.of("outcome", "SUCCESS"));
73-
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(2)
74-
.contains(KeyValue.of("client.name", "none"), KeyValue.of("http.url", "/resource/42"));
72+
KeyValue.of("status", "200"), KeyValue.of("client.name", "none"), KeyValue.of("outcome", "SUCCESS"));
73+
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("http.url", "/resource/42"));
7574
}
7675

7776
@Test
7877
void addsKeyValuesForRequestWithoutUriTemplate() {
7978
ClientRequestObservationContext context = createContext(
8079
new MockClientHttpRequest(HttpMethod.GET, "/resource/42"), new MockClientHttpResponse());
8180
assertThat(this.observationConvention.getLowCardinalityKeyValues(context))
82-
.contains(KeyValue.of("method", "GET"), KeyValue.of("uri", "none"));
83-
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(2)
84-
.contains(KeyValue.of("http.url", "/resource/42"));
81+
.contains(KeyValue.of("method", "GET"), KeyValue.of("client.name", "none"), KeyValue.of("uri", "none"));
82+
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("http.url", "/resource/42"));
8583
}
8684

8785
@Test
8886
void addsClientNameForRequestWithHost() {
8987
ClientRequestObservationContext context = createContext(
9088
new MockClientHttpRequest(HttpMethod.GET, "https://localhost:8080/resource/42"),
9189
new MockClientHttpResponse());
92-
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("client.name", "localhost"));
90+
assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("client.name", "localhost"));
9391
}
9492

9593
@Test

‎spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ClientHttpObservationDocumentation.java

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -47,7 +47,7 @@ public KeyName[] getLowCardinalityKeyNames() {
4747

4848
@Override
4949
public KeyName[] getHighCardinalityKeyNames() {
50-
return HighCardinalityKeyNames.values();
50+
return new KeyName[] {HighCardinalityKeyNames.HTTP_URL};
5151
}
5252

5353
};
@@ -86,6 +86,16 @@ public String asString() {
8686
}
8787
},
8888

89+
/**
90+
* Client name derived from the request URI host.
91+
*/
92+
CLIENT_NAME {
93+
@Override
94+
public String asString() {
95+
return "client.name";
96+
}
97+
},
98+
8999
/**
90100
* Name of the exception thrown during the exchange, or {@value KeyValue#NONE_VALUE} if no exception happened.
91101
*/
@@ -124,7 +134,10 @@ public String asString() {
124134

125135
/**
126136
* Client name derived from the request URI host.
137+
* @deprecated in favor of {@link LowCardinalityKeyNames#CLIENT_NAME}.
138+
* This will be available both as a low and high cardinality key value.
127139
*/
140+
@Deprecated(since = "6.0.5", forRemoval = true)
128141
CLIENT_NAME {
129142
@Override
130143
public String asString() {

‎spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConvention.java

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,12 @@ public class DefaultClientRequestObservationConvention implements ClientRequestO
5050

5151
private static final KeyValue HTTP_OUTCOME_UNKNOWN = KeyValue.of(LowCardinalityKeyNames.OUTCOME, "UNKNOWN");
5252

53+
private static final KeyValue CLIENT_NAME_NONE = KeyValue.of(LowCardinalityKeyNames.CLIENT_NAME, KeyValue.NONE_VALUE);
54+
5355
private static final KeyValue EXCEPTION_NONE = KeyValue.of(LowCardinalityKeyNames.EXCEPTION, KeyValue.NONE_VALUE);
5456

5557
private static final KeyValue HTTP_URL_NONE = KeyValue.of(HighCardinalityKeyNames.HTTP_URL, KeyValue.NONE_VALUE);
5658

57-
private static final KeyValue CLIENT_NAME_NONE = KeyValue.of(HighCardinalityKeyNames.CLIENT_NAME, KeyValue.NONE_VALUE);
5859

5960
private final String name;
6061

@@ -86,7 +87,7 @@ public String getContextualName(ClientRequestObservationContext context) {
8687

8788
@Override
8889
public KeyValues getLowCardinalityKeyValues(ClientRequestObservationContext context) {
89-
return KeyValues.of(uri(context), method(context), status(context), exception(context), outcome(context));
90+
return KeyValues.of(uri(context), method(context), status(context), clientName(context), exception(context), outcome(context));
9091
}
9192

9293
protected KeyValue uri(ClientRequestObservationContext context) {
@@ -119,6 +120,13 @@ protected KeyValue status(ClientRequestObservationContext context) {
119120
return STATUS_CLIENT_ERROR;
120121
}
121122

123+
protected KeyValue clientName(ClientRequestObservationContext context) {
124+
if (context.getRequest() != null && context.getRequest().url().getHost() != null) {
125+
return KeyValue.of(LowCardinalityKeyNames.CLIENT_NAME, context.getRequest().url().getHost());
126+
}
127+
return CLIENT_NAME_NONE;
128+
}
129+
122130
protected KeyValue exception(ClientRequestObservationContext context) {
123131
Throwable error = context.getError();
124132
if (error != null) {
@@ -141,7 +149,7 @@ protected KeyValue outcome(ClientRequestObservationContext context) {
141149

142150
@Override
143151
public KeyValues getHighCardinalityKeyValues(ClientRequestObservationContext context) {
144-
return KeyValues.of(httpUrl(context), clientName(context));
152+
return KeyValues.of(httpUrl(context));
145153
}
146154

147155
protected KeyValue httpUrl(ClientRequestObservationContext context) {
@@ -151,13 +159,6 @@ protected KeyValue httpUrl(ClientRequestObservationContext context) {
151159
return HTTP_URL_NONE;
152160
}
153161

154-
protected KeyValue clientName(ClientRequestObservationContext context) {
155-
if (context.getRequest() != null && context.getRequest().url().getHost() != null) {
156-
return KeyValue.of(HighCardinalityKeyNames.CLIENT_NAME, context.getRequest().url().getHost());
157-
}
158-
return CLIENT_NAME_NONE;
159-
}
160-
161162
static class HttpOutcome {
162163

163164
static KeyValue forStatus(HttpStatusCode statusCode) {

‎spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientRequestObservationConventionTests.java

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -58,22 +58,23 @@ void shouldOnlySupportWebClientObservationContext() {
5858
@Test
5959
void shouldAddKeyValuesForNullExchange() {
6060
ClientRequestObservationContext context = new ClientRequestObservationContext();
61-
assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).hasSize(5)
61+
assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).hasSize(6)
6262
.contains(KeyValue.of("method", "none"), KeyValue.of("uri", "none"), KeyValue.of("status", "CLIENT_ERROR"),
63+
KeyValue.of("client.name", "none"),
6364
KeyValue.of("exception", "none"), KeyValue.of("outcome", "UNKNOWN"));
64-
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(2)
65-
.contains(KeyValue.of("client.name", "none"), KeyValue.of("http.url", "none"));
65+
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(1)
66+
.contains(KeyValue.of("http.url", "none"));
6667
}
6768

6869
@Test
6970
void shouldAddKeyValuesForExchangeWithException() {
7071
ClientRequestObservationContext context = new ClientRequestObservationContext();
7172
context.setError(new IllegalStateException("Could not create client request"));
72-
assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).hasSize(5)
73+
assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).hasSize(6)
7374
.contains(KeyValue.of("method", "none"), KeyValue.of("uri", "none"), KeyValue.of("status", "CLIENT_ERROR"),
74-
KeyValue.of("exception", "IllegalStateException"), KeyValue.of("outcome", "UNKNOWN"));
75-
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(2)
76-
.contains(KeyValue.of("client.name", "none"), KeyValue.of("http.url", "none"));
75+
KeyValue.of("client.name", "none"), KeyValue.of("exception", "IllegalStateException"), KeyValue.of("outcome", "UNKNOWN"));
76+
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(1)
77+
.contains(KeyValue.of("http.url", "none"));
7778
}
7879

7980
@Test
@@ -85,9 +86,9 @@ void shouldAddKeyValuesForRequestWithUriTemplate() {
8586
context.setRequest(context.getCarrier().build());
8687
assertThat(this.observationConvention.getLowCardinalityKeyValues(context))
8788
.contains(KeyValue.of("exception", "none"), KeyValue.of("method", "GET"), KeyValue.of("uri", "/resource/{id}"),
88-
KeyValue.of("status", "200"), KeyValue.of("outcome", "SUCCESS"));
89-
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(2)
90-
.contains(KeyValue.of("client.name", "none"), KeyValue.of("http.url", "/resource/42"));
89+
KeyValue.of("status", "200"), KeyValue.of("client.name", "none"), KeyValue.of("outcome", "SUCCESS"));
90+
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(1)
91+
.contains(KeyValue.of("http.url", "/resource/42"));
9192
}
9293

9394
@Test
@@ -96,14 +97,14 @@ void shouldAddKeyValuesForRequestWithoutUriTemplate() {
9697
context.setRequest(context.getCarrier().build());
9798
assertThat(this.observationConvention.getLowCardinalityKeyValues(context))
9899
.contains(KeyValue.of("method", "GET"), KeyValue.of("uri", "none"));
99-
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(2).contains(KeyValue.of("http.url", "/resource/42"));
100+
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).hasSize(1).contains(KeyValue.of("http.url", "/resource/42"));
100101
}
101102

102103
@Test
103104
void shouldAddClientNameKeyValueForRequestWithHost() {
104105
ClientRequestObservationContext context = createContext(ClientRequest.create(HttpMethod.GET, URI.create("https://localhost:8080/resource/42")));
105106
context.setRequest(context.getCarrier().build());
106-
assertThat(this.observationConvention.getHighCardinalityKeyValues(context)).contains(KeyValue.of("client.name", "localhost"));
107+
assertThat(this.observationConvention.getLowCardinalityKeyValues(context)).contains(KeyValue.of("client.name", "localhost"));
107108
}
108109

109110
private ClientRequestObservationContext createContext(ClientRequest.Builder request) {

0 commit comments

Comments
 (0)