Close active span if an exception is thrown in client request processing

tracing/jersey-client/pom.xml

@@ -62,5 +62,15 @@
             <artifactId>helidon-jersey-client</artifactId>
             <scope>provided</scope>
         </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-api</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.hamcrest</groupId>
+            <artifactId>hamcrest-all</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>

tracing/jersey-client/src/main/java/io/helidon/tracing/jersey/client/ClientTracingAutoDiscoverable.java

@@ -25,10 +25,14 @@
  */
 @ConstrainedTo(RuntimeType.CLIENT)
 public class ClientTracingAutoDiscoverable implements AutoDiscoverable {
+
+    static final int CLIENT_TRACING_PRIORITY = 10;
+
     @Override
     public void configure(FeatureContext context) {
         if (!context.getConfiguration().isRegistered(ClientTracingFilter.class)) {
-            context.register(ClientTracingFilter.class, 10);
+            context.register(ClientTracingFilter.class, CLIENT_TRACING_PRIORITY);
+            context.register(ClientTracingInterceptor.class, CLIENT_TRACING_PRIORITY);
         }
     }
 }

tracing/jersey-client/src/main/java/io/helidon/tracing/jersey/client/ClientTracingFilter.java

@@ -259,6 +259,7 @@ public void filter(ClientRequestContext requestContext, ClientResponseContext re
                                                  (status < HTTP_STATUS_SERVER_ERROR_THRESHOLD) ? "ClientError" : "ServerError"));
             }
             span.finish();
+            requestContext.removeProperty(SPAN_PROPERTY_NAME);
         }
     }
 

tracing/jersey-client/src/main/java/io/helidon/tracing/jersey/client/ClientTracingInterceptor.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright (c) 2021 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.helidon.tracing.jersey.client;
+
+import java.util.Map;
+
+import io.opentracing.Span;
+import io.opentracing.tag.Tags;
+import jakarta.ws.rs.client.ClientRequestContext;
+import jakarta.ws.rs.client.ClientResponseContext;
+import org.glassfish.jersey.client.spi.PostInvocationInterceptor;
+
+import static io.helidon.tracing.jersey.client.ClientTracingFilter.SPAN_PROPERTY_NAME;
+
+/**
+ * A post-invocation client interceptor. If an exception (e.g. a connection timeout)
+ * is thrown while executing a client request, this interceptor will ensure
+ * that an active tracing span is properly finished --given that client response
+ * filters will not be executed if an exception is thrown.
+ */
+public class ClientTracingInterceptor implements PostInvocationInterceptor {
+
+    @Override
+    public void afterRequest(ClientRequestContext requestContext, ClientResponseContext responseContext) {
+        // no-op
+    }
+
+    /**
+     * Upon encountering a client exception, and if there's an active span created by a
+     * tracing filter, finish the span.
+     *
+     * @param requestContext the request context
+     * @param exceptionContext the exception context
+     */
+    @Override
+    public void onException(ClientRequestContext requestContext, ExceptionContext exceptionContext) {
+        Object property = requestContext.getProperty(SPAN_PROPERTY_NAME);
+        if (property instanceof Span span) {
+            Tags.ERROR.set(span, true);
+            span.log(Map.of("event",
+                    "error",
+                    "message",
+                    "Exception executing client request: " + exceptionContext.getThrowables().pop(),
+                    "error.kind", "ClientError"));
+            span.finish();
+            requestContext.removeProperty(SPAN_PROPERTY_NAME);
+        }
+    }
+}

tracing/jersey-client/src/test/java/io/helidon/tracing/jersey/client/ClientTracingInterceptorTest.java

@@ -0,0 +1,76 @@
+/*
+ * Copyright (c) 2021 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.helidon.tracing.jersey.client;
+
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+import jakarta.ws.rs.client.Client;
+import jakarta.ws.rs.client.ClientBuilder;
+import jakarta.ws.rs.client.ClientRequestContext;
+import jakarta.ws.rs.client.ClientResponseContext;
+import org.glassfish.jersey.client.spi.PostInvocationInterceptor;
+import org.junit.jupiter.api.Test;
+
+import static io.helidon.tracing.jersey.client.ClientTracingAutoDiscoverable.CLIENT_TRACING_PRIORITY;
+import static io.helidon.tracing.jersey.client.ClientTracingFilter.SPAN_PROPERTY_NAME;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+class ClientTracingInterceptorTest {
+
+    private static AtomicBoolean isSpanFinished = new AtomicBoolean();
+
+    @Test
+    public void test() {
+        Client client = ClientBuilder.newBuilder()
+                .connectTimeout(1, TimeUnit.MILLISECONDS)
+                .readTimeout(1, TimeUnit.MILLISECONDS)
+                .register(AfterClientTracingInterceptor.class, CLIENT_TRACING_PRIORITY + 1)
+                .build();
+
+        CompletableFuture<?> future = client.target("https://example.org")
+                .request()
+                .rx()
+                .get()
+                .toCompletableFuture();
+
+        try {
+           future.join();
+        } catch (Throwable t) {
+            assertThat(isSpanFinished.get(), is(true));
+        }
+    }
+
+    /**
+     * This interceptor executes after {@link ClientTracingInterceptor} and ensures the span
+     * created in {@link ClientTracingFilter} is finished and no longer in the context.
+     */
+    static class AfterClientTracingInterceptor implements PostInvocationInterceptor {
+
+        @Override
+        public void afterRequest(ClientRequestContext requestContext, ClientResponseContext responseContext) {
+            // no-op
+        }
+
+        @Override
+        public void onException(ClientRequestContext requestContext, ExceptionContext exceptionContext) {
+            Object span = requestContext.getProperty(SPAN_PROPERTY_NAME);
+            isSpanFinished.set(span == null);
+        }
+    }
+}