From 8413344e8818ce8432c380b17e5b5d3aaad459ba Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Mon, 28 Mar 2016 21:03:08 +0800 Subject: [PATCH] Tightens Log to forbid null events and fixes Span json serialization The Log object should never have a null entry as it is defined. This change fixes that, backfills equals/hashCode for convenience, and adds Jackson annotations that use the validating constructor. This also adds a json annotation to span that allows logs to be serialized. This is tested by round-tripping the json. --- .../org/springframework/cloud/sleuth/Log.java | 35 ++++++++++- .../springframework/cloud/sleuth/Span.java | 2 + .../springframework/cloud/sleuth/LogTest.java | 59 +++++++++++++++++++ .../cloud/sleuth/SpanTest.java | 17 +++++- 4 files changed, 110 insertions(+), 3 deletions(-) create mode 100644 spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/LogTest.java diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Log.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Log.java index 7600c4ead..a283f2f38 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Log.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Log.java @@ -16,6 +16,9 @@ package org.springframework.cloud.sleuth; +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; + /** * Represents an event in time associated with a span. Every span has zero or more Logs, * each of which being a timestamped event name. @@ -30,7 +33,7 @@ public class Log { private final long timestamp; /** - * Event (if not null) should be the stable name of some notable moment in the lifetime of a span. + * Event should be the stable name of some notable moment in the lifetime of a span. * For instance, a span representing a browser page load might add an Event for each of the * Performance.timing moments here: https://developer.mozilla.org/en-US/docs/Web/API/PerformanceTiming * @@ -40,7 +43,12 @@ public class Log { */ private final String event; - public Log(long timestamp, String event) { + @JsonCreator + public Log( + @JsonProperty(value = "timestamp", required = true) long timestamp, + @JsonProperty(value = "event", required = true) String event + ) { + if (event == null) throw new NullPointerException("event"); this.timestamp = timestamp; this.event = event; } @@ -53,6 +61,29 @@ public class Log { return this.event; } + @Override + public boolean equals(Object o) { + if (o == this) { + return true; + } + if (o instanceof Log) { + Log that = (Log) o; + return (this.timestamp == that.timestamp) + && (this.event.equals(that.event)); + } + return false; + } + + @Override + public int hashCode() { + int h = 1; + h *= 1000003; + h ^= (this.timestamp >>> 32) ^ this.timestamp; + h *= 1000003; + h ^= this.event.hashCode(); + return h; + } + @Override public String toString() { return "Log{" + "timestamp=" + this.timestamp + diff --git a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java index cb07d86a3..ecd0e192f 100644 --- a/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java +++ b/spring-cloud-sleuth-core/src/main/java/org/springframework/cloud/sleuth/Span.java @@ -16,6 +16,7 @@ package org.springframework.cloud.sleuth; +import com.fasterxml.jackson.annotation.JsonAutoDetect; import java.math.BigInteger; import java.util.ArrayList; import java.util.Collection; @@ -66,6 +67,7 @@ import com.fasterxml.jackson.annotation.JsonInclude; * like scoped tracers. Sleuth spans are DTOs, whose sole responsibility is the current * span in the trace tree. */ +@JsonAutoDetect(fieldVisibility = JsonAutoDetect.Visibility.ANY) @JsonInclude(JsonInclude.Include.NON_DEFAULT) public class Span { diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/LogTest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/LogTest.java new file mode 100644 index 000000000..68ffa0e1f --- /dev/null +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/LogTest.java @@ -0,0 +1,59 @@ +/* + * Copyright 2013-2016 the original author or 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 org.springframework.cloud.sleuth; + +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.IOException; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import static org.assertj.core.api.BDDAssertions.then; + +/** + * @author Adrian Cole + */ +public class LogTest { + + @Rule + public ExpectedException thrown = ExpectedException.none(); + + ObjectMapper objectMapper = new ObjectMapper(); + + @Test public void ctor_missing_event() throws IOException { + thrown.expect(NullPointerException.class); + thrown.expectMessage("event"); + + new Log(1234L, null); + } + + @Test public void serialization_round_trip() throws IOException { + Log log = new Log(1234L, "cs"); + + String serialized = objectMapper.writeValueAsString(log); + Log deserialized = objectMapper.readValue(serialized, Log.class); + + then(deserialized).isEqualTo(log); + } + + @Test public void deserialize_missing_event() throws IOException { + thrown.expect(JsonMappingException.class); + + objectMapper.readValue("{\"timestamp\": 1234}", Log.class); + } +} diff --git a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/SpanTest.java b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/SpanTest.java index 7f9db5274..cb3879983 100644 --- a/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/SpanTest.java +++ b/spring-cloud-sleuth-core/src/test/java/org/springframework/cloud/sleuth/SpanTest.java @@ -16,6 +16,7 @@ package org.springframework.cloud.sleuth; +import java.io.IOException; import java.util.Collections; import org.junit.Test; @@ -80,4 +81,18 @@ public class SpanTest { then(serializedName).isNotEmpty(); } -} \ No newline at end of file + + @Test public void should_properly_serialize_logs() throws IOException { + Span span = new Span(1, 2, "http:name", 1L, + Collections.emptyList(), 2L, true, true, "process"); + span.logEvent("cs"); + + ObjectMapper objectMapper = new ObjectMapper(); + + String serialized = objectMapper.writeValueAsString(span); + Span deserialized = objectMapper.readValue(serialized, Span.class); + + then(deserialized.logs()) + .isEqualTo(span.logs()); + } +}