prevent absence of scala runtime and akka from disabling field injection of runnables

[richardstartin]

These well intentioned optimisations were preventing field injection of runnables, because of a bit of technical debt in FieldBackedProvider

      for (final Map.Entry<String, String> entry : contextStore.entrySet()) {
        /*
         * For each context store defined in a current instrumentation we create an agent builder
         * that injects necessary fields.
         * Note: this synchronization should not have any impact on performance
         * since this is done when agent builder is being made, it doesn't affect actual
         * class transformation.
         */
        synchronized (INSTALLED_CONTEXT_MATCHERS) {
          // FIXME: This makes an assumption that class loader matchers for instrumenters that use
          // same context classes should be the same - which seems reasonable, but is not checked.
          // Addressing this properly requires some notion of 'compound intrumenters' which we
          // currently do not have.
          if (INSTALLED_CONTEXT_MATCHERS.contains(entry)) {
            log.debug("Skipping builder for {} {}", instrumenter.getClass().getName(), entry);
            continue;
          }
          log.debug("Making builder for {} {}", instrumenter.getClass().getName(), entry);
          INSTALLED_CONTEXT_MATCHERS.add(entry);

This led to injection for the Runnable instrumentation being skipped whenever scala and akka weren't present. We figured this out because all runnables in the play smoke test get field injected (see the logs at #1908) but none for springboot or springboot-grpc, and then there were these logs.

❯ ag "making builder" dd-smoke-tests/springboot-grpc/build/reports/testProcess.datadog.smoketest.SpringBootGrpcCompletableFutureTest.log | grep Runnable
9:[dd.trace 2020-09-24 12:18:11:760 +0200] [main] DEBUG datadog.trace.agent.tooling.context.FieldBackedProvider - Making builder for datadog.trace.instrumentation.akka.concurrent.AkkaForkJoinTaskInstrumentation java.lang.Runnable=datadog.trace.bootstrap.instrumentation.java.concurrent.State
❯ ag "skipping builder" dd-smoke-tests/springboot-grpc/build/reports/testProcess.datadog.smoketest.SpringBootGrpcCompletableFutureTest.log | grep Runnable
61:[dd.trace 2020-09-24 12:18:12:049 +0200] [main] DEBUG datadog.trace.agent.tooling.context.FieldBackedProvider - Skipping builder for datadog.trace.instrumentation.guava10.ListenableFutureInstrumentation java.lang.Runnable=datadog.trace.bootstrap.instrumentation.java.concurrent.State
110:[dd.trace 2020-09-24 12:18:12:806 +0200] [main] DEBUG datadog.trace.agent.tooling.context.FieldBackedProvider - Skipping builder for datadog.trace.instrumentation.java.concurrent.JavaExecutorInstrumentation java.lang.Runnable=datadog.trace.bootstrap.instrumentation.java.concurrent.State
114:[dd.trace 2020-09-24 12:18:12:882 +0200] [main] DEBUG datadog.trace.agent.tooling.context.FieldBackedProvider - Skipping builder for datadog.trace.instrumentation.java.concurrent.JavaForkJoinTaskInstrumentation java.lang.Runnable=datadog.trace.bootstrap.instrumentation.java.concurrent.State
116:[dd.trace 2020-09-24 12:18:12:915 +0200] [main] DEBUG datadog.trace.agent.tooling.context.FieldBackedProvider - Skipping builder for datadog.trace.instrumentation.java.concurrent.NonStandardExecutorInstrumentation java.lang.Runnable=datadog.trace.bootstrap.instrumentation.java.concurrent.State
118:[dd.trace 2020-09-24 12:18:12:945 +0200] [main] DEBUG datadog.trace.agent.tooling.context.FieldBackedProvider - Skipping builder for datadog.trace.instrumentation.java.concurrent.RunnableInstrumentation java.lang.Runnable=datadog.trace.bootstrap.instrumentation.java.concurrent.State
205:[dd.trace 2020-09-24 12:18:13:314 +0200] [main] DEBUG datadog.trace.agent.tooling.context.FieldBackedProvider - Skipping builder for datadog.trace.instrumentation.scala.concurrent.ScalaForkJoinTaskInstrumentation java.lang.Runnable=datadog.trace.bootstrap.instrumentation.java.concurrent.State

[richardstartin]

ClassLoader matchers added in version 0.45.0 seem to have made no appreciable impact on startup time, according to these numbers at least.

[bantonsson]

So this only affects the _normal _ instrumentation. The other one that should be changed is in datadog.trace.agent.tooling.context.FieldBackedProvider.additionalInstrumentation(agentBuilder)

[richardstartin]

Thanks

[richardstartin]

I tested this (see last commit) and there was a difference of less than 1 S.D.

[devinsba]

The numbers don't lie

[richardstartin]

The numbers don't lie

Well, on a MacBook Pro 13" they can be pretty misleading so I'll take some measurements on another machine. Also, @bantonsson is working on an alternative approach to just ignoring classloader matchers (which I still think is the right thing to do - I don't think they're an optimisation) so I won't merge this unless he buys in to this approach.

[devinsba]

Based on my startup time testing there is a small increase over master but I think we have already made that back up with the other changes you have been making

[richardstartin]

I noticed that it slowed down a bit when I used any() in FieldBackedProvider, but when I overrode the class loader matcher in Instrumenter.Default it was quite a bit faster, maybe 1.5-2 standard deviations. I think it's possible we're overestimating the amortised cost of some of the matchers we're trying to avoid with class loader matchers, and perhaps underestimating the cost of the class loader matchers themselves. I'll dig in to it a bit more next week.

[richardstartin]

we can have field injection and class loader matchers with #1920