Merge pull request #7987 from Ladicek/opentracing-tests-and-fixes

fix OpenTracing trace context propagation with Fault Tolerance @asynchronous methods
quarkusio · Apr 14, 2020 · 8ca1d0d · 8ca1d0d
2 parents 0769017 + 1b676f3
commit 8ca1d0d
Show file tree

Hide file tree

Showing 11 changed files with 133 additions and 35 deletions.
diff --git a/bom/runtime/pom.xml b/bom/runtime/pom.xml
@@ -37,7 +37,7 @@
         <smallrye-metrics.version>2.4.1</smallrye-metrics.version>
         <smallrye-open-api.version>1.2.1</smallrye-open-api.version>
         <smallrye-opentracing.version>1.3.4</smallrye-opentracing.version>
-        <smallrye-fault-tolerance.version>4.1.1</smallrye-fault-tolerance.version>
+        <smallrye-fault-tolerance.version>4.2.0</smallrye-fault-tolerance.version>
         <smallrye-jwt.version>2.1.1</smallrye-jwt.version>
         <smallrye-context-propagation.version>1.0.11</smallrye-context-propagation.version>
         <smallrye-reactive-streams-operators.version>1.0.10</smallrye-reactive-streams-operators.version>
@@ -1453,6 +1453,11 @@
                 <artifactId>smallrye-fault-tolerance-context-propagation</artifactId>
                 <version>${smallrye-fault-tolerance.version}</version>
             </dependency>
+            <dependency>
+                <groupId>io.smallrye</groupId>
+                <artifactId>smallrye-fault-tolerance-tracing-propagation</artifactId>
+                <version>${smallrye-fault-tolerance.version}</version>
+            </dependency>
             <dependency>
                 <groupId>io.smallrye</groupId>
                 <artifactId>smallrye-context-propagation</artifactId>

diff --git a/extensions/jaeger/deployment/src/main/java/io/quarkus/jaeger/deployment/JaegerProcessor.java b/extensions/jaeger/deployment/src/main/java/io/quarkus/jaeger/deployment/JaegerProcessor.java
@@ -2,6 +2,7 @@
 
 import javax.inject.Inject;
 
+import io.quarkus.deployment.Capabilities;
 import io.quarkus.deployment.annotations.BuildProducer;
 import io.quarkus.deployment.annotations.BuildStep;
 import io.quarkus.deployment.annotations.ExecutionTime;
@@ -21,13 +22,15 @@ public class JaegerProcessor {
     @BuildStep
     @Record(ExecutionTime.RUNTIME_INIT)
     void setupTracer(JaegerDeploymentRecorder jdr, JaegerBuildTimeConfig buildTimeConfig, JaegerConfig jaeger,
-            ApplicationConfig appConfig) {
+            ApplicationConfig appConfig, Capabilities capabilities) {
 
         // Indicates that this extension would like the SSL support to be enabled
         extensionSslNativeSupport.produce(new ExtensionSslNativeSupportBuildItem(FeatureBuildItem.JAEGER));
 
         if (buildTimeConfig.enabled) {
-            jdr.registerTracer(jaeger, appConfig);
+            boolean metricsEnabled = capabilities.isCapabilityPresent(Capabilities.METRICS);
+
+            jdr.registerTracer(jaeger, appConfig, metricsEnabled);
         }
     }
 

diff --git a/extensions/jaeger/runtime/pom.xml b/extensions/jaeger/runtime/pom.xml
@@ -18,7 +18,7 @@
             <groupId>io.quarkus</groupId>
             <artifactId>quarkus-core</artifactId>
         </dependency>
-       <dependency>
+        <dependency>
             <groupId>io.jaegertracing</groupId>
             <artifactId>jaeger-core</artifactId>
         </dependency>
@@ -31,12 +31,9 @@
             <artifactId>svm</artifactId>
         </dependency>
         <dependency>
-            <groupId>io.smallrye</groupId>
-            <artifactId>smallrye-metrics</artifactId>
-        </dependency>
-        <dependency>
-            <groupId>org.eclipse.microprofile.metrics</groupId>
-            <artifactId>microprofile-metrics-api</artifactId>
+            <groupId>io.quarkus</groupId>
+            <artifactId>quarkus-smallrye-metrics</artifactId>
+            <optional>true</optional>
         </dependency>
     </dependencies>
 

diff --git a/...ions/jaeger/runtime/src/main/java/io/quarkus/jaeger/runtime/JaegerDeploymentRecorder.java b/...ions/jaeger/runtime/src/main/java/io/quarkus/jaeger/runtime/JaegerDeploymentRecorder.java
@@ -15,7 +15,7 @@ public class JaegerDeploymentRecorder {
     private static final Optional UNKNOWN_SERVICE_NAME = Optional.of("quarkus/unknown");
     private static final QuarkusJaegerTracer quarkusTracer = new QuarkusJaegerTracer();
 
-    synchronized public void registerTracer(JaegerConfig jaeger, ApplicationConfig appConfig) {
+    synchronized public void registerTracer(JaegerConfig jaeger, ApplicationConfig appConfig, boolean metricsEnabled) {
         if (!jaeger.serviceName.isPresent()) {
             if (appConfig.name.isPresent()) {
                 jaeger.serviceName = appConfig.name;
@@ -24,6 +24,7 @@ synchronized public void registerTracer(JaegerConfig jaeger, ApplicationConfig a
             }
         }
         initTracerConfig(jaeger);
+        quarkusTracer.setMetricsEnabled(metricsEnabled);
         quarkusTracer.reset();
         // register Quarkus tracer to GlobalTracer.
         // Usually the tracer will be registered only here, although consumers

diff --git a/extensions/jaeger/runtime/src/main/java/io/quarkus/jaeger/runtime/QuarkusJaegerTracer.java b/extensions/jaeger/runtime/src/main/java/io/quarkus/jaeger/runtime/QuarkusJaegerTracer.java
@@ -2,6 +2,8 @@
 
 import io.jaegertracing.Configuration;
 import io.jaegertracing.internal.JaegerTracer;
+import io.jaegertracing.internal.metrics.NoopMetricsFactory;
+import io.jaegertracing.spi.MetricsFactory;
 import io.opentracing.ScopeManager;
 import io.opentracing.Span;
 import io.opentracing.SpanContext;
@@ -14,11 +16,16 @@ public class QuarkusJaegerTracer implements Tracer {
     private volatile JaegerTracer tracer;
 
     private boolean logTraceContext;
+    private boolean metricsEnabled;
 
     void setLogTraceContext(boolean logTraceContext) {
         this.logTraceContext = logTraceContext;
     }
 
+    void setMetricsEnabled(boolean metricsEnabled) {
+        this.metricsEnabled = metricsEnabled;
+    }
+
     @Override
     public String toString() {
         return tracer().toString();
@@ -35,8 +42,11 @@ private Tracer tracer() {
         if (tracer == null) {
             synchronized (this) {
                 if (tracer == null) {
+                    MetricsFactory metricsFactory = metricsEnabled
+                            ? new QuarkusJaegerMetricsFactory()
+                            : new NoopMetricsFactory();
                     tracer = Configuration.fromEnv()
-                            .withMetricsFactory(new QuarkusJaegerMetricsFactory())
+                            .withMetricsFactory(metricsFactory)
                             .getTracerBuilder()
                             .withScopeManager(getScopeManager())
                             .build();

diff --git a/extensions/smallrye-opentracing/deployment/pom.xml b/extensions/smallrye-opentracing/deployment/pom.xml
@@ -39,6 +39,11 @@
             <artifactId>quarkus-jsonp-deployment</artifactId>
         </dependency>
 
+        <dependency>
+            <groupId>io.quarkus</groupId>
+            <artifactId>quarkus-resteasy-jackson</artifactId>
+            <scope>test</scope>
+        </dependency>
         <dependency>
             <groupId>io.quarkus</groupId>
             <artifactId>quarkus-rest-client-deployment</artifactId>

diff --git a/...cing/deployment/src/test/java/io/quarkus/smallrye/opentracing/deployment/RestService.java b/...cing/deployment/src/test/java/io/quarkus/smallrye/opentracing/deployment/RestService.java
@@ -1,11 +1,14 @@
 package io.quarkus.smallrye.opentracing.deployment;
 
+import java.util.List;
+import java.util.concurrent.CompletionStage;
+
 import javax.ws.rs.GET;
 import javax.ws.rs.Path;
+import javax.ws.rs.Produces;
+import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 
-import org.eclipse.microprofile.opentracing.Traced;
-
 @Path("/")
 public interface RestService {
 
@@ -22,7 +25,11 @@ public interface RestService {
     Response restClient();
 
     @GET
-    @Traced(false)
     @Path("/faultTolerance")
-    Response faultTolerance();
+    CompletionStage<String> faultTolerance();
+
+    @GET
+    @Path("/jpa")
+    @Produces(MediaType.APPLICATION_JSON)
+    List<Fruit> jpa();
 }
diff --git a/...ntracing/deployment/src/test/java/io/quarkus/smallrye/opentracing/deployment/Service.java b/...ntracing/deployment/src/test/java/io/quarkus/smallrye/opentracing/deployment/Service.java
@@ -1,36 +1,53 @@
 package io.quarkus.smallrye.opentracing.deployment;
 
 import java.time.temporal.ChronoUnit;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionStage;
 
 import javax.enterprise.context.ApplicationScoped;
 import javax.inject.Inject;
+import javax.persistence.EntityManager;
 
+import org.eclipse.microprofile.faulttolerance.Asynchronous;
 import org.eclipse.microprofile.faulttolerance.Fallback;
 import org.eclipse.microprofile.faulttolerance.Retry;
 import org.eclipse.microprofile.faulttolerance.Timeout;
 import org.eclipse.microprofile.opentracing.Traced;
 
 import io.opentracing.Tracer;
 
-@Traced
 @ApplicationScoped
 public class Service {
 
     @Inject
     Tracer tracer;
 
+    @Inject
+    EntityManager em;
+
+    @Traced
     public void foo() {
     }
 
+    // @Asynchronous methods (and their fallback methods) shouldn't be @Traced
+    // because https://github.com/eclipse/microprofile-opentracing/issues/189
+    @Asynchronous
     @Fallback(fallbackMethod = "fallback")
     @Timeout(value = 20L, unit = ChronoUnit.MILLIS)
     @Retry(delay = 10L, maxRetries = 2)
-    public String faultTolerance() {
+    public CompletionStage<String> faultTolerance() {
         tracer.buildSpan("ft").start().finish();
         throw new RuntimeException();
     }
 
-    public String fallback() {
-        return "fallback";
+    public CompletionStage<String> fallback() {
+        tracer.buildSpan("fallback").start().finish();
+        return CompletableFuture.completedFuture("fallback");
+    }
+
+    @Traced
+    public List<Fruit> getFruits() {
+        return em.createNamedQuery("Fruits.findAll", Fruit.class).getResultList();
     }
 }
diff --git a/...ing/deployment/src/test/java/io/quarkus/smallrye/opentracing/deployment/TestResource.java b/...ing/deployment/src/test/java/io/quarkus/smallrye/opentracing/deployment/TestResource.java
@@ -1,5 +1,8 @@
 package io.quarkus.smallrye.opentracing.deployment;
 
+import java.util.List;
+import java.util.concurrent.CompletionStage;
+
 import javax.inject.Inject;
 import javax.ws.rs.Path;
 import javax.ws.rs.core.Context;
@@ -38,8 +41,11 @@ public Response restClient() {
     }
 
     @Override
-    public Response faultTolerance() {
-        String ret = service.faultTolerance();
-        return Response.ok(ret).build();
+    public CompletionStage<String> faultTolerance() {
+        return service.faultTolerance();
+    }
+
+    public List<Fruit> jpa() {
+        return service.getFruits();
     }
 }
diff --git a/...cing/deployment/src/test/java/io/quarkus/smallrye/opentracing/deployment/TracingTest.java b/...cing/deployment/src/test/java/io/quarkus/smallrye/opentracing/deployment/TracingTest.java
@@ -1,5 +1,8 @@
 package io.quarkus.smallrye.opentracing.deployment;
 
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasSize;
+
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
@@ -20,7 +23,6 @@
 import io.quarkus.test.QuarkusUnitTest;
 import io.restassured.RestAssured;
 import io.restassured.parsing.Parser;
-import io.restassured.response.Response;
 
 public class TracingTest {
 
@@ -41,7 +43,7 @@ public class TracingTest {
     }
 
     @BeforeEach
-    public void after() {
+    public void before() {
         mockTracer.reset();
     }
 
@@ -104,9 +106,10 @@ public void testMPRestClient() {
     public void testContextPropagationInFaultTolerance() {
         try {
             RestAssured.defaultParser = Parser.TEXT;
-            Response response = RestAssured.when().get("/faultTolerance");
-            response.then().statusCode(200);
-            Assertions.assertEquals("fallback", response.body().asString());
+            RestAssured.when().get("/faultTolerance")
+                    .then()
+                    .statusCode(200)
+                    .body(equalTo("fallback"));
             Awaitility.await().atMost(5, TimeUnit.SECONDS)
                     .until(() -> mockTracer.finishedSpans().size() == 5);
             List<MockSpan> spans = mockTracer.finishedSpans();
@@ -115,13 +118,47 @@ public void testContextPropagationInFaultTolerance() {
             for (MockSpan mockSpan : spans) {
                 Assertions.assertEquals(spans.get(0).context().traceId(), mockSpan.context().traceId());
             }
-            Assertions.assertEquals("ft", mockTracer.finishedSpans().get(0).operationName());
-            Assertions.assertEquals("ft", mockTracer.finishedSpans().get(1).operationName());
-            Assertions.assertEquals("ft", mockTracer.finishedSpans().get(2).operationName());
-            Assertions.assertEquals("io.quarkus.smallrye.opentracing.deployment.Service.fallback",
-                    mockTracer.finishedSpans().get(3).operationName());
-            Assertions.assertEquals("io.quarkus.smallrye.opentracing.deployment.Service.faultTolerance",
-                    mockTracer.finishedSpans().get(4).operationName());
+
+            // if timeout occurs, subsequent retries/fallback can be interleaved with the execution that timed out,
+            // resulting in varying span order
+            Assertions.assertEquals(3, countSpansWithOperationName(spans, "ft"));
+            Assertions.assertEquals(1, countSpansWithOperationName(spans, "fallback"));
+            Assertions.assertEquals(1, countSpansWithOperationName(spans,
+                    "GET:io.quarkus.smallrye.opentracing.deployment.TestResource.faultTolerance"));
+        } finally {
+            RestAssured.reset();
+        }
+    }
+
+    private long countSpansWithOperationName(List<MockSpan> spans, String operationName) {
+        return spans.stream().filter(span -> span.operationName().equals(operationName)).count();
+    }
+
+    @Test
+    public void testJPA() {
+        try {
+            RestAssured.defaultParser = Parser.JSON;
+            RestAssured.when().get("/jpa")
+                    .then()
+                    .statusCode(200)
+                    .body("", hasSize(3))
+                    .body("name[0]", equalTo("Apple"))
+                    .body("name[1]", equalTo("Banana"))
+                    .body("name[2]", equalTo("Cherry"));
+            List<MockSpan> spans = mockTracer.finishedSpans();
+
+            Assertions.assertEquals(3, spans.size());
+            for (MockSpan mockSpan : spans) {
+                Assertions.assertEquals(spans.get(0).context().traceId(), mockSpan.context().traceId());
+            }
+            MockSpan firstSpan = mockTracer.finishedSpans().get(0);
+            Assertions.assertEquals("Query", firstSpan.operationName());
+            Assertions.assertTrue(firstSpan.tags().containsKey("db.statement"));
+            Assertions.assertTrue(firstSpan.tags().get("db.statement").toString().contains("known_fruits"));
+            Assertions.assertEquals("io.quarkus.smallrye.opentracing.deployment.Service.getFruits",
+                    mockTracer.finishedSpans().get(1).operationName());
+            Assertions.assertEquals("GET:io.quarkus.smallrye.opentracing.deployment.TestResource.jpa",
+                    mockTracer.finishedSpans().get(2).operationName());
         } finally {
             RestAssured.reset();
         }

diff --git a/extensions/smallrye-opentracing/runtime/pom.xml b/extensions/smallrye-opentracing/runtime/pom.xml
@@ -32,6 +32,16 @@
                 </exclusion>
             </exclusions>
         </dependency>
+        <dependency>
+            <groupId>io.smallrye</groupId>
+            <artifactId>smallrye-fault-tolerance-tracing-propagation</artifactId>
+            <exclusions>
+                <exclusion>
+                    <groupId>*</groupId>
+                    <artifactId>*</artifactId>
+                </exclusion>
+            </exclusions>
+        </dependency>
         <dependency>
             <groupId>jakarta.inject</groupId>
             <artifactId>jakarta.inject-api</artifactId>