Record errors thrown by custom handler in RestTemplate observations

Prior to this commit, the `RestTemplate` observation instrumentation
would only record `RestClientException` and `IOException` as errors in
the observation. Other types of errors can be thrown by custom
components, such as `ResponseErrorHandler` and in this case they aren't
recorded with the observation.
Also, the current instrumentation does not create any observation scope
around the execution. While this would have a limited benefit as no
application code is executed there, developers could set up custom
components (such as, again, `ResponseErrorHandler`) that could use
contextual logging with trace ids.

This commit ensures that all `Throwable` are recorded as errors with the
observations and that an observation `Scope` is created around the
execution of the client exchange.

Fixes gh-32060
This commit is contained in:
Brian Clozel
2024-01-22 11:03:57 +01:00
parent 5856d2e54e
commit 70d9f7c62c
2 changed files with 51 additions and 2 deletions

View File

@@ -39,9 +39,11 @@ import org.springframework.http.client.ClientHttpRequestFactory;
import org.springframework.http.client.ClientHttpResponse;
import org.springframework.http.client.observation.ClientRequestObservationContext;
import org.springframework.http.converter.HttpMessageConverter;
import org.springframework.lang.Nullable;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
@@ -157,6 +159,31 @@ class RestTemplateObservationTests {
assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "UNKNOWN");
}
@Test // gh-32060
void executeShouldRecordErrorsThrownByErrorHandler() throws Exception {
mockSentRequest(GET, "https://example.org");
mockResponseStatus(HttpStatus.OK);
mockResponseBody("Hello World", MediaType.TEXT_PLAIN);
given(errorHandler.hasError(any())).willThrow(new IllegalStateException("error handler"));
assertThatIllegalStateException().isThrownBy(() ->
template.execute("https://example.org", GET, null, null));
assertThatHttpObservation().hasLowCardinalityKeyValue("exception", "IllegalStateException");
}
@Test // gh-32060
void executeShouldCreateObservationScope() throws Exception {
mockSentRequest(GET, "https://example.org");
mockResponseStatus(HttpStatus.OK);
mockResponseBody("Hello World", MediaType.TEXT_PLAIN);
ObservationErrorHandler observationErrorHandler = new ObservationErrorHandler(observationRegistry);
template.setErrorHandler(observationErrorHandler);
template.execute("https://example.org", GET, null, null);
assertThat(observationErrorHandler.currentObservation).isNotNull();
}
private void mockSentRequest(HttpMethod method, String uri) throws Exception {
mockSentRequest(method, uri, new HttpHeaders());
@@ -204,4 +231,26 @@ class RestTemplateObservationTests {
}
}
static class ObservationErrorHandler implements ResponseErrorHandler {
final TestObservationRegistry observationRegistry;
@Nullable
Observation currentObservation;
public ObservationErrorHandler(TestObservationRegistry observationRegistry) {
this.observationRegistry = observationRegistry;
}
@Override
public boolean hasError(ClientHttpResponse response) throws IOException {
return true;
}
@Override
public void handleError(ClientHttpResponse response) throws IOException {
currentObservation = this.observationRegistry.getCurrentObservation();
}
}
}