Introduce Clock interface for time measurements (#50)

* Introduce Clock interface for time measurements * Use nano-time for measuring duration * Fix the tests, remove powermock dependency * Add more tests, upgrade to OT 0.15
jaegertracing · Sep 12, 2016 · b93744a · b93744a
1 parent d095ae8
commit b93744a
Show file tree

Hide file tree

Showing 16 changed files with 350 additions and 136 deletions.
diff --git a/build.gradle b/build.gradle
@@ -7,7 +7,7 @@ plugins {
 }
 
 
-ext.opentracingVersion = '0.13.0'
+ext.opentracingVersion = '0.15.0'
 ext.guavaVersion = '18.0'
 ext.apacheThriftVersion = '0.9.3'
 ext.jerseyVersion = '2.22.2'
@@ -16,7 +16,6 @@ ext.jacksonVersion = '2.7.4'
 
 ext.junitVersion = '4.12'
 ext.mockitoVersion = '2.0.2-beta'
-ext.powermockVersion = '1.6.4'
 
 allprojects {
     apply plugin: 'idea' // intellij support

diff --git a/...com/uber/jaeger/context/CallableTest.java → ...com/uber/jaeger/context/CallableTest.java b/...com/uber/jaeger/context/CallableTest.java → ...com/uber/jaeger/context/CallableTest.java
@@ -19,11 +19,8 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
-package com.uber.jaeger.filters.jaxrs2;
+package com.uber.jaeger.context;
 
-import com.uber.jaeger.Tracer;
-import com.uber.jaeger.reporters.InMemoryReporter;
-import com.uber.jaeger.samplers.ConstSampler;
 import io.opentracing.Span;
 import org.junit.Before;
 import org.junit.Test;
@@ -35,14 +32,12 @@
 import static org.mockito.Mockito.when;
 
 public class CallableTest {
-    Tracer tracer;
     TraceContext traceContext;
     Span span;
 
     @Before
     public void setUp() {
-        tracer = new Tracer.Builder("raza", new InMemoryReporter(), new ConstSampler(true)).build();
-        span = tracer.buildSpan("firefly").start();
+        span = mock(Span.class);
         traceContext = mock(TraceContext.class);
         when(traceContext.getCurrentSpan()).thenReturn(span);
         when(traceContext.pop()).thenReturn(span);

diff --git a/...com/uber/jaeger/context/RunnableTest.java → ...com/uber/jaeger/context/RunnableTest.java b/...com/uber/jaeger/context/RunnableTest.java → ...com/uber/jaeger/context/RunnableTest.java
@@ -19,11 +19,8 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
-package com.uber.jaeger.filters.jaxrs2;
+package com.uber.jaeger.context;
 
-import com.uber.jaeger.Tracer;
-import com.uber.jaeger.reporters.InMemoryReporter;
-import com.uber.jaeger.samplers.ConstSampler;
 import io.opentracing.Span;
 import org.junit.Before;
 import org.junit.Test;
@@ -35,14 +32,12 @@
 import static org.mockito.Mockito.when;
 
 public class RunnableTest {
-    Tracer tracer;
     TraceContext traceContext;
     Span span;
 
     @Before
     public void setUp() {
-        tracer = new Tracer.Builder("death-star", new InMemoryReporter(), new ConstSampler(true)).build();
-        span = tracer.buildSpan("destroyer").start();
+        span = mock(Span.class);
         traceContext = mock(TraceContext.class);
         when(traceContext.getCurrentSpan()).thenReturn(span);
         when(traceContext.pop()).thenReturn(span);

diff --git a/...er/context/TracedExecutorServiceTest.java → ...er/context/TracedExecutorServiceTest.java b/...er/context/TracedExecutorServiceTest.java → ...er/context/TracedExecutorServiceTest.java
@@ -19,20 +19,17 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
  * THE SOFTWARE.
  */
-package com.uber.jaeger.filters.jaxrs2;
+package com.uber.jaeger.context;
+
+import io.opentracing.Span;
+import org.junit.Before;
+import org.junit.Test;
 
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.TimeUnit;
 
-import com.uber.jaeger.Tracer;
-import com.uber.jaeger.reporters.InMemoryReporter;
-import com.uber.jaeger.samplers.ConstSampler;
-import io.opentracing.Span;
-import org.junit.Before;
-import org.junit.Test;
-
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
@@ -41,22 +38,21 @@
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
-import static org.powermock.api.mockito.PowerMockito.when;
+import static org.mockito.Mockito.when;
 
 public class TracedExecutorServiceTest {
 
     TimeUnit TIME_UNIT = TimeUnit.MILLISECONDS;
     TracedExecutorService tracedExecutorService;
     ExecutorService wrappedExecutorService;
-    Tracer tracer = new Tracer.Builder("test-executor-service", new InMemoryReporter(), new ConstSampler(true)).build();
     Span span;
     TraceContext traceContext;
     List<java.util.concurrent.Callable<Span>> callableList;
 
     @Before
     public void setUp() {
         wrappedExecutorService = mock(ExecutorService.class);
-        span = tracer.buildSpan("span-op").start();
+        span = mock(Span.class);
         traceContext = mock(TraceContext.class);
         when(traceContext.pop()).thenReturn(span);
         when(traceContext.getCurrentSpan()).thenReturn(span);

diff --git a/jaeger-core/build.gradle b/jaeger-core/build.gradle
@@ -20,10 +20,6 @@ dependencies {
 
     testCompile group: 'junit', name: 'junit', version: junitVersion
     testCompile group: 'org.mockito', name: 'mockito-all', version: mockitoVersion
-    // TODO use only one mocking framework
-    testCompile group: 'org.powermock', name: 'powermock-api-mockito', version: powermockVersion
-    testCompile group: 'org.powermock', name: 'powermock-core', version: powermockVersion
-    testCompile group: 'org.powermock', name: 'powermock-module-junit4', version: powermockVersion
 }
 
 compileThrift {

diff --git a/jaeger-core/src/main/java/com/uber/jaeger/Span.java b/jaeger-core/src/main/java/com/uber/jaeger/Span.java
@@ -22,32 +22,43 @@
 package com.uber.jaeger;
 
 import com.twitter.zipkin.thriftjava.Endpoint;
-import com.uber.jaeger.utils.Utils;
 import io.opentracing.tag.Tags;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
 public class Span implements io.opentracing.Span {
     private final Tracer tracer;
-    private final long start; // time in microseconds
-    private final String operationName;
+    private final long startTimeMicroseconds;
+    private final long startTimeNanoTicks;
+    private final boolean computeDurationViaNanoTicks;
+    private long durationMicroseconds; // span durationMicroseconds
+    private String operationName;
     private SpanContext context;
     private Endpoint peer;
-    private long duration; // time in microseconds
-    private Map<String,Object> tags;
+    private Map<String, Object> tags;
     private List<LogData> logs;
     private String localComponent;
     private boolean isClient;
     private boolean isRPC;
 
-    Span(Tracer tracer, String operationName, SpanContext context, long start, Map<String, Object> tags) {
+    Span(Tracer tracer,
+         String operationName,
+         SpanContext context,
+         long startTimeMicroseconds,
+         long startTimeNanoTicks,
+         boolean computeDurationViaNanoTicks,
+         Map<String, Object> tags
+    ) {
         this.tracer = tracer;
         this.operationName = operationName;
         this.context = context;
-        this.start = start;
+        this.startTimeMicroseconds = startTimeMicroseconds;
+        this.startTimeNanoTicks = startTimeNanoTicks;
+        this.computeDurationViaNanoTicks = computeDurationViaNanoTicks;
         this.tags = tags;
     }
 
@@ -56,12 +67,12 @@ public String getLocalComponent() {
     }
 
     public long getStart() {
-        return start;
+        return startTimeMicroseconds;
     }
 
     public long getDuration() {
         synchronized (this) {
-            return duration;
+            return durationMicroseconds;
         }
     }
 
@@ -87,23 +98,30 @@ private Endpoint getOrMakePeer() {
 
     public Map<String, Object> getTags() {
         synchronized (this) {
-            // TODO wasteful allocation and copying
-            return new HashMap<>(tags);
+            return Collections.unmodifiableMap(tags);
         }
     }
 
+    @Override
+    public Span setOperationName(String operationName) {
+        synchronized (this) {
+            this.operationName = operationName;
+        }
+        return this;
+    }
+
     public String getOperationName() {
-        return operationName;
+        synchronized (this) {
+            return operationName;
+        }
     }
 
     public List<LogData> getLogs() {
         synchronized (this) {
             if (logs == null) {
                 return null;
             }
-
-            // TODO wasteful allocation and copying
-            return new ArrayList<>(logs);
+            return Collections.unmodifiableList(logs);
         }
     }
 
@@ -143,13 +161,22 @@ public io.opentracing.SpanContext context() {
 
     @Override
     public void finish() {
-        finish(Utils.getMicroseconds());
+        if (computeDurationViaNanoTicks) {
+            long nanoDuration = tracer.clock().currentNanoTicks() - startTimeNanoTicks;
+            finishWithDuration(nanoDuration / 1000);
+        } else {
+            finish(tracer.clock().currentTimeMicros());
+        }
     }
 
     @Override
     public void finish(long finishMicros) {
+        finishWithDuration(finishMicros - startTimeMicroseconds);
+    }
+
+    private void finishWithDuration(long durationMicros) {
         synchronized(this) {
-            this.duration = finishMicros - start;
+            this.durationMicroseconds = durationMicros;
         }
 
         if (context.isSampled()) {
@@ -238,7 +265,7 @@ private Span setTagAsObject(String key, Object value) {
 
     @Override
     public Span log(String message, /* @Nullable */ Object payload) {
-        return log(Utils.getMicroseconds(), message, payload);
+        return log(tracer.clock().currentTimeMicros(), message, payload);
     }
 
     @Override