Fix JDBC instrumentation: recursive statements inside Connection#getMetaData
MODIFY
instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java
instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java
@@ -17,8 +17,10 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
+import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap;
import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
import java.sql.PreparedStatement;
+import java.sql.Statement;
import java.util.Map;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.method.MethodDescription;
@@ -51,6 +53,17 @@ public class PreparedStatementInstrumentation implements TypeInstrumentation {
@Advice.This PreparedStatement statement,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
+ // Connection#getMetaData() may execute a Statement or PreparedStatement to retrieve DB info
+ // this happens before the DB CLIENT span is started (and put in the current context), so this
+ // instrumentation runs again and the shouldStartSpan() check always returns true - and so on
+ // until we get a StackOverflowError
+ // 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
+ if (CallDepthThreadLocalMap.getCallDepth(Statement.class).getAndIncrement() > 0) {
+ return;
+ }
+
Context parentContext = currentContext();
if (!tracer().shouldStartSpan(parentContext)) {
return;
@@ -68,6 +81,7 @@ public class PreparedStatementInstrumentation implements TypeInstrumentation {
if (scope == null) {
return;
}
+ CallDepthThreadLocalMap.reset(Statement.class);
scope.close();
if (throwable == null) {

MODIFY
instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java
instrumentation/jdbc/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java
@@ -17,6 +17,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
+import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap;
import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
import java.sql.Statement;
import java.util.Map;
@@ -52,6 +53,17 @@ public class StatementInstrumentation implements TypeInstrumentation {
@Advice.This Statement statement,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
+ // Connection#getMetaData() may execute a Statement or PreparedStatement to retrieve DB info
+ // this happens before the DB CLIENT span is started (and put in the current context), so this
+ // instrumentation runs again and the shouldStartSpan() check always returns true - and so on
+ // until we get a StackOverflowError
+ // 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
+ if (CallDepthThreadLocalMap.getCallDepth(Statement.class).getAndIncrement() > 0) {
+ return;
+ }
+
Context parentContext = currentContext();
if (!tracer().shouldStartSpan(parentContext)) {
return;
@@ -69,6 +81,7 @@ public class StatementInstrumentation implements TypeInstrumentation {
if (scope == null) {
return;
}
+ CallDepthThreadLocalMap.reset(Statement.class);
scope.close();
if (throwable == null) {

MODIFY
instrumentation/jdbc/javaagent/src/test/groovy/JdbcInstrumentationTest.groovy
instrumentation/jdbc/javaagent/src/test/groovy/JdbcInstrumentationTest.groovy
@@ -14,8 +14,10 @@ import io.opentelemetry.instrumentation.test.AgentInstrumentationSpecification
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
import java.sql.CallableStatement
import java.sql.Connection
+import java.sql.DatabaseMetaData
import java.sql.PreparedStatement
import java.sql.ResultSet
+import java.sql.SQLException
import java.sql.Statement
import javax.sql.DataSource
import org.apache.derby.jdbc.EmbeddedDataSource
@@ -724,4 +726,63 @@ class JdbcInstrumentationTest extends AgentInstrumentationSpecification {
"tomcat" | _
"c3p0" | _
}
+
+ // regression test for https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/2644
+ @Unroll
+ def "should handle recursive Statements inside Connection.getMetaData(): #desc"() {
+ given:
+ def connection = new DbCallingConnection(usePreparedStatementInConnection)
+
+ when:
+ runUnderTrace("parent") {
+ executeQueryFunction(connection, "SELECT * FROM table")
+ }
+
+ then:
+ assertTraces(1) {
+ trace(0, 2) {
+ basicSpan(it, 0, "parent")
+ span(1) {
+ name "SELECT table"
+ kind CLIENT
+ childOf span(0)
+ errored false
+ attributes {
+ "$SemanticAttributes.DB_SYSTEM.key" "testdb"
+ "$SemanticAttributes.DB_CONNECTION_STRING.key" "testdb://localhost"
+ "$SemanticAttributes.DB_STATEMENT.key" "SELECT * FROM table"
+ "$SemanticAttributes.DB_OPERATION.key" "SELECT"
+ "$SemanticAttributes.DB_SQL_TABLE.key" "table"
+ }
+ }
+ }
+ }
+
+ 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
+
+ DbCallingConnection(boolean usePreparedStatement) {
+ super(false)
+ this.usePreparedStatement = usePreparedStatement
+ }
+
+ @Override
+ DatabaseMetaData getMetaData() throws SQLException {
+ // simulate retrieving DB metadata from the DB itself
+ if (usePreparedStatement) {
+ prepareStatement("SELECT * from DB_METADATA").executeQuery()
+ } else {
+ createStatement().executeQuery("SELECT * from DB_METADATA")
+ }
+ return super.getMetaData()
+ }
+ }
}

MODIFY
instrumentation/jdbc/javaagent/src/test/groovy/test/TestConnection.groovy
instrumentation/jdbc/javaagent/src/test/groovy/test/TestConnection.groovy
@@ -41,7 +41,7 @@ class TestConnection implements Connection {
@Override
PreparedStatement prepareStatement(String sql) throws SQLException {
- return null
+ return new TestPreparedStatement(this)
}
@Override

ADD
instrumentation/jdbc/javaagent/src/test/groovy/test/TestPreparedStatement.groovy
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package test
+
+import java.sql.Array
+import java.sql.Blob
+import java.sql.Clob
+import java.sql.Connection
+import java.sql.Date
+import java.sql.NClob
+import java.sql.ParameterMetaData
+import java.sql.PreparedStatement
+import java.sql.Ref
+import java.sql.ResultSet
+import java.sql.ResultSetMetaData
+import java.sql.RowId
+import java.sql.SQLException
+import java.sql.SQLXML
+import java.sql.Time
+import java.sql.Timestamp
+
+class TestPreparedStatement extends TestStatement implements PreparedStatement {
+ TestPreparedStatement(Connection connection) {
+ super(connection)
+ }
+
+ @Override
+ ResultSet executeQuery() throws SQLException {
+ return null
+ }
+
+ @Override
+ int executeUpdate() throws SQLException {
+ return 0
+ }
+
+ @Override
+ void setNull(int parameterIndex, int sqlType) throws SQLException {
+
+ }
+
+ @Override
+ void setBoolean(int parameterIndex, boolean x) throws SQLException {
+
+ }
+
+ @Override
+ void setByte(int parameterIndex, byte x) throws SQLException {
+
+ }
+
+ @Override
+ void setShort(int parameterIndex, short x) throws SQLException {
+
+ }
+
+ @Override
+ void setInt(int parameterIndex, int x) throws SQLException {
+
+ }
+
+ @Override
+ void setLong(int parameterIndex, long x) throws SQLException {
+
+ }
+
+ @Override
+ void setFloat(int parameterIndex, float x) throws SQLException {
+
+ }
+
+ @Override
+ void setDouble(int parameterIndex, double x) throws SQLException {
+
+ }
+
+ @Override
+ void setBigDecimal(int parameterIndex, BigDecimal x) throws SQLException {
+
+ }
+
+ @Override
+ void setString(int parameterIndex, String x) throws SQLException {
+
+ }
+
+ @Override
+ void setBytes(int parameterIndex, byte[] x) throws SQLException {
+
+ }
+
+ @Override
+ void setDate(int parameterIndex, Date x) throws SQLException {
+
+ }
+
+ @Override
+ void setTime(int parameterIndex, Time x) throws SQLException {
+
+ }
+
+ @Override
+ void setTimestamp(int parameterIndex, Timestamp x) throws SQLException {
+
+ }
+
+ @Override
+ void setAsciiStream(int parameterIndex, InputStream x, int length) throws SQLException {
+
+ }
+
+ @Override
+ void setUnicodeStream(int parameterIndex, InputStream x, int length) throws SQLException {
+
+ }
+
+ @Override
+ void setBinaryStream(int parameterIndex, InputStream x, int length) throws SQLException {
+
+ }
+
+ @Override
+ void clearParameters() throws SQLException {
+
+ }
+
+ @Override
+ void setObject(int parameterIndex, Object x, int targetSqlType) throws SQLException {
+
+ }
+
+ @Override
+ void setObject(int parameterIndex, Object x) throws SQLException {
+
+ }
+
+ @Override
+ boolean execute() throws SQLException {
+ return false
+ }
+
+ @Override
+ void addBatch() throws SQLException {
+
+ }
+
+ @Override
+ void setCharacterStream(int parameterIndex, Reader reader, int length) throws SQLException {
+
+ }
+
+ @Override
+ void setRef(int parameterIndex, Ref x) throws SQLException {
+
+ }
+
+ @Override
+ void setBlob(int parameterIndex, Blob x) throws SQLException {
+
+ }
+
+ @Override
+ void setClob(int parameterIndex, Clob x) throws SQLException {
+
+ }
+
+ @Override
+ void setArray(int parameterIndex, Array x) throws SQLException {
+
+ }
+
+ @Override
+ ResultSetMetaData getMetaData() throws SQLException {
+ return null
+ }
+
+ @Override
+ void setDate(int parameterIndex, Date x, Calendar cal) throws SQLException {
+
+ }
+
+ @Override
+ void setTime(int parameterIndex, Time x, Calendar cal) throws SQLException {
+
+ }
+
+ @Override
+ void setTimestamp(int parameterIndex, Timestamp x, Calendar cal) throws SQLException {
+
+ }
+
+ @Override
+ void setNull(int parameterIndex, int sqlType, String typeName) throws SQLException {
+
+ }
+
+ @Override
+ void setURL(int parameterIndex, URL x) throws SQLException {
+
+ }
+
+ @Override
+ ParameterMetaData getParameterMetaData() throws SQLException {
+ return null
+ }
+
+ @Override
+ void setRowId(int parameterIndex, RowId x) throws SQLException {
+
+ }
+
+ @Override
+ void setNString(int parameterIndex, String value) throws SQLException {
+
+ }
+
+ @Override
+ void setNCharacterStream(int parameterIndex, Reader value, long length) throws SQLException {
+
+ }
+
+ @Override
+ void setNClob(int parameterIndex, NClob value) throws SQLException {
+
+ }
+
+ @Override
+ void setClob(int parameterIndex, Reader reader, long length) throws SQLException {
+
+ }
+
+ @Override
+ void setBlob(int parameterIndex, InputStream inputStream, long length) throws SQLException {
+
+ }
+
+ @Override
+ void setNClob(int parameterIndex, Reader reader, long length) throws SQLException {
+
+ }
+
+ @Override
+ void setSQLXML(int parameterIndex, SQLXML xmlObject) throws SQLException {
+
+ }
+
+ @Override
+ void setObject(int parameterIndex, Object x, int targetSqlType, int scaleOrLength) throws SQLException {
+
+ }
+
+ @Override
+ void setAsciiStream(int parameterIndex, InputStream x, long length) throws SQLException {
+
+ }
+
+ @Override
+ void setBinaryStream(int parameterIndex, InputStream x, long length) throws SQLException {
+
+ }
+
+ @Override
+ void setCharacterStream(int parameterIndex, Reader reader, long length) throws SQLException {
+
+ }
+
+ @Override
+ void setAsciiStream(int parameterIndex, InputStream x) throws SQLException {
+
+ }
+
+ @Override
+ void setBinaryStream(int parameterIndex, InputStream x) throws SQLException {
+
+ }
+
+ @Override
+ void setCharacterStream(int parameterIndex, Reader reader) throws SQLException {
+
+ }
+
+ @Override
+ void setNCharacterStream(int parameterIndex, Reader value) throws SQLException {
+
+ }
+
+ @Override
+ void setClob(int parameterIndex, Reader reader) throws SQLException {
+
+ }
+
+ @Override
+ void setBlob(int parameterIndex, InputStream inputStream) throws SQLException {
+
+ }
+
+ @Override
+ void setNClob(int parameterIndex, Reader reader) throws SQLException {
+
+ }
+}

Fix JDBC instrumentation: recursive statements inside Connection#getMetaData (#2756)