Exceptions thrown by custom error handlers are not recorded in RestTemplate observations

[Vsevolod123]

RestTemplate set error to observation only for IOException and RestClientException and there is no way to customize that.

I tried error handler like this

@RequiredArgsConstructor
public class DefaultResponseRestTemplateErrorHandler extends DefaultResponseErrorHandler {
    private final ObservationRegistry observationRegistry;

    @Override
    public void handleError(@NonNull ClientHttpResponse clientHttpResponse) throws IOException {
        final var status = (HttpStatus) clientHttpResponse.getStatusCode();
        final var exception = switch (status) {
            case BAD_REQUEST -> new RuntimeException("bad request");
            case UNAUTHORIZED -> new RuntimeException("unauthorized");
            case FORBIDDEN -> new RuntimeException("forbidden");
            case GONE -> new RuntimeException("gone");
            default -> new RuntimeException("other");
        };
        
        Optional.ofNullable(observationRegistry.getCurrentObservation())
                                .map(Observation::getContext)
                                .filter(ClientRequestObservationContext.class::isInstance)
                                .map(ClientRequestObservationContext.class::cast)
                                .ifPresent(context -> context.setError(exception));
                                
        throw exception;
    }
}

But observationRegistry#getCurrentObservation returns parent Observation (in my case http.server.requests).
We use spring-web 6.0.14.

I think there should be some way for customization.

[bclozel]

I'm not sure I understand the goal of this customization. You can throw RestClientException instances from the error handler and they will be recorded as errors in the observation. As for why you can't get the observation as current in the error handler, this is because we didn't open a scope for the processing of the request; this could be useful if developers want to log custom error messages in their error handler and expect trace ids to be present.

Before repurposing this issue in that direction, I would like to get your feedback about throwing RestClientException instead. Thanks!

[Vsevolod123]

Unfortunately, it's very complicated to redesign 200+ applications in that way.

In fact, I've already been able to achive that what I need with ObservationHandler like this

public class RestTemplateObservationHandler implements ObservationHandler<ClientRequestObservationContext> {
    private static final ThreadLocal<ClientRequestObservationContext> CONTEXT_THREAD_LOCAL = new ThreadLocal<>();

    public static Optional<ClientRequestObservationContext> getCurrentContext() {
        return Optional.ofNullable(CONTEXT_THREAD_LOCAL.get());
    }

    @Override
    public void onStart(@NonNull ClientRequestObservationContext context) {
        CONTEXT_THREAD_LOCAL.set(context);
    }

    @Override
    public void onStop(@NonNull ClientRequestObservationContext context) {
        CONTEXT_THREAD_LOCAL.remove();
    }

    @Override
    public boolean supportsContext(@NonNull Observation.Context context) {
        return context instanceof ClientRequestObservationContext;
    }
}

and RestTemplateCustomizer (from spring-boot) like this

@Bean
RestTemplateCustomizer addObservationErrorHandlerRestTemplateBuilderCustomizer() {
    return restTemplate -> {
        final var originalErrorHandler = restTemplate.getErrorHandler();
        final var observationErrorHandler = new ResponseErrorHandler() {
            @Override
            public boolean hasError(@NonNull ClientHttpResponse response) throws IOException {
                return originalErrorHandler.hasError(response);
            }

            @Override
            public void handleError(@NonNull ClientHttpResponse response) throws IOException {
                try {
                    originalErrorHandler.handleError(response);
                } catch (Exception e) {
                    RestTemplateObservationHandler.getCurrentContext()
                            .ifPresent(context -> context.setError(e));
                    throw e;
                }
            }
        };

        restTemplate.setErrorHandler(observationErrorHandler);
    };
 }

But it seems strange that WebClient handles all exception types while RestTemplate handles only RestClientException.

I don't even know if it's worth closing the task.

[bclozel]

So you mean that if all exceptions would be recorded as errors by the observations this would solve the issue for you? I thought the main problem was about getting the current observation.

[Vsevolod123]

Yes, you are right.
I think that problem with currentObservation is on the micrometer side.

[bclozel]

No, that's not what I said.
Currently the instrumentation in RestTemplate records only IOException and RestClientException exception as errors in the observation. This is because the actual HTTP exchange can only throw such exceptions. This would be a bit artificial, but we could instead record all Throwable as errors - considering that error handlers throwing another type of exception should also be recorded as an error. Would this solve the problem for you?

[Vsevolod123]

Custom ResponseErrorHandler could throw any Throwable.

Yes, this will solve problem for me.