Skip to content

Commit a9c1795

Browse files
adriancoleadriancole
authored
Adds toString to Annotation and BinaryAnnotation (openzipkin#1652)
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
1 parent bf5fa50 commit a9c1795

File tree

6 files changed

+131
-12
lines changed

6 files changed

+131
-12
lines changed

‎benchmarks/src/main/java/zipkin/benchmarks/SpanBenchmarks.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2015-2016 The OpenZipkin Authors
2+
* Copyright 2015-2017 The OpenZipkin Authors
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
55
* in compliance with the License. You may obtain a copy of the License at

‎zipkin/src/main/java/zipkin/Annotation.java

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2015-2016 The OpenZipkin Authors
2+
* Copyright 2015-2017 The OpenZipkin Authors
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
55
* in compliance with the License. You may obtain a copy of the License at
@@ -13,8 +13,10 @@
1313
*/
1414
package zipkin;
1515

16+
import zipkin.internal.JsonCodec;
1617
import zipkin.internal.Nullable;
1718

19+
import static zipkin.internal.Util.UTF_8;
1820
import static zipkin.internal.Util.checkNotNull;
1921
import static zipkin.internal.Util.equal;
2022

@@ -100,9 +102,7 @@ public Annotation build() {
100102

101103
@Override
102104
public boolean equals(Object o) {
103-
if (o == this) {
104-
return true;
105-
}
105+
if (o == this) return true;
106106
if (o instanceof Annotation) {
107107
Annotation that = (Annotation) o;
108108
return (this.timestamp == that.timestamp)
@@ -132,4 +132,8 @@ public int compareTo(Annotation that) {
132132
if (byTimestamp != 0) return byTimestamp;
133133
return value.compareTo(that.value);
134134
}
135+
136+
@Override public String toString() {
137+
return new String(JsonCodec.writeAnnotation(this), UTF_8);
138+
}
135139
}

‎zipkin/src/main/java/zipkin/BinaryAnnotation.java

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* Copyright 2015-2016 The OpenZipkin Authors
2+
* Copyright 2015-2017 The OpenZipkin Authors
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
55
* in compliance with the License. You may obtain a copy of the License at
@@ -14,9 +14,11 @@
1414
package zipkin;
1515

1616
import java.util.Arrays;
17+
import zipkin.internal.JsonCodec;
1718
import zipkin.internal.Nullable;
1819
import zipkin.internal.Util;
1920

21+
import static zipkin.internal.Util.UTF_8;
2022
import static zipkin.internal.Util.checkNotNull;
2123
import static zipkin.internal.Util.equal;
2224

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

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

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

131135
BinaryAnnotation(String key, byte[] value, Type type, Endpoint endpoint) {
132-
this.key = checkNotNull(key, "key");
133-
this.value = checkNotNull(value, "value");
134-
this.type = checkNotNull(type, "type");
136+
checkNotNull(key, "key");
137+
if (value == null) throw new NullPointerException("value of " + key);
138+
if (type == null) throw new NullPointerException("type of " + key);
139+
this.key = key;
140+
this.value = value;
141+
this.type = type;
135142
this.endpoint = endpoint;
136143
}
137144

@@ -197,9 +204,7 @@ public BinaryAnnotation build() {
197204

198205
@Override
199206
public boolean equals(Object o) {
200-
if (o == this) {
201-
return true;
202-
}
207+
if (o == this) return true;
203208
if (o instanceof BinaryAnnotation) {
204209
BinaryAnnotation that = (BinaryAnnotation) o;
205210
return (this.key.equals(that.key))
@@ -230,4 +235,8 @@ public int compareTo(BinaryAnnotation that) {
230235
if (this == that) return 0;
231236
return key.compareTo(that.key);
232237
}
238+
239+
@Override public String toString() {
240+
return new String(JsonCodec.writeBinaryAnnotation(this), UTF_8);
241+
}
233242
}

‎zipkin/src/main/java/zipkin/internal/JsonCodec.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,16 @@ public static byte[] writeEndpoint(Endpoint value) {
460460
return write(ENDPOINT_WRITER, value);
461461
}
462462

463+
/** Exposed for {@link Annotation#toString()} */
464+
public static byte[] writeAnnotation(Annotation value) {
465+
return write(ANNOTATION_WRITER, value);
466+
}
467+
468+
/** Exposed for {@link BinaryAnnotation#toString()} */
469+
public static byte[] writeBinaryAnnotation(BinaryAnnotation value) {
470+
return write(BINARY_ANNOTATION_WRITER, value);
471+
}
472+
463473
/** Exposed for ElasticSearch HttpBulkIndexer */
464474
public static String escape(String value) {
465475
return Buffer.jsonEscape(value);

‎zipkin/src/test/java/zipkin/AnnotationTest.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
/**
2+
* Copyright 2015-2017 The OpenZipkin Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
5+
* in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License
10+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
11+
* or implied. See the License for the specific language governing permissions and limitations under
12+
* the License.
13+
*/
14+
package zipkin;
15+
16+
import org.junit.Rule;
17+
import org.junit.Test;
18+
import org.junit.rules.ExpectedException;
19+
20+
import static org.assertj.core.api.Assertions.assertThat;
21+
import static zipkin.TestObjects.APP_ENDPOINT;
22+
23+
public class AnnotationTest {
24+
@Rule public ExpectedException thrown = ExpectedException.none();
25+
26+
@Test public void messageWhenMissingValue() {
27+
thrown.expect(NullPointerException.class);
28+
thrown.expectMessage("value");
29+
30+
Annotation.create(1L, null, null);
31+
}
32+
33+
@Test public void toStringIsJson_withEndpoint() {
34+
assertThat(Annotation.create(1L, "foo", APP_ENDPOINT))
35+
.hasToString(
36+
"{\"timestamp\":1,\"value\":\"foo\",\"endpoint\":{\"serviceName\":\"app\",\"ipv4\":\"172.17.0.2\",\"port\":8080}}");
37+
}
38+
39+
// simple spans will not have endpoint defined eventhough normal ones will
40+
@Test public void toStringIsJson_withoutEndpoint() {
41+
assertThat(Annotation.create(1L, "foo", null))
42+
.hasToString("{\"timestamp\":1,\"value\":\"foo\"}");
43+
}
44+
}

‎zipkin/src/test/java/zipkin/BinaryAnnotationTest.java

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/**
2+
* Copyright 2015-2017 The OpenZipkin Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
5+
* in compliance with the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License
10+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
11+
* or implied. See the License for the specific language governing permissions and limitations under
12+
* the License.
13+
*/
14+
package zipkin;
15+
16+
import org.junit.Rule;
17+
import org.junit.Test;
18+
import org.junit.rules.ExpectedException;
19+
20+
import static org.assertj.core.api.Assertions.assertThat;
21+
import static zipkin.TestObjects.APP_ENDPOINT;
22+
23+
public class BinaryAnnotationTest {
24+
@Rule public ExpectedException thrown = ExpectedException.none();
25+
26+
@Test public void messageWhenMissingKey() {
27+
thrown.expect(NullPointerException.class);
28+
thrown.expectMessage("key");
29+
30+
BinaryAnnotation.create(null, null, null);
31+
}
32+
33+
@Test public void messageWhenMissingValue() {
34+
thrown.expect(NullPointerException.class);
35+
thrown.expectMessage("value of foo");
36+
37+
BinaryAnnotation.create("foo", null, null);
38+
}
39+
40+
@Test public void messageWhenMissingType() {
41+
thrown.expect(NullPointerException.class);
42+
thrown.expectMessage("type of foo");
43+
44+
BinaryAnnotation.builder().key("foo").value(new byte[0]).build();
45+
}
46+
47+
@Test public void toStringIsJson() {
48+
assertThat(BinaryAnnotation.create("foo", "bar", APP_ENDPOINT))
49+
.hasToString(
50+
"{\"key\":\"foo\",\"value\":\"bar\",\"endpoint\":{\"serviceName\":\"app\",\"ipv4\":\"172.17.0.2\",\"port\":8080}}");
51+
}
52+
}

0 commit comments

Comments
 (0)