Upgrade otel to 1.33.0 (#402)

* 1st attempt otel upgrade

* temp trying

* test using javaagent

* fix client tests

* otel 1.33.0, and fix netty and blocking extension

* fix netty 4.1

* fix servlet-3.0

* fix servlet-3.0, rw

* fix spark

* fix spring-webflux

* fix struts and undertow

* fix vertx

* fix grpc

* fix tests

* cleanup

* spotless apply

* fix grpc 1.30.0

* fix micronaut

* fix smoke tests

* fix smoke tests

* fix netty client

* fix smoke tests

* fix smoke tests

* debug

* increase timeout for flake test

* increase timeout for flaky test, try to fix vertx test

* disable flaky test

.gitmodules

@@ -1,3 +1,7 @@
-[submodule "javaagent-core/src/main/proto"]
+[submodule "otel-extensions/src/main/proto"]
 	path = otel-extensions/src/main/proto
 	url = https://github.com/hypertrace/agent-config.git
+
+[submodule "testing-common/src/main/proto"]
+	path = testing-common/src/main/proto
+	url = https://github.com/open-telemetry/opentelemetry-proto.git

build.gradle.kts

@@ -54,14 +54,15 @@ subprojects {
     extra.set("versions", mapOf(
         // when updating these values, some values must also be updated in buildSrc as this map
         // cannot be accessed there
-        "opentelemetry" to "1.24.0",
-        "opentelemetry_semconv" to "1.24.0-alpha",
-        "opentelemetry_proto" to "0.11.0-alpha",
-        "opentelemetry_java_agent" to "1.24.0-alpha",
-        "opentelemetry_java_agent_all" to "1.24.0",
-        "opentelemetry_java_agent-tooling" to "1.24.0-alpha",
+        "opentelemetry" to "1.33.0",
+        "opentelemetry_semconv" to "1.21.0-alpha",
+        "opentelemetry_api_semconv" to "1.33.0-alpha",
+        "opentelemetry_proto" to "0.20.0-alpha",
+        "opentelemetry_java_agent" to "1.33.0-alpha",
+        "opentelemetry_java_agent_all" to "1.33.0",
+        "opentelemetry_java_agent-tooling" to "1.33.0-alpha",
 
-        "opentelemetry_gradle_plugin" to "1.24.0-alpha",
+        "opentelemetry_gradle_plugin" to "1.33.0-alpha",
         "byte_buddy" to "1.12.10",
         "slf4j" to "2.0.7"
     ))

buildSrc/build.gradle.kts

@@ -33,7 +33,7 @@ tasks {
 dependencies {
   compileOnly(gradleApi())
   implementation(localGroovy())
-  val otelInstrumentationVersion = "1.24.0-alpha"
+  val otelInstrumentationVersion = "1.33.0-alpha"
   implementation("io.opentelemetry.javaagent:opentelemetry-muzzle:$otelInstrumentationVersion")
   implementation("io.opentelemetry.instrumentation.muzzle-generation:io.opentelemetry.instrumentation.muzzle-generation.gradle.plugin:$otelInstrumentationVersion")
   implementation("io.opentelemetry.instrumentation.muzzle-check:io.opentelemetry.instrumentation.muzzle-check.gradle.plugin:$otelInstrumentationVersion")

buildSrc/src/main/java/io/opentelemetry/instrumentation/gradle/AutoInstrumentationPlugin.java

@@ -33,20 +33,6 @@ public void apply(Project project) {
     project.getPlugins().apply(JavaLibraryPlugin.class);
     createLibraryConfiguration(project);
     addDependencies(project);
-    project
-        .getTasks()
-        .withType(
-            Test.class,
-            task -> {
-              task.dependsOn(":testing-bootstrap:shadowJar");
-              File testingBootstrapJar =
-                  new File(
-                      project.project(":testing-bootstrap").getBuildDir(),
-                      "libs/testing-bootstrap.jar");
-              // Make sure tests get rerun if the contents of the testing-bootstrap.jar change
-              task.getInputs().property("testing-bootstrap-jar", testingBootstrapJar);
-              task.getJvmArgumentProviders().add(new InstrumentationTestArgs(testingBootstrapJar));
-            });
   }
 
   private void addDependencies(Project project) {

instrumentation/apache-httpasyncclient-4.1/build.gradle.kts

@@ -32,9 +32,6 @@ dependencies {
     api(project(":instrumentation:apache-httpclient-4.0"))
 
     implementation("io.opentelemetry.javaagent.instrumentation:opentelemetry-javaagent-apache-httpasyncclient-4.1:${versions["opentelemetry_java_agent"]}")
-    testImplementation("io.opentelemetry.javaagent.instrumentation:opentelemetry-javaagent-apache-httpclient-4.0:${versions["opentelemetry_java_agent"]}")
-    testImplementation("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api-semconv:${versions["opentelemetry_semconv"]}")
     library("org.apache.httpcomponents:httpasyncclient:4.1")
-     testImplementation(testFixtures(project(":testing-common")))
+    testImplementation(project(":testing-common"))
 }
-

instrumentation/apache-httpasyncclient-4.1/src/test/java/io/opentelemetry/instrumentation/hypertrace/apachehttpasyncclient/ApacheAsyncClientInstrumentationModuleTest.java

@@ -16,7 +16,7 @@
 
 package io.opentelemetry.instrumentation.hypertrace.apachehttpasyncclient;
 
-import io.opentelemetry.sdk.trace.data.SpanData;
+import io.opentelemetry.proto.trace.v1.Span;
 import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
@@ -41,7 +41,6 @@
 import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
 import org.apache.http.impl.nio.client.HttpAsyncClients;
 import org.apache.http.message.BasicHeader;
-import org.hypertrace.agent.core.instrumentation.HypertraceSemanticAttributes;
 import org.hypertrace.agent.testing.AbstractInstrumenterTest;
 import org.hypertrace.agent.testing.TestHttpServer;
 import org.hypertrace.agent.testing.TestHttpServer.GetJsonHandler;
@@ -84,27 +83,33 @@ public void getJson()
     Assertions.assertEquals(GetJsonHandler.RESPONSE_BODY, responseBody);
 
     TEST_WRITER.waitForTraces(1);
-    // TODO : It needs some time to create second span for responseBody
-    TEST_WRITER.waitForSpans(2);
-    List<List<SpanData>> traces = TEST_WRITER.getTraces();
+    // exclude server spans
+    List<List<Span>> traces =
+        TEST_WRITER.waitForSpans(
+            2, span -> span.getKind().equals(Span.SpanKind.SPAN_KIND_SERVER), 123);
     Assertions.assertEquals(1, traces.size());
     Assertions.assertEquals(2, traces.get(0).size());
-    SpanData clientSpan = traces.get(0).get(0);
+    Span clientSpan = traces.get(0).get(1);
+    Span responseBodySpan = traces.get(0).get(0);
+    if (traces.get(0).get(0).getKind().equals(Span.SpanKind.SPAN_KIND_CLIENT)) {
+      clientSpan = traces.get(0).get(0);
+      responseBodySpan = traces.get(0).get(1);
+    }
 
     Assertions.assertEquals(
         "test-value",
-        clientSpan
-            .getAttributes()
-            .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header")));
+        TEST_WRITER
+            .getAttributesMap(clientSpan)
+            .get("http.response.header.test-response-header")
+            .getStringValue());
     Assertions.assertEquals(
         "bar",
-        clientSpan.getAttributes().get(HypertraceSemanticAttributes.httpRequestHeader("foo")));
-    Assertions.assertNull(
-        clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY));
-    SpanData responseBodySpan = traces.get(0).get(1);
+        TEST_WRITER.getAttributesMap(clientSpan).get("http.request.header.foo").getStringValue());
+    Assertions.assertNull(TEST_WRITER.getAttributesMap(clientSpan).get("http.request.body"));
+
     Assertions.assertEquals(
         GetJsonHandler.RESPONSE_BODY,
-        responseBodySpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY));
+        TEST_WRITER.getAttributesMap(responseBodySpan).get("http.response.body").getStringValue());
   }
 
   @Test
@@ -136,22 +141,24 @@ public void postJsonEntity(HttpEntity entity)
     Assertions.assertEquals(204, response.getStatusLine().getStatusCode());
 
     TEST_WRITER.waitForTraces(1);
-    List<List<SpanData>> traces = TEST_WRITER.getTraces();
+    // exclude server spans
+    List<List<Span>> traces =
+        TEST_WRITER.waitForSpans(1, span -> span.getKind().equals(Span.SpanKind.SPAN_KIND_SERVER));
     Assertions.assertEquals(1, traces.size());
     Assertions.assertEquals(1, traces.get(0).size());
-    SpanData clientSpan = traces.get(0).get(0);
+    Span clientSpan = traces.get(0).get(0);
 
     String requestBody = readInputStream(entity.getContent());
     Assertions.assertEquals(
         "test-value",
-        clientSpan
-            .getAttributes()
-            .get(HypertraceSemanticAttributes.httpResponseHeader("test-response-header")));
+        TEST_WRITER
+            .getAttributesMap(clientSpan)
+            .get("http.response.header.test-response-header")
+            .getStringValue());
     Assertions.assertEquals(
         requestBody,
-        clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_REQUEST_BODY));
-    Assertions.assertNull(
-        clientSpan.getAttributes().get(HypertraceSemanticAttributes.HTTP_RESPONSE_BODY));
+        TEST_WRITER.getAttributesMap(clientSpan).get("http.request.body").getStringValue());
+    Assertions.assertNull(TEST_WRITER.getAttributesMap(clientSpan).get("http.response.body"));
   }
 
   private static String readInputStream(InputStream inputStream) throws IOException {
@@ -168,7 +175,7 @@ private static String readInputStream(InputStream inputStream) throws IOExceptio
     return textBuilder.toString();
   }
 
-  class NonRepeatableStringEntity extends StringEntity {
+  static class NonRepeatableStringEntity extends StringEntity {
 
     public NonRepeatableStringEntity(String s) throws UnsupportedEncodingException {
       super(s);

instrumentation/apache-httpclient-4.0/build.gradle.kts

@@ -41,9 +41,6 @@ val versions: Map<String, String> by extra
 
 dependencies {
     api(project(":instrumentation:java-streams"))
-    testImplementation("io.opentelemetry.javaagent.instrumentation:opentelemetry-javaagent-apache-httpclient-4.0:${versions["opentelemetry_java_agent"]}")
     library("org.apache.httpcomponents:httpclient:4.0")
-
-     testImplementation(testFixtures(project(":testing-common")))
-     testImplementation("io.opentelemetry.instrumentation:opentelemetry-instrumentation-api-semconv:${versions["opentelemetry_semconv"]}")
+    testImplementation(project(":testing-common"))
 }

instrumentation/build.gradle.kts

@@ -62,3 +62,111 @@ tasks {
         relocate("io.opentelemetry.extension.aws", "io.opentelemetry.javaagent.shaded.io.opentelemetry.extension.aws")
     }
 }
+
+subprojects {
+    class JavaagentTestArgumentsProvider(
+        @InputFile
+        @PathSensitive(org.gradle.api.tasks.PathSensitivity.RELATIVE)
+        val agentShadowJar: File,
+
+        @InputFile
+        @PathSensitive(org.gradle.api.tasks.PathSensitivity.RELATIVE)
+        val shadowJar: File,
+
+        @InputFile
+        @PathSensitive(org.gradle.api.tasks.PathSensitivity.RELATIVE)
+        val extensionJar: File,
+    ) : CommandLineArgumentProvider {
+        override fun asArguments(): Iterable<String> = listOf(
+            "-Dotel.javaagent.debug=true",
+            "-javaagent:${agentShadowJar.absolutePath}",
+            "-Dht.javaagent.filter.jar.paths=${extensionJar.absolutePath}",
+            "-Dotel.exporter.otlp.protocol=http/protobuf",
+            "-Dotel.exporter.otlp.traces.endpoint=http://localhost:4318/v1/traces",
+            "-Dotel.metrics.exporter=none",
+            // make the path to the javaagent available to tests
+            "-Dotel.javaagent.testing.javaagent-jar-path=${agentShadowJar.absolutePath}",
+            "-Dotel.javaagent.experimental.initializer.jar=${shadowJar.absolutePath}",
+
+            // prevent sporadic gradle deadlocks, see SafeLogger for more details
+            "-Dotel.javaagent.testing.transform-safe-logging.enabled=true",
+            // Reduce noise in assertion messages since we don't need to verify this in most tests. We check
+            // in smoke tests instead.
+            "-Dotel.javaagent.add-thread-details=false",
+            // suppress repeated logging of "No metric data to export - skipping export."
+            // since PeriodicMetricReader is configured with a short interval
+            "-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.opentelemetry.sdk.metrics.export.PeriodicMetricReader=INFO",
+            // suppress a couple of verbose ClassNotFoundException stack traces logged at debug level
+            "-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.grpc.internal.ServerImplBuilder=INFO",
+            "-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.grpc.internal.ManagedChannelImplBuilder=INFO",
+            "-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.perfmark.PerfMark=INFO",
+            "-Dio.opentelemetry.javaagent.slf4j.simpleLogger.log.io.grpc.Context=INFO"
+        )
+    }
+
+    tasks.withType<Test>().configureEach {
+        val instShadowTask: Jar = project(":instrumentation").tasks.named<Jar>("shadowJar").get()
+        inputs.files(layout.files(instShadowTask))
+        val instShadowJar = instShadowTask.archiveFile.get().asFile
+
+        val shadowTask: Jar = project(":javaagent").tasks.named<Jar>("shadowJar").get()
+        inputs.files(layout.files(shadowTask))
+        val agentShadowJar = shadowTask.archiveFile.get().asFile
+
+        val extensionBuild: Jar = project(":tests-extension").tasks.named<Jar>("shadowJar").get()
+        inputs.files(layout.files(extensionBuild))
+        val extensionJar = extensionBuild.archiveFile.get().asFile
+
+        dependsOn(":instrumentation:shadowJar")
+
+        dependsOn(":javaagent:shadowJar")
+
+        dependsOn(":tests-extension:shadowJar")
+
+        jvmArgumentProviders.add(JavaagentTestArgumentsProvider(agentShadowJar, instShadowJar, extensionJar))
+
+        // We do fine-grained filtering of the classpath of this codebase's sources since Gradle's
+        // configurations will include transitive dependencies as well, which tests do often need.
+        classpath = classpath.filter {
+            if (file(layout.buildDirectory.dir("resources/main")).equals(it) || file(layout.buildDirectory.dir("classes/java/main")).equals(
+                    it
+                )
+            ) {
+                // The sources are packaged into the testing jar, so we need to exclude them from the test
+                // classpath, which automatically inherits them, to ensure our shaded versions are used.
+                return@filter false
+            }
+
+            val lib = it.absoluteFile
+            if (lib.name.startsWith("opentelemetry-javaagent-")) {
+                // These dependencies are packaged into the testing jar, so we need to exclude them from the test
+                // classpath, which automatically inherits them, to ensure our shaded versions are used.
+                return@filter false
+            }
+            if (lib.name.startsWith("javaagent-core")) {
+                // These dependencies are packaged into the testing jar, so we need to exclude them from the test
+                // classpath, which automatically inherits them, to ensure our shaded versions are used.
+                return@filter false
+            }
+            if (lib.name.startsWith("filter-api")) {
+                // These dependencies are packaged into the testing jar, so we need to exclude them from the test
+                // classpath, which automatically inherits them, to ensure our shaded versions are used.
+                return@filter false
+            }
+            if (lib.name.startsWith("opentelemetry-") && lib.name.contains("-autoconfigure-")) {
+                // These dependencies should not be on the test classpath, because they will auto-instrument
+                // the library and the tests could pass even if the javaagent instrumentation fails to apply
+                return@filter false
+            }
+            return@filter true
+        }
+    }
+
+    configurations.configureEach {
+        if (name.endsWith("testruntimeclasspath", ignoreCase = true)) {
+            // Added by agent, don't let Gradle bring it in when running tests.
+            exclude("io.opentelemetry.javaagent", "opentelemetry-javaagent-bootstrap")
+        }
+    }
+
+}

instrumentation/grpc-1.6/build.gradle.kts

@@ -35,8 +35,6 @@ idea {
     }
 }
 
-val testGrpcVersion = "1.30.0"
-
 protobuf {
   protoc {
     // The artifact spec for the Protobuf Compiler
@@ -61,7 +59,6 @@ val grpcVersion = "1.6.0"
 
 dependencies {
     api("io.opentelemetry.instrumentation:opentelemetry-grpc-1.6:${versions["opentelemetry_java_agent"]}")
-    testImplementation("io.opentelemetry.javaagent.instrumentation:opentelemetry-javaagent-grpc-1.6:${versions["opentelemetry_java_agent"]}")
     implementation(project(":instrumentation:grpc-common"))
     implementation(project(":shaded-protobuf-java-util", "shadow"))
 
@@ -74,7 +71,7 @@ dependencies {
 
     implementation("javax.annotation:javax.annotation-api:1.3.2")
 
-    testImplementation(testFixtures(project(":testing-common")))
+    testImplementation(project(":testing-common"))
 
     testImplementation(files(project(":instrumentation:grpc-shaded-netty-1.9").artifacts))
 
@@ -128,20 +125,52 @@ for (version in listOf("1.30.0")) {
         extendsFrom(configurations.runtimeClasspath.get())
     }
     dependencies {
-        versionedConfiguration(testFixtures(project(":testing-common")))
-        versionedConfiguration("io.opentelemetry.javaagent.instrumentation:opentelemetry-javaagent-grpc-1.6:${versions["opentelemetry_java_agent"]}")
-        versionedConfiguration("io.opentelemetry.instrumentation:opentelemetry-grpc-1.6:${versions["opentelemetry_java_agent"]}")
+        versionedConfiguration(project(":testing-common"))
         versionedConfiguration(project(":instrumentation:grpc-shaded-netty-1.9"))
         versionedConfiguration(platform("io.grpc:grpc-bom:$version"))
         versionedConfiguration("io.grpc:grpc-core")
         versionedConfiguration("io.grpc:grpc-protobuf")
         versionedConfiguration("io.grpc:grpc-stub")
         versionedConfiguration("io.grpc:grpc-netty")
-
     }
     val versionedTest = task<Test>("test_${version}") {
         group = "verification"
         classpath = versionedConfiguration + sourceSets.main.get().output + sourceSets.test.get().output + sourceSets.named(computeSourceSetNameForVersion(version)).get().output
+        // We do fine-grained filtering of the classpath of this codebase's sources since Gradle's
+        // configurations will include transitive dependencies as well, which tests do often need.
+        classpath = classpath.filter {
+            if (file(layout.buildDirectory.dir("resources/main")).equals(it) || file(layout.buildDirectory.dir("classes/java/main")).equals(
+                    it
+                )
+            ) {
+                // The sources are packaged into the testing jar, so we need to exclude them from the test
+                // classpath, which automatically inherits them, to ensure our shaded versions are used.
+                return@filter false
+            }
+
+            val lib = it.absoluteFile
+            if (lib.name.startsWith("opentelemetry-javaagent-")) {
+                // These dependencies are packaged into the testing jar, so we need to exclude them from the test
+                // classpath, which automatically inherits them, to ensure our shaded versions are used.
+                return@filter false
+            }
+            if (lib.name.startsWith("javaagent-core")) {
+                // These dependencies are packaged into the testing jar, so we need to exclude them from the test
+                // classpath, which automatically inherits them, to ensure our shaded versions are used.
+                return@filter false
+            }
+            if (lib.name.startsWith("filter-api")) {
+                // These dependencies are packaged into the testing jar, so we need to exclude them from the test
+                // classpath, which automatically inherits them, to ensure our shaded versions are used.
+                return@filter false
+            }
+            if (lib.name.startsWith("opentelemetry-") && lib.name.contains("-autoconfigure-")) {
+                // These dependencies should not be on the test classpath, because they will auto-instrument
+                // the library and the tests could pass even if the javaagent instrumentation fails to apply
+                return@filter false
+            }
+            return@filter true
+        }
         useJUnitPlatform()
     }
     tasks.check { dependsOn(versionedTest) }