Skip to content

Commit

Permalink
Add contextualName to HTTP Observations
Browse files Browse the repository at this point in the history
This commit ensures that all HTTP `ObservationConvention`
implementations provide a consistent contextual name for observations.
This name should be like "http get" where only the HTTP verb changes
depending on the request.

Fixes gh-29231
  • Loading branch information
bclozel committed Sep 30, 2022
1 parent 0cc3c6b commit b9070ae
Show file tree
Hide file tree
Showing 8 changed files with 57 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ public String getName() {
return this.name;
}

@Override
public String getContextualName(ClientHttpObservationContext context) {
return "http " + context.getCarrier().getMethod().name().toLowerCase();
}

@Override
public KeyValues getLowCardinalityKeyValues(ClientHttpObservationContext context) {
return KeyValues.of(uri(context), method(context), status(context), exception(context), outcome(context));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ public String getName() {
return this.name;
}

@Override
public String getContextualName(HttpRequestsObservationContext context) {
return "http " + context.getCarrier().getMethod().toLowerCase();
}

@Override
public KeyValues getLowCardinalityKeyValues(HttpRequestsObservationContext context) {
return KeyValues.of(method(context), uri(context), status(context), exception(context), outcome(context));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ public String getName() {
return this.name;
}

@Override
public String getContextualName(HttpRequestsObservationContext context) {
return "http " + context.getCarrier().getMethod().name().toLowerCase();
}

@Override
public KeyValues getLowCardinalityKeyValues(HttpRequestsObservationContext context) {
return KeyValues.of(method(context), uri(context), status(context), exception(context), outcome(context));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,18 @@ class DefaultClientHttpObservationConventionTests {

private final DefaultClientHttpObservationConvention observationConvention = new DefaultClientHttpObservationConvention();

@Test
void shouldHaveName() {
assertThat(this.observationConvention.getName()).isEqualTo("http.client.requests");
}

@Test
void shouldHaveContextualName() {
ClientHttpObservationContext context = new ClientHttpObservationContext();
context.setCarrier(new MockClientHttpRequest(HttpMethod.GET, "/test"));
assertThat(this.observationConvention.getContextualName(context)).isEqualTo("http get");
}

@Test
void supportsOnlyClientHttpObservationContext() {
assertThat(this.observationConvention.supportsContext(new ClientHttpObservationContext())).isTrue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class DefaultHttpRequestsObservationConventionTests {

private final DefaultHttpRequestsObservationConvention convention = new DefaultHttpRequestsObservationConvention();

private final MockHttpServletRequest request = new MockHttpServletRequest();
private final MockHttpServletRequest request = new MockHttpServletRequest("GET", "/test/resource");

private final MockHttpServletResponse response = new MockHttpServletResponse();

Expand All @@ -45,6 +45,11 @@ void shouldHaveDefaultName() {
assertThat(convention.getName()).isEqualTo("http.server.requests");
}

@Test
void shouldHaveContextualName() {
assertThat(convention.getContextualName(this.context)).isEqualTo("http get");
}

@Test
void supportsOnlyHttpRequestsObservationContext() {
assertThat(this.convention.supportsContext(this.context)).isTrue();
Expand All @@ -66,7 +71,6 @@ void addsKeyValuesForExchange() {

@Test
void addsKeyValuesForExchangeWithPathPattern() {
this.request.setMethod("GET");
this.request.setRequestURI("/test/resource");
this.request.setPathInfo("/test/resource");
this.context.setPathPattern("/test/{name}");
Expand All @@ -80,7 +84,6 @@ void addsKeyValuesForExchangeWithPathPattern() {

@Test
void addsKeyValuesForErrorExchange() {
this.request.setMethod("GET");
this.request.setRequestURI("/test/resource");
this.request.setPathInfo("/test/resource");
this.context.setError(new IllegalArgumentException("custom error"));
Expand All @@ -95,7 +98,6 @@ void addsKeyValuesForErrorExchange() {

@Test
void addsKeyValuesForRedirectExchange() {
this.request.setMethod("GET");
this.request.setRequestURI("/test/redirect");
this.request.setPathInfo("/test/redirect");
this.response.setStatus(302);
Expand All @@ -110,7 +112,6 @@ void addsKeyValuesForRedirectExchange() {

@Test
void addsKeyValuesForNotFoundExchange() {
this.request.setMethod("GET");
this.request.setRequestURI("/test/notFound");
this.request.setPathInfo("/test/notFound");
this.response.setStatus(404);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,13 @@ void shouldHaveDefaultName() {
assertThat(convention.getName()).isEqualTo("http.server.requests");
}

@Test
void shouldHaveContextualName() {
ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/test/resource"));
HttpRequestsObservationContext context = new HttpRequestsObservationContext(exchange);
assertThat(convention.getContextualName(context)).isEqualTo("http get");
}

@Test
void supportsOnlyHttpRequestsObservationContext() {
ServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.post("/test/resource"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ public String getName() {
return this.name;
}

@Override
public String getContextualName(ClientObservationContext context) {
return "http " + context.getCarrier().method().name().toLowerCase();
}

@Override
public KeyValues getLowCardinalityKeyValues(ClientObservationContext context) {
return KeyValues.of(uri(context), method(context), status(context), exception(context), outcome(context));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,18 @@ class DefaultClientObservationConventionTests {

private DefaultClientObservationConvention observationConvention = new DefaultClientObservationConvention();

@Test
void shouldHaveName() {
assertThat(this.observationConvention.getName()).isEqualTo("http.client.requests");
}

@Test
void shouldHaveContextualName() {
ClientObservationContext context = new ClientObservationContext();
context.setCarrier(ClientRequest.create(HttpMethod.GET, URI.create("/test")).build());
assertThat(this.observationConvention.getContextualName(context)).isEqualTo("http get");
}

@Test
void shouldOnlySupportWebClientObservationContext() {
assertThat(this.observationConvention.supportsContext(new ClientObservationContext())).isTrue();
Expand Down

0 comments on commit b9070ae

Please sign in to comment.