diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java index 8f38606ee5..1380b2753c 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java @@ -1,8 +1,5 @@ package datadog.opentracing; -import static io.opentracing.log.Fields.ERROR_OBJECT; -import static io.opentracing.log.Fields.MESSAGE; - import datadog.trace.api.DDTags; import datadog.trace.api.interceptor.MutableSpan; import datadog.trace.api.sampling.PrioritySampling; @@ -158,18 +155,6 @@ public class DDSpan implements Span, MutableSpan { setTag(DDTags.ERROR_STACK, errorString.toString()); } - private boolean extractError(final Map map) { - if (map.get(ERROR_OBJECT) instanceof Throwable) { - final Throwable error = (Throwable) map.get(ERROR_OBJECT); - setErrorMeta(error); - return true; - } else if (map.get(MESSAGE) instanceof String) { - setTag(DDTags.ERROR_MSG, (String) map.get(MESSAGE)); - return true; - } - return false; - } - /* (non-Javadoc) * @see io.opentracing.BaseSpan#setTag(java.lang.String, java.lang.String) */ @@ -242,8 +227,7 @@ public class DDSpan implements Span, MutableSpan { */ @Override public final DDSpan log(final Map map) { - extractError(map); - logHandler.log(map); + logHandler.log(map, this); return this; } @@ -252,8 +236,7 @@ public class DDSpan implements Span, MutableSpan { */ @Override public final DDSpan log(final long l, final Map map) { - extractError(map); - logHandler.log(l, map); + logHandler.log(l, map, this); return this; } @@ -262,7 +245,7 @@ public class DDSpan implements Span, MutableSpan { */ @Override public final DDSpan log(final String s) { - logHandler.log(s); + logHandler.log(s, this); return this; } @@ -271,7 +254,7 @@ public class DDSpan implements Span, MutableSpan { */ @Override public final DDSpan log(final long l, final String s) { - logHandler.log(l, s); + logHandler.log(l, s, this); return this; } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java index 8f24842df4..76ccb4b32f 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java @@ -672,8 +672,9 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace } public DDSpanBuilder withLogHandler(final LogHandler logHandler) { - assert logHandler != null : "LogHandler must not be null"; - this.logHandler = logHandler; + if (logHandler != null) { + this.logHandler = logHandler; + } return this; } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DefaultLogHandler.java b/dd-trace-ot/src/main/java/datadog/opentracing/DefaultLogHandler.java index 10d403d6e8..0f7cf2ba63 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DefaultLogHandler.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DefaultLogHandler.java @@ -1,5 +1,9 @@ package datadog.opentracing; +import static io.opentracing.log.Fields.ERROR_OBJECT; +import static io.opentracing.log.Fields.MESSAGE; + +import datadog.trace.api.DDTags; import java.util.Map; import lombok.extern.slf4j.Slf4j; @@ -7,22 +11,33 @@ import lombok.extern.slf4j.Slf4j; @Slf4j public class DefaultLogHandler implements LogHandler { @Override - public void log(Map fields) { + public void log(Map fields, DDSpan span) { + extractError(fields, span); log.debug("`log` method is not implemented. Doing nothing"); } @Override - public void log(long timestampMicroseconds, Map fields) { + public void log(long timestampMicroseconds, Map fields, DDSpan span) { + extractError(fields, span); log.debug("`log` method is not implemented. Doing nothing"); } @Override - public void log(String event) { + public void log(String event, DDSpan span) { log.debug("`log` method is not implemented. Provided log: {}", event); } @Override - public void log(long timestampMicroseconds, String event) { + public void log(long timestampMicroseconds, String event, DDSpan span) { log.debug("`log` method is not implemented. Provided log: {}", event); } + + private void extractError(final Map map, DDSpan span) { + if (map.get(ERROR_OBJECT) instanceof Throwable) { + final Throwable error = (Throwable) map.get(ERROR_OBJECT); + span.setErrorMeta(error); + } else if (map.get(MESSAGE) instanceof String) { + span.setTag(DDTags.ERROR_MSG, (String) map.get(MESSAGE)); + } + } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/LogHandler.java b/dd-trace-ot/src/main/java/datadog/opentracing/LogHandler.java index f1e5d43b68..03bc25029e 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/LogHandler.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/LogHandler.java @@ -9,8 +9,9 @@ public interface LogHandler { * * @param fields key:value log fields. Tracer implementations should support String, numeric, and * boolean values; some may also support arbitrary Objects. + * @param span from which the call was made */ - void log(Map fields); + void log(Map fields, DDSpan span); /** * Handles the log implementation in the Span. @@ -18,16 +19,17 @@ public interface LogHandler { * @param timestampMicroseconds The explicit timestamp for the log record. Must be greater than or * equal to the Span's start timestamp. * @param fields key:value log fields. Tracer implementations should support String, numeric, and - * boolean values; some may also support arbitrary Objects. + * @param span from which the call was made */ - void log(long timestampMicroseconds, Map fields); + void log(long timestampMicroseconds, Map fields, DDSpan span); /** * Handles the log implementation in the Span.. * * @param event the event value; often a stable identifier for a moment in the Span lifecycle + * @param span from which the call was made */ - void log(String event); + void log(String event, DDSpan span); /** * Handles the log implementation in the Span. @@ -35,6 +37,7 @@ public interface LogHandler { * @param timestampMicroseconds The explicit timestamp for the log record. Must be greater than or * equal to the Span's start timestamp. * @param event the event value; often a stable identifier for a moment in the Span lifecycle + * @param span from which the call was made */ - void log(long timestampMicroseconds, String event); + void log(long timestampMicroseconds, String event, DDSpan span); } diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy index 56b795999f..ad1ce537ad 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy @@ -499,6 +499,23 @@ class DDSpanBuilderTest extends DDSpecification { span.log(timeStamp, fieldsMap) } + def "sanity test when passed log handler is null"() { + setup: + final String expectedName = "fakeName" + final DDSpan span = tracer + .buildSpan(expectedName) + .withLogHandler(null) + .start() + final String expectedLogEvent = "fakeEvent" + final timeStamp = System.currentTimeMillis() + final Map fieldsMap = new HashMap<>() + + span.log(expectedLogEvent) + span.log(timeStamp, expectedLogEvent) + span.log(fieldsMap) + span.log(timeStamp, fieldsMap) + } + def "should delegate simple logs to logHandler"() { setup: @@ -516,7 +533,7 @@ class DDSpanBuilderTest extends DDSpecification { span.log(timeStamp, expectedLogEvent) expect: - logHandler.assertLogCalledWithArgs(timeStamp, expectedLogEvent) + logHandler.assertLogCalledWithArgs(timeStamp, expectedLogEvent, span) } def "should delegate simple logs with timestamp to logHandler"() { @@ -534,7 +551,7 @@ class DDSpanBuilderTest extends DDSpecification { span.log(expectedLogEvent) expect: - logHandler.assertLogCalledWithArgs(expectedLogEvent) + logHandler.assertLogCalledWithArgs(expectedLogEvent, span) } @@ -553,7 +570,7 @@ class DDSpanBuilderTest extends DDSpecification { span.log(fieldsMap) expect: - logHandler.assertLogCalledWithArgs(fieldsMap) + logHandler.assertLogCalledWithArgs(fieldsMap, span) } @@ -573,50 +590,41 @@ class DDSpanBuilderTest extends DDSpecification { span.log(timeStamp, fieldsMap) expect: - logHandler.assertLogCalledWithArgs(timeStamp, fieldsMap) + logHandler.assertLogCalledWithArgs(timeStamp, fieldsMap, span) } - def "test assertion not null LogHandler"() { - setup: - final String expectedName = "fakeName" - - when: - tracer - .buildSpan(expectedName) - .withLogHandler(null) - - then: - thrown(AssertionError) - } - private static class TestLogHandler implements LogHandler { Object[] arguments = null @Override - void log(Map fields) { - arguments = new Object[1] + void log(Map fields, DDSpan span) { + arguments = new Object[2] arguments[0] = fields + arguments[1] = span } @Override - void log(long timestampMicroseconds, Map fields) { - arguments = new Object[2] + void log(long timestampMicroseconds, Map fields, DDSpan span) { + arguments = new Object[3] arguments[0] = timestampMicroseconds arguments[1] = fields + arguments[2] = span } @Override - void log(String event) { - arguments = new Object[1] - arguments[0] = event - } - - @Override - void log(long timestampMicroseconds, String event) { + void log(String event, DDSpan span) { arguments = new Object[2] + arguments[0] = event + arguments[1] = span + } + + @Override + void log(long timestampMicroseconds, String event, DDSpan span) { + arguments = new Object[3] arguments[0] = timestampMicroseconds arguments[1] = event + arguments[2] = span } boolean assertLogCalledWithArgs(Object... args) { diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/DefaultLogHandlerTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/DefaultLogHandlerTest.groovy new file mode 100644 index 0000000000..4241ffd645 --- /dev/null +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/DefaultLogHandlerTest.groovy @@ -0,0 +1,80 @@ +package datadog.opentracing + +import datadog.trace.api.DDTags +import datadog.trace.common.writer.ListWriter +import io.opentracing.log.Fields +import datadog.trace.util.test.DDSpecification + + +class DefaultLogHandlerTest extends DDSpecification { + def writer = new ListWriter() + def tracer = DDTracer.builder().writer(writer).build() + + def "handles correctly the error passed in the fields"() { + setup: + final LogHandler underTest = new DefaultLogHandler() + final DDSpan span = tracer.buildSpan("op name").withServiceName("foo").start() + final String errorMessage = "errorMessage" + final String differentMessage = "differentMessage" + final Throwable throwable = new Throwable(errorMessage) + final Map fields = new HashMap<>() + fields.put(Fields.ERROR_OBJECT, throwable) + fields.put(Fields.MESSAGE, differentMessage) + + when: + underTest.log(fields, span) + + then: + span.getTags().get(DDTags.ERROR_MSG) == throwable.getMessage() + span.getTags().get(DDTags.ERROR_TYPE) == throwable.getClass().getName() + } + + def "handles correctly the error passed in the fields when called with timestamp"() { + setup: + final LogHandler underTest = new DefaultLogHandler() + final DDSpan span = tracer.buildSpan("op name").withServiceName("foo").start() + final String errorMessage = "errorMessage" + final String differentMessage = "differentMessage" + final Throwable throwable = new Throwable(errorMessage) + final Map fields = new HashMap<>() + fields.put(Fields.ERROR_OBJECT, throwable) + fields.put(Fields.MESSAGE, differentMessage) + + when: + underTest.log(System.currentTimeMillis(), fields, span) + + then: + span.getTags().get(DDTags.ERROR_MSG) == throwable.getMessage() + span.getTags().get(DDTags.ERROR_TYPE) == throwable.getClass().getName() + } + + def "handles correctly the message passed in the fields"() { + setup: + final LogHandler underTest = new DefaultLogHandler() + final DDSpan span = tracer.buildSpan("op name").withServiceName("foo").start() + final String errorMessage = "errorMessage" + final Map fields = new HashMap<>() + fields.put(Fields.MESSAGE, errorMessage) + + when: + underTest.log(fields, span) + + then: + span.getTags().get(DDTags.ERROR_MSG) == errorMessage + } + + def "handles correctly the message passed in the fields when called with timestamp"() { + setup: + final LogHandler underTest = new DefaultLogHandler() + final DDSpan span = tracer.buildSpan("op name").withServiceName("foo").start() + final String errorMessage = "errorMessage" + final Map fields = new HashMap<>() + fields.put(Fields.MESSAGE, errorMessage) + + when: + underTest.log(System.currentTimeMillis(), fields, span) + + then: + span.getTags().get(DDTags.ERROR_MSG) == errorMessage + } +}