From b24ae8d6387868c7a69ed601a79277e1d9688d63 Mon Sep 17 00:00:00 2001 From: Marius Constantin Date: Thu, 27 Feb 2020 11:04:52 +0100 Subject: [PATCH] Add the DefaultLogHandler implementation --- .../main/java/datadog/opentracing/DDSpan.java | 36 +++++----------- .../java/datadog/opentracing/DDTracer.java | 9 ++-- .../opentracing/DefaultLogHandler.java | 28 +++++++++++++ .../{LogsHandler.java => LogHandler.java} | 2 +- .../opentracing/DDSpanBuilderTest.groovy | 41 ++++++++++++------- 5 files changed, 72 insertions(+), 44 deletions(-) create mode 100644 dd-trace-ot/src/main/java/datadog/opentracing/DefaultLogHandler.java rename dd-trace-ot/src/main/java/datadog/opentracing/{LogsHandler.java => LogHandler.java} (97%) 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 0c6cffabed..8f38606ee5 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDSpan.java @@ -51,7 +51,7 @@ public class DDSpan implements Span, MutableSpan { private final AtomicLong durationNano = new AtomicLong(); /** Delegates to for handling the logs if present. */ - private final LogsHandler logsHandler; + private final LogHandler logHandler; /** Implementation detail. Stores the weak reference to this span. Used by TraceCollection. */ volatile WeakReference ref; @@ -63,7 +63,7 @@ public class DDSpan implements Span, MutableSpan { * @param context the context used for the span */ DDSpan(final long timestampMicro, final DDSpanContext context) { - this(timestampMicro, context, null); + this(timestampMicro, context, new DefaultLogHandler()); } /** @@ -71,11 +71,11 @@ public class DDSpan implements Span, MutableSpan { * * @param timestampMicro if greater than zero, use this time instead of the current time * @param context the context used for the span - * @param logsHandler as the handler where to delegate the log actions + * @param logHandler as the handler where to delegate the log actions */ - DDSpan(final long timestampMicro, final DDSpanContext context, final LogsHandler logsHandler) { + DDSpan(final long timestampMicro, final DDSpanContext context, final LogHandler logHandler) { this.context = context; - this.logsHandler = logsHandler; + this.logHandler = logHandler; if (timestampMicro <= 0L) { // record the start time @@ -242,11 +242,8 @@ public class DDSpan implements Span, MutableSpan { */ @Override public final DDSpan log(final Map map) { - if (!extractError(map) && logsHandler != null) { - logsHandler.log(map); - } else { - log.debug("`log` method is not implemented. Doing nothing"); - } + extractError(map); + logHandler.log(map); return this; } @@ -255,11 +252,8 @@ public class DDSpan implements Span, MutableSpan { */ @Override public final DDSpan log(final long l, final Map map) { - if (!extractError(map) && logsHandler != null) { - logsHandler.log(l, map); - } else { - log.debug("`log` method is not implemented. Doing nothing"); - } + extractError(map); + logHandler.log(l, map); return this; } @@ -268,11 +262,7 @@ public class DDSpan implements Span, MutableSpan { */ @Override public final DDSpan log(final String s) { - if (logsHandler != null) { - logsHandler.log(s); - } else { - log.debug("`log` method is not implemented. Provided log: {}", s); - } + logHandler.log(s); return this; } @@ -281,11 +271,7 @@ public class DDSpan implements Span, MutableSpan { */ @Override public final DDSpan log(final long l, final String s) { - if (logsHandler != null) { - logsHandler.log(l, s); - } else { - log.debug("`log` method is not implemented. Provided log: {}", s); - } + logHandler.log(l, s); 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 f5ccdbdd5a..8f24842df4 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java @@ -580,7 +580,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace private boolean errorFlag; private String spanType; private boolean ignoreScope = false; - private LogsHandler logsHandler = null; + private LogHandler logHandler = new DefaultLogHandler(); public DDSpanBuilder(final String operationName, final ScopeManager scopeManager) { this.operationName = operationName; @@ -594,7 +594,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace } private Span startSpan() { - return new DDSpan(timestampMicro, buildSpanContext(), logsHandler); + return new DDSpan(timestampMicro, buildSpanContext(), logHandler); } @Override @@ -671,8 +671,9 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace return parent.baggageItems(); } - public DDSpanBuilder withLogsHandler(LogsHandler logsHandler) { - this.logsHandler = logsHandler; + public DDSpanBuilder withLogHandler(final LogHandler logHandler) { + assert logHandler != null : "LogHandler must not be 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 new file mode 100644 index 0000000000..10d403d6e8 --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DefaultLogHandler.java @@ -0,0 +1,28 @@ +package datadog.opentracing; + +import java.util.Map; +import lombok.extern.slf4j.Slf4j; + +/** The default implementation of the LogHandler. */ +@Slf4j +public class DefaultLogHandler implements LogHandler { + @Override + public void log(Map fields) { + log.debug("`log` method is not implemented. Doing nothing"); + } + + @Override + public void log(long timestampMicroseconds, Map fields) { + log.debug("`log` method is not implemented. Doing nothing"); + } + + @Override + public void log(String event) { + log.debug("`log` method is not implemented. Provided log: {}", event); + } + + @Override + public void log(long timestampMicroseconds, String event) { + log.debug("`log` method is not implemented. Provided log: {}", event); + } +} diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/LogsHandler.java b/dd-trace-ot/src/main/java/datadog/opentracing/LogHandler.java similarity index 97% rename from dd-trace-ot/src/main/java/datadog/opentracing/LogsHandler.java rename to dd-trace-ot/src/main/java/datadog/opentracing/LogHandler.java index a6069980eb..f1e5d43b68 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/LogsHandler.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/LogHandler.java @@ -2,7 +2,7 @@ package datadog.opentracing; import java.util.Map; -public interface LogsHandler { +public interface LogHandler { /** * Handles the log implementation in the 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 e68a756647..56b795999f 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy @@ -480,7 +480,7 @@ class DDSpanBuilderTest extends DDSpecification { "a:1,b-c:d" | [a: "1", "b-c": "d"] } - def "sanity test for logs if logsHandler is null"() { + def "sanity test for logs if logHandler is null"() { setup: final String expectedName = "fakeName" @@ -502,13 +502,13 @@ class DDSpanBuilderTest extends DDSpecification { def "should delegate simple logs to logHandler"() { setup: - final LogsHandler logsHandler = new TestLogsHandler() + final LogHandler logHandler = new TestLogHandler() final String expectedName = "fakeName" final DDSpan span = tracer .buildSpan(expectedName) - .withLogsHandler(logsHandler) + .withLogHandler(logHandler) .withServiceName("foo") .start() final String expectedLogEvent = "fakeEvent" @@ -516,56 +516,56 @@ class DDSpanBuilderTest extends DDSpecification { span.log(timeStamp, expectedLogEvent) expect: - logsHandler.assertLogCalledWithArgs(timeStamp, expectedLogEvent) + logHandler.assertLogCalledWithArgs(timeStamp, expectedLogEvent) } def "should delegate simple logs with timestamp to logHandler"() { setup: - final LogsHandler logsHandler = new TestLogsHandler() + final LogHandler logHandler = new TestLogHandler() final String expectedName = "fakeName" final DDSpan span = tracer .buildSpan(expectedName) - .withLogsHandler(logsHandler) + .withLogHandler(logHandler) .withServiceName("foo") .start() final String expectedLogEvent = "fakeEvent" span.log(expectedLogEvent) expect: - logsHandler.assertLogCalledWithArgs(expectedLogEvent) + logHandler.assertLogCalledWithArgs(expectedLogEvent) } def "should delegate logs with fields to logHandler"() { setup: - final LogsHandler logsHandler = new TestLogsHandler() + final LogHandler logHandler = new TestLogHandler() final String expectedName = "fakeName" final DDSpan span = tracer .buildSpan(expectedName) - .withLogsHandler(logsHandler) + .withLogHandler(logHandler) .withServiceName("foo") .start() final Map fieldsMap = new HashMap<>() span.log(fieldsMap) expect: - logsHandler.assertLogCalledWithArgs(fieldsMap) + logHandler.assertLogCalledWithArgs(fieldsMap) } def "should delegate logs with fields and timestamp to logHandler"() { setup: - final LogsHandler logsHandler = new TestLogsHandler() + final LogHandler logHandler = new TestLogHandler() final String expectedName = "fakeName" final DDSpan span = tracer .buildSpan(expectedName) - .withLogsHandler(logsHandler) + .withLogHandler(logHandler) .withServiceName("foo") .start() final Map fieldsMap = new HashMap<>() @@ -573,11 +573,24 @@ class DDSpanBuilderTest extends DDSpecification { span.log(timeStamp, fieldsMap) expect: - logsHandler.assertLogCalledWithArgs(timeStamp, fieldsMap) + logHandler.assertLogCalledWithArgs(timeStamp, fieldsMap) } - private static class TestLogsHandler implements LogsHandler { + 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