diff --git a/instrumentation/jdbc/javaagent/build.gradle.kts b/instrumentation/jdbc/javaagent/build.gradle.kts index 2b3b41e5c165..269966d91b1e 100644 --- a/instrumentation/jdbc/javaagent/build.gradle.kts +++ b/instrumentation/jdbc/javaagent/build.gradle.kts @@ -33,4 +33,7 @@ dependencies { tasks.withType().configureEach { jvmArgs("-Dotel.instrumentation.jdbc-datasource.enabled=true") + // this is needed to guarantee that if a layered JDBC driver is excluded then + // the underlying driver is still correctly instrumented + jvmArgs("-Dotel.javaagent.exclude-classes=io.opentelemetry.javaagent.instrumentation.jdbc.excluded.*") } diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java index 418a8e7344c3..d4d37878a93a 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java @@ -21,6 +21,7 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.CallDepth; import java.sql.PreparedStatement; +import java.sql.SQLException; import java.sql.Statement; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; @@ -62,7 +63,16 @@ public static void onEnter( // using CallDepth prevents this, because this check happens before Connection#getMetadata() // is called - the first recursive Statement call is just skipped and we do not create a span // for it - callDepth = CallDepth.forClass(Statement.class); + // the call depth should be anchored to the specific connection of this statement to avoid + // suppressing spans for layered JDBC drivers (that should be performed by semantic + // suppression) + // it's possible however that getting the connection throws an SQLException hence falling back + // to the call depth of a generic Statement. + try { + callDepth = CallDepth.forClass(statement.getConnection().getClass()); + } catch (SQLException e) { + callDepth = CallDepth.forClass(Statement.class); + } if (callDepth.getAndIncrement() > 0) { return; } diff --git a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java index 5f61b2e2f472..81b799cb16f8 100644 --- a/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java +++ b/instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java @@ -20,6 +20,7 @@ import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; import io.opentelemetry.javaagent.instrumentation.api.CallDepth; +import java.sql.SQLException; import java.sql.Statement; import net.bytebuddy.asm.Advice; import net.bytebuddy.description.type.TypeDescription; @@ -62,7 +63,16 @@ public static void onEnter( // using CallDepth prevents this, because this check happens before Connection#getMetadata() // is called - the first recursive Statement call is just skipped and we do not create a span // for it - callDepth = CallDepth.forClass(Statement.class); + // the call depth should be anchored to the specific connection of this statement to avoid + // suppressing spans for layered JDBC drivers (that should be performed by semantic + // suppression) + // it's possible however that getting the connection throws an SQLException hence falling back + // to the call depth of a generic Statement. + try { + callDepth = CallDepth.forClass(statement.getConnection().getClass()); + } catch (SQLException e) { + callDepth = CallDepth.forClass(Statement.class); + } if (callDepth.getAndIncrement() > 0) { return; } diff --git a/instrumentation/jdbc/javaagent/src/test/groovy/JdbcInstrumentationTest.groovy b/instrumentation/jdbc/javaagent/src/test/groovy/JdbcInstrumentationTest.groovy index 561f56cc7c89..26c60e7cb89d 100644 --- a/instrumentation/jdbc/javaagent/src/test/groovy/JdbcInstrumentationTest.groovy +++ b/instrumentation/jdbc/javaagent/src/test/groovy/JdbcInstrumentationTest.groovy @@ -10,6 +10,7 @@ import io.opentelemetry.api.trace.SpanKind import io.opentelemetry.instrumentation.jdbc.TestConnection import io.opentelemetry.instrumentation.jdbc.TestDriver import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification +import io.opentelemetry.javaagent.instrumentation.jdbc.excluded.TwoLayerDbCallingConnection import io.opentelemetry.semconv.trace.attributes.SemanticAttributes import org.apache.derby.jdbc.EmbeddedDataSource import org.apache.derby.jdbc.EmbeddedDriver @@ -806,6 +807,53 @@ class JdbcInstrumentationTest extends AgentInstrumentationSpecification { "getMetaData() uses PreparedStatement, test PreparedStatement" | true | { con, query -> con.prepareStatement(query).executeQuery() } } + // regression test for https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/4974 + def "call depth suppression should not hide layered drivers: #desc"() { + given: + def innerConnection = new DbCallingConnection(usePreparedStatementInConnection) + innerConnection.url = "jdbc:testdb://localhost/inner" + + def outerConnection = new TwoLayerDbCallingConnection(innerConnection) + outerConnection.url = "jdbc:testdb://localhost/outer" + + when: + runWithSpan("parent") { + executeQueryFunction(outerConnection, "SELECT * FROM table") + } + + then: + assertTraces(1) { + trace(0, 2) { + span(0) { + name "parent" + kind SpanKind.INTERNAL + hasNoParent() + } + span(1) { + name "SELECT inner.table" + kind CLIENT + childOf span(0) + attributes { + "$SemanticAttributes.DB_SYSTEM" "testdb" + "$SemanticAttributes.DB_CONNECTION_STRING" "testdb://localhost" + "$SemanticAttributes.DB_NAME" "inner" + "$SemanticAttributes.DB_STATEMENT" "SELECT * FROM table" + "$SemanticAttributes.DB_OPERATION" "SELECT" + "$SemanticAttributes.DB_SQL_TABLE" "table" + "$SemanticAttributes.NET_PEER_NAME" "localhost" + } + } + } + } + + where: + desc | usePreparedStatementInConnection | executeQueryFunction + "getMetaData() uses Statement, test Statement" | false | { con, query -> con.createStatement().executeQuery(query) } + "getMetaData() uses PreparedStatement, test Statement" | true | { con, query -> con.createStatement().executeQuery(query) } + "getMetaData() uses Statement, test PreparedStatement" | false | { con, query -> con.prepareStatement(query).executeQuery() } + "getMetaData() uses PreparedStatement, test PreparedStatement" | true | { con, query -> con.prepareStatement(query).executeQuery() } + } + class DbCallingConnection extends TestConnection { final boolean usePreparedStatement diff --git a/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/excluded/TwoLayerDbCallingConnection.java b/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/excluded/TwoLayerDbCallingConnection.java new file mode 100644 index 000000000000..9cec73b220f9 --- /dev/null +++ b/instrumentation/jdbc/javaagent/src/test/java/io/opentelemetry/javaagent/instrumentation/jdbc/excluded/TwoLayerDbCallingConnection.java @@ -0,0 +1,43 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.javaagent.instrumentation.jdbc.excluded; + +import io.opentelemetry.instrumentation.jdbc.TestConnection; +import io.opentelemetry.instrumentation.jdbc.TestPreparedStatement; +import io.opentelemetry.instrumentation.jdbc.TestStatement; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.sql.Statement; + +class TwoLayerDbCallingConnection extends TestConnection { + final Connection inner; + + TwoLayerDbCallingConnection(Connection inner) { + this.inner = inner; + } + + @Override + public Statement createStatement() throws SQLException { + return new TestStatement(this) { + @Override + public ResultSet executeQuery(String sql) throws SQLException { + return inner.createStatement().executeQuery(sql); + } + }; + } + + @Override + public PreparedStatement prepareStatement(String sql) throws SQLException { + return new TestPreparedStatement(this) { + @Override + public ResultSet executeQuery() throws SQLException { + return inner.prepareStatement(sql).executeQuery(sql); + } + }; + } +}