Merge branch 'master' into INPLAT-614

Author: betterengineering

.gitlab-ci.yml

@@ -137,9 +137,12 @@ default:
     KUBERNETES_MEMORY_REQUEST: 8Gi
     KUBERNETES_MEMORY_LIMIT: 8Gi
     CACHE_TYPE: lib #default
+    FF_USE_FASTZIP: "true"
+    CACHE_COMPRESSION_LEVEL: "slowest"
+
     RUNTIME_AVAILABLE_PROCESSORS_OVERRIDE: 4 # Runtime.getRuntime().availableProcessors() returns incorrect or very high values in Kubernetes
   cache:
-    - key: '$CI_SERVER_VERSION-$CACHE_TYPE' # Dependencies cache. Reset the cache every time gitlab is upgraded.  ~Every couple months
+    - key: dependency-$CACHE_TYPE # Dependencies cache
       paths:
         # Cached dependencies and wrappers for gradle
         - .gradle/wrapper
@@ -148,8 +151,8 @@ default:
       policy: $DEPENDENCY_CACHE_POLICY
       unprotect: true
       fallback_keys: # Use fallback keys because all cache types are not populated. See note under: populate_dep_cache
-        - '$CI_SERVER_VERSION-base'
-        - '$CI_SERVER_VERSION-lib'
+        - 'dependency-base'
+        - 'dependency-lib'
     - key: $CI_PIPELINE_ID-$CACHE_TYPE # Incremental build cache. Shared by all jobs in the pipeline of the same type
       paths:
         - .gradle/caches/$GRADLE_VERSION
@@ -837,7 +840,7 @@ package-oci:
 
 configure_system_tests:
   variables:
-    SYSTEM_TESTS_SCENARIOS_GROUPS: "simple_onboarding,simple_onboarding_profiling,docker-ssi,lib-injection"
+    SYSTEM_TESTS_SCENARIOS_GROUPS: "simple_onboarding,simple_onboarding_profiling,simple_onboarding_appsec,docker-ssi,lib-injection"
 
 create_key:
   stage: generate-signing-key

communication/src/main/java/datadog/communication/ddagent/DDAgentFeaturesDiscovery.java

@@ -1,13 +1,16 @@
 package datadog.communication.ddagent;
 
+import static datadog.communication.http.OkHttpUtils.DATADOG_CONTAINER_ID;
+import static datadog.communication.http.OkHttpUtils.DATADOG_CONTAINER_TAGS_HASH;
 import static datadog.communication.serialization.msgpack.MsgPackWriter.FIXARRAY;
-import static java.util.Collections.emptyList;
+import static java.util.Collections.emptySet;
 import static java.util.Collections.singletonList;
-import static java.util.Collections.unmodifiableList;
+import static java.util.Collections.unmodifiableSet;
 
 import com.squareup.moshi.JsonAdapter;
 import com.squareup.moshi.Moshi;
 import com.squareup.moshi.Types;
+import datadog.common.container.ContainerInfo;
 import datadog.communication.http.OkHttpUtils;
 import datadog.communication.monitor.DDAgentStatsDClientManager;
 import datadog.communication.monitor.Monitoring;
@@ -88,8 +91,8 @@ public class DDAgentFeaturesDiscovery implements DroppingPolicy {
   private volatile String evpProxyEndpoint;
   private volatile String version;
   private volatile String telemetryProxyEndpoint;
-  private volatile List<String> peerTags = emptyList();
-  private volatile List<String> spanKindsToComputedStats = emptyList();
+  private volatile Set<String> peerTags = emptySet();
+  private volatile Set<String> spanKindsToComputedStats = emptySet();
 
   private long lastTimeDiscovered;
 
@@ -123,8 +126,8 @@ private void reset() {
     version = null;
     lastTimeDiscovered = 0;
     telemetryProxyEndpoint = null;
-    peerTags = emptyList();
-    spanKindsToComputedStats = emptyList();
+    peerTags = emptySet();
+    spanKindsToComputedStats = emptySet();
   }
 
   /** Run feature discovery, unconditionally. */
@@ -157,11 +160,15 @@ private void doDiscovery() {
     // 3. fallback if the endpoint couldn't be found or the response couldn't be parsed
     try (Recording recording = discoveryTimer.start()) {
       boolean fallback = true;
-      try (Response response =
-          client
-              .newCall(new Request.Builder().url(agentBaseUrl.resolve("info").url()).build())
-              .execute()) {
+      final Request.Builder requestBuilder =
+          new Request.Builder().url(agentBaseUrl.resolve("info").url());
+      final String containerId = ContainerInfo.get().getContainerId();
+      if (containerId != null) {
+        requestBuilder.header(DATADOG_CONTAINER_ID, containerId);
+      }
+      try (Response response = client.newCall(requestBuilder.build()).execute()) {
         if (response.isSuccessful()) {
+          processInfoResponseHeaders(response);
           fallback = !processInfoResponse(response.body().string());
         }
       } catch (Throwable error) {
@@ -222,6 +229,10 @@ private String probeTracesEndpoint(String[] endpoints) {
     return V3_ENDPOINT;
   }
 
+  private void processInfoResponseHeaders(Response response) {
+    ContainerInfo.get().setContainerTagsHash(response.header(DATADOG_CONTAINER_TAGS_HASH));
+  }
+
   @SuppressWarnings("unchecked")
   private boolean processInfoResponse(String response) {
     try {
@@ -295,11 +306,16 @@ private boolean processInfoResponse(String response) {
                     || Boolean.TRUE.equals(canDrop));
 
         Object peer_tags = map.get("peer_tags");
-        peerTags = peer_tags == null ? emptyList() : unmodifiableList((List<String>) peer_tags);
+        peerTags =
+            peer_tags instanceof List
+                ? unmodifiableSet(new HashSet<>((List<String>) peer_tags))
+                : emptySet();
 
         Object span_kinds = map.get("span_kinds_stats_computed");
         spanKindsToComputedStats =
-            span_kinds == null ? emptyList() : unmodifiableList((List<String>) span_kinds);
+            span_kinds instanceof List
+                ? unmodifiableSet(new HashSet<>((List<String>) span_kinds))
+                : emptySet();
       }
       try {
         state = Strings.sha256(response);
@@ -357,11 +373,11 @@ public boolean supportsLongRunning() {
     return supportsLongRunning;
   }
 
-  public List<String> peerTags() {
+  public Set<String> peerTags() {
     return peerTags;
   }
 
-  public List<String> spanKindsToComputedStats() {
+  public Set<String> spanKindsToComputedStats() {
     return spanKindsToComputedStats;
   }
 

communication/src/main/java/datadog/communication/http/OkHttpUtils.java

@@ -43,8 +43,9 @@ public final class OkHttpUtils {
   private static final String DATADOG_META_LANG_INTERPRETER = "Datadog-Meta-Lang-Interpreter";
   private static final String DATADOG_META_LANG_INTERPRETER_VENDOR =
       "Datadog-Meta-Lang-Interpreter-Vendor";
-  private static final String DATADOG_CONTAINER_ID = "Datadog-Container-ID";
+  public static final String DATADOG_CONTAINER_ID = "Datadog-Container-ID";
   private static final String DATADOG_ENTITY_ID = "Datadog-Entity-ID";
+  public static final String DATADOG_CONTAINER_TAGS_HASH = "Datadog-Container-Tags-Hash";
 
   private static final String DD_API_KEY = "DD-API-KEY";
 

communication/src/test/groovy/datadog/communication/ddagent/DDAgentFeaturesDiscoveryTest.groovy

@@ -1,9 +1,11 @@
 package datadog.communication.ddagent
 
+import datadog.common.container.ContainerInfo
 import datadog.communication.monitor.Monitoring
 import datadog.trace.test.util.DDSpecification
 import datadog.trace.util.Strings
 import okhttp3.Call
+import okhttp3.Headers
 import okhttp3.HttpUrl
 import okhttp3.MediaType
 import okhttp3.OkHttpClient
@@ -19,6 +21,8 @@ import java.nio.file.Paths
 import static datadog.communication.ddagent.DDAgentFeaturesDiscovery.V01_DATASTREAMS_ENDPOINT
 import static datadog.communication.ddagent.DDAgentFeaturesDiscovery.V6_METRICS_ENDPOINT
 import static datadog.communication.ddagent.DDAgentFeaturesDiscovery.V7_CONFIG_ENDPOINT
+import static datadog.communication.http.OkHttpUtils.DATADOG_CONTAINER_ID
+import static datadog.communication.http.OkHttpUtils.DATADOG_CONTAINER_TAGS_HASH
 
 class DDAgentFeaturesDiscoveryTest extends DDSpecification {
 
@@ -466,13 +470,37 @@ class DDAgentFeaturesDiscoveryTest extends DDSpecification {
       )
   }
 
-  def infoResponse(Request request, String json) {
+  def "should send container id as header on the info request and parse the hash in the response"() {
+    setup:
+    OkHttpClient client = Mock(OkHttpClient)
+    DDAgentFeaturesDiscovery features = new DDAgentFeaturesDiscovery(client, monitoring, agentUrl, true, true)
+    def oldContainerId = ContainerInfo.get().getContainerId()
+    def oldContainerTagsHash = ContainerInfo.get().getContainerTagsHash()
+    ContainerInfo.get().setContainerId("test")
+
+    when: "/info requested"
+    features.discover()
+
+    then:
+    1 * client.newCall(_) >> { Request request ->
+      assert request.header(DATADOG_CONTAINER_ID) == "test"
+      infoResponse(request, INFO_RESPONSE, Headers.of(DATADOG_CONTAINER_TAGS_HASH, "test-hash"))
+    }
+    and:
+    assert ContainerInfo.get().getContainerTagsHash() == "test-hash"
+    cleanup:
+    ContainerInfo.get().setContainerId(oldContainerId)
+    ContainerInfo.get().setContainerTagsHash(oldContainerTagsHash)
+  }
+
+  def infoResponse(Request request, String json, Headers headers = new Headers.Builder().build()) {
     return Mock(Call) {
       it.execute() >> new Response.Builder()
         .code(200)
         .request(request)
         .protocol(Protocol.HTTP_1_1)
         .message("")
+        .headers(headers)
         .body(ResponseBody.create(MediaType.get("application/json"), json))
         .build()
     }

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/matcher/ClassLoaderMatchers.java

@@ -42,11 +42,19 @@ public static boolean canSkipClassLoaderByName(final ClassLoader loader) {
       case "org.codehaus.groovy.runtime.callsite.CallSiteClassLoader":
       case "sun.reflect.DelegatingClassLoader":
       case "jdk.internal.reflect.DelegatingClassLoader":
+      case "org.jvnet.hk2.internal.DelegatingClassLoader":
       case "clojure.lang.DynamicClassLoader":
       case "org.apache.cxf.common.util.ASMHelper$TypeHelperClassLoader":
       case "com.ibm.xml.xlxp2.jaxb.codegen.AbstractGeneratedStubFactory$RootStubClassLoader":
       case "sun.misc.Launcher$ExtClassLoader":
+      case "org.springframework.context.support.ContextTypeMatchClassLoader$ContextOverridingClassLoader":
+      case "org.openjdk.nashorn.internal.runtime.ScriptLoader":
+      case "jdk.nashorn.internal.runtime.ScriptLoader":
+      case "org.codehaus.janino.ByteArrayClassLoader":
+      case "org.eclipse.persistence.internal.jaxb.JaxbClassLoader":
+      case "com.alibaba.fastjson.util.ASMClassLoader":
       case "datadog.trace.bootstrap.DatadogClassLoader":
+      case "datadog.trace.bootstrap.InstrumentationClassLoader":
         return true;
     }
     if (CHECK_EXCLUDES) {

dd-java-agent/instrumentation/karate/src/test/groovy/KarateTest.groovy

@@ -21,7 +21,7 @@ import java.util.concurrent.CopyOnWriteArrayList
 
 import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass
 
-@IgnoreIf(reason = "Karate instrumentation is creating an unclosed span of kind test_suite_end specifically for Java 24. Skip until we have a fix.", value = {
+@IgnoreIf(reason = "Karate does not support Java 24+ yet: https://github.com/karatelabs/karate/blob/master/.github/workflows/jdk-compat.yml#L18", value = {
   JavaVirtualMachine.isJavaVersionAtLeast(24)
 })
 @DisableTestTrace(reason = "avoid self-tracing")

dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/base/HttpClientTest.groovy

@@ -293,6 +293,7 @@ abstract class HttpClientTest extends VersionedNamingTestBase {
     status == 200
     assertTraces(2) {
       trace(size(2)) {
+        sortSpansByStart()
         basicSpan(it, "parent")
         clientSpan(it, span(0), method)
       }
@@ -439,6 +440,7 @@ abstract class HttpClientTest extends VersionedNamingTestBase {
     // only one trace (client).
     assertTraces(1) {
       trace(size(2)) {
+        sortSpansByStart()
         basicSpan(it, "parent")
         clientSpan(it, span(0), method, renameService)
       }
@@ -482,6 +484,7 @@ abstract class HttpClientTest extends VersionedNamingTestBase {
     assertTraces(1) {
       sortSpansByStart()
       trace(size(3)) {
+        sortSpansByStart()
         basicSpan(it, "parent")
         clientSpan(it, span(0), method)
         basicSpan(it, "child", span(0))
@@ -721,6 +724,7 @@ abstract class HttpClientTest extends VersionedNamingTestBase {
     def thrownException = ex instanceof ExecutionException ? ex.cause : ex
     assertTraces(1) {
       trace(size(2)) {
+        sortSpansByStart()
         basicSpan(it, "parent", null, thrownException)
         clientSpan(it, span(0), method, false, false, uri, null, true, thrownException)
       }

dd-smoke-tests/play-2.4/src/test/groovy/datadog/smoketest/IastPlayNettySmokeTest.groovy

@@ -1,5 +1,6 @@
 package datadog.smoketest
 
+import datadog.trace.test.util.Flaky
 import static java.util.concurrent.TimeUnit.SECONDS
 import okhttp3.FormBody
 import okhttp3.Request
@@ -8,6 +9,7 @@ import spock.lang.Shared
 import java.nio.file.Files
 
 
+@Flaky("https://datadoghq.atlassian.net/browse/APPSEC-58301")
 class IastPlayNettySmokeTest extends AbstractIastServerSmokeTest {
 
   @Shared

dd-trace-core/src/main/java/datadog/trace/common/metrics/ConflatingMetricsAggregator.java

@@ -2,6 +2,7 @@
 
 import static datadog.communication.ddagent.DDAgentFeaturesDiscovery.V6_METRICS_ENDPOINT;
 import static datadog.trace.api.Functions.UTF8_ENCODE;
+import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND;
 import static datadog.trace.common.metrics.AggregateMetric.ERROR_TAG;
 import static datadog.trace.common.metrics.AggregateMetric.TOP_LEVEL_TAG;
 import static datadog.trace.common.metrics.SignalItem.ReportSignal.REPORT;
@@ -230,7 +231,14 @@ public boolean publish(List<? extends CoreSpan<?>> trace) {
   }
 
   private boolean shouldComputeMetric(CoreSpan<?> span) {
-    return (span.isMeasured() || span.isTopLevel()) && span.getDurationNano() > 0;
+    return (span.isMeasured() || span.isTopLevel() || spanKindEligible(span))
+        && span.getDurationNano() > 0;
+  }
+
+  private boolean spanKindEligible(CoreSpan<?> span) {
+    final Object spanKind = span.getTag(SPAN_KIND);
+    // use toString since it could be a CharSequence...
+    return spanKind != null && features.spanKindsToComputedStats().contains(spanKind.toString());
   }
 
   private boolean publish(CoreSpan<?> span, boolean isTopLevel) {

dd-trace-core/src/test/groovy/datadog/trace/common/metrics/ConflatingMetricAggregatorTest.groovy

@@ -13,6 +13,7 @@ import java.util.concurrent.TimeUnit
 import java.util.concurrent.TimeoutException
 import java.util.function.Supplier
 
+import static datadog.trace.bootstrap.instrumentation.api.Tags.SPAN_KIND
 import static java.util.concurrent.TimeUnit.MILLISECONDS
 import static java.util.concurrent.TimeUnit.SECONDS
 
@@ -32,7 +33,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
     Sink sink = Mock(Sink)
     DDAgentFeaturesDiscovery features = Mock(DDAgentFeaturesDiscovery)
     features.supportsMetrics() >> true
-    WellKnownTags wellKnownTags = new WellKnownTags("runtimeid", "hostname", "env", "service", "version","language")
+    WellKnownTags wellKnownTags = new WellKnownTags("runtimeid", "hostname", "env", "service", "version", "language")
     ConflatingMetricsAggregator aggregator = new ConflatingMetricsAggregator(
       wellKnownTags,
       empty,
@@ -61,7 +62,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
     Sink sink = Mock(Sink)
     DDAgentFeaturesDiscovery features = Mock(DDAgentFeaturesDiscovery)
     features.supportsMetrics() >> true
-    WellKnownTags wellKnownTags = new WellKnownTags("runtimeid", "hostname", "env", "service", "version","language")
+    WellKnownTags wellKnownTags = new WellKnownTags("runtimeid", "hostname", "env", "service", "version", "language")
     ConflatingMetricsAggregator aggregator = new ConflatingMetricsAggregator(
       wellKnownTags,
       [ignoredResourceName].toSet(),
@@ -120,6 +121,44 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
     aggregator.close()
   }
 
+  def "should compute stats for span kind #kind"() {
+    setup:
+    MetricWriter writer = Mock(MetricWriter)
+    Sink sink = Stub(Sink)
+    DDAgentFeaturesDiscovery features = Mock(DDAgentFeaturesDiscovery)
+    features.supportsMetrics() >> true
+    features.spanKindsToComputedStats() >> ["client", "server", "producer", "consumer"]
+    ConflatingMetricsAggregator aggregator = new ConflatingMetricsAggregator(empty,
+      features, sink, writer, 10, queueSize, reportingInterval, SECONDS)
+    aggregator.start()
+
+    when:
+    CountDownLatch latch = new CountDownLatch(1)
+    def span = Spy(new SimpleSpan("service", "operation", "resource", "type", false, false, false, 0, 100, HTTP_OK))
+    span.getTag(SPAN_KIND) >> kind
+    aggregator.publish([span])
+    aggregator.report()
+    def latchTriggered = latch.await(2, SECONDS)
+
+    then:
+    latchTriggered == statsComputed
+    (statsComputed ? 1 : 0) * writer.startBucket(1, _, _)
+    (statsComputed ? 1 : 0) * writer.add(new MetricKey("resource", "service", "operation", "type", HTTP_OK, false), _) >> { MetricKey key, AggregateMetric value ->
+      value.getHitCount() == 1 && value.getTopLevelCount() == 0 && value.getDuration() == 100
+    }
+    (statsComputed ? 1 : 0) * writer.finishBucket() >> { latch.countDown() }
+
+    cleanup:
+    aggregator.close()
+
+    where:
+    kind                             | statsComputed
+    "client"                         | true
+    UTF8BytesString.create("server") | true
+    "internal"                       | false
+    null                             | false
+  }
+
   def "measured spans do not contribute to top level count"() {
     setup:
     MetricWriter writer = Mock(MetricWriter)
@@ -472,7 +511,7 @@ class ConflatingMetricAggregatorTest extends DDSpecification {
     ConflatingMetricsAggregator aggregator = new ConflatingMetricsAggregator(empty,
       features, sink, writer, 10, queueSize, 200, MILLISECONDS)
     final spans = [
-      new SimpleSpan("service" , "operation", "resource", "type", false, true, false, 0, 10, HTTP_OK)
+      new SimpleSpan("service", "operation", "resource", "type", false, true, false, 0, 10, HTTP_OK)
     ]
     aggregator.start()