add adjuster property to always apply timestamp and duration if missing

adjuster/finagle/src/main/java/zipkin/sparkstreaming/adjuster/finagle/FinagleAdjuster.java

@@ -28,11 +28,14 @@ public abstract class FinagleAdjuster extends Adjuster {
 
   public static Builder newBuilder() {
     return new AutoValue_FinagleAdjuster.Builder()
-        .applyTimestampAndDuration(true);
+        .applyTimestampAndDuration(true)
+        .spanModelTimestampAndDuration(false);
   }
 
   abstract boolean applyTimestampAndDuration();
 
+  abstract boolean spanModelTimestampAndDuration();
+
   @AutoValue.Builder
   public interface Builder {
     /**
@@ -44,21 +47,30 @@ public interface Builder {
      */
     Builder applyTimestampAndDuration(boolean applyTimestampAndDuration);
 
+    /**
+     * This back fills timestamp and duration for Spans coming from Finagle Span model, even if
+     * they are generated by non-finagle libraries.
+     */
+    Builder spanModelTimestampAndDuration(boolean spanModelTimestampAndDuration);
+
     FinagleAdjuster build();
   }
 
   @Override protected boolean shouldAdjust(Span span) {
-    for (BinaryAnnotation b : span.binaryAnnotations) {
-      if (b.key.indexOf("finagle.version") != -1) return true;
+    if (spanModelTimestampAndDuration() && (span.timestamp == null && span.duration == null)) {
+      return true;
+    }
+    if (applyTimestampAndDuration()) {
+      for (BinaryAnnotation b : span.binaryAnnotations) {
+        if (b.key.indexOf("finagle.version") != -1)
+          return true;
+      }
     }
     return false;
   }
 
   @Override protected Span adjust(Span span) {
-    if (applyTimestampAndDuration()) {
-      return ApplyTimestampAndDuration.apply(span);
-    }
-    return span;
+    return ApplyTimestampAndDuration.apply(span);
   }
 
   FinagleAdjuster() {

adjuster/finagle/src/test/java/zipkin/sparkstreaming/adjuster/finagle/FinagleAdjusterTest.java

@@ -34,35 +34,59 @@ public class FinagleAdjusterTest {
   // finagle often sets to the client endpoint to the same as the local endpoint
   Endpoint remoteEndpoint = localEndpoint.toBuilder().port(63840).build();
 
-  Span serverSpan = Span.builder()
+  Span serverSpanWithoutFinagleAnnotation = Span.builder()
       .traceId(-6054243957716233329L)
       .name("my-span")
       .id(-3615651937927048332L)
       .parentId(-6054243957716233329L)
       .addAnnotation(Annotation.create(1442493420635000L, Constants.SERVER_RECV, localEndpoint))
       .addAnnotation(Annotation.create(1442493422680000L, Constants.SERVER_SEND, localEndpoint))
-      .addBinaryAnnotation(BinaryAnnotation.create("srv/finagle.version", "6.28.0", localEndpoint0))
       .addBinaryAnnotation(BinaryAnnotation.address(Constants.SERVER_ADDR, localEndpoint))
       .addBinaryAnnotation(BinaryAnnotation.address(Constants.CLIENT_ADDR, remoteEndpoint))
       .build();
 
+  Span serverSpanWithFinagleAnnotation = serverSpanWithoutFinagleAnnotation.toBuilder()
+      .addBinaryAnnotation(BinaryAnnotation.create("srv/finagle.version", "6.28.0", localEndpoint0))
+      .build();
+
   /** Default is to apply timestamp and duration */
   @Test
   public void adjustsFinagleSpans() throws Exception {
-    Iterable<Span> adjusted = adjuster.adjust(asList(serverSpan));
-    assertThat(adjusted).containsExactly(ApplyTimestampAndDuration.apply(serverSpan));
+    Iterable<Span> adjusted = adjuster.adjust(asList(serverSpanWithFinagleAnnotation));
+    assertThat(adjusted)
+        .containsExactly(ApplyTimestampAndDuration.apply(serverSpanWithFinagleAnnotation));
   }
 
   @Test
   public void applyTimestampAndDuration_disabled() throws Exception {
     adjuster = FinagleAdjuster.newBuilder().applyTimestampAndDuration(false).build();
-    Iterable<Span> adjusted = adjuster.adjust(asList(serverSpan));
-    assertThat(adjusted).containsExactly(serverSpan);
+    Iterable<Span> adjusted = adjuster.adjust(asList(serverSpanWithFinagleAnnotation));
+    assertThat(adjusted).containsExactly(serverSpanWithFinagleAnnotation);
   }
 
   @Test
   public void doesntAdjustNonFinagleSpans() throws Exception {
     Iterable<Span> adjusted = adjuster.adjust(TestObjects.TRACE);
     assertThat(adjusted).containsExactlyElementsOf(TestObjects.TRACE);
   }
+
+  @Test
+  public void adjustsFinagleSpansWithoutFinagleAnnotation() throws Exception {
+    adjuster = FinagleAdjuster.newBuilder().applyTimestampAndDuration(false)
+        .spanModelTimestampAndDuration(true)
+        .build();
+    Iterable<Span> adjusted = adjuster.adjust(asList(serverSpanWithoutFinagleAnnotation));
+    assertThat(adjusted)
+        .containsExactly(ApplyTimestampAndDuration.apply(serverSpanWithoutFinagleAnnotation));
+  }
+
+  @Test
+  public void allPropertiesDisabled() throws Exception {
+    adjuster = FinagleAdjuster.newBuilder()
+        .applyTimestampAndDuration(false)
+        .spanModelTimestampAndDuration(false)
+        .build();
+    Iterable<Span> adjusted = adjuster.adjust(asList(serverSpanWithFinagleAnnotation));
+    assertThat(adjusted).containsExactly(serverSpanWithFinagleAnnotation);
+  }
 }

autoconfigure/adjuster-finagle/README.md

@@ -34,4 +34,5 @@ have a prefix of "zipkin.sparkstreaming.adjuster.finagle"
 Property | Default | Description | Fix
 --- | --- | --- | ---
 apply-timestamp-and-duration | true | Backfill span.timestamp and duration based on annotations. | [Use zipkin-finagle](https://github.com/openzipkin/zipkin-finagle/issues/10)
+span-model-timestamp-and-duration | false | Backfill span.timestamp and duration if missing. | [Use zipkin-finagle](https://github.com/openzipkin/zipkin-finagle/issues/10)
 adjust-issue343 | false | Drops "finagle.flush" annotation, to rectify [finagle memcached bug](https://github.com/twitter/finagle/issues/343). | Use finagle version 6.36.0 or higher

autoconfigure/adjuster-finagle/src/main/java/zipkin/sparkstreaming/autoconfigure/adjuster/finagle/ZipkinFinagleAdjusterProperties.java

@@ -19,6 +19,7 @@
 @ConfigurationProperties("zipkin.sparkstreaming.adjuster.finagle")
 public class ZipkinFinagleAdjusterProperties {
   private boolean applyTimestampAndDuration = true;
+  private boolean spanModelTimestampAndDuration = false;
 
   public boolean isApplyTimestampAndDuration() {
     return applyTimestampAndDuration;
@@ -28,8 +29,17 @@ public void setApplyTimestampAndDuration(boolean applyTimestampAndDuration) {
     this.applyTimestampAndDuration = applyTimestampAndDuration;
   }
 
+  public boolean isSpanModelTimestampAndDuration() {
+    return spanModelTimestampAndDuration;
+  }
+
+  public void setSpanModelTimestampAndDuration(boolean spanModelTimestampAndDuration) {
+    this.spanModelTimestampAndDuration = spanModelTimestampAndDuration;
+  }
+
   FinagleAdjuster.Builder toBuilder() {
     return FinagleAdjuster.newBuilder()
-        .applyTimestampAndDuration(applyTimestampAndDuration);
+        .applyTimestampAndDuration(applyTimestampAndDuration)
+        .spanModelTimestampAndDuration(spanModelTimestampAndDuration);
   }
 }

autoconfigure/adjuster-finagle/src/test/java/zipkin/sparkstreaming/adjuster/finagle/ZipkinFinagleAdjusterAutoConfigurationTest.java

@@ -58,6 +58,7 @@ public void providesAdjusterWhenEnabled() {
 
     FinagleAdjuster adjuster = context.getBean(FinagleAdjuster.class);
     assertThat(adjuster.applyTimestampAndDuration()).isTrue();
+    assertThat(adjuster.spanModelTimestampAndDuration()).isFalse();
   }
 
   @Test
@@ -73,6 +74,19 @@ public void disableTimestampAndDuration() {
     assertThat(adjuster.applyTimestampAndDuration()).isFalse();
   }
 
+  @Test
+  public void enableSpanModelTimestampAndDuration() {
+    addEnvironment(context,
+        "zipkin.sparkstreaming.adjuster.finagle.enabled:" + true,
+        "zipkin.sparkstreaming.adjuster.finagle.span-model-timestamp-and-duration:" + true);
+    context.register(PropertyPlaceholderAutoConfiguration.class,
+        ZipkinFinagleAdjusterAutoConfiguration.class);
+    context.refresh();
+
+    FinagleAdjuster adjuster = context.getBean(FinagleAdjuster.class);
+    assertThat(adjuster.spanModelTimestampAndDuration()).isTrue();
+  }
+
   @Test
   public void doesntProvideIssue343AdjusterWhenFinagleDisabled() {
     context.register(PropertyPlaceholderAutoConfiguration.class,