From 7f8b81457e0dff4facc97bcfc1278ec9bcb7a5e1 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 12 Oct 2017 17:24:27 +0200 Subject: [PATCH 1/5] Adjust tag naming for better consistency --- dd-trace/src/main/java/com/datadoghq/trace/DDTags.java | 10 +++++----- .../groovy/com/datadoghq/trace/writer/DDApiTest.groovy | 5 +++-- .../com/datadoghq/trace/DDSpanSerializationTest.java | 4 ++-- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/dd-trace/src/main/java/com/datadoghq/trace/DDTags.java b/dd-trace/src/main/java/com/datadoghq/trace/DDTags.java index e543fec7132..8e50283fa89 100644 --- a/dd-trace/src/main/java/com/datadoghq/trace/DDTags.java +++ b/dd-trace/src/main/java/com/datadoghq/trace/DDTags.java @@ -1,10 +1,10 @@ package com.datadoghq.trace; public class DDTags { - public static final String SPAN_TYPE = "span-type"; - public static final String SERVICE_NAME = "service-name"; - public static final String RESOURCE_NAME = "resource-name"; - public static final String THREAD_NAME = "thread-name"; - public static final String THREAD_ID = "thread-id"; + public static final String SPAN_TYPE = "span.type"; + public static final String SERVICE_NAME = "service.name"; + public static final String RESOURCE_NAME = "resource.name"; + public static final String THREAD_NAME = "thread.name"; + public static final String THREAD_ID = "thread.id"; public static final String DB_STATEMENT = "sql.query"; } diff --git a/dd-trace/src/test/groovy/com/datadoghq/trace/writer/DDApiTest.groovy b/dd-trace/src/test/groovy/com/datadoghq/trace/writer/DDApiTest.groovy index 4afaff9fcba..76b00db1e00 100644 --- a/dd-trace/src/test/groovy/com/datadoghq/trace/writer/DDApiTest.groovy +++ b/dd-trace/src/test/groovy/com/datadoghq/trace/writer/DDApiTest.groovy @@ -1,5 +1,6 @@ package com.datadoghq.trace.writer +import com.datadoghq.trace.DDTags import com.datadoghq.trace.Service import com.datadoghq.trace.SpanFactory import com.fasterxml.jackson.core.type.TypeReference @@ -89,7 +90,7 @@ class DDApiTest extends Specification { [SpanFactory.newSpanOf(1L)] | [new TreeMap<>([ "duration" : 0, "error" : 0, - "meta" : ["thread-name": Thread.currentThread().getName(), "thread-id": "${Thread.currentThread().id}"], + "meta" : [(DDTags.THREAD_NAME): Thread.currentThread().getName(), (DDTags.THREAD_ID): "${Thread.currentThread().id}"], "name" : "fakeOperation", "parent_id": 0, "resource" : "fakeResource", @@ -102,7 +103,7 @@ class DDApiTest extends Specification { [SpanFactory.newSpanOf(100L)] | [new TreeMap<>([ "duration" : 0, "error" : 0, - "meta" : ["thread-name": Thread.currentThread().getName(), "thread-id": "${Thread.currentThread().id}"], + "meta" : [(DDTags.THREAD_NAME): Thread.currentThread().getName(), (DDTags.THREAD_ID): "${Thread.currentThread().id}"], "name" : "fakeOperation", "parent_id": 0, "resource" : "fakeResource", diff --git a/dd-trace/src/test/java/com/datadoghq/trace/DDSpanSerializationTest.java b/dd-trace/src/test/java/com/datadoghq/trace/DDSpanSerializationTest.java index 5fa85ec238e..2e2cc7f872f 100644 --- a/dd-trace/src/test/java/com/datadoghq/trace/DDSpanSerializationTest.java +++ b/dd-trace/src/test/java/com/datadoghq/trace/DDSpanSerializationTest.java @@ -50,8 +50,8 @@ public void setUp() throws Exception { null, null); - baggage.put("thread-name", Thread.currentThread().getName()); - baggage.put("thread-id", String.valueOf(Thread.currentThread().getId())); + baggage.put(DDTags.THREAD_NAME, Thread.currentThread().getName()); + baggage.put(DDTags.THREAD_ID, String.valueOf(Thread.currentThread().getId())); span = new DDSpan(100L, context); span.finish(133L); From af5ef5e624f80d58dfc0170bbaf55848b8566e6c Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 13 Oct 2017 14:08:03 +0200 Subject: [PATCH 2/5] Fix name overloading confusion. references to entries below were using the wrong instance and thus not being cleaned properly. --- .../com/datadoghq/agent/ClassLoaderIntegrationInjector.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/src/main/java/com/datadoghq/agent/ClassLoaderIntegrationInjector.java b/dd-java-agent/src/main/java/com/datadoghq/agent/ClassLoaderIntegrationInjector.java index e6ef832da10..4418c69217a 100644 --- a/dd-java-agent/src/main/java/com/datadoghq/agent/ClassLoaderIntegrationInjector.java +++ b/dd-java-agent/src/main/java/com/datadoghq/agent/ClassLoaderIntegrationInjector.java @@ -15,8 +15,8 @@ public class ClassLoaderIntegrationInjector { private final Map entries; private final Map invocationPoints = Maps.newConcurrentMap(); - public ClassLoaderIntegrationInjector(final Map entries) { - this.entries = Maps.newHashMap(entries); + public ClassLoaderIntegrationInjector(final Map allEntries) { + this.entries = Maps.newHashMap(allEntries); for (final Iterator> it = entries.entrySet().iterator(); it.hasNext(); ) { From aac945747b874cfac368e31c604f7ba627370059 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 13 Oct 2017 14:15:00 +0200 Subject: [PATCH 3/5] =?UTF-8?q?Rename=20=E2=80=9Cservice-name=E2=80=9D=20t?= =?UTF-8?q?o=20=E2=80=9Cmy-service-name=E2=80=9D=20to=20reduce=20confusion?= =?UTF-8?q?.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit same for resource-name --- .../com/datadoghq/trace/writer/DDApiTest.groovy | 2 +- .../trace/sampling/ExampleWithDDAgentWriter.java | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/dd-trace/src/test/groovy/com/datadoghq/trace/writer/DDApiTest.groovy b/dd-trace/src/test/groovy/com/datadoghq/trace/writer/DDApiTest.groovy index 76b00db1e00..ffe21673acf 100644 --- a/dd-trace/src/test/groovy/com/datadoghq/trace/writer/DDApiTest.groovy +++ b/dd-trace/src/test/groovy/com/datadoghq/trace/writer/DDApiTest.groovy @@ -186,7 +186,7 @@ class DDApiTest extends Specification { where: services | expectedRequestBody [:] | [:] - ["service-name": new Service("service-name", "app-name", Service.AppType.CUSTOM)] | ["service-name": new TreeMap<>([ + ["my-service-name": new Service("my-service-name", "app-name", Service.AppType.CUSTOM)] | ["my-service-name": new TreeMap<>([ "app" : "app-name", "app_type": "custom"]) ] diff --git a/dd-trace/src/test/java/com/datadoghq/trace/sampling/ExampleWithDDAgentWriter.java b/dd-trace/src/test/java/com/datadoghq/trace/sampling/ExampleWithDDAgentWriter.java index 826d99728b5..fb60275ce0e 100644 --- a/dd-trace/src/test/java/com/datadoghq/trace/sampling/ExampleWithDDAgentWriter.java +++ b/dd-trace/src/test/java/com/datadoghq/trace/sampling/ExampleWithDDAgentWriter.java @@ -7,24 +7,24 @@ public class ExampleWithDDAgentWriter { - public static void main(String[] args) throws Exception { + public static void main(final String[] args) throws Exception { // Instantiate the DDWriter // By default, traces are written to localhost:8126 (the ddagent) - Writer writer = new DDAgentWriter(); + final Writer writer = new DDAgentWriter(); // Instantiate the proper Sampler // - RateSampler if you want to keep `ratio` traces // - AllSampler to keep all traces - Sampler sampler = new AllSampler(); + final Sampler sampler = new AllSampler(); // Create the tracer - DDTracer tracer = new DDTracer(writer, sampler); + final DDTracer tracer = new DDTracer(writer, sampler); - Span parent = + final Span parent = tracer .buildSpan("hello-world") - .withServiceName("service-name") + .withServiceName("my-service-name") .withSpanType("web") .startManual(); @@ -32,11 +32,11 @@ public static void main(String[] args) throws Exception { parent.setBaggageItem("a-baggage", "value"); - Span child = + final Span child = tracer .buildSpan("hello-world") .asChildOf(parent) - .withResourceName("resource-name") + .withResourceName("my-resource-name") .startManual(); Thread.sleep(100); From 4ab32b0b9520fc38169f1158bd3fc9d49f6cf1be Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 13 Oct 2017 15:52:44 +0200 Subject: [PATCH 4/5] Add test for setting service/resource name via tag. --- .../com/datadoghq/trace/SpanFactory.groovy | 2 +- .../datadoghq/trace/writer/DDApiTest.groovy | 25 +++++++++---------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/dd-trace/src/test/groovy/com/datadoghq/trace/SpanFactory.groovy b/dd-trace/src/test/groovy/com/datadoghq/trace/SpanFactory.groovy index 7565b058a7e..45970d33f53 100644 --- a/dd-trace/src/test/groovy/com/datadoghq/trace/SpanFactory.groovy +++ b/dd-trace/src/test/groovy/com/datadoghq/trace/SpanFactory.groovy @@ -14,7 +14,7 @@ class SpanFactory { "fakeType", Collections.emptyMap(), null, - null) + new DDTracer()) return new DDSpan(timestampMicro, context) } diff --git a/dd-trace/src/test/groovy/com/datadoghq/trace/writer/DDApiTest.groovy b/dd-trace/src/test/groovy/com/datadoghq/trace/writer/DDApiTest.groovy index ffe21673acf..c5c55655c29 100644 --- a/dd-trace/src/test/groovy/com/datadoghq/trace/writer/DDApiTest.groovy +++ b/dd-trace/src/test/groovy/com/datadoghq/trace/writer/DDApiTest.groovy @@ -1,6 +1,5 @@ package com.datadoghq.trace.writer -import com.datadoghq.trace.DDTags import com.datadoghq.trace.Service import com.datadoghq.trace.SpanFactory import com.fasterxml.jackson.core.type.TypeReference @@ -85,28 +84,28 @@ class DDApiTest extends Specification { // Populate thread info dynamically as it is different when run via gradle vs idea. where: - traces | expectedRequestBody - [] | [] - [SpanFactory.newSpanOf(1L)] | [new TreeMap<>([ + traces | expectedRequestBody + [] | [] + [SpanFactory.newSpanOf(1L).setTag("service.name", "my-service")] | [new TreeMap<>([ "duration" : 0, "error" : 0, - "meta" : [(DDTags.THREAD_NAME): Thread.currentThread().getName(), (DDTags.THREAD_ID): "${Thread.currentThread().id}"], + "meta" : ["thread.name": Thread.currentThread().getName(), "thread.id": "${Thread.currentThread().id}"], "name" : "fakeOperation", "parent_id": 0, "resource" : "fakeResource", - "service" : "fakeService", + "service" : "my-service", "span_id" : 1, "start" : 1000, "trace_id" : 1, "type" : "fakeType" ])] - [SpanFactory.newSpanOf(100L)] | [new TreeMap<>([ + [SpanFactory.newSpanOf(100L).setTag("resource.name", "my-resource")] | [new TreeMap<>([ "duration" : 0, "error" : 0, - "meta" : [(DDTags.THREAD_NAME): Thread.currentThread().getName(), (DDTags.THREAD_ID): "${Thread.currentThread().id}"], + "meta" : ["thread.name": Thread.currentThread().getName(), "thread.id": "${Thread.currentThread().id}"], "name" : "fakeOperation", "parent_id": 0, - "resource" : "fakeResource", + "resource" : "my-resource", "service" : "fakeService", "span_id" : 1, "start" : 100000, @@ -184,10 +183,10 @@ class DDApiTest extends Specification { // Populate thread info dynamically as it is different when run via gradle vs idea. where: - services | expectedRequestBody - [:] | [:] - ["my-service-name": new Service("my-service-name", "app-name", Service.AppType.CUSTOM)] | ["my-service-name": new TreeMap<>([ - "app" : "app-name", + services | expectedRequestBody + [:] | [:] + ["my-service-name": new Service("my-service-name", "my-app-name", Service.AppType.CUSTOM)] | ["my-service-name": new TreeMap<>([ + "app" : "my-app-name", "app_type": "custom"]) ] } From 2775dd988eee514957a5d0110cdca36a544f30e7 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Fri, 13 Oct 2017 16:03:19 +0200 Subject: [PATCH 5/5] Hardcode additional tag names --- .../integration/servlet/JettyServletTest.groovy | 16 +++++++--------- .../integration/servlet/TomcatServletTest.groovy | 16 +++++++--------- .../agent/TraceAnnotationsManagerTest.java | 3 +-- .../trace/resolver/TracerResolverTest.java | 5 ++--- 4 files changed, 17 insertions(+), 23 deletions(-) diff --git a/dd-java-agent-ittests/src/test/groovy/com/datadoghq/agent/integration/servlet/JettyServletTest.groovy b/dd-java-agent-ittests/src/test/groovy/com/datadoghq/agent/integration/servlet/JettyServletTest.groovy index bbff95be0a8..a8ddd35d75e 100644 --- a/dd-java-agent-ittests/src/test/groovy/com/datadoghq/agent/integration/servlet/JettyServletTest.groovy +++ b/dd-java-agent-ittests/src/test/groovy/com/datadoghq/agent/integration/servlet/JettyServletTest.groovy @@ -1,10 +1,8 @@ package com.datadoghq.agent.integration.servlet import com.datadoghq.trace.DDBaseSpan -import com.datadoghq.trace.DDTags import com.datadoghq.trace.DDTracer import com.datadoghq.trace.writer.ListWriter -import io.opentracing.tag.Tags import io.opentracing.util.GlobalTracer import okhttp3.Interceptor import okhttp3.OkHttpClient @@ -101,13 +99,13 @@ class JettyServletTest extends Specification { span.context().operationName == "servlet.request" !span.context().getErrorFlag() span.context().parentId != 0 // parent should be the okhttp call. - span.context().tags[Tags.HTTP_URL.key] == "http://localhost:$PORT/$path" - span.context().tags[Tags.HTTP_METHOD.key] == "GET" - span.context().tags[Tags.SPAN_KIND.key] == Tags.SPAN_KIND_SERVER - span.context().tags[Tags.COMPONENT.key] == "java-web-servlet" - span.context().tags[Tags.HTTP_STATUS.key] == 200 - span.context().tags[DDTags.THREAD_NAME] != null - span.context().tags[DDTags.THREAD_ID] != null + span.context().tags["http.url"] == "http://localhost:$PORT/$path" + span.context().tags["http.method"] == "GET" + span.context().tags["span.kind"] == "server" + span.context().tags["component"] == "java-web-servlet" + span.context().tags["http.status_code"] == 200 + span.context().tags["thread.name"] != null + span.context().tags["thread.id"] != null span.context().tags.size() == 7 where: diff --git a/dd-java-agent-ittests/src/test/groovy/com/datadoghq/agent/integration/servlet/TomcatServletTest.groovy b/dd-java-agent-ittests/src/test/groovy/com/datadoghq/agent/integration/servlet/TomcatServletTest.groovy index 2f494f52eb4..336581f2c15 100644 --- a/dd-java-agent-ittests/src/test/groovy/com/datadoghq/agent/integration/servlet/TomcatServletTest.groovy +++ b/dd-java-agent-ittests/src/test/groovy/com/datadoghq/agent/integration/servlet/TomcatServletTest.groovy @@ -1,10 +1,8 @@ package com.datadoghq.agent.integration.servlet -import com.datadoghq.trace.DDTags import com.datadoghq.trace.DDTracer import com.datadoghq.trace.writer.ListWriter import com.google.common.io.Files -import io.opentracing.tag.Tags import io.opentracing.util.GlobalTracer import okhttp3.OkHttpClient import okhttp3.Request @@ -101,13 +99,13 @@ class TomcatServletTest extends Specification { span.context().operationName == "servlet.request" !span.context().getErrorFlag() span.context().parentId != 0 // parent should be the okhttp call. - span.context().tags[Tags.HTTP_URL.key] == "http://localhost:$PORT/$path" - span.context().tags[Tags.HTTP_METHOD.key] == "GET" - span.context().tags[Tags.SPAN_KIND.key] == Tags.SPAN_KIND_SERVER - span.context().tags[Tags.COMPONENT.key] == "java-web-servlet" - span.context().tags[Tags.HTTP_STATUS.key] == 200 - span.context().tags[DDTags.THREAD_NAME] != null - span.context().tags[DDTags.THREAD_ID] != null + span.context().tags["http.url"] == "http://localhost:$PORT/$path" + span.context().tags["http.method"] == "GET" + span.context().tags["span.kind"] == "server" + span.context().tags["component"] == "java-web-servlet" + span.context().tags["http.status_code"] == 200 + span.context().tags["thread.name"] != null + span.context().tags["thread.id"] != null span.context().tags.size() == 7 where: diff --git a/dd-java-agent-ittests/src/test/java/com/datadoghq/agent/TraceAnnotationsManagerTest.java b/dd-java-agent-ittests/src/test/java/com/datadoghq/agent/TraceAnnotationsManagerTest.java index f290c63645c..1a390678f6c 100644 --- a/dd-java-agent-ittests/src/test/java/com/datadoghq/agent/TraceAnnotationsManagerTest.java +++ b/dd-java-agent-ittests/src/test/java/com/datadoghq/agent/TraceAnnotationsManagerTest.java @@ -6,7 +6,6 @@ import com.datadoghq.trace.DDTracer; import com.datadoghq.trace.integration.ErrorFlag; import com.datadoghq.trace.writer.ListWriter; -import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; import java.lang.reflect.Field; import org.junit.Before; @@ -74,7 +73,7 @@ public void testExceptionExit() { // DO NOTHING } assertThat(writer.firstTrace().get(0).getOperationName()).isEqualTo("ERROR"); - assertThat(writer.firstTrace().get(0).getTags().get(Tags.ERROR.getKey())).isEqualTo("true"); + assertThat(writer.firstTrace().get(0).getTags().get("error")).isEqualTo("true"); assertThat(writer.firstTrace().get(0).getError()).isEqualTo(1); } } diff --git a/dd-trace/src/test/java/com/datadoghq/trace/resolver/TracerResolverTest.java b/dd-trace/src/test/java/com/datadoghq/trace/resolver/TracerResolverTest.java index 520c6e94229..015d37d211e 100644 --- a/dd-trace/src/test/java/com/datadoghq/trace/resolver/TracerResolverTest.java +++ b/dd-trace/src/test/java/com/datadoghq/trace/resolver/TracerResolverTest.java @@ -9,7 +9,6 @@ import io.opentracing.NoopTracerFactory; import io.opentracing.Tracer; import io.opentracing.contrib.tracerresolver.TracerResolver; -import io.opentracing.tag.Tags; import io.opentracing.util.GlobalTracer; import java.lang.reflect.Field; import java.util.List; @@ -23,7 +22,7 @@ public void testResolve() { final DDTracer tracer = (DDTracer) tracerResolver.resolve(); // for HTTP decorators - List decorators = tracer.getSpanContextDecorators(Tags.COMPONENT.getKey()); + List decorators = tracer.getSpanContextDecorators("component"); assertThat(decorators.size()).isEqualTo(2); AbstractDecorator decorator = decorators.get(0); @@ -34,7 +33,7 @@ public void testResolve() { assertThat(httpServiceDecorator.getSetValue()).isEqualTo("world"); // for URL decorators - decorators = tracer.getSpanContextDecorators(Tags.HTTP_URL.getKey()); + decorators = tracer.getSpanContextDecorators("http.url"); assertThat(decorators.size()).isEqualTo(1); decorator = decorators.get(0);