diff --git a/.circleci/collect_artifacts.sh b/.circleci/collect_artifacts.sh deleted file mode 100755 index 2d9cd40f6b2..00000000000 --- a/.circleci/collect_artifacts.sh +++ /dev/null @@ -1,53 +0,0 @@ -#!/bin/bash - -# Save all important reports and artifacts into (project-root)/build -# This folder will be saved by circleci and available after test runs. - -REPORTS_DIR=./reports -mkdir -p $REPORTS_DIR >/dev/null 2>&1 - -ARTIFACT_DIR=./artifacts/ -mkdir -p $ARTIFACT_DIR >/dev/null 2>&1 - -function save_reports () { - project_to_save=$1 - if [ -d workspace/$project_to_save/build/reports ]; then - report_path=$REPORTS_DIR/$project_to_save/reports - mkdir -p $report_path - cp -r workspace/$project_to_save/build/reports/* $report_path/ - fi -} - -function save_libs () { - project_to_save=$1 - if [ -d workspace/$project_to_save/build/libs ]; then - libs_path=$ARTIFACT_DIR/$project_to_save/libs - mkdir -p $libs_path - cp -r workspace/$project_to_save/build/libs/* $libs_path/ - fi -} - - -function save_results () { - if [ -d workspace/$project_to_save/build/test-results ]; then - mkdir -p $REPORTS_DIR/results - find workspace/**/build/test-results -name \*.xml -exec cp {} $REPORTS_DIR/results \; - fi -} - -save_reports dd-java-agent -save_reports dd-java-agent/tooling -save_reports dd-java-agent/testing -# Save reports for all instrumentation projects -for integration_path in dd-java-agent/instrumentation/*; do - save_reports $integration_path -done -save_reports dd-java-agent-ittests -save_reports dd-trace-api -save_reports dd-trace-ot - -save_libs dd-java-agent -save_libs dd-trace-api -save_libs dd-trace-ot - -save_results diff --git a/.circleci/collect_libs.sh b/.circleci/collect_libs.sh new file mode 100755 index 00000000000..015e9651aa3 --- /dev/null +++ b/.circleci/collect_libs.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash + +# Save all important libs into (project-root)/libs +# This folder will be saved by circleci and available after test runs. + +set -e + +LIBS_DIR=./libs/ +mkdir -p $LIBS_DIR >/dev/null 2>&1 + +for lib_path in workspace/**/build/libs; do + echo "saving libs in $lib_path" + cp $lib_path/*.jar $LIBS_DIR/ +done diff --git a/.circleci/collect_reports.sh b/.circleci/collect_reports.sh new file mode 100755 index 00000000000..72af9d73e1a --- /dev/null +++ b/.circleci/collect_reports.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash + +# Save all important reports into (project-root)/reports +# This folder will be saved by circleci and available after test runs. + +set -e + +REPORTS_DIR=./reports +mkdir -p $REPORTS_DIR >/dev/null 2>&1 + +function save_reports () { + project_to_save=$1 + echo "saving reports for $project_to_save" + + report_path=$REPORTS_DIR/$project_to_save + mkdir -p $report_path + cp -r workspace/$project_to_save/build/reports/* $report_path/ +} + +shopt -s globstar + +for report_path in workspace/**/build/reports; do + report_path=${report_path//workspace\//} + report_path=${report_path//\/build\/reports/} + save_reports $report_path +done diff --git a/.circleci/collect_results.sh b/.circleci/collect_results.sh new file mode 100755 index 00000000000..13072caf047 --- /dev/null +++ b/.circleci/collect_results.sh @@ -0,0 +1,13 @@ +#!/usr/bin/env bash + +# Save all important reports and artifacts into (project-root)/results +# This folder will be saved by circleci and available after test runs. + +set -e + +TEST_RESULTS_DIR=./results +mkdir -p $TEST_RESULTS_DIR >/dev/null 2>&1 + +echo "saving test results" +mkdir -p $TEST_RESULTS_DIR/results +find workspace/**/build/test-results -name \*.xml -exec cp {} $TEST_RESULTS_DIR \; diff --git a/.circleci/config.yml b/.circleci/config.yml index d7183e864d4..6c5945ff308 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -29,7 +29,23 @@ jobs: - run: name: Build Project - command: GRADLE_OPTS="-Dorg.gradle.jvmargs=-Xmx2G -Xms512M" ./gradlew clean compileTestGroovy compileTestScala compileTestJava check -x test -x traceAgentTest --build-cache --parallel --stacktrace --no-daemon --max-workers=6 + command: GRADLE_OPTS="-Dorg.gradle.jvmargs=-Xmx2G -Xms512M" ./gradlew clean :dd-java-agent:shadowJar compileTestGroovy compileTestScala compileTestJava check -x test -x traceAgentTest --build-cache --parallel --stacktrace --no-daemon --max-workers=6 + + - run: + name: Collect Libs + when: always + command: .circleci/collect_libs.sh + + - store_artifacts: + path: ./libs + + - run: + name: Collect Reports + when: always + command: .circleci/collect_reports.sh + + - store_artifacts: + path: ./reports - persist_to_workspace: root: . @@ -69,15 +85,20 @@ jobs: command: GRADLE_OPTS="-Dorg.gradle.jvmargs=-Xmx2G -Xms512M" ./gradlew $TEST_TASK --build-cache --parallel --stacktrace --no-daemon --max-workers=3 - run: - name: Collect Artifacts + name: Collect Reports when: always - command: .circleci/collect_artifacts.sh + command: .circleci/collect_reports.sh - - store_test_results: + - store_artifacts: path: ./reports - - store_artifacts: - path: ./artifacts + - run: + name: Collect Test Results + when: always + command: .circleci/collect_results.sh + + - store_test_results: + path: ./results test_7: <<: *default_test_job @@ -128,13 +149,21 @@ jobs: command: ./gradlew traceAgentTest --build-cache --parallel --stacktrace --no-daemon --max-workers=6 - run: - name: Collect Artifacts + name: Collect Reports when: always - command: .circleci/collect_artifacts.sh + command: .circleci/collect_reports.sh - - store_test_results: + - store_artifacts: path: ./reports + - run: + name: Collect Test Results + when: always + command: .circleci/collect_results.sh + + - store_test_results: + path: ./results + scan_versions: <<: *defaults steps: @@ -150,17 +179,6 @@ jobs: name: Verify Version Scan command: ./gradlew verifyVersionScan --parallel --stacktrace --no-daemon --max-workers=6 - - run: - name: Collect Artifacts - when: always - command: .circleci/collect_artifacts.sh - - - store_test_results: - path: ./reports - - - store_artifacts: - path: ./artifacts - - save_cache: key: dd-trace-java-version-scan-{{ checksum "dd-trace-java.gradle" }} paths: ~/.gradle diff --git a/dd-java-agent/instrumentation/servlet-2/servlet-2.gradle b/dd-java-agent/instrumentation/servlet-2/servlet-2.gradle index f4f1d268529..9ca823870c5 100644 --- a/dd-java-agent/instrumentation/servlet-2/servlet-2.gradle +++ b/dd-java-agent/instrumentation/servlet-2/servlet-2.gradle @@ -28,6 +28,5 @@ dependencies { testCompile group: 'org.eclipse.jetty', name: 'jetty-server', version: '7.0.0.v20091005' testCompile group: 'org.eclipse.jetty', name: 'jetty-servlet', version: '7.0.0.v20091005' - testCompile project(':dd-java-agent:instrumentation:okhttp-3') // used in the tests testCompile group: 'com.squareup.okhttp3', name: 'okhttp', version: '3.6.0' } diff --git a/dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServletTest.groovy b/dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServletTest.groovy index 0e095240a1e..2d1425515b7 100644 --- a/dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServletTest.groovy +++ b/dd-java-agent/instrumentation/servlet-2/src/test/groovy/JettyServletTest.groovy @@ -93,7 +93,8 @@ class JettyServletTest extends AgentTestRunner { expect: response.body().string().trim() == expectedResponse - writer.size() == 2 // second (parent) trace is the okhttp call above... + writer.waitForTraces(1) + writer.size() == 1 def trace = writer.firstTrace() trace.size() == 1 def span = trace[0] @@ -103,7 +104,7 @@ class JettyServletTest extends AgentTestRunner { span.context().resourceName == "GET /$path" span.context().spanType == DDSpanTypes.WEB_SERVLET !span.context().getErrorFlag() - span.context().parentId != 0 // parent should be the okhttp call. + span.context().parentId == 0 span.context().tags["http.url"] == "http://localhost:$PORT/$path" span.context().tags["http.method"] == "GET" span.context().tags["span.kind"] == "server" @@ -129,7 +130,8 @@ class JettyServletTest extends AgentTestRunner { expect: response.body().string().trim() != expectedResponse - writer.size() == 2 // second (parent) trace is the okhttp call above... + writer.waitForTraces(1) + writer.size() == 1 def trace = writer.firstTrace() trace.size() == 1 def span = trace[0] @@ -138,7 +140,7 @@ class JettyServletTest extends AgentTestRunner { span.context().resourceName == "GET /$path" span.context().spanType == DDSpanTypes.WEB_SERVLET span.context().getErrorFlag() - span.context().parentId != 0 // parent should be the okhttp call. + span.context().parentId == 0 span.context().tags["http.url"] == "http://localhost:$PORT/$path" span.context().tags["http.method"] == "GET" span.context().tags["span.kind"] == "server" diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServletTest.groovy b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServletTest.groovy index b6fcf4a85f0..2fd8a18f233 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServletTest.groovy +++ b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/JettyServletTest.groovy @@ -94,6 +94,7 @@ class JettyServletTest extends AgentTestRunner { expect: response.body().string().trim() == expectedResponse + writer.waitForTraces(1) writer.size() == 1 def trace = writer.firstTrace() trace.size() == 1 @@ -131,6 +132,7 @@ class JettyServletTest extends AgentTestRunner { expect: response.body().string().trim() != expectedResponse + writer.waitForTraces(1) writer.size() == 1 def trace = writer.firstTrace() trace.size() == 1 @@ -172,6 +174,7 @@ class JettyServletTest extends AgentTestRunner { expect: response.body().string().trim() != expectedResponse + writer.waitForTraces(1) writer.size() == 1 def trace = writer.firstTrace() trace.size() == 1 diff --git a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServletTest.groovy b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServletTest.groovy index cde141b5fe7..247feb06e62 100644 --- a/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServletTest.groovy +++ b/dd-java-agent/instrumentation/servlet-3/src/test/groovy/TomcatServletTest.groovy @@ -93,6 +93,7 @@ class TomcatServletTest extends AgentTestRunner { expect: response.body().string().trim() == expectedResponse + writer.waitForTraces(1) writer.size() == 1 def trace = writer.firstTrace() trace.size() == 1 @@ -130,6 +131,7 @@ class TomcatServletTest extends AgentTestRunner { expect: response.body().string().trim() != expectedResponse + writer.waitForTraces(1) writer.size() == 1 def trace = writer.firstTrace() trace.size() == 1 @@ -171,6 +173,7 @@ class TomcatServletTest extends AgentTestRunner { expect: response.body().string().trim() != expectedResponse + writer.waitForTraces(1) writer.size() == 1 def trace = writer.firstTrace() trace.size() == 1 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 4b4ea6b1cb2..793297ac091 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java @@ -1,6 +1,5 @@ package datadog.opentracing; -import com.fasterxml.jackson.annotation.JsonIgnore; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import datadog.opentracing.decorators.AbstractDecorator; @@ -15,7 +14,6 @@ import datadog.trace.api.interceptor.TraceInterceptor; import datadog.trace.api.sampling.PrioritySampling; import datadog.trace.common.DDTraceConfig; -import datadog.trace.common.Service; import datadog.trace.common.sampling.AllSampler; import datadog.trace.common.sampling.RateByServiceSampler; import datadog.trace.common.sampling.Sampler; @@ -41,6 +39,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentSkipListSet; import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.atomic.AtomicInteger; import lombok.extern.slf4j.Slf4j; /** DDTracer makes it easy to send traces and span to DD using the OpenTracing API. */ @@ -74,7 +73,8 @@ public int compare(final TraceInterceptor o1, final TraceInterceptor o2) { } }); private final CodecRegistry registry; - private final Map services = new HashMap<>(); + + private final AtomicInteger traceCount = new AtomicInteger(0); /** By default, report to local agent and collect all traces. */ public DDTracer() { @@ -115,6 +115,7 @@ public DDTracer( if (this.writer instanceof DDAgentWriter && sampler instanceof DDApi.ResponseListener) { final DDApi api = ((DDAgentWriter) this.writer).getApi(); api.addResponseListener((DDApi.ResponseListener) this.sampler); + api.addTraceCounter(traceCount); } registerClassLoader(ClassLoader.getSystemClassLoader()); @@ -256,6 +257,7 @@ void write(final PendingTrace trace) { } } } + traceCount.incrementAndGet(); if (!writtenTrace.isEmpty() && this.sampler.sample(writtenTrace.get(0))) { this.writer.write(writtenTrace); } @@ -280,33 +282,6 @@ public String toString() { + '}'; } - /** - * Register additional information about a service. Service additional information are a Datadog - * feature only. Services are reported through a specific Datadog endpoint. - * - * @param service additional service information - */ - public void addServiceInfo(final Service service) { - services.put(service.getName(), service); - // Update the writer - try { - // We don't bother to send multiple times the list of services at this time - writer.writeServices(services); - } catch (final Throwable ex) { - log.warn("Failed to report additional service information, reason: {}", ex.getMessage()); - } - } - - /** - * Return the list of additional service information registered - * - * @return the list of additional service information - */ - @JsonIgnore - public Map getServiceInfo() { - return services; - } - private static class CodecRegistry { private final Map, Codec> codecs = new HashMap<>(); diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java index 1ef15eb2c4e..ac785d4ee7b 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDAgentWriter.java @@ -3,9 +3,7 @@ import com.google.auto.service.AutoService; import com.google.common.util.concurrent.ThreadFactoryBuilder; import datadog.opentracing.DDSpan; -import datadog.trace.common.Service; import java.util.List; -import java.util.Map; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -91,28 +89,6 @@ public void write(final List trace) { queueFullReported = false; } - /* (non-Javadoc) - * @see datadog.trace.Writer#writeServices(java.util.List) - */ - @Override - public void writeServices(final Map services) { - - final Runnable task = - new Runnable() { - @Override - public void run() { - // SEND the payload to the agent - log.debug("Async writer about to write {} services", services.size()); - if (api.sendServices(services)) { - log.debug("Async writer just sent {} services", services.size()); - } else { - log.warn("Failed for Async writer to send {} services", services.size()); - } - } - }; - executor.submit(task); - } - /* (non-Javadoc) * @see Writer#start() */ diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java index 203c1507b30..a426b72c155 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/DDApi.java @@ -5,7 +5,6 @@ import com.google.common.util.concurrent.RateLimiter; import datadog.opentracing.DDSpan; import datadog.opentracing.DDTraceOTInfo; -import datadog.trace.common.Service; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStreamReader; @@ -15,46 +14,44 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import lombok.extern.slf4j.Slf4j; import org.msgpack.jackson.dataformat.MessagePackFactory; /** The API pointing to a DD agent */ @Slf4j public class DDApi { + private static final String DATADOG_META_LANG = "Datadog-Meta-Lang"; + private static final String DATADOG_META_LANG_VERSION = "Datadog-Meta-Lang-Version"; + private static final String DATADOG_META_LANG_INTERPRETER = "Datadog-Meta-Lang-Interpreter"; + private static final String DATADOG_META_TRACER_VERSION = "Datadog-Meta-Tracer-Version"; + private static final String X_DATADOG_TRACE_COUNT = "X-Datadog-Trace-Count"; private static final String TRACES_ENDPOINT_V3 = "/v0.3/traces"; - private static final String SERVICES_ENDPOINT_V3 = "/v0.3/services"; private static final String TRACES_ENDPOINT_V4 = "/v0.4/traces"; - private static final String SERVICES_ENDPOINT_V4 = "/v0.4/services"; private static final long SECONDS_BETWEEN_ERROR_LOG = TimeUnit.MINUTES.toSeconds(5); private final String tracesEndpoint; - private final String servicesEndpoint; private final List responseListeners = new ArrayList<>(); + private AtomicInteger traceCount; + private final RateLimiter loggingRateLimiter = RateLimiter.create(1.0 / SECONDS_BETWEEN_ERROR_LOG); private static final ObjectMapper objectMapper = new ObjectMapper(new MessagePackFactory()); public DDApi(final String host, final int port) { - this( - host, - port, - traceEndpointAvailable("http://" + host + ":" + port + TRACES_ENDPOINT_V4) - && serviceEndpointAvailable("http://" + host + ":" + port + SERVICES_ENDPOINT_V4)); + this(host, port, traceEndpointAvailable("http://" + host + ":" + port + TRACES_ENDPOINT_V4)); } DDApi(final String host, final int port, final boolean v4EndpointsAvailable) { if (v4EndpointsAvailable) { this.tracesEndpoint = "http://" + host + ":" + port + TRACES_ENDPOINT_V4; - this.servicesEndpoint = "http://" + host + ":" + port + SERVICES_ENDPOINT_V4; } else { log.debug("API v0.4 endpoints not available. Downgrading to v0.3"); this.tracesEndpoint = "http://" + host + ":" + port + TRACES_ENDPOINT_V3; - this.servicesEndpoint = "http://" + host + ":" + port + SERVICES_ENDPOINT_V3; } } @@ -64,6 +61,10 @@ public void addResponseListener(final ResponseListener listener) { } } + public void addTraceCounter(final AtomicInteger traceCount) { + this.traceCount = traceCount; + } + /** * Send traces to the DD agent * @@ -71,34 +72,13 @@ public void addResponseListener(final ResponseListener listener) { * @return the staus code returned */ public boolean sendTraces(final List> traces) { - return putContent("traces", tracesEndpoint, traces, traces.size()); - } - - /** - * Send service extra information to the services endpoint - * - * @param services the services to be sent - */ - public boolean sendServices(final Map services) { - if (services == null) { - return true; - } - return putContent("services", servicesEndpoint, services, services.size()); - } - - /** - * PUT to an endpoint the provided JSON content - * - * @param content - * @return the status code - */ - private boolean putContent( - final String type, final String endpoint, final Object content, final int size) { + final int totalSize = traceCount == null ? traces.size() : traceCount.getAndSet(0); try { - final HttpURLConnection httpCon = getHttpURLConnection(endpoint); + final HttpURLConnection httpCon = getHttpURLConnection(tracesEndpoint); + httpCon.setRequestProperty(X_DATADOG_TRACE_COUNT, String.valueOf(totalSize)); final OutputStream out = httpCon.getOutputStream(); - objectMapper.writeValue(out, content); + objectMapper.writeValue(out, traces); out.flush(); out.close(); @@ -121,16 +101,16 @@ private boolean putContent( if (responseCode != 200) { if (log.isDebugEnabled()) { log.debug( - "Error while sending {} {} to the DD agent. Status: {}, ResponseMessage: ", - size, - type, + "Error while sending {} of {} traces to the DD agent. Status: {}, ResponseMessage: ", + traces.size(), + totalSize, responseCode, httpCon.getResponseMessage()); } else if (loggingRateLimiter.tryAcquire()) { log.warn( - "Error while sending {} {} to the DD agent. Status: {} (going silent for {} seconds)", - size, - type, + "Error while sending {} of {} traces to the DD agent. Status: {} (going silent for {} seconds)", + traces.size(), + totalSize, responseCode, httpCon.getResponseMessage(), SECONDS_BETWEEN_ERROR_LOG); @@ -138,7 +118,7 @@ private boolean putContent( return false; } - log.debug("Succesfully sent {} {} to the DD agent.", size, type); + log.debug("Succesfully sent {} of {} traces to the DD agent.", traces.size(), totalSize); try { if (null != responseString @@ -146,7 +126,7 @@ private boolean putContent( && !"OK".equalsIgnoreCase(responseString.trim())) { final JsonNode response = objectMapper.readTree(responseString); for (final ResponseListener listener : responseListeners) { - listener.onResponse(endpoint, response); + listener.onResponse(tracesEndpoint, response); } } } catch (final IOException e) { @@ -156,12 +136,18 @@ private boolean putContent( } catch (final IOException e) { if (log.isDebugEnabled()) { - log.debug("Error while sending " + size + " " + type + " to the DD agent.", e); + log.debug( + "Error while sending " + + traces.size() + + " of " + + totalSize + + " traces to the DD agent.", + e); } else if (loggingRateLimiter.tryAcquire()) { log.warn( - "Error while sending {} {} to the DD agent. {}: {} (going silent for {} seconds)", - size, - type, + "Error while sending {} of {} traces to the DD agent. {}: {} (going silent for {} seconds)", + traces.size(), + totalSize, e.getClass().getName(), e.getMessage(), SECONDS_BETWEEN_ERROR_LOG); @@ -208,10 +194,11 @@ private static HttpURLConnection getHttpURLConnection(final String endpoint) thr httpCon.setDoInput(true); httpCon.setRequestMethod("PUT"); httpCon.setRequestProperty("Content-Type", "application/msgpack"); - httpCon.setRequestProperty("Datadog-Meta-Lang", "java"); - httpCon.setRequestProperty("Datadog-Meta-Lang-Version", DDTraceOTInfo.JAVA_VERSION); - httpCon.setRequestProperty("Datadog-Meta-Lang-Interpreter", DDTraceOTInfo.JAVA_VM_NAME); - httpCon.setRequestProperty("Datadog-Meta-Tracer-Version", DDTraceOTInfo.VERSION); + httpCon.setRequestProperty(DATADOG_META_LANG, "java"); + httpCon.setRequestProperty(DATADOG_META_LANG_VERSION, DDTraceOTInfo.JAVA_VERSION); + httpCon.setRequestProperty(DATADOG_META_LANG_INTERPRETER, DDTraceOTInfo.JAVA_VM_NAME); + httpCon.setRequestProperty(DATADOG_META_TRACER_VERSION, DDTraceOTInfo.VERSION); + return httpCon; } diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ListWriter.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ListWriter.java index ec77537c8b4..14846e1b0bb 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/ListWriter.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/ListWriter.java @@ -1,10 +1,8 @@ package datadog.trace.common.writer; import datadog.opentracing.DDSpan; -import datadog.trace.common.Service; import java.util.LinkedList; import java.util.List; -import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -45,11 +43,6 @@ public void waitForTraces(final int number) throws InterruptedException, Timeout } } - @Override - public void writeServices(final Map services) { - throw new UnsupportedOperationException(); - } - @Override public void start() { close(); 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 edfa595b64c..1a541401d31 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 @@ -3,9 +3,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.auto.service.AutoService; import datadog.opentracing.DDSpan; -import datadog.trace.common.Service; import java.util.List; -import java.util.Map; import lombok.extern.slf4j.Slf4j; @Slf4j @@ -17,16 +15,11 @@ public class LoggingWriter implements Writer { public void write(final List trace) { try { log.info("write(trace): {}", serializer.writeValueAsString(trace)); - } catch (Exception e) { + } catch (final Exception e) { log.error("error writing(trace): {}", trace); } } - @Override - public void writeServices(final Map services) { - log.info("additional service information: {}", services.values()); - } - @Override public void close() { log.info("close()"); diff --git a/dd-trace-ot/src/main/java/datadog/trace/common/writer/Writer.java b/dd-trace-ot/src/main/java/datadog/trace/common/writer/Writer.java index afa89fddaa8..52df8fd0f4a 100644 --- a/dd-trace-ot/src/main/java/datadog/trace/common/writer/Writer.java +++ b/dd-trace-ot/src/main/java/datadog/trace/common/writer/Writer.java @@ -2,9 +2,7 @@ import datadog.opentracing.DDSpan; import datadog.trace.common.DDTraceConfig; -import datadog.trace.common.Service; import java.util.List; -import java.util.Map; import java.util.Properties; import lombok.extern.slf4j.Slf4j; @@ -20,13 +18,6 @@ public interface Writer { */ void write(List trace); - /** - * Report additional service information to the endpoint - * - * @param services a list of extra information about services - */ - void writeServices(Map services); - /** Start the writer */ void start(); diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/PendingTraceTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/PendingTraceTest.groovy index 2f653e1018e..f83da97aee4 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/PendingTraceTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/PendingTraceTest.groovy @@ -8,6 +8,7 @@ import spock.lang.Subject class PendingTraceTest extends Specification { def writer = new ListWriter() def tracer = new DDTracer(writer) + def traceCount = tracer.traceCount def traceId = System.identityHashCode(this) @@ -30,6 +31,7 @@ class PendingTraceTest extends Specification { expect: trace.asList() == [rootSpan] writer == [[rootSpan]] + traceCount.get() == 1 } def "child finishes before parent"() { @@ -57,6 +59,7 @@ class PendingTraceTest extends Specification { trace.weakReferences.size() == 0 trace.asList() == [rootSpan, child] writer == [[rootSpan, child]] + traceCount.get() == 1 } def "parent finishes before child which holds up trace"() { @@ -84,6 +87,7 @@ class PendingTraceTest extends Specification { trace.weakReferences.size() == 0 trace.asList() == [child, rootSpan] writer == [[child, rootSpan]] + traceCount.get() == 1 } def "trace reported when unfinished child discarded"() { @@ -108,6 +112,7 @@ class PendingTraceTest extends Specification { trace.weakReferences.size() == 0 trace.asList() == [rootSpan] writer == [[rootSpan]] + traceCount.get() == 1 } def "add unfinished span to trace fails"() { @@ -118,6 +123,7 @@ class PendingTraceTest extends Specification { trace.pendingReferenceCount.get() == 1 trace.weakReferences.size() == 1 trace.asList() == [] + traceCount.get() == 0 } def "register span to wrong trace fails"() { diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/ServiceTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/ServiceTest.groovy deleted file mode 100644 index b2e82b67e1a..00000000000 --- a/dd-trace-ot/src/test/groovy/datadog/trace/ServiceTest.groovy +++ /dev/null @@ -1,72 +0,0 @@ -package datadog.trace - -import datadog.opentracing.DDTracer -import datadog.trace.common.Service -import datadog.trace.common.sampling.AllSampler -import datadog.trace.common.writer.DDAgentWriter -import spock.lang.Specification -import spock.lang.Timeout - -import static org.mockito.ArgumentMatchers.any -import static org.mockito.Mockito.* - -@Timeout(1) -class ServiceTest extends Specification { - - - def "getter and setter"() { - - setup: - def service = new Service("api-intake", "kafka", Service.AppType.CUSTOM) - - expect: - service.getName() == "api-intake" - service.getAppName() == "kafka" - service.getAppType() == Service.AppType.CUSTOM - service.toString() == "Service { name='api-intake', appName='kafka', appType=custom }" - - } - - def "enum"() { - - expect: - Service.AppType.values().size() == 5 - Service.AppType.DB.toString() == "db" - Service.AppType.WEB.toString() == "web" - Service.AppType.CUSTOM.toString() == "custom" - Service.AppType.WORKER.toString() == "worker" - Service.AppType.CACHE.toString() == "cache" - - } - - def "add extra info about a specific service"() { - - setup: - def tracer = new DDTracer() - def service = new Service("api-intake", "kafka", Service.AppType.CUSTOM) - - when: - tracer.addServiceInfo(service) - - then: - tracer.getServiceInfo().size() == 1 - tracer.getServiceInfo().get("api-intake") == service - - } - - def "add a extra info is reported to the writer"() { - - setup: - def writer = spy(new DDAgentWriter()) - def tracer = new DDTracer(DDTracer.UNASSIGNED_DEFAULT_SERVICE_NAME, writer, new AllSampler()) - - - when: - tracer.addServiceInfo(new Service("api-intake", "kafka", Service.AppType.CUSTOM)) - - then: - verify(writer, times(1)).writeServices(any(Map)) - - } - -} diff --git a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDApiTest.groovy b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDApiTest.groovy index 90f996e4264..82b632311df 100644 --- a/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDApiTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/trace/api/writer/DDApiTest.groovy @@ -4,7 +4,6 @@ import com.fasterxml.jackson.core.type.TypeReference import com.fasterxml.jackson.databind.JsonNode import com.fasterxml.jackson.databind.ObjectMapper import datadog.opentracing.SpanFactory -import datadog.trace.common.Service import datadog.trace.common.writer.DDApi import datadog.trace.common.writer.DDApi.ResponseListener import org.msgpack.jackson.dataformat.MessagePackFactory @@ -15,6 +14,7 @@ import spock.lang.Specification import spock.lang.Timeout import spock.lang.Unroll +import java.util.concurrent.atomic.AtomicInteger import java.util.concurrent.atomic.AtomicReference import static ratpack.groovy.test.embed.GroovyEmbeddedApp.ratpack @@ -33,17 +33,12 @@ class DDApiTest extends Specification { def status = request.contentLength > 0 ? 200 : 500 response.status(status).send() } - put("v0.4/services") { - def status = request.contentLength > 0 ? 200 : 500 - response.status(status).send() - } } } def client = new DDApi("localhost", agent.address.port) expect: client.tracesEndpoint == "http://localhost:${agent.address.port}/v0.4/traces" - client.servicesEndpoint == "http://localhost:${agent.address.port}/v0.4/services" client.sendTraces([]) cleanup: @@ -57,17 +52,12 @@ class DDApiTest extends Specification { put("v0.4/traces") { response.status(404).send() } - put("v0.4/services") { - def status = request.contentLength > 0 ? 200 : 500 - response.status(status).send() - } } } def client = new DDApi("localhost", agent.address.port) expect: client.tracesEndpoint == "http://localhost:${agent.address.port}/v0.3/traces" - client.servicesEndpoint == "http://localhost:${agent.address.port}/v0.3/services" !client.sendTraces([]) cleanup: @@ -89,22 +79,18 @@ class DDApiTest extends Specification { response.send() } } - put("v0.4/services") { - def status = request.contentLength > 0 ? 200 : 500 - response.status(status).send() - } } } def client = new DDApi("localhost", agent.address.port) expect: client.tracesEndpoint == "http://localhost:${agent.address.port}/v0.4/traces" - client.servicesEndpoint == "http://localhost:${agent.address.port}/v0.4/services" client.sendTraces(traces) requestContentType.get().type == "application/msgpack" requestHeaders.get().get("Datadog-Meta-Lang") == "java" requestHeaders.get().get("Datadog-Meta-Lang-Version") == System.getProperty("java.version", "unknown") requestHeaders.get().get("Datadog-Meta-Tracer-Version") == "Stubbed-Test-Version" + requestHeaders.get().get("X-Datadog-Trace-Count") == "${traces.size()}" convertList(requestBody.get()) == expectedRequestBody cleanup: @@ -142,141 +128,36 @@ class DDApiTest extends Specification { ])] } - // Services endpoint - def "sending an empty map of services returns no errors"() { - setup: - def agent = ratpack { - handlers { - put("v0.4/traces") { - def status = request.contentLength > 0 ? 200 : 500 - response.status(status).send() - } - put("v0.4/services") { - def status = request.contentLength > 0 ? 200 : 500 - response.status(status).send() - } - } - } - def client = new DDApi("localhost", agent.address.port) - - expect: - client.tracesEndpoint == "http://localhost:${agent.address.port}/v0.4/traces" - client.servicesEndpoint == "http://localhost:${agent.address.port}/v0.4/services" - client.sendServices() - - cleanup: - agent.close() - } - - def "non-200 response results in false returned for services endpoint"() { - setup: - def agent = ratpack { - handlers { - put("v0.4/traces") { - def status = request.contentLength > 0 ? 200 : 500 - response.status(status).send() - } - put("v0.4/services") { - response.status(404).send() - } - } - } - def client = new DDApi("localhost", agent.address.port) - - expect: - client.tracesEndpoint == "http://localhost:${agent.address.port}/v0.3/traces" - client.servicesEndpoint == "http://localhost:${agent.address.port}/v0.3/services" - !client.sendServices([:]) - - cleanup: - agent.close() - } - - def "services content is sent as MSGPACK"() { - setup: - def requestContentType = new AtomicReference() - def requestHeaders = new AtomicReference() - def requestBody = new AtomicReference() - def agent = ratpack { - handlers { - put("v0.4/traces") { - def status = request.contentLength > 0 ? 200 : 500 - response.status(status).send() - } - put("v0.4/services") { - requestContentType.set(request.contentType) - requestHeaders.set(request.headers) - request.body.then { - requestBody.set(it.bytes) - response.send() - } - } - } - } - def client = new DDApi("localhost", agent.address.port) - - expect: - client.tracesEndpoint == "http://localhost:${agent.address.port}/v0.4/traces" - client.servicesEndpoint == "http://localhost:${agent.address.port}/v0.4/services" - client.sendServices(services) - requestContentType.get().type == "application/msgpack" - requestHeaders.get().get("Datadog-Meta-Lang") == "java" - requestHeaders.get().get("Datadog-Meta-Lang-Version") == System.getProperty("java.version", "unknown") - requestHeaders.get().get("Datadog-Meta-Tracer-Version") == "Stubbed-Test-Version" - convertMap(requestBody.get()) == expectedRequestBody - - cleanup: - agent.close() - - // 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", "my-app-name", Service.AppType.CUSTOM)] | ["my-service-name": new TreeMap<>([ - "app" : "my-app-name", - "app_type": "custom"]) - ] - } - def "Api ResponseListeners see 200 responses"() { setup: def agentResponse = new AtomicReference(null) + def requestHeaders = new AtomicReference() ResponseListener responseListener = { String endpoint, JsonNode responseJson -> agentResponse.set(responseJson.toString()) } - boolean servicesAvailable = true def agent = ratpack { handlers { put("v0.4/traces") { + requestHeaders.set(request.headers) def status = request.contentLength > 0 ? 200 : 500 response.status(status).send('{"hello":"test"}') } - put("v0.4/services") { - if (servicesAvailable) { - def status = request.contentLength > 0 ? 200 : 500 - response.status(status).send('{"service-response":"from-test"}') - } else { - response.status(404).send('{"service-response":"from-test"}') - } - } } } def client = new DDApi("localhost", agent.address.port) client.addResponseListener(responseListener) - def services = ["my-service-name": new Service("my-service-name", "my-app-name", Service.AppType.CUSTOM)] + def traceCounter = new AtomicInteger(3) + client.addTraceCounter(traceCounter) when: client.sendTraces([]) then: agentResponse.get() == '{"hello":"test"}' - - when: - servicesAvailable = false - agentResponse.set('not-set') - client.sendServices(services) - then: - // response not seen because of non-200 status - agentResponse.get() == 'not-set' + requestHeaders.get().get("Datadog-Meta-Lang") == "java" + requestHeaders.get().get("Datadog-Meta-Lang-Version") == System.getProperty("java.version", "unknown") + requestHeaders.get().get("Datadog-Meta-Tracer-Version") == "Stubbed-Test-Version" + requestHeaders.get().get("X-Datadog-Trace-Count") == "3" // false data shows the value provided via traceCounter. + traceCounter.get() == 0 cleanup: agent.close() @@ -290,19 +171,13 @@ class DDApiTest extends Specification { def status = request.contentLength > 0 ? 200 : 500 response.status(status).send() } - put("v0.3/services") { - def status = request.contentLength > 0 ? 200 : 500 - response.status(status).send() - } } } def client = new DDApi("localhost", v3Agent.address.port) expect: client.tracesEndpoint == "http://localhost:${v3Agent.address.port}/v0.3/traces" - client.servicesEndpoint == "http://localhost:${v3Agent.address.port}/v0.3/services" client.sendTraces([]) - client.sendServices() cleanup: v3Agent.close() @@ -310,7 +185,7 @@ class DDApiTest extends Specification { @Timeout(5) @Unroll - def "Api Downgrades to v3 if timeout exceeded (#delayTrace, #delayServices, #badPort)"() { + def "Api Downgrades to v3 if timeout exceeded (#delayTrace, #badPort)"() { // This test is unfortunately only exercising the read timeout, not the connect timeout. setup: def agent = ratpack { @@ -319,10 +194,6 @@ class DDApiTest extends Specification { def status = request.contentLength > 0 ? 200 : 500 response.status(status).send() } - put("v0.3/services") { - def status = request.contentLength > 0 ? 200 : 500 - response.status(status).send() - } put("v0.4/traces") { Blocking.exec { Thread.sleep(delayTrace) @@ -330,13 +201,6 @@ class DDApiTest extends Specification { response.status(status).send() } } - put("v0.4/services") { - Blocking.exec { - Thread.sleep(delayServices) - def status = request.contentLength > 0 ? 200 : 500 - response.status(status).send() - } - } } } def port = badPort ? 999 : agent.address.port @@ -344,19 +208,16 @@ class DDApiTest extends Specification { expect: client.tracesEndpoint == "http://localhost:${port}/$endpointVersion/traces" - client.servicesEndpoint == "http://localhost:${port}/$endpointVersion/services" cleanup: agent.close() where: - endpointVersion | delayTrace | delayServices | badPort - "v0.4" | 0 | 0 | false - "v0.3" | 0 | 0 | true - "v0.4" | 500 | 0 | false - "v0.4" | 0 | 500 | false - "v0.3" | 30000 | 0 | false - "v0.3" | 0 | 30000 | false + endpointVersion | delayTrace | badPort + "v0.4" | 0 | false + "v0.3" | 0 | true + "v0.4" | 500 | false + "v0.3" | 30000 | false } static List> convertList(byte[] bytes) { diff --git a/dd-trace-ot/src/traceAgentTest/groovy/DDApiIntegrationTest.groovy b/dd-trace-ot/src/traceAgentTest/groovy/DDApiIntegrationTest.groovy index 7451f673640..d5477e8e795 100644 --- a/dd-trace-ot/src/traceAgentTest/groovy/DDApiIntegrationTest.groovy +++ b/dd-trace-ot/src/traceAgentTest/groovy/DDApiIntegrationTest.groovy @@ -3,7 +3,6 @@ import datadog.opentracing.DDSpan import datadog.opentracing.DDSpanContext import datadog.opentracing.DDTracer import datadog.opentracing.PendingTrace -import datadog.trace.common.Service import datadog.trace.api.sampling.PrioritySampling import datadog.trace.common.writer.DDAgentWriter import datadog.trace.common.writer.DDApi @@ -68,18 +67,6 @@ class DDApiIntegrationTest { [[new DDSpan(TimeUnit.MILLISECONDS.toMicros(System.currentTimeMillis()), CONTEXT)]] | 4 } - def "Sending services succeeds"() { - expect: - api.sendServices(services) - endpoint.get() == null - agentResponse.get() == null - - where: - services | _ - [:] | _ - ['app': new Service("name", "appName", Service.AppType.WEB)] | _ - } - @Unroll def "Sending bad trace fails (test #test)"() { expect: