Merge pull request #423 from DataDog/mar-kolya/fix-span-cleaner-bug

Fix span cleaner executor to use correct span cleaner reference
DataDog · Aug 3, 2018 · 5289204 · 5289204
2 parents 911ad5f + e517080
commit 5289204
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 24 deletions.
diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/PendingTrace.java b/dd-trace-ot/src/main/java/datadog/opentracing/PendingTrace.java
@@ -22,12 +22,7 @@
 
 @Slf4j
 public class PendingTrace extends ConcurrentLinkedDeque<DDSpan> {
-  private static final SpanCleaner SPAN_CLEANER;
-
-  static {
-    SPAN_CLEANER = new SpanCleaner();
-    SPAN_CLEANER.start();
-  }
+  private static final SpanCleaner SPAN_CLEANER = new SpanCleaner();
 
   private final DDTracer tracer;
   private final String traceId;
@@ -56,8 +51,8 @@ public class PendingTrace extends ConcurrentLinkedDeque<DDSpan> {
     this.traceId = traceId;
     this.serviceNameMappings = serviceNameMappings;
 
-    this.startTimeNano = Clock.currentNanoTime();
-    this.startNanoTicks = Clock.currentNanoTicks();
+    startTimeNano = Clock.currentNanoTime();
+    startNanoTicks = Clock.currentNanoTicks();
 
     SPAN_CLEANER.pendingTraces.add(this);
   }
@@ -198,7 +193,7 @@ private void write() {
     if (isWritten.compareAndSet(false, true)) {
       SPAN_CLEANER.pendingTraces.remove(this);
       if (!isEmpty()) {
-        log.debug("Writing {} spans to {}.", this.size(), tracer.writer);
+        log.debug("Writing {} spans to {}.", size(), tracer.writer);
         tracer.write(this);
       }
     }
@@ -254,20 +249,8 @@ public Thread newThread(final Runnable r) {
     private final Set<PendingTrace> pendingTraces =
         Collections.newSetFromMap(new ConcurrentHashMap<PendingTrace, Boolean>());
 
-    void start() {
-      executorService.scheduleAtFixedRate(new SpanCleaner(), 0, CLEAN_FREQUENCY, TimeUnit.SECONDS);
-      try {
-        Runtime.getRuntime()
-            .addShutdownHook(
-                new Thread() {
-                  @Override
-                  public void run() {
-                    PendingTrace.SpanCleaner.this.close();
-                  }
-                });
-      } catch (final IllegalStateException ex) {
-        // The JVM is already shutting down.
-      }
+    public SpanCleaner() {
+      executorService.scheduleAtFixedRate(this, 0, CLEAN_FREQUENCY, TimeUnit.SECONDS);
     }
 
     @Override
@@ -285,6 +268,9 @@ public void close() {
       } catch (final InterruptedException e) {
         log.info("Writer properly closed and async writer interrupted.");
       }
+
+      // Make sure that whatever was left over gets cleaned up
+      run();
     }
   }
 }
diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/PendingTraceTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/PendingTraceTest.groovy
@@ -6,7 +6,6 @@ import spock.lang.Subject
 
 import java.util.concurrent.TimeUnit
 
-
 class PendingTraceTest extends Specification {
   def writer = new ListWriter()
   def tracer = new DDTracer(writer)