Merge branch 'master' into rgs/profiler-cleanup

.circleci/config.continue.yml.j2

@@ -36,7 +36,7 @@ instrumentation_modules: &instrumentation_modules "dd-java-agent/instrumentation
 debugger_modules: &debugger_modules "dd-java-agent/agent-debugger|dd-java-agent/agent-bootstrap|dd-java-agent/agent-builder|internal-api|communication|dd-trace-core"
 profiling_modules: &profiling_modules "dd-java-agent/agent-profiling"
 
-default_system_tests_commit: &default_system_tests_commit edfea31b7a9ceaed03b705de34a4e525853444c0
+default_system_tests_commit: &default_system_tests_commit 97bada5205ff411ec62ba9c2c15e66cd5a49fbb0
 
 parameters:
   nightly:

buildSrc/src/main/groovy/InstrumentPlugin.groovy

@@ -116,7 +116,9 @@ abstract class InstrumentTask extends DefaultTask {
       parameters.buildStartedTime.set(invocationDetails.buildStartedTime)
       parameters.pluginClassPath.setFrom(project.configurations.findByName('instrumentPluginClasspath') ?: [])
       parameters.plugins.set(extension.plugins)
-      parameters.instrumentingClassPath.setFrom(project.configurations.compileClasspath.findAll {
+      def matcher = instrumentTask.name =~ /instrument([A-Z].+)Java/
+      def cfgName = matcher.matches() ? "${matcher.group(1).uncapitalize()}CompileClasspath" : 'compileClasspath'
+      parameters.instrumentingClassPath.setFrom(project.configurations[cfgName].findAll {
         it.name != 'previous-compilation-data.bin' && !it.name.endsWith(".gz")
       } + sourceDirectory + (extension.additionalClasspath[instrumentTask.name] ?: [])*.get())
       parameters.sourceDirectory.set(sourceDirectory.asFile)

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java

@@ -150,7 +150,13 @@ public static void start(final Instrumentation inst, final URL agentJarURL, Stri
     createAgentClassloader(agentJarURL);
 
     if (Platform.isNativeImageBuilder()) {
+      // these default services are not used during native-image builds
+      jmxFetchEnabled = false;
+      remoteConfigEnabled = false;
+      telemetryEnabled = false;
+      // apply trace instrumentation, but skip starting other services
       startDatadogAgent(inst);
+      StaticEventLogger.end("Agent.start");
       return;
     }
 

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecorator.java

@@ -250,7 +250,7 @@ public AgentSpan onRequest(
     }
 
     String inferredAddressStr = null;
-    if (clientIpResolverEnabled) {
+    if (clientIpResolverEnabled && context != null) {
       InetAddress inferredAddress = ClientIpAddressResolver.resolve(context, span);
       // the peer address should be used if:
       // 1. the headers yield nothing, regardless of whether it is public or not
@@ -269,6 +269,17 @@ public AgentSpan onRequest(
         inferredAddressStr = inferredAddress.getHostAddress();
         span.setTag(Tags.HTTP_CLIENT_IP, inferredAddressStr);
       }
+    } else if (clientIpResolverEnabled && span.getLocalRootSpan() != span) {
+      // in this case context == null
+      // If there is no context we can't do anything but use the peer addr.
+      // Additionally, context == null arises on subspans for which the resolution
+      // likely already happened on the top span, so we don't need to do the resolution
+      // again. Instead, copy from the top span, should it exist
+      AgentSpan localRootSpan = span.getLocalRootSpan();
+      Object clientIp = localRootSpan.getTag(Tags.HTTP_CLIENT_IP);
+      if (clientIp != null) {
+        span.setTag(Tags.HTTP_CLIENT_IP, clientIp);
+      }
     }
 
     if (peerIp != null) {

dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/HttpServerDecoratorTest.groovy

@@ -23,6 +23,7 @@ import datadog.trace.core.datastreams.DataStreamsMonitoring
 import java.util.function.Function
 import java.util.function.Supplier
 
+import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_DECODED_RESOURCE_PRESERVE_SPACES
 import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_RAW_QUERY_STRING
 import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_RAW_RESOURCE
 import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_SERVER_TAG_QUERY_STRING
@@ -63,6 +64,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
     } else {
       1 * this.span.getRequestContext()
     }
+    _ * this.span.getLocalRootSpan() >> this.span
     0 * _
 
     where:
@@ -102,6 +104,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
       1 * this.span.setResourceName({ it as String == expectedPath })
     }
     1 * this.span.setTag(Tags.HTTP_METHOD, null)
+    _ * this.span.getLocalRootSpan() >> this.span
     0 * _
 
     where:
@@ -141,18 +144,34 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
     2 * this.span.getRequestContext()
     1 * this.span.setResourceName({ it as String == expectedResource }, ResourceNamePriorities.HTTP_PATH_NORMALIZER)
     1 * this.span.setTag(Tags.HTTP_METHOD, null)
+    _ * this.span.getLocalRootSpan() >> this.span
     0 * _
 
     where:
     rawQuery | rawResource | url                             | expectedUrl           | expectedQuery | expectedResource
-    false    | false       | "http://host/p%20ath?query%3F?" | "http://host/p ath"   | "query??"     | "/path"
+    false    | false       | "http://host/p%20ath?query%3F?" | "http://host/p ath"   | "query??"     | "/p ath"
     false    | true        | "http://host/p%20ath?query%3F?" | "http://host/p%20ath" | "query??"     | "/p%20ath"
-    true     | false       | "http://host/p%20ath?query%3F?" | "http://host/p ath"   | "query%3F?"   | "/path"
+    true     | false       | "http://host/p%20ath?query%3F?" | "http://host/p ath"   | "query%3F?"   | "/p ath"
     true     | true        | "http://host/p%20ath?query%3F?" | "http://host/p%20ath" | "query%3F?"   | "/p%20ath"
 
     req = [url: url == null ? null : new URI(url)]
   }
 
+  void 'url handling without space preservation'() {
+    setup:
+    injectSysConfig(HTTP_SERVER_RAW_RESOURCE, 'false')
+    injectSysConfig(HTTP_SERVER_DECODED_RESOURCE_PRESERVE_SPACES, 'false')
+    def decorator = newDecorator()
+
+    when:
+    decorator.onRequest(this.span, null, [url: new URI('http://host/p%20ath')], null)
+
+    then:
+    1 * this.span.setResourceName({ it as String == '/path' }, ResourceNamePriorities.HTTP_PATH_NORMALIZER)
+    _ * this.span.getLocalRootSpan() >> this.span
+    _ * _
+  }
+
   def "test onConnection"() {
     setup:
     def ctx = Mock(AgentSpan.Context.Extracted)
@@ -269,6 +288,7 @@ class HttpServerDecoratorTest extends ServerDecoratorTest {
     when:
     decorator.onRequest(this.span, [peerIp: '4.4.4.4'], null, ctx)
 
+    _ * this.span.getLocalRootSpan() >> this.span
     then:
     2 * ctx.getXForwardedFor() >> '2.3.4.5'
     1 * this.span.setTag(Tags.HTTP_CLIENT_IP, '2.3.4.5')

dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/CapturedContext.java

@@ -329,7 +329,7 @@ public Status evaluate(
     this.thisClassName = thisClassName;
     boolean shouldEvaluate = resolveEvaluateAt(probeImplementation, methodLocation);
     if (shouldEvaluate) {
-      probeImplementation.evaluate(this, status);
+      probeImplementation.evaluate(this, status, methodLocation);
     }
     return status;
   }
@@ -340,11 +340,7 @@ private static boolean resolveEvaluateAt(
       // line probe, no evaluation of probe's evaluateAt
       return true;
     }
-    MethodLocation localEvaluateAt = probeImplementation.getEvaluateAt();
-    if (methodLocation == MethodLocation.ENTRY) {
-      return localEvaluateAt == MethodLocation.DEFAULT || localEvaluateAt == MethodLocation.ENTRY;
-    }
-    return localEvaluateAt == methodLocation;
+    return MethodLocation.isSame(methodLocation, probeImplementation.getEvaluateAt());
   }
 
   public Status getStatus(String probeId) {

dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/DebuggerContext.java

@@ -191,19 +191,9 @@ public static DebuggerSpan createSpan(String operationName, String[] tags) {
    *
    * @return true if can proceed to capture data
    */
-  public static boolean isReadyToCapture(String... probeIds) {
-    // TODO provide overloaded version without string array
+  public static boolean isReadyToCapture(Class<?> callingClass, String... probeIds) {
     try {
-      if (probeIds == null || probeIds.length == 0) {
-        return false;
-      }
-      boolean result = false;
-      for (String probeId : probeIds) {
-        // if all probes are rate limited, we don't capture
-        result |= ProbeRateLimiter.tryProbe(probeId);
-      }
-      result = result && checkAndSetInProbe();
-      return result;
+      return checkAndSetInProbe();
     } catch (Exception ex) {
       LOGGER.debug("Error in isReadyToCapture: ", ex);
       return false;

dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/MethodLocation.java

@@ -3,5 +3,12 @@
 public enum MethodLocation {
   DEFAULT,
   ENTRY,
-  EXIT
+  EXIT;
+
+  public static boolean isSame(MethodLocation methodLocation, MethodLocation evaluateAt) {
+    if (methodLocation == MethodLocation.ENTRY) {
+      return evaluateAt == MethodLocation.DEFAULT || evaluateAt == MethodLocation.ENTRY;
+    }
+    return methodLocation == evaluateAt;
+  }
 }

dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/ProbeImplementation.java

@@ -16,7 +16,8 @@ public interface ProbeImplementation {
 
   String getStrTags();
 
-  void evaluate(CapturedContext context, CapturedContext.Status status);
+  void evaluate(
+      CapturedContext context, CapturedContext.Status status, MethodLocation methodLocation);
 
   void commit(
       CapturedContext entryContext,
@@ -84,7 +85,8 @@ public String getStrTags() {
     }
 
     @Override
-    public void evaluate(CapturedContext context, CapturedContext.Status status) {}
+    public void evaluate(
+        CapturedContext context, CapturedContext.Status status, MethodLocation methodLocation) {}
 
     @Override
     public void commit(

dd-java-agent/agent-debugger/debugger-bootstrap/src/main/java/datadog/trace/bootstrap/debugger/ProbeRateLimiter.java

@@ -7,6 +7,7 @@
 import java.time.temporal.ChronoUnit;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
+import java.util.function.DoubleFunction;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -23,6 +24,7 @@ public class ProbeRateLimiter {
       new ConcurrentHashMap<>();
   private static Sampler GLOBAL_SNAPSHOT_SAMPLER = createSampler(DEFAULT_GLOBAL_SNAPSHOT_RATE);
   private static Sampler GLOBAL_LOG_SAMPLER = createSampler(DEFAULT_GLOBAL_LOG_RATE);
+  private static DoubleFunction<Sampler> samplerSupplier = ProbeRateLimiter::createSampler;
 
   public static boolean tryProbe(String probeId) {
     RateLimitInfo rateLimitInfo =
@@ -37,19 +39,19 @@ public static boolean tryProbe(String probeId) {
 
   private static RateLimitInfo getDefaultRateLimitInfo(String probeId) {
     LOGGER.debug("Setting sampling with default snapshot rate for probeId={}", probeId);
-    return new RateLimitInfo(createSampler(DEFAULT_SNAPSHOT_RATE), true);
+    return new RateLimitInfo(samplerSupplier.apply(DEFAULT_SNAPSHOT_RATE), true);
   }
 
   public static void setRate(String probeId, double rate, boolean isCaptureSnapshot) {
-    PROBE_SAMPLERS.put(probeId, new RateLimitInfo(createSampler(rate), isCaptureSnapshot));
+    PROBE_SAMPLERS.put(probeId, new RateLimitInfo(samplerSupplier.apply(rate), isCaptureSnapshot));
   }
 
   public static void setGlobalSnapshotRate(double rate) {
-    GLOBAL_SNAPSHOT_SAMPLER = createSampler(rate);
+    GLOBAL_SNAPSHOT_SAMPLER = samplerSupplier.apply(rate);
   }
 
   public static void setGlobalLogRate(double rate) {
-    GLOBAL_LOG_SAMPLER = createSampler(rate);
+    GLOBAL_LOG_SAMPLER = samplerSupplier.apply(rate);
   }
 
   public static void resetRate(String probeId) {
@@ -65,6 +67,11 @@ public static void resetAll() {
     resetGlobalRate();
   }
 
+  public static void setSamplerSupplier(DoubleFunction<Sampler> samplerSupplier) {
+    ProbeRateLimiter.samplerSupplier =
+        samplerSupplier != null ? samplerSupplier : ProbeRateLimiter::createSampler;
+  }
+
   private static Sampler createSampler(double rate) {
     if (rate < 0) {
       return new ConstantSampler(true);

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/ConfigurationUpdater.java

@@ -258,11 +258,6 @@ private void applyRateLimiter(ConfigurationComparer changes) {
                 : getDefaultRateLimitPerProbe(probe),
             probe.isCaptureSnapshot());
       }
-      if (addedDefinitions instanceof SpanDecorationProbe) {
-        // Span decoration probes use the same instrumentation as log probes, but we don't want
-        // to sample here.
-        ProbeRateLimiter.setRate(addedDefinitions.getId(), -1, false);
-      }
     }
     // remove rate for all removed probes
     for (ProbeDefinition removedDefinition : changes.getRemovedDefinitions()) {

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java

@@ -4,6 +4,7 @@
 
 import com.datadog.debugger.sink.DebuggerSink;
 import com.datadog.debugger.sink.Sink;
+import com.datadog.debugger.symbol.SymbolExtractionTransformer;
 import com.datadog.debugger.uploader.BatchUploader;
 import datadog.communication.ddagent.DDAgentFeaturesDiscovery;
 import datadog.communication.ddagent.SharedCommunicationObjects;
@@ -91,6 +92,10 @@ public static synchronized void run(
     } else {
       log.debug("No configuration poller available from SharedCommunicationObjects");
     }
+    if (config.getDebuggerSymbolEnabled()) {
+      instrumentation.addTransformer(
+          new SymbolExtractionTransformer(debuggerSink.getSymbolSink(), config));
+    }
   }
 
   private static void setupSourceFileTracking(

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/ASMHelper.java

@@ -9,6 +9,7 @@
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
 import java.util.stream.Collectors;
 import org.objectweb.asm.Opcodes;
@@ -20,6 +21,7 @@
 import org.objectweb.asm.tree.InsnList;
 import org.objectweb.asm.tree.InsnNode;
 import org.objectweb.asm.tree.LdcInsnNode;
+import org.objectweb.asm.tree.LocalVariableNode;
 import org.objectweb.asm.tree.MethodInsnNode;
 import org.objectweb.asm.tree.TypeInsnNode;
 
@@ -203,6 +205,43 @@ private static String getReflectiveMethodName(int sort) {
     }
   }
 
+  public static List<LocalVariableNode> sortLocalVariables(List<LocalVariableNode> localVariables) {
+    List<LocalVariableNode> sortedLocalVars = new ArrayList<>(localVariables);
+    sortedLocalVars.sort(Comparator.comparingInt(o -> o.index));
+    return sortedLocalVars;
+  }
+
+  public static LocalVariableNode[] createLocalVarNodes(List<LocalVariableNode> sortedLocalVars) {
+    int maxIndex = sortedLocalVars.get(sortedLocalVars.size() - 1).index;
+    LocalVariableNode[] localVars = new LocalVariableNode[maxIndex + 1];
+    for (LocalVariableNode localVariableNode : sortedLocalVars) {
+      localVars[localVariableNode.index] = localVariableNode;
+    }
+    return localVars;
+  }
+
+  public static void adjustLocalVarsBasedOnArgs(
+      boolean isStatic,
+      LocalVariableNode[] localVars,
+      org.objectweb.asm.Type[] argTypes,
+      List<LocalVariableNode> sortedLocalVars) {
+    // assume that first local variables matches method arguments
+    // as stated into the JVM spec:
+    // https://docs.oracle.com/javase/specs/jvms/se8/html/jvms-2.html#jvms-2.6.1
+    // so we reassigned local var in arg slots if they are empty
+    if (argTypes.length < localVars.length) {
+      int slot = isStatic ? 0 : 1;
+      int localVarTableIdx = slot;
+      for (org.objectweb.asm.Type t : argTypes) {
+        if (localVars[slot] == null && localVarTableIdx < sortedLocalVars.size()) {
+          localVars[slot] = sortedLocalVars.get(localVarTableIdx);
+        }
+        slot += t.getSize();
+        localVarTableIdx++;
+      }
+    }
+  }
+
   /** Wraps ASM's {@link org.objectweb.asm.Type} with associated generic types */
   public static class Type {
     private final org.objectweb.asm.Type mainType;

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java

@@ -127,13 +127,16 @@ private boolean addLineCaptures(LineMap lineMap) {
       }
       if (beforeLabel != null) {
         InsnList insnList = new InsnList();
+        ldc(insnList, Type.getObjectType(classNode.name));
+        // stack [class, array]
         pushProbesIds(insnList);
         // stack [array]
         invokeStatic(
             insnList,
             DEBUGGER_CONTEXT_TYPE,
             "isReadyToCapture",
             Type.BOOLEAN_TYPE,
+            CLASS_TYPE,
             STRING_ARRAY_TYPE);
         // stack [boolean]
         LabelNode targetNode = new LabelNode();
@@ -314,10 +317,17 @@ private void instrumentMethodEnter() {
       methodNode.instructions.insert(methodEnterLabel, insnList);
       return;
     }
+    ldc(insnList, Type.getObjectType(classNode.name));
+    // stack [class]
     pushProbesIds(insnList);
-    // stack [array]
+    // stack [class, array]
     invokeStatic(
-        insnList, DEBUGGER_CONTEXT_TYPE, "isReadyToCapture", Type.BOOLEAN_TYPE, STRING_ARRAY_TYPE);
+        insnList,
+        DEBUGGER_CONTEXT_TYPE,
+        "isReadyToCapture",
+        Type.BOOLEAN_TYPE,
+        CLASS_TYPE,
+        STRING_ARRAY_TYPE);
     // stack [boolean]
     LabelNode targetNode = new LabelNode();
     LabelNode gotoNode = new LabelNode();