Skip to content

Commit

Permalink
Adds toString to Annotation and BinaryAnnotation (#1652)
Browse files Browse the repository at this point in the history
If you use Span.toString, embedded annotations and binary annotations
look good. However, in tests or debugger they don't. This fixes this.

Thx @SirTyro for reporting
  • Loading branch information
adriancole committed Jul 17, 2017
1 parent bf5fa50 commit a9c1795
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2015-2016 The OpenZipkin Authors
* Copyright 2015-2017 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand Down
12 changes: 8 additions & 4 deletions zipkin/src/main/java/zipkin/Annotation.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2015-2016 The OpenZipkin Authors
* Copyright 2015-2017 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand All @@ -13,8 +13,10 @@
*/
package zipkin;

import zipkin.internal.JsonCodec;
import zipkin.internal.Nullable;

import static zipkin.internal.Util.UTF_8;
import static zipkin.internal.Util.checkNotNull;
import static zipkin.internal.Util.equal;

Expand Down Expand Up @@ -100,9 +102,7 @@ public Annotation build() {

@Override
public boolean equals(Object o) {
if (o == this) {
return true;
}
if (o == this) return true;
if (o instanceof Annotation) {
Annotation that = (Annotation) o;
return (this.timestamp == that.timestamp)
Expand Down Expand Up @@ -132,4 +132,8 @@ public int compareTo(Annotation that) {
if (byTimestamp != 0) return byTimestamp;
return value.compareTo(that.value);
}

@Override public String toString() {
return new String(JsonCodec.writeAnnotation(this), UTF_8);
}
}
23 changes: 16 additions & 7 deletions zipkin/src/main/java/zipkin/BinaryAnnotation.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright 2015-2016 The OpenZipkin Authors
* Copyright 2015-2017 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
Expand All @@ -14,9 +14,11 @@
package zipkin;

import java.util.Arrays;
import zipkin.internal.JsonCodec;
import zipkin.internal.Nullable;
import zipkin.internal.Util;

import static zipkin.internal.Util.UTF_8;
import static zipkin.internal.Util.checkNotNull;
import static zipkin.internal.Util.equal;

Expand Down Expand Up @@ -93,6 +95,8 @@ public static BinaryAnnotation address(String key, Endpoint endpoint) {

/** String values are the only queryable type of binary annotation. */
public static BinaryAnnotation create(String key, String value, @Nullable Endpoint endpoint) {
checkNotNull(key, "key");
if (value == null) throw new NullPointerException("value of " + key);
return new BinaryAnnotation(key, value.getBytes(Util.UTF_8), Type.STRING, endpoint);
}

Expand Down Expand Up @@ -129,9 +133,12 @@ public static BinaryAnnotation create(String key, byte[] value, Type type, @Null
public final Endpoint endpoint;

BinaryAnnotation(String key, byte[] value, Type type, Endpoint endpoint) {
this.key = checkNotNull(key, "key");
this.value = checkNotNull(value, "value");
this.type = checkNotNull(type, "type");
checkNotNull(key, "key");
if (value == null) throw new NullPointerException("value of " + key);
if (type == null) throw new NullPointerException("type of " + key);
this.key = key;
this.value = value;
this.type = type;
this.endpoint = endpoint;
}

Expand Down Expand Up @@ -197,9 +204,7 @@ public BinaryAnnotation build() {

@Override
public boolean equals(Object o) {
if (o == this) {
return true;
}
if (o == this) return true;
if (o instanceof BinaryAnnotation) {
BinaryAnnotation that = (BinaryAnnotation) o;
return (this.key.equals(that.key))
Expand Down Expand Up @@ -230,4 +235,8 @@ public int compareTo(BinaryAnnotation that) {
if (this == that) return 0;
return key.compareTo(that.key);
}

@Override public String toString() {
return new String(JsonCodec.writeBinaryAnnotation(this), UTF_8);
}
}
10 changes: 10 additions & 0 deletions zipkin/src/main/java/zipkin/internal/JsonCodec.java
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,16 @@ public static byte[] writeEndpoint(Endpoint value) {
return write(ENDPOINT_WRITER, value);
}

/** Exposed for {@link Annotation#toString()} */
public static byte[] writeAnnotation(Annotation value) {
return write(ANNOTATION_WRITER, value);
}

/** Exposed for {@link BinaryAnnotation#toString()} */
public static byte[] writeBinaryAnnotation(BinaryAnnotation value) {
return write(BINARY_ANNOTATION_WRITER, value);
}

/** Exposed for ElasticSearch HttpBulkIndexer */
public static String escape(String value) {
return Buffer.jsonEscape(value);
Expand Down
44 changes: 44 additions & 0 deletions zipkin/src/test/java/zipkin/AnnotationTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/**
* Copyright 2015-2017 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
package zipkin;

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

import static org.assertj.core.api.Assertions.assertThat;
import static zipkin.TestObjects.APP_ENDPOINT;

public class AnnotationTest {
@Rule public ExpectedException thrown = ExpectedException.none();

@Test public void messageWhenMissingValue() {
thrown.expect(NullPointerException.class);
thrown.expectMessage("value");

Annotation.create(1L, null, null);
}

@Test public void toStringIsJson_withEndpoint() {
assertThat(Annotation.create(1L, "foo", APP_ENDPOINT))
.hasToString(
"{\"timestamp\":1,\"value\":\"foo\",\"endpoint\":{\"serviceName\":\"app\",\"ipv4\":\"172.17.0.2\",\"port\":8080}}");
}

// simple spans will not have endpoint defined eventhough normal ones will
@Test public void toStringIsJson_withoutEndpoint() {
assertThat(Annotation.create(1L, "foo", null))
.hasToString("{\"timestamp\":1,\"value\":\"foo\"}");
}
}
52 changes: 52 additions & 0 deletions zipkin/src/test/java/zipkin/BinaryAnnotationTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/**
* Copyright 2015-2017 The OpenZipkin Authors
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
* in compliance with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software distributed under the License
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
* or implied. See the License for the specific language governing permissions and limitations under
* the License.
*/
package zipkin;

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

import static org.assertj.core.api.Assertions.assertThat;
import static zipkin.TestObjects.APP_ENDPOINT;

public class BinaryAnnotationTest {
@Rule public ExpectedException thrown = ExpectedException.none();

@Test public void messageWhenMissingKey() {
thrown.expect(NullPointerException.class);
thrown.expectMessage("key");

BinaryAnnotation.create(null, null, null);
}

@Test public void messageWhenMissingValue() {
thrown.expect(NullPointerException.class);
thrown.expectMessage("value of foo");

BinaryAnnotation.create("foo", null, null);
}

@Test public void messageWhenMissingType() {
thrown.expect(NullPointerException.class);
thrown.expectMessage("type of foo");

BinaryAnnotation.builder().key("foo").value(new byte[0]).build();
}

@Test public void toStringIsJson() {
assertThat(BinaryAnnotation.create("foo", "bar", APP_ENDPOINT))
.hasToString(
"{\"key\":\"foo\",\"value\":\"bar\",\"endpoint\":{\"serviceName\":\"app\",\"ipv4\":\"172.17.0.2\",\"port\":8080}}");
}
}

0 comments on commit a9c1795

Please sign in to comment.