Updates to Zipkin 1.3 and rounds up Span duration to one when < 1μ (#326)
Duration of 0 is confusing to plot and can be misinterpreted as null. See https://github.com/openzipkin/zipkin/issues/1174 See https://github.com/openzipkin/zipkin/releases/tag/1.3.0
This commit is contained in:
@@ -190,7 +190,7 @@ public class Span {
|
||||
this.startNanos = null; // don't know the start tick
|
||||
this.begin = begin;
|
||||
} else {
|
||||
this.startNanos = System.nanoTime();
|
||||
this.startNanos = nanoTime();
|
||||
this.begin = System.currentTimeMillis();
|
||||
}
|
||||
if (end > 0) {
|
||||
@@ -226,7 +226,7 @@ public class Span {
|
||||
this.end = System.currentTimeMillis();
|
||||
}
|
||||
if (this.startNanos != null) { // set a precise duration
|
||||
this.durationMicros = (System.nanoTime() - this.startNanos) / 1000;
|
||||
this.durationMicros = Math.max(1, (nanoTime() - this.startNanos) / 1000);
|
||||
} else {
|
||||
this.durationMicros = (this.end - this.begin) * 1000;
|
||||
}
|
||||
@@ -248,6 +248,8 @@ public class Span {
|
||||
/**
|
||||
* Return the total amount of time elapsed since start was called, if running, or
|
||||
* difference between stop and start, in microseconds.
|
||||
*
|
||||
* @return zero if not running, or a positive number of microseconds.
|
||||
*/
|
||||
@JsonIgnore
|
||||
public synchronized long getAccumulatedMicros() {
|
||||
@@ -258,13 +260,19 @@ public class Span {
|
||||
return 0;
|
||||
}
|
||||
if (this.startNanos != null) {
|
||||
return (System.nanoTime() - this.startNanos) / 1000;
|
||||
return Math.max(1, (nanoTime() - this.startNanos) / 1000);
|
||||
} else {
|
||||
return (System.currentTimeMillis() - this.begin) * 1000;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Visible for testing
|
||||
@JsonIgnore
|
||||
long nanoTime() {
|
||||
return System.nanoTime();
|
||||
}
|
||||
|
||||
/**
|
||||
* Has the span been started and not yet stopped?
|
||||
*/
|
||||
|
||||
@@ -18,6 +18,7 @@ package org.springframework.cloud.sleuth;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.Collections;
|
||||
import java.util.concurrent.atomic.AtomicLong;
|
||||
|
||||
import com.fasterxml.jackson.core.JsonProcessingException;
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
@@ -135,4 +136,43 @@ public class SpanTests {
|
||||
assertThat(deserialized.getAccumulatedMicros())
|
||||
.isEqualTo(span.getAccumulatedMicros());
|
||||
}
|
||||
|
||||
// Duration of 0 is confusing to plot and can be misinterpreted as null
|
||||
@Test public void getAccumulatedMicros_roundsUpToOneWhenRunning() throws IOException {
|
||||
AtomicLong nanoTime = new AtomicLong();
|
||||
|
||||
// starts the span, recording its initial tick as zero
|
||||
Span span = new Span(0, 0, "http:name", 1L, Collections.<Long>emptyList(), 2L, true,
|
||||
true, "process", null) {
|
||||
@Override long nanoTime() {
|
||||
return nanoTime.get();
|
||||
}
|
||||
};
|
||||
|
||||
// When only 100 nanoseconds passed
|
||||
nanoTime.set(100L);
|
||||
|
||||
// We round so that we don't confuse "not started" with a short span.
|
||||
assertThat(span.getAccumulatedMicros()).isEqualTo(1L);
|
||||
}
|
||||
|
||||
// Duration of 0 is confusing to plot and can be misinterpreted as null
|
||||
@Test public void getAccumulatedMicros_roundsUpToOneWhenStopped() throws IOException {
|
||||
AtomicLong nanoTime = new AtomicLong();
|
||||
|
||||
// starts the span, recording its initial tick as zero
|
||||
Span span = new Span(0, 0, "http:name", 1L, Collections.<Long>emptyList(), 2L, true,
|
||||
true, "process", null) {
|
||||
@Override long nanoTime() {
|
||||
return nanoTime.get();
|
||||
}
|
||||
};
|
||||
|
||||
// When only 100 nanoseconds passed
|
||||
nanoTime.set(100L);
|
||||
span.stop();
|
||||
|
||||
// We round so that we don't confuse "not started" with a short span.
|
||||
assertThat(span.getAccumulatedMicros()).isEqualTo(1L);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -16,7 +16,7 @@
|
||||
<properties>
|
||||
<spring-cloud-netflix.version>1.2.0.BUILD-SNAPSHOT</spring-cloud-netflix.version>
|
||||
<aspectj.version>1.8.4</aspectj.version>
|
||||
<zipkin.version>1.1.5</zipkin.version>
|
||||
<zipkin.version>1.3.0</zipkin.version>
|
||||
</properties>
|
||||
<dependencyManagement>
|
||||
<dependencies>
|
||||
|
||||
@@ -59,12 +59,12 @@
|
||||
<dependency>
|
||||
<groupId>io.zipkin.java</groupId>
|
||||
<artifactId>zipkin</artifactId>
|
||||
<version>1.1.5</version>
|
||||
<version>1.3.0</version>
|
||||
</dependency>
|
||||
<dependency>
|
||||
<groupId>io.zipkin.java</groupId>
|
||||
<artifactId>zipkin-server</artifactId>
|
||||
<version>1.1.5</version>
|
||||
<version>1.3.0</version>
|
||||
</dependency>
|
||||
</dependencies>
|
||||
</dependencyManagement>
|
||||
|
||||
Reference in New Issue
Block a user