Fix instrumentation for the latest version of ucp (#12052)

laurit · web-flow · commit ec0223de59a6 · 2024-08-20T12:11:24.000-07:00
diff --git a/instrumentation/oracle-ucp-11.2/javaagent/build.gradle.kts b/instrumentation/oracle-ucp-11.2/javaagent/build.gradle.kts
@@ -13,10 +13,15 @@ muzzle {
 
 dependencies {
   library("com.oracle.database.jdbc:ucp:11.2.0.4")
+  library("com.oracle.database.jdbc:ojdbc8:12.2.0.1")
 
   implementation(project(":instrumentation:oracle-ucp-11.2:library"))
 
   testImplementation(project(":instrumentation:oracle-ucp-11.2:testing"))
+}
 
-  latestDepTestLibrary("com.oracle.database.jdbc:ucp:21.9.0.0")
+tasks {
+  test {
+    usesService(gradle.sharedServices.registrations["testcontainersBuildService"].service)
+  }
 }
diff --git a/instrumentation/oracle-ucp-11.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/oracleucp/v11_2/UniversalConnectionPoolInstrumentation.java b/instrumentation/oracle-ucp-11.2/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/oracleucp/v11_2/UniversalConnectionPoolInstrumentation.java
@@ -8,10 +8,11 @@
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
 import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface;
 import static io.opentelemetry.javaagent.instrumentation.oracleucp.v11_2.OracleUcpSingletons.telemetry;
+import static net.bytebuddy.matcher.ElementMatchers.isPublic;
 import static net.bytebuddy.matcher.ElementMatchers.named;
-import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
 import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
 
+import io.opentelemetry.javaagent.bootstrap.CallDepth;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
 import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
 import net.bytebuddy.asm.Advice;
@@ -34,9 +35,7 @@ public ElementMatcher<TypeDescription> typeMatcher() {
   @Override
   public void transform(TypeTransformer transformer) {
     transformer.applyAdviceToMethod(
-        named("start")
-            .and(takesArguments(0).or(takesArguments(1).and(takesArgument(0, boolean.class)))),
-        this.getClass().getName() + "$StartAdvice");
+        named("start").and(isPublic()), this.getClass().getName() + "$StartAdvice");
     transformer.applyAdviceToMethod(
         named("stop").and(takesArguments(0)), this.getClass().getName() + "$StopAdvice");
   }
@@ -53,8 +52,20 @@ public static void onExit(@Advice.This UniversalConnectionPool connectionPool) {
   @SuppressWarnings("unused")
   public static class StopAdvice {
 
+    @Advice.OnMethodEnter(suppress = Throwable.class)
+    public static void onEnter(@Advice.Local("otelCallDepth") CallDepth callDepth) {
+      callDepth = CallDepth.forClass(UniversalConnectionPool.class);
+      callDepth.getAndIncrement();
+    }
+
     @Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
-    public static void onExit(@Advice.This UniversalConnectionPool connectionPool) {
+    public static void onExit(
+        @Advice.This UniversalConnectionPool connectionPool,
+        @Advice.Local("otelCallDepth") CallDepth callDepth) {
+      if (callDepth == null || callDepth.decrementAndGet() > 0) {
+        return;
+      }
+
       telemetry().unregisterMetrics(connectionPool);
     }
   }
diff --git a/instrumentation/oracle-ucp-11.2/library/build.gradle.kts b/instrumentation/oracle-ucp-11.2/library/build.gradle.kts
@@ -5,8 +5,13 @@ plugins {
 
 dependencies {
   library("com.oracle.database.jdbc:ucp:11.2.0.4")
+  library("com.oracle.database.jdbc:ojdbc8:12.2.0.1")
 
   testImplementation(project(":instrumentation:oracle-ucp-11.2:testing"))
+}
 
-  latestDepTestLibrary("com.oracle.database.jdbc:ucp:21.9.0.0")
+tasks {
+  test {
+    usesService(gradle.sharedServices.registrations["testcontainersBuildService"].service)
+  }
 }
diff --git a/instrumentation/oracle-ucp-11.2/testing/build.gradle.kts b/instrumentation/oracle-ucp-11.2/testing/build.gradle.kts
@@ -4,8 +4,7 @@ plugins {
 
 dependencies {
   api(project(":testing-common"))
-  api("org.mockito:mockito-core")
-  api("org.mockito:mockito-junit-jupiter")
+  implementation("org.testcontainers:oracle-free")
 
   compileOnly("com.oracle.database.jdbc:ucp:11.2.0.4")
 }
diff --git a/instrumentation/oracle-ucp-11.2/testing/src/main/java/io/opentelemetry/instrumentation/oracleucp/AbstractOracleUcpInstrumentationTest.java b/instrumentation/oracle-ucp-11.2/testing/src/main/java/io/opentelemetry/instrumentation/oracleucp/AbstractOracleUcpInstrumentationTest.java
@@ -9,25 +9,31 @@
 
 import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension;
 import io.opentelemetry.instrumentation.testing.junit.db.DbConnectionPoolMetricsAssertions;
-import io.opentelemetry.instrumentation.testing.junit.db.MockDriver;
 import java.sql.Connection;
-import java.sql.SQLException;
+import java.time.Duration;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import oracle.ucp.admin.UniversalConnectionPoolManagerImpl;
 import oracle.ucp.jdbc.PoolDataSource;
 import oracle.ucp.jdbc.PoolDataSourceFactory;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
-import org.mockito.junit.jupiter.MockitoExtension;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.testcontainers.containers.output.Slf4jLogConsumer;
+import org.testcontainers.oracle.OracleContainer;
 
-@ExtendWith(MockitoExtension.class)
 public abstract class AbstractOracleUcpInstrumentationTest {
+  private static final Logger logger =
+      LoggerFactory.getLogger(AbstractOracleUcpInstrumentationTest.class);
+
   private static final String INSTRUMENTATION_NAME = "io.opentelemetry.orcale-ucp-11.2";
+  private static OracleContainer oracle;
 
   protected abstract InstrumentationExtension testing();
 
@@ -36,17 +42,42 @@ public abstract class AbstractOracleUcpInstrumentationTest {
   protected abstract void shutdown(PoolDataSource connectionPool) throws Exception;
 
   @BeforeAll
-  static void setUpMocks() throws SQLException {
-    MockDriver.register();
+  static void setUp() {
+    // This docker image does not work on arm mac. To run this test on arm mac read
+    // https://blog.jdriven.com/2022/07/running-oracle-xe-with-testcontainers-on-apple-silicon/
+    // install colima with brew install colima
+    // colima start --arch x86_64 --memory 4
+    // export TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE=/var/run/docker.sock
+    // export DOCKER_HOST="unix://${HOME}/.colima/docker.sock"
+    String dockerHost = System.getenv("DOCKER_HOST");
+    if (!"aarch64".equals(System.getProperty("os.arch"))
+        || (dockerHost != null && dockerHost.contains("colima"))) {
+      oracle =
+          new OracleContainer("gvenzl/oracle-free:23.4-slim-faststart")
+              .withLogConsumer(new Slf4jLogConsumer(logger))
+              .withStartupTimeout(Duration.ofMinutes(2));
+      oracle.start();
+    }
+  }
+
+  @AfterAll
+  static void cleanUp() {
+    if (oracle != null) {
+      oracle.stop();
+    }
   }
 
   @ParameterizedTest
   @ValueSource(booleans = {true, false})
   void shouldReportMetrics(boolean setExplicitPoolName) throws Exception {
+    Assumptions.assumeTrue(oracle != null);
+
     // given
     PoolDataSource connectionPool = PoolDataSourceFactory.getPoolDataSource();
-    connectionPool.setConnectionFactoryClassName(MockDriver.class.getName());
-    connectionPool.setURL("jdbc:mock:testDatabase");
+    connectionPool.setConnectionFactoryClassName("oracle.jdbc.pool.OracleDataSource");
+    connectionPool.setURL(oracle.getJdbcUrl());
+    connectionPool.setUser(oracle.getUsername());
+    connectionPool.setPassword(oracle.getPassword());
     if (setExplicitPoolName) {
       connectionPool.setConnectionPoolName("testPool");
     }

Original file line number	Diff line number	Diff line change
`@@ -4,8 +4,7 @@ plugins {`
`4`	`4`
`5`	`5`	`dependencies {`
`6`	`6`	`api(project(":testing-common"))`
`7`		`- api("org.mockito:mockito-core")`
`8`		`- api("org.mockito:mockito-junit-jupiter")`
	`7`	`+ implementation("org.testcontainers:oracle-free")`
`9`	`8`
`10`	`9`	`compileOnly("com.oracle.database.jdbc:ucp:11.2.0.4")`
`11`	`10`	`}`