Provide full request URL for "http.url" keyvalue

Prior to this commit, the Observation filter for Servlet applications
would only use the request pathInfo as an "http.url" high cardinality
keyvalue. This commit ensures that we're using the full request URL as a
value there.

This also polishes gh-29254.

Fixes gh-29257
See gh-29254
This commit is contained in:
Brian Clozel 2022-10-05 11:40:28 +02:00
parent 7dd6afc263
commit 681cf0dae7
5 changed files with 13 additions and 19 deletions

View File

@ -46,7 +46,7 @@ public class DefaultClientHttpObservationConvention implements ClientHttpObserva
private static final KeyValue EXCEPTION_NONE = KeyValue.of(ClientHttpObservation.LowCardinalityKeyNames.EXCEPTION, "none");
private static final KeyValue URI_EXPANDED_NONE = KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.HTTP_URL, "none");
private static final KeyValue HTTP_URL_NONE = KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.HTTP_URL, "none");
private static final KeyValue CLIENT_NAME_NONE = KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.CLIENT_NAME, "none");
@ -141,7 +141,7 @@ public class DefaultClientHttpObservationConvention implements ClientHttpObserva
if (context.getCarrier() != null) {
return KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.HTTP_URL, context.getCarrier().getURI().toASCIIString());
}
return URI_EXPANDED_NONE;
return HTTP_URL_NONE;
}
protected KeyValue clientName(ClientHttpObservationContext context) {

View File

@ -47,7 +47,7 @@ public class DefaultHttpRequestsObservationConvention implements HttpRequestsObs
private static final KeyValue EXCEPTION_NONE = KeyValue.of(HttpRequestsObservation.LowCardinalityKeyNames.EXCEPTION, "none");
private static final KeyValue URI_EXPANDED_UNKNOWN = KeyValue.of(HttpRequestsObservation.HighCardinalityKeyNames.HTTP_URL, "UNKNOWN");
private static final KeyValue HTTP_URL_UNKNOWN = KeyValue.of(HttpRequestsObservation.HighCardinalityKeyNames.HTTP_URL, "UNKNOWN");
private final String name;
@ -138,10 +138,9 @@ public class DefaultHttpRequestsObservationConvention implements HttpRequestsObs
protected KeyValue uriExpanded(HttpRequestsObservationContext context) {
if (context.getCarrier() != null) {
String uriExpanded = (context.getCarrier().getPathInfo() != null) ? context.getCarrier().getPathInfo() : "/";
return KeyValue.of(HttpRequestsObservation.HighCardinalityKeyNames.HTTP_URL, uriExpanded);
return KeyValue.of(HttpRequestsObservation.HighCardinalityKeyNames.HTTP_URL, context.getCarrier().getRequestURI());
}
return URI_EXPANDED_UNKNOWN;
return HTTP_URL_UNKNOWN;
}
}

View File

@ -48,7 +48,7 @@ public class DefaultHttpRequestsObservationConvention implements HttpRequestsObs
private static final KeyValue EXCEPTION_NONE = KeyValue.of(HttpRequestsObservation.LowCardinalityKeyNames.EXCEPTION, "none");
private static final KeyValue URI_EXPANDED_UNKNOWN = KeyValue.of(HttpRequestsObservation.HighCardinalityKeyNames.HTTP_URL, "UNKNOWN");
private static final KeyValue HTTP_URL_UNKNOWN = KeyValue.of(HttpRequestsObservation.HighCardinalityKeyNames.HTTP_URL, "UNKNOWN");
private final String name;
@ -85,7 +85,7 @@ public class DefaultHttpRequestsObservationConvention implements HttpRequestsObs
@Override
public KeyValues getHighCardinalityKeyValues(HttpRequestsObservationContext context) {
return KeyValues.of(uriExpanded(context));
return KeyValues.of(httpUrl(context));
}
protected KeyValue method(HttpRequestsObservationContext context) {
@ -143,12 +143,12 @@ public class DefaultHttpRequestsObservationConvention implements HttpRequestsObs
return HttpOutcome.UNKNOWN.asKeyValue();
}
protected KeyValue uriExpanded(HttpRequestsObservationContext context) {
protected KeyValue httpUrl(HttpRequestsObservationContext context) {
if (context.getCarrier() != null) {
String uriExpanded = context.getCarrier().getPath().toString();
return KeyValue.of(HttpRequestsObservation.HighCardinalityKeyNames.HTTP_URL, uriExpanded);
}
return URI_EXPANDED_UNKNOWN;
return HTTP_URL_UNKNOWN;
}
}

View File

@ -60,7 +60,6 @@ class DefaultHttpRequestsObservationConventionTests {
void addsKeyValuesForExchange() {
this.request.setMethod("POST");
this.request.setRequestURI("/test/resource");
this.request.setPathInfo("/test/resource");
assertThat(this.convention.getLowCardinalityKeyValues(this.context)).hasSize(5)
.contains(KeyValue.of("method", "POST"), KeyValue.of("uri", "UNKNOWN"), KeyValue.of("status", "200"),
@ -72,7 +71,6 @@ class DefaultHttpRequestsObservationConventionTests {
@Test
void addsKeyValuesForExchangeWithPathPattern() {
this.request.setRequestURI("/test/resource");
this.request.setPathInfo("/test/resource");
this.context.setPathPattern("/test/{name}");
assertThat(this.convention.getLowCardinalityKeyValues(this.context)).hasSize(5)
@ -85,7 +83,6 @@ class DefaultHttpRequestsObservationConventionTests {
@Test
void addsKeyValuesForErrorExchange() {
this.request.setRequestURI("/test/resource");
this.request.setPathInfo("/test/resource");
this.context.setError(new IllegalArgumentException("custom error"));
this.response.setStatus(500);
@ -99,7 +96,6 @@ class DefaultHttpRequestsObservationConventionTests {
@Test
void addsKeyValuesForRedirectExchange() {
this.request.setRequestURI("/test/redirect");
this.request.setPathInfo("/test/redirect");
this.response.setStatus(302);
this.response.addHeader("Location", "https://example.org/other");
@ -113,7 +109,6 @@ class DefaultHttpRequestsObservationConventionTests {
@Test
void addsKeyValuesForNotFoundExchange() {
this.request.setRequestURI("/test/notFound");
this.request.setPathInfo("/test/notFound");
this.response.setStatus(404);
assertThat(this.convention.getLowCardinalityKeyValues(this.context)).hasSize(5)

View File

@ -47,7 +47,7 @@ public class DefaultClientObservationConvention implements ClientObservationConv
private static final KeyValue EXCEPTION_NONE = KeyValue.of(ClientObservation.LowCardinalityKeyNames.EXCEPTION, "none");
private static final KeyValue URI_EXPANDED_NONE = KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.HTTP_URL, "none");
private static final KeyValue HTTP_URL_NONE = KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.HTTP_URL, "none");
private static final KeyValue CLIENT_NAME_NONE = KeyValue.of(ClientHttpObservation.HighCardinalityKeyNames.CLIENT_NAME, "none");
@ -135,14 +135,14 @@ public class DefaultClientObservationConvention implements ClientObservationConv
@Override
public KeyValues getHighCardinalityKeyValues(ClientObservationContext context) {
return KeyValues.of(uriExpanded(context), clientName(context));
return KeyValues.of(httpUrl(context), clientName(context));
}
protected KeyValue uriExpanded(ClientObservationContext context) {
protected KeyValue httpUrl(ClientObservationContext context) {
if (context.getCarrier() != null) {
return KeyValue.of(ClientObservation.HighCardinalityKeyNames.HTTP_URL, context.getCarrier().url().toASCIIString());
}
return URI_EXPANDED_NONE;
return HTTP_URL_NONE;
}
protected KeyValue clientName(ClientObservationContext context) {