Fix JDK http client propagation of non-sampled traces (#3736)

* Fix JDK http client propagation of non-sampled traces

* Lower wait time

instrumentation/java-http-client/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/httpclient/HttpHeadersInstrumentation.java

@@ -13,7 +13,6 @@
 
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
-import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
 import java.net.http.HttpHeaders;
 import net.bytebuddy.asm.Advice;
 import net.bytebuddy.description.type.TypeDescription;
@@ -40,9 +39,7 @@ public static class HeadersAdvice {
 
     @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
     public static void methodExit(@Advice.Return(readOnly = false) HttpHeaders headers) {
-      if (Java8BytecodeBridge.currentSpan().isRecording()) {
-        headers = tracer().inject(headers);
-      }
+      headers = tracer().inject(headers);
     }
   }
 }

testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/base/HttpClientTest.groovy

@@ -263,6 +263,11 @@ abstract class HttpClientTest<REQUEST> extends InstrumentationSpecification {
     method << BODY_METHODS
   }
 
+  def "basic GET request with not sampled parent"() {
+    expect:
+    junitTest.successfulRequestWithNotSampledParent()
+  }
+
   def "should suppress nested CLIENT span if already under parent CLIENT span (#method)"() {
     assumeTrue(testWithClientParent())
     expect:
@@ -272,7 +277,6 @@ abstract class HttpClientTest<REQUEST> extends InstrumentationSpecification {
     method << BODY_METHODS
   }
 
-
   //FIXME: add tests for POST with large/chunked data
 
   def "trace request with callback and parent"() {

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/AgentTestRunner.java

@@ -105,4 +105,10 @@ public <T, E extends Throwable> T runWithServerSpan(
       String spanName, ThrowingSupplier<T, E> callback) throws E {
     return testInstrumenters.runWithServerSpan(spanName, callback);
   }
+
+  @Override
+  public <T, E extends Throwable> T runWithNonRecordingSpan(ThrowingSupplier<T, E> callback)
+      throws E {
+    return testInstrumenters.runWithNonRecordingSpan(callback);
+  }
 }

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java

@@ -142,4 +142,7 @@ default <E extends Throwable> void runWithServerSpan(
    */
   <T, E extends Throwable> T runWithServerSpan(String spanName, ThrowingSupplier<T, E> callback)
       throws E;
+
+  /** Runs the provided {@code callback} inside the scope of a non-recording span. */
+  <T, E extends Throwable> T runWithNonRecordingSpan(ThrowingSupplier<T, E> callback) throws E;
 }

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/LibraryTestRunner.java

@@ -124,6 +124,12 @@ public <T, E extends Throwable> T runWithServerSpan(
     return testInstrumenters.runWithServerSpan(spanName, callback);
   }
 
+  @Override
+  public <T, E extends Throwable> T runWithNonRecordingSpan(ThrowingSupplier<T, E> callback)
+      throws E {
+    return testInstrumenters.runWithNonRecordingSpan(callback);
+  }
+
   private static class FlushTrackingSpanProcessor implements SpanProcessor {
     @Override
     public void onStart(Context parentContext, ReadWriteSpan span) {}

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/TestInstrumenters.java

@@ -6,6 +6,10 @@
 package io.opentelemetry.instrumentation.testing;
 
 import io.opentelemetry.api.OpenTelemetry;
+import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.api.trace.SpanContext;
+import io.opentelemetry.api.trace.TraceFlags;
+import io.opentelemetry.api.trace.TraceState;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.context.Scope;
 import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
@@ -58,6 +62,19 @@ <T, E extends Throwable> T runWithServerSpan(String spanName, ThrowingSupplier<T
     return runWithInstrumenter(spanName, testServerInstrumenter, callback);
   }
 
+  /** Runs the provided {@code callback} inside the scope of a non-recording span. */
+  <T, E extends Throwable> T runWithNonRecordingSpan(ThrowingSupplier<T, E> callback) throws E {
+    SpanContext spanContext =
+        SpanContext.create(
+            "12345678123456781234567812345678",
+            "1234567812345678",
+            TraceFlags.getDefault(),
+            TraceState.getDefault());
+    try (Scope ignored = Span.wrap(spanContext).makeCurrent()) {
+      return callback.get();
+    }
+  }
+
   private static <T, E extends Throwable> T runWithInstrumenter(
       String spanName, Instrumenter<String, Void> instrumenter, ThrowingSupplier<T, E> callback)
       throws E {

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/junit/http/AbstractHttpClientTest.java

@@ -257,6 +257,20 @@ void successfulRequestWithParent(String method) {
         });
   }
 
+  @Test
+  void successfulRequestWithNotSampledParent() throws InterruptedException {
+    String method = "GET";
+    URI uri = resolveAddress("/success");
+    int responseCode = testing.runWithNonRecordingSpan(() -> doRequest(method, uri));
+
+    assertThat(responseCode).isEqualTo(200);
+
+    // sleep to ensure no spans are emitted
+    Thread.sleep(200);
+
+    assertThat(testing.traces()).isEmpty();
+  }
+
   @ParameterizedTest
   @ValueSource(strings = {"PUT", "POST"})
   void shouldSuppressNestedClientSpanIfAlreadyUnderParentClientSpan(String method) {