fix: Pass HttpResponse from HttpClientResponseException to OpenTelemetry client instrumenter (#710)

* Added test to demonstrate missing http error status_code in client spans

* Pass the HttpResponse to OTel instrumentation when client call throws HttpClientResponseException

Author: lgathy

tracing-opentelemetry-http/src/main/java/io/micronaut/tracing/opentelemetry/instrument/http/client/OpenTelemetryClientFilter.java

@@ -20,6 +20,7 @@
 import io.micronaut.core.annotation.Nullable;
 import io.micronaut.core.propagation.PropagatedContext;
 import io.micronaut.http.HttpResponse;
+import io.micronaut.http.HttpResponseProvider;
 import io.micronaut.http.MutableHttpRequest;
 import io.micronaut.http.annotation.Filter;
 import io.micronaut.http.filter.ClientFilterChain;
@@ -38,6 +39,8 @@
 import org.reactivestreams.Publisher;
 import reactor.core.publisher.Mono;
 
+import java.util.Optional;
+
 import static io.micronaut.http.HttpAttributes.INVOCATION_CONTEXT;
 import static io.micronaut.tracing.opentelemetry.instrument.http.client.OpenTelemetryClientFilter.CLIENT_PATH;
 
@@ -93,13 +96,21 @@ public Publisher<? extends HttpResponse<?>> doFilter(MutableHttpRequest<?> reque
                         Span span = Span.fromContext(context);
                         span.recordException(throwable);
                         span.setStatus(StatusCode.ERROR);
-                        instrumenter.end(context, request, null, throwable);
+                        HttpResponse<?> response = findResponseInThrowable(throwable).orElse(null);
+                        instrumenter.end(context, request, response, throwable);
                     });
 
             }
         }
     }
 
+    private Optional<HttpResponse<?>> findResponseInThrowable(@Nullable Throwable throwable) {
+        if (throwable instanceof HttpResponseProvider httpResponseProvider) {
+            return Optional.ofNullable(httpResponseProvider.getResponse());
+        }
+        return Optional.empty();
+    }
+
     private void handleContinueSpan(MutableHttpRequest<?> request) {
         Object invocationContext = request.getAttribute(INVOCATION_CONTEXT).orElse(null);
         if (invocationContext instanceof MethodInvocationContext<?, ?> context) {

tracing-opentelemetry-http/src/test/groovy/io/micronaut/tracing/opentelemetry/instrument/http/OpenTelemetryHttpSpec.groovy

@@ -108,6 +108,9 @@ class OpenTelemetryHttpSpec extends Specification {
         assert serverSpans.every {it.attributes.stream().any { it.get(HttpAttributes.HTTP_REQUEST_METHOD) }}
         assert !hasRoute || serverSpans.every {it.attributes.stream().any { it.get(HttpAttributes.HTTP_ROUTE) }}
         assert serverSpans.every {it.attributes.stream().any {x -> Optional.ofNullable(x.get(HttpAttributes.HTTP_RESPONSE_STATUS_CODE)).map { it.intValue() == httpStatus.code }.orElse(false) }}
+        def clientSpans = exporter.finishedSpanItems.findAll { it -> it.kind == SpanKind.CLIENT }
+        assert clientSpans.every {it.attributes.stream().any { it.get(HttpAttributes.HTTP_REQUEST_METHOD) }}
+        assert clientSpans.every {it.attributes.stream().any {x -> Optional.ofNullable(x.get(HttpAttributes.HTTP_RESPONSE_STATUS_CODE)).map { it.intValue() == httpStatus.code }.orElse(false) }}
     }
 
     void 'test map WithSpan annotation'() {
@@ -232,6 +235,34 @@ class OpenTelemetryHttpSpec extends Specification {
         '/error/completionStageErrorContinueSpan' | 1         | 'inside normal method continueSpan'
     }
 
+    void 'test client error #desc, path=/error/#variant'() {
+        def errorClient = context.getBean(ErrorClient)
+
+        when:
+        String responseBody = errorClient.get(variant)
+
+        then:
+        def e = thrown(HttpClientResponseException)
+        e.message == "Internal Server Error"
+        conditions.eventually {
+            hasSpans(hasInternalSpan ? 1 : 0, 1, 1)
+            exporter.finishedSpanItems.events.any { it.size() > 0 && it.get(0).name == "exception" }
+            exporter.finishedSpanItems.stream().allMatch(span -> span.status.statusCode == StatusCode.ERROR)
+            hasHttpSemanticAttributes(e.status)
+        }
+        cleanup:
+        exporter.reset()
+        where:
+        variant                            | hasInternalSpan | desc
+        'publisher'                        | true            | 'inside publisher'
+        'publisherErrorContinueSpan'       | false           | 'inside continueSpan publisher'
+        'mono'                             | true            | 'propagated through publisher'
+        'sync'                             | true            | 'inside normal function'
+        'completionStage'                  | true            | 'inside completionStage'
+        'completionStagePropagation'       | true            | 'propagated through  completionStage'
+        'completionStageErrorContinueSpan' | false           | 'inside normal method continueSpan'
+    }
+
     void 'client with tracing annotations'() {
         def warehouseClient = embeddedServer.applicationContext.getBean(WarehouseClient)
         def serverSpanCount = 2
@@ -641,6 +672,13 @@ class OpenTelemetryHttpSpec extends Specification {
         }
     }
 
+    @Client("/error")
+    static interface ErrorClient {
+
+        @Get("/{variant}")
+        String get(@PathVariable String variant)
+    }
+
     @Controller('/exclude')
     static class ExcludeController {