From d3e0323b04a2e5cf09a9db420bafbd5faecec113 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 30 Jul 2018 14:55:10 +1000 Subject: [PATCH 1/2] Allow DDTracerResolver to be disabled. --- .../java/datadog/opentracing/DDTracer.java | 14 ++-- .../resolver/DDTracerResolver.java | 13 +++- .../datadog/trace/common/DDTraceConfig.java | 34 +--------- .../datadog/trace/common/util/Config.java | 39 +++++++++++ .../resolver/TracerResolverTest.groovy | 65 +++++++++++++++++++ .../datadog/trace/DDTraceConfigTest.groovy | 7 +- .../resolver/TracerResolverTest.java | 44 ------------- 7 files changed, 128 insertions(+), 88 deletions(-) create mode 100644 dd-trace-ot/src/main/java/datadog/trace/common/util/Config.java create mode 100644 dd-trace-ot/src/test/groovy/datadog/opentracing/resolver/TracerResolverTest.groovy delete mode 100644 dd-trace-ot/src/test/java/datadog/opentracing/resolver/TracerResolverTest.java 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 0b4a74dd74..fe2d763128 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java @@ -1,5 +1,7 @@ package datadog.opentracing; +import static datadog.trace.common.util.Config.parseMap; + import datadog.opentracing.decorators.AbstractDecorator; import datadog.opentracing.decorators.DDDecoratorsFactory; import datadog.opentracing.propagation.Codec; @@ -90,9 +92,9 @@ public class DDTracer implements io.opentracing.Tracer { config.getProperty(DDTraceConfig.SERVICE_NAME), Writer.Builder.forConfig(config), Sampler.Builder.forConfig(config), - DDTraceConfig.parseMap(config.getProperty(DDTraceConfig.SPAN_TAGS)), - DDTraceConfig.parseMap(config.getProperty(DDTraceConfig.SERVICE_MAPPING)), - DDTraceConfig.parseMap(config.getProperty(DDTraceConfig.HEADER_TAGS))); + parseMap(config.getProperty(DDTraceConfig.SPAN_TAGS)), + parseMap(config.getProperty(DDTraceConfig.SERVICE_MAPPING)), + parseMap(config.getProperty(DDTraceConfig.HEADER_TAGS))); log.debug("Using config: {}", config); } @@ -161,9 +163,9 @@ public class DDTracer implements io.opentracing.Tracer { UNASSIGNED_DEFAULT_SERVICE_NAME, writer, new AllSampler(), - DDTraceConfig.parseMap(new DDTraceConfig().getProperty(DDTraceConfig.SPAN_TAGS)), - DDTraceConfig.parseMap(new DDTraceConfig().getProperty(DDTraceConfig.SERVICE_MAPPING)), - DDTraceConfig.parseMap(new DDTraceConfig().getProperty(DDTraceConfig.HEADER_TAGS))); + parseMap(new DDTraceConfig().getProperty(DDTraceConfig.SPAN_TAGS)), + parseMap(new DDTraceConfig().getProperty(DDTraceConfig.SERVICE_MAPPING)), + parseMap(new DDTraceConfig().getProperty(DDTraceConfig.HEADER_TAGS))); } /** diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/resolver/DDTracerResolver.java b/dd-trace-ot/src/main/java/datadog/opentracing/resolver/DDTracerResolver.java index 98fee6a923..b440b44684 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/resolver/DDTracerResolver.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/resolver/DDTracerResolver.java @@ -2,6 +2,7 @@ package datadog.opentracing.resolver; import com.google.auto.service.AutoService; import datadog.opentracing.DDTracer; +import datadog.trace.common.util.Config; import io.opentracing.Tracer; import io.opentracing.contrib.tracerresolver.TracerResolver; import io.opentracing.noop.NoopTracerFactory; @@ -11,6 +12,7 @@ import lombok.extern.slf4j.Slf4j; @Slf4j @AutoService(TracerResolver.class) public class DDTracerResolver extends TracerResolver { + static final String CONFIG_KEY = "dd.trace.resolver.enabled"; public static Tracer registerTracer() { final Tracer tracer = TracerResolver.resolveTracer(); @@ -27,8 +29,13 @@ public class DDTracerResolver extends TracerResolver { @Override protected Tracer resolve() { - log.info("Creating the Datadog Tracer from the resolver"); - - return new DDTracer(); + final boolean enabled = !"false".equalsIgnoreCase(Config.getPropOrEnv(CONFIG_KEY)); + if (enabled) { + log.info("Creating DDTracer with DDTracerResolver"); + return new DDTracer(); + } else { + log.info("DDTracerResolver disabled"); + return null; + } } } diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/DDTraceConfig.java b/dd-trace-ot/src/main/java/datadog/trace/common/DDTraceConfig.java index c1c4285db8..4587a8dd4b 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/DDTraceConfig.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/DDTraceConfig.java @@ -1,11 +1,10 @@ package datadog.trace.common; +import static datadog.trace.common.util.Config.getPropOrEnv; + import datadog.opentracing.DDTracer; import datadog.trace.common.writer.DDAgentWriter; import datadog.trace.common.writer.Writer; -import java.util.Collections; -import java.util.HashMap; -import java.util.Map; import java.util.Properties; import lombok.extern.slf4j.Slf4j; @@ -71,33 +70,4 @@ public class DDTraceConfig extends Properties { setProperty(key, value); } } - - private String getPropOrEnv(final String name) { - return System.getProperty(name, System.getenv(propToEnvName(name))); - } - - static String propToEnvName(final String name) { - return name.toUpperCase().replace(".", "_"); - } - - public static Map parseMap(final String str) { - if (str == null || str.trim().isEmpty()) { - return Collections.emptyMap(); - } - if (!str.matches("(([^,:]+:[^,:]+,)*([^,:]+:[^,:]+),?)?")) { - log.warn("Invalid config '{}'. Must match 'key1:value1,key2:value2'.", str); - return Collections.emptyMap(); - } - - final String[] tokens = str.split(",", -1); - final Map map = new HashMap<>(tokens.length + 1, 1f); - - for (final String token : tokens) { - final String[] keyValue = token.split(":", -1); - if (keyValue.length == 2) { - map.put(keyValue[0].trim(), keyValue[1].trim()); - } - } - return Collections.unmodifiableMap(map); - } } diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/util/Config.java b/dd-trace-ot/src/main/java/datadog/trace/common/util/Config.java new file mode 100644 index 0000000000..9c885785e5 --- /dev/null +++ b/dd-trace-ot/src/main/java/datadog/trace/common/util/Config.java @@ -0,0 +1,39 @@ +package datadog.trace.common.util; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import lombok.extern.slf4j.Slf4j; + +@Slf4j +public abstract class Config { + + public static String getPropOrEnv(final String name) { + return System.getProperty(name, System.getenv(propToEnvName(name))); + } + + public static String propToEnvName(final String name) { + return name.toUpperCase().replace(".", "_"); + } + + public static Map parseMap(final String str) { + if (str == null || str.trim().isEmpty()) { + return Collections.emptyMap(); + } + if (!str.matches("(([^,:]+:[^,:]+,)*([^,:]+:[^,:]+),?)?")) { + log.warn("Invalid config '{}'. Must match 'key1:value1,key2:value2'.", str); + return Collections.emptyMap(); + } + + final String[] tokens = str.split(",", -1); + final Map map = new HashMap<>(tokens.length + 1, 1f); + + for (final String token : tokens) { + final String[] keyValue = token.split(":", -1); + if (keyValue.length == 2) { + map.put(keyValue[0].trim(), keyValue[1].trim()); + } + } + return Collections.unmodifiableMap(map); + } +} diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/resolver/TracerResolverTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/resolver/TracerResolverTest.groovy new file mode 100644 index 0000000000..53410b7481 --- /dev/null +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/resolver/TracerResolverTest.groovy @@ -0,0 +1,65 @@ +package datadog.opentracing.resolver + +import datadog.opentracing.DDTracer +import io.opentracing.Tracer +import io.opentracing.contrib.tracerresolver.TracerResolver +import io.opentracing.noop.NoopTracer +import io.opentracing.noop.NoopTracerFactory +import io.opentracing.util.GlobalTracer +import spock.lang.Specification + +import java.lang.reflect.Field + +class TracerResolverTest extends Specification { + + def setup() { + setTracer(null) + assert !GlobalTracer.isRegistered() + } + + def "test resolveTracer"() { + when: + def tracer = TracerResolver.resolveTracer() + + then: + !GlobalTracer.isRegistered() + tracer instanceof DDTracer + } + + def "test registerTracer"() { + when: + def tracer = DDTracerResolver.registerTracer() + + then: + GlobalTracer.isRegistered() + tracer instanceof DDTracer + } + + def "test disable DDTracerResolver"() { + setup: + System.setProperty("dd.trace.resolver.enabled", "false") + + when: + def tracer = TracerResolver.resolveTracer() + + then: + !GlobalTracer.isRegistered() + tracer == null + + when: + tracer = DDTracerResolver.registerTracer() + + then: + !GlobalTracer.isRegistered() + tracer instanceof NoopTracer + + cleanup: + System.clearProperty("dd.trace.resolver.enabled") + } + + def setTracer(Tracer tracer) { + final Field tracerField = GlobalTracer.getDeclaredField("tracer") + tracerField.setAccessible(true) + tracerField.set(tracer, NoopTracerFactory.create()) + } +} diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/DDTraceConfigTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/DDTraceConfigTest.groovy index fdd35123f3..ed925628d7 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/DDTraceConfigTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/DDTraceConfigTest.groovy @@ -19,7 +19,8 @@ import static datadog.trace.common.DDTraceConfig.SERVICE_MAPPING import static datadog.trace.common.DDTraceConfig.SERVICE_NAME import static datadog.trace.common.DDTraceConfig.SPAN_TAGS import static datadog.trace.common.DDTraceConfig.WRITER_TYPE -import static datadog.trace.common.DDTraceConfig.propToEnvName +import static datadog.trace.common.util.Config.parseMap +import static datadog.trace.common.util.Config.propToEnvName class DDTraceConfigTest extends Specification { @Rule @@ -150,7 +151,7 @@ class DDTraceConfigTest extends Specification { def "parsing valid string returns a map"() { expect: - DDTraceConfig.parseMap(str) == map + parseMap(str) == map where: str | map @@ -163,7 +164,7 @@ class DDTraceConfigTest extends Specification { def "parsing an invalid string returns an empty map"() { expect: - DDTraceConfig.parseMap(str) == [:] + parseMap(str) == [:] where: str | _ diff --git a/dd-trace-ot/src/test/java/datadog/opentracing/resolver/TracerResolverTest.java b/dd-trace-ot/src/test/java/datadog/opentracing/resolver/TracerResolverTest.java deleted file mode 100644 index c45ca488c1..0000000000 --- a/dd-trace-ot/src/test/java/datadog/opentracing/resolver/TracerResolverTest.java +++ /dev/null @@ -1,44 +0,0 @@ -package datadog.opentracing.resolver; - -import static org.assertj.core.api.Assertions.assertThat; - -import datadog.opentracing.DDTracer; -import io.opentracing.Tracer; -import io.opentracing.contrib.tracerresolver.TracerResolver; -import io.opentracing.noop.NoopTracerFactory; -import io.opentracing.util.GlobalTracer; -import java.lang.reflect.Field; -import org.junit.Test; - -public class TracerResolverTest { - - @Test - public void testResolveTracer() throws Exception { - final Field tracerField = GlobalTracer.class.getDeclaredField("tracer"); - tracerField.setAccessible(true); - tracerField.set(null, NoopTracerFactory.create()); - - assertThat(GlobalTracer.isRegistered()).isFalse(); - - final Tracer tracer = TracerResolver.resolveTracer(); - - assertThat(GlobalTracer.isRegistered()).isFalse(); - assertThat(tracer).isInstanceOf(DDTracer.class); - } - - @Test - public void testRegisterTracer() throws Exception { - final Field tracerField = GlobalTracer.class.getDeclaredField("tracer"); - tracerField.setAccessible(true); - tracerField.set(null, NoopTracerFactory.create()); - - assertThat(GlobalTracer.isRegistered()).isFalse(); - - DDTracerResolver.registerTracer(); - - assertThat(GlobalTracer.isRegistered()).isTrue(); - assertThat(tracerField.get(null)).isInstanceOf(DDTracer.class); - - tracerField.set(null, NoopTracerFactory.create()); - } -} From 2dd62bbab10971346c78f7102e140f55fc8ad782 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Tue, 31 Jul 2018 10:17:05 +1000 Subject: [PATCH 2/2] Optimization to avoid serialization when logging disabled. --- .../datadog/trace/common/writer/LoggingWriter.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/LoggingWriter.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/LoggingWriter.java index c1bb70c0a0..0da4839d1d 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/LoggingWriter.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/LoggingWriter.java @@ -11,10 +11,12 @@ public class LoggingWriter implements Writer { @Override public void write(final List trace) { - try { - log.info("write(trace): {}", serializer.writeValueAsString(trace)); - } catch (final Exception e) { - log.error("error writing(trace): {}", trace); + if (log.isInfoEnabled()) { + try { + log.info("write(trace): {}", serializer.writeValueAsString(trace)); + } catch (final Exception e) { + log.error("error writing(trace): {}", trace); + } } }