Merge pull request #783 from DataDog/tyler/fix-content-length

Fix content length calculation for larger trace counts
DataDog · Mar 27, 2019 · 1100461 · 1100461
2 parents 722e6c4 + 2052647
commit 1100461
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 2 deletions.
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
@@ -122,9 +122,9 @@ public long contentLength() {
               if (traceCount < (1 << 4)) {
                 return sizeInBytes + 1; // byte
               } else if (traceCount < (1 << 16)) {
-                return sizeInBytes + 2; // short
+                return sizeInBytes + 3; // byte + short
               } else {
-                return sizeInBytes + 4; // int
+                return sizeInBytes + 5; // byte + int
               }
             }
 

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
@@ -7,6 +7,7 @@ import datadog.trace.common.writer.DDApi
 import datadog.trace.common.writer.DDApi.ResponseListener
 import spock.lang.Specification
 
+import java.util.concurrent.atomic.AtomicLong
 import java.util.concurrent.atomic.AtomicReference
 
 import static datadog.trace.agent.test.server.http.TestHttpServer.httpServer
@@ -199,6 +200,38 @@ class DDApiTest extends Specification {
     "v0.3"          | 30000      | false
   }
 
+  def "verify content length"() {
+    setup:
+    def receivedContentLength = new AtomicLong()
+    def agent = httpServer {
+      handlers {
+        put("v0.4/traces") {
+          receivedContentLength.set(request.contentLength)
+          response.status(200).send()
+        }
+      }
+    }
+    def client = new DDApi("localhost", agent.address.port, null)
+
+    when:
+    def success = client.sendTraces(traces)
+    then:
+    success
+    receivedContentLength.get() == expectedLength
+
+    cleanup:
+    agent.close()
+
+    where:
+    expectedLength | traces
+    1              | []
+    3              | [[], []]
+    16             | (1..15).collect { [] }
+    19             | (1..16).collect { [] }
+    65538          | (1..((1 << 16) - 1)).collect { [] }
+    65541          | (1..(1 << 16)).collect { [] }
+  }
+
   static List<List<TreeMap<String, Object>>> convertList(byte[] bytes) {
     return mapper.readValue(bytes, new TypeReference<List<List<TreeMap<String, Object>>>>() {})
   }

diff --git a/dd-trace-ot/src/traceAgentTest/groovy/DDApiIntegrationTest.groovy b/dd-trace-ot/src/traceAgentTest/groovy/DDApiIntegrationTest.groovy
@@ -134,6 +134,11 @@ class DDApiIntegrationTest {
       [[], []]                                                                            | 2
       [[new DDSpan(1, CONTEXT)]]                                                          | 3
       [[new DDSpan(TimeUnit.MILLISECONDS.toMicros(System.currentTimeMillis()), CONTEXT)]] | 4
+      (1..15).collect { [] }                                                              | 5
+      (1..16).collect { [] }                                                              | 6
+      // Larger traces take more than 1 second to send to the agent and get a timeout exception:
+//      (1..((1 << 16) - 1)).collect { [] }                                                 | 7
+//      (1..(1 << 16)).collect { [] }                                                       | 8
     }
 
     def "Sending traces to unix domain socket succeeds (test #test)"() {

diff --git a/dd-trace/src/test/groovy/datadog/trace/tracer/writer/AgentWriterTest.groovy b/dd-trace/src/test/groovy/datadog/trace/tracer/writer/AgentWriterTest.groovy
@@ -1,6 +1,7 @@
 package datadog.trace.tracer.writer
 
 import datadog.trace.tracer.Trace
+import spock.lang.Retry
 import spock.lang.Specification
 
 import java.util.concurrent.ExecutorService
@@ -138,6 +139,7 @@ class AgentWriterTest extends Specification {
     sampleRateByService == SampleRateByService.EMPTY_INSTANCE
   }
 
+  @Retry
   def "test start/#closeMethod"() {
     setup:
     def writer = new AgentWriter(client)
@@ -178,6 +180,7 @@ class AgentWriterTest extends Specification {
   }
 
   boolean isWriterThreadRunning() {
+    // This is known to fail sometimes.
     return Thread.getAllStackTraces().keySet().any { t -> t.getName() == "dd-agent-writer" }
   }
 }