Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates to zipkin 1.31 and removes more internal usages #477

Merged
merged 1 commit into from
Sep 1, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

import java.util.Random;

import static zipkin.internal.Util.checkArgument;

/**
* This sampler is appropriate for high-traffic instrumentation (ex edge web servers that each
* receive >100K requests) who provision random trace ids, and make the sampling decision only once.
Expand All @@ -28,7 +26,9 @@ public final class BoundarySampler extends Sampler {
public static Sampler create(float rate) {
if (rate == 0) return Sampler.NEVER_SAMPLE;
if (rate == 1.0) return ALWAYS_SAMPLE;
checkArgument(rate >= 0.0001f && rate < 1, "rate should be between 0.0001 and 1: was %s", rate);
if (rate < 0.0001f || rate > 1) {
throw new IllegalArgumentException("rate should be between 0.0001 and 1: was " + rate);
}
final long boundary = (long) (rate * 10000); // safe cast as less <= 1
return new BoundarySampler(boundary);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

import static com.github.kristofa.brave.InetAddressUtilities.getLocalHostLANAddress;
import static com.github.kristofa.brave.InetAddressUtilities.toInt;
import static zipkin.internal.Util.checkNotNull;

/** @deprecated Replaced by {@link brave.Tracing} */
@Deprecated
Expand Down Expand Up @@ -150,7 +149,8 @@ public Builder traceSampler(Sampler sampler) {
* <p>See https://github.com/openzipkin/zipkin-reporter-java
*/
public Builder reporter(Reporter<zipkin.Span> reporter) {
this.reporter = checkNotNull(reporter, "reporter");
if (reporter == null) throw new NullPointerException("reporter == null");
this.reporter = reporter;
return this;
}

Expand All @@ -175,7 +175,8 @@ public Builder spanCollector(SpanCollector spanCollector) {
}

public Builder clock(Clock clock) {
this.clock = checkNotNull(clock, "clock");
if (clock == null) throw new NullPointerException("clock == null");
this.clock = clock;
return this;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
import java.util.BitSet;
import java.util.Random;

import static zipkin.internal.Util.checkArgument;

/**
* This sampler is appropriate for low-traffic instrumentation (ex servers that each receive <100K
* requests), or those who do not provision random trace ids. It not appropriate for collectors as
Expand All @@ -25,7 +23,9 @@ public final class CountingSampler extends Sampler {
public static Sampler create(final float rate) {
if (rate == 0) return NEVER_SAMPLE;
if (rate == 1.0) return ALWAYS_SAMPLE;
checkArgument(rate >= 0.01f && rate < 1, "rate should be between 0.01 and 1: was %s", rate);
if (rate < 0.01f || rate > 1) {
throw new IllegalArgumentException("rate should be between 0.01 and 1: was " + rate);
}
return new CountingSampler(rate);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
import javax.annotation.Generated;
import javax.annotation.Nullable;

import static zipkin.internal.Util.equal;

/**
* An annotation is similar to a log statement. It includes a host field which
* allows these events to be attributed properly, and also aggregatable.
Expand Down Expand Up @@ -52,6 +50,10 @@ public boolean equals(Object o) {
return false;
}

static boolean equal(Object a, Object b) {
return a == b || a != null && a.equals(b);
}

@Override
public int hashCode() {
int h = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import static com.github.kristofa.brave.internal.Util.checkNotNull;
import static com.github.kristofa.brave.internal.Util.equal;
import static zipkin.internal.Util.checkArgument;

/**
* Indicates the network context of a service recording an annotation with two
Expand Down Expand Up @@ -121,7 +120,7 @@ public Endpoint.Builder ipv4(int ipv4) {
/** @see Endpoint#ipv6 */
public Endpoint.Builder ipv6(byte[] ipv6) {
if (ipv6 != null) {
checkArgument(ipv6.length == 16, "ipv6 addresses are 16 bytes: " + ipv6.length);
if (ipv6.length != 16) throw new IllegalArgumentException("ipv6.length != 16");
this.ipv6 = ipv6;
}
return this;
Expand All @@ -138,7 +137,7 @@ public Endpoint.Builder ipv6(byte[] ipv6) {
* @see Endpoint#port
*/
public Endpoint.Builder port(int port) {
checkArgument(port >= 0 && port <= 0xffff, "invalid port %s", port);
if (port < 0 || port > 0xffff) throw new IllegalArgumentException("invalid port " + port);
this.port = port == 0 ? null : (short) (port & 0xffff);
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.github.kristofa.brave.SpanId;
import com.github.kristofa.brave.TracerAdapter;
import com.twitter.zipkin.gen.Span;
import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicLong;
Expand All @@ -18,9 +19,9 @@
import static com.github.kristofa.brave.TracerAdapter.toSpan;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.tuple;
import static zipkin.internal.Util.UTF_8;

public class TracerAdapterTest {
static final Charset UTF_8 = Charset.forName("UTF-8");

List<zipkin.Span> spans = new ArrayList<>();
AtomicLong epochMicros = new AtomicLong();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import zipkin.Endpoint;
import zipkin.Span;
import zipkin.TraceKeys;
import zipkin.internal.Util;
import zipkin.storage.InMemoryStorage;

import static java.util.Arrays.asList;
Expand Down Expand Up @@ -275,7 +274,7 @@ public void httpUrlTagIncludesQueryParams() throws Exception {
assertThat(collectedSpans())
.flatExtracting(s -> s.binaryAnnotations)
.filteredOn(b -> b.key.equals(TraceKeys.HTTP_URL))
.extracting(b -> new String(b.value, Util.UTF_8))
.extracting(b -> new String(b.value, "UTF-8"))
.containsExactly(url(path).toString());
}

Expand Down
6 changes: 4 additions & 2 deletions brave/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,12 @@
</artifactSet>
<filters>
<filter>
<!-- Shade references to span2 until zipkin2 is out -->
<!-- Shade references to v2 until zipkin2 is out -->
<artifact>io.zipkin.java:zipkin</artifact>
<includes>
<include>zipkin/internal/*Span2*.class</include>
<include>zipkin/internal/v2/*Span*.class</include>
<include>zipkin/internal/V2SpanConverter*.class</include>
<!-- TODO: v2 currently indirectly references Util -->
<include>zipkin/internal/Util.class</include>
</includes>
<excludes>
Expand Down
9 changes: 4 additions & 5 deletions brave/src/main/java/brave/internal/recorder/MutableSpan.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,16 @@
import brave.propagation.TraceContext;
import javax.annotation.Nullable;
import zipkin.Endpoint;
import zipkin.internal.Span2;

final class MutableSpan {
final Span2.Builder span;
final zipkin.internal.v2.Span.Builder span;
boolean finished;
long timestamp;

// Since this is not exposed, this class could be refactored later as needed to act in a pool
// to reduce GC churn. This would involve calling span.clear and resetting the fields below.
MutableSpan(TraceContext context, Endpoint localEndpoint) {
this.span = Span2.builder()
this.span = zipkin.internal.v2.Span.builder()
.traceIdHigh(context.traceIdHigh())
.traceId(context.traceId())
.parentId(context.parentId())
Expand All @@ -37,7 +36,7 @@ synchronized MutableSpan name(String name) {

synchronized MutableSpan kind(Span.Kind kind) {
try {
span.kind(Span2.Kind.valueOf(kind.name()));
span.kind(zipkin.internal.v2.Span.Kind.valueOf(kind.name()));
} catch (IllegalArgumentException e) {
// TODO: log
}
Expand Down Expand Up @@ -70,7 +69,7 @@ synchronized MutableSpan finish(@Nullable Long finishTimestamp) {
return this;
}

synchronized Span2 toSpan() {
synchronized zipkin.internal.v2.Span toSpan() {
return span.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import java.util.logging.Logger;
import javax.annotation.Nullable;
import zipkin.Endpoint;
import zipkin.internal.Span2Converter;
import zipkin.internal.V2SpanConverter;
import zipkin.reporter.Reporter;

/**
Expand Down Expand Up @@ -80,7 +80,7 @@ void reportOrphanedSpans() {
if (value == null || noop.get()) continue;
try {
value.annotate(clock.currentTimeMicroseconds(), "brave.flush");
reporter.report(Span2Converter.toSpan(value.toSpan()));
reporter.report(V2SpanConverter.toSpan(value.toSpan()));
} catch (RuntimeException e) {
// don't crash the caller if there was a problem reporting an unrelated span.
if (context != null && logger.isLoggable(Level.FINE)) {
Expand Down
6 changes: 3 additions & 3 deletions brave/src/main/java/brave/internal/recorder/Recorder.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import java.util.concurrent.atomic.AtomicBoolean;
import javax.annotation.Nullable;
import zipkin.Endpoint;
import zipkin.internal.Span2Converter;
import zipkin.internal.V2SpanConverter;
import zipkin.reporter.Reporter;

/** Dispatches mutations on a span to a shared object per trace/span id. */
Expand Down Expand Up @@ -81,7 +81,7 @@ public void finish(TraceContext context, long finishTimestamp) {
if (span == null || noop.get()) return;
synchronized (span) {
span.finish(finishTimestamp);
reporter.report(Span2Converter.toSpan(span.toSpan()));
reporter.report(V2SpanConverter.toSpan(span.toSpan()));
}
}

Expand All @@ -96,7 +96,7 @@ public void flush(TraceContext context) {
if (span == null || noop.get()) return;
synchronized (span) {
span.finish(null);
reporter.report(Span2Converter.toSpan(span.toSpan()));
reporter.report(V2SpanConverter.toSpan(span.toSpan()));
}
}
}
6 changes: 3 additions & 3 deletions brave/src/main/java/brave/sampler/BoundarySampler.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

import java.util.Random;

import static zipkin.internal.Util.checkArgument;

/**
* This sampler is appropriate for high-traffic instrumentation (ex edge web servers that each
* receive >100K requests) who provision random trace ids, and make the sampling decision only once.
Expand All @@ -28,7 +26,9 @@ public final class BoundarySampler extends Sampler {
public static Sampler create(float rate) {
if (rate == 0) return Sampler.NEVER_SAMPLE;
if (rate == 1.0) return ALWAYS_SAMPLE;
checkArgument(rate >= 0.0001f && rate < 1, "rate should be between 0.0001 and 1: was %s", rate);
if (rate < 0.0001f || rate > 1) {
throw new IllegalArgumentException("rate should be between 0.0001 and 1: was " + rate);
}
final long boundary = (long) (rate * 10000); // safe cast as less <= 1
return new BoundarySampler(boundary);
}
Expand Down
6 changes: 3 additions & 3 deletions brave/src/main/java/brave/sampler/CountingSampler.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
import java.util.BitSet;
import java.util.Random;

import static zipkin.internal.Util.checkArgument;

/**
* This sampler is appropriate for low-traffic instrumentation (ex servers that each receive <100K
* requests), or those who do not provision random trace ids. It not appropriate for collectors as
Expand All @@ -25,7 +23,9 @@ public final class CountingSampler extends Sampler {
public static Sampler create(final float rate) {
if (rate == 0) return NEVER_SAMPLE;
if (rate == 1.0) return ALWAYS_SAMPLE;
checkArgument(rate >= 0.01f && rate < 1, "rate should be between 0.01 and 1: was %s", rate);
if (rate < 0.01f || rate > 1) {
throw new IllegalArgumentException("rate should be between 0.01 and 1: was " + rate);
}
return new CountingSampler(rate);
}

Expand Down
31 changes: 15 additions & 16 deletions brave/src/test/java/brave/internal/recorder/MutableSpanTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@
import org.junit.Test;
import zipkin.Annotation;
import zipkin.Endpoint;
import zipkin.internal.Span2;
import zipkin.internal.Span2Converter;
import zipkin.internal.V2SpanConverter;

import static brave.Span.Kind.CLIENT;
import static brave.Span.Kind.SERVER;
Expand Down Expand Up @@ -54,57 +53,57 @@ public class MutableSpanTest {
}

@Test public void finished_client() {
finish(Span.Kind.CLIENT, Span2.Kind.CLIENT);
finish(Span.Kind.CLIENT, zipkin.internal.v2.Span.Kind.CLIENT);
}

@Test public void finished_server() {
finish(Span.Kind.SERVER, Span2.Kind.SERVER);
finish(Span.Kind.SERVER, zipkin.internal.v2.Span.Kind.SERVER);
}

@Test public void finished_producer() {
finish(Span.Kind.PRODUCER, Span2.Kind.PRODUCER);
finish(Span.Kind.PRODUCER, zipkin.internal.v2.Span.Kind.PRODUCER);
}

@Test public void finished_consumer() {
finish(Span.Kind.CONSUMER, Span2.Kind.CONSUMER);
finish(Span.Kind.CONSUMER, zipkin.internal.v2.Span.Kind.CONSUMER);
}

private void finish(Span.Kind braveKind, Span2.Kind span2Kind) {
private void finish(Span.Kind braveKind, zipkin.internal.v2.Span.Kind span2Kind) {
MutableSpan span = newSpan();
span.kind(braveKind);
span.start(1L);
span.finish(2L);

Span2 span2 = span.toSpan();
zipkin.internal.v2.Span span2 = span.toSpan();
assertThat(span2.annotations()).isEmpty();
assertThat(span2.timestamp()).isEqualTo(1L);
assertThat(span2.duration()).isEqualTo(1L);
assertThat(span2.kind()).isEqualTo(span2Kind);
}

@Test public void flushed_client() {
flush(Span.Kind.CLIENT, Span2.Kind.CLIENT);
flush(Span.Kind.CLIENT, zipkin.internal.v2.Span.Kind.CLIENT);
}

@Test public void flushed_server() {
flush(Span.Kind.SERVER, Span2.Kind.SERVER);
flush(Span.Kind.SERVER, zipkin.internal.v2.Span.Kind.SERVER);
}

@Test public void flushed_producer() {
flush(Span.Kind.PRODUCER, Span2.Kind.PRODUCER);
flush(Span.Kind.PRODUCER, zipkin.internal.v2.Span.Kind.PRODUCER);
}

@Test public void flushed_consumer() {
flush(Span.Kind.CONSUMER, Span2.Kind.CONSUMER);
flush(Span.Kind.CONSUMER, zipkin.internal.v2.Span.Kind.CONSUMER);
}

private void flush(Span.Kind braveKind, Span2.Kind span2Kind) {
private void flush(Span.Kind braveKind, zipkin.internal.v2.Span.Kind span2Kind) {
MutableSpan span = newSpan();
span.kind(braveKind);
span.start(1L);
span.finish(null);

Span2 span2 = span.toSpan();
zipkin.internal.v2.Span span2 = span.toSpan();
assertThat(span2.annotations()).isEmpty();
assertThat(span2.timestamp()).isEqualTo(1L);
assertThat(span2.duration()).isNull();
Expand All @@ -131,12 +130,12 @@ private void flush(Span.Kind braveKind, Span2.Kind span2Kind) {
span.kind(SERVER);
span.finish(2L);

assertThat(Span2Converter.toSpan(span.toSpan())).extracting(s -> s.timestamp, s -> s.duration)
assertThat(V2SpanConverter.toSpan(span.toSpan())).extracting(s -> s.timestamp, s -> s.duration)
.allSatisfy(u -> assertThat(u).isNull());
}

@Test public void flushUnstartedNeitherSetsTimestampNorDuration() {
Span2 flushed = newSpan().finish(null).toSpan();
zipkin.internal.v2.Span flushed = newSpan().finish(null).toSpan();
assertThat(flushed).extracting(s -> s.timestamp(), s -> s.duration())
.allSatisfy(u -> assertThat(u).isNull());
}
Expand Down
4 changes: 2 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@
<jetty-servlet25.version>7.6.21.v20160908</jetty-servlet25.version>
<!-- Note: 3.1.x requires Java 8; 3.0.20.Final is broken -->
<resteasy.version>3.1.3.Final</resteasy.version>
<zipkin.version>1.30.3</zipkin.version>
<zipkin-reporter.version>1.0.1</zipkin-reporter.version>
<zipkin.version>1.31.0</zipkin.version>
<zipkin-reporter.version>1.0.2</zipkin-reporter.version>
<finagle.version>6.45.0</finagle.version>
<log4j.version>2.8.2</log4j.version>
<okhttp.version>3.8.1</okhttp.version>
Expand Down