Don't wrap null ResultSet in jdbc library instrumentation (#13758)

Author: laurit

instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryPreparedStatement.java

@@ -59,7 +59,7 @@ public OpenTelemetryPreparedStatement(
 
   @Override
   public ResultSet executeQuery() throws SQLException {
-    return new OpenTelemetryResultSet(wrapCall(query, delegate::executeQuery), this);
+    return OpenTelemetryResultSet.wrap(wrapCall(query, delegate::executeQuery), this);
   }
 
   @Override

instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryResultSet.java

@@ -34,11 +34,18 @@ class OpenTelemetryResultSet implements ResultSet {
   private final ResultSet delegate;
   private final Statement statement;
 
-  OpenTelemetryResultSet(ResultSet delegate, Statement statement) {
+  private OpenTelemetryResultSet(ResultSet delegate, Statement statement) {
     this.delegate = delegate;
     this.statement = statement;
   }
 
+  public static ResultSet wrap(ResultSet delegate, Statement statement) {
+    if (delegate == null) {
+      return null;
+    }
+    return new OpenTelemetryResultSet(delegate, statement);
+  }
+
   @Override
   public boolean next() throws SQLException {
     return delegate.next();

instrumentation/jdbc/library/src/main/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryStatement.java

@@ -176,7 +176,7 @@ public void setCursorName(String name) throws SQLException {
 
   @Override
   public ResultSet getResultSet() throws SQLException {
-    return new OpenTelemetryResultSet(delegate.getResultSet(), this);
+    return OpenTelemetryResultSet.wrap(delegate.getResultSet(), this);
   }
 
   @Override
@@ -246,7 +246,7 @@ public Connection getConnection() {
 
   @Override
   public ResultSet getGeneratedKeys() throws SQLException {
-    return new OpenTelemetryResultSet(delegate.getGeneratedKeys(), this);
+    return OpenTelemetryResultSet.wrap(delegate.getGeneratedKeys(), this);
   }
 
   @Override

instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/OpenTelemetryConnectionTest.java

@@ -105,6 +105,29 @@ void testVerifyPrepareStatement() throws SQLException {
     connection.close();
   }
 
+  @Test
+  void testVerifyPrepareStatementUpdate() throws SQLException {
+    Instrumenter<DbRequest, Void> instrumenter =
+        createStatementInstrumenter(testing.getOpenTelemetry());
+    DbInfo dbInfo = getDbInfo();
+    OpenTelemetryConnection connection =
+        new OpenTelemetryConnection(new TestConnection(), dbInfo, instrumenter);
+    String query = "UPDATE users SET name = name";
+    PreparedStatement statement = connection.prepareStatement(query);
+
+    testing.runWithSpan(
+        "parent",
+        () -> {
+          statement.executeUpdate();
+          assertThat(statement.getResultSet()).isNull();
+        });
+
+    jdbcTraceAssertion(dbInfo, query, "UPDATE");
+
+    statement.close();
+    connection.close();
+  }
+
   @Test
   void testVerifyPrepareStatementQuery() throws SQLException {
     Instrumenter<DbRequest, Void> instrumenter =
@@ -214,14 +237,18 @@ private static DbInfo getDbInfo() {
         .build();
   }
 
-  @SuppressWarnings("deprecation") // old semconv
   private static void jdbcTraceAssertion(DbInfo dbInfo, String query) {
+    jdbcTraceAssertion(dbInfo, query, "SELECT");
+  }
+
+  @SuppressWarnings("deprecation") // old semconv
+  private static void jdbcTraceAssertion(DbInfo dbInfo, String query, String operation) {
     testing.waitAndAssertTraces(
         trace ->
             trace.hasSpansSatisfyingExactly(
                 span -> span.hasName("parent").hasKind(SpanKind.INTERNAL).hasNoParent(),
                 span ->
-                    span.hasName("SELECT my_name.users")
+                    span.hasName(operation + " my_name.users")
                         .hasKind(SpanKind.CLIENT)
                         .hasParent(trace.getSpan(0))
                         .hasAttributesSatisfyingExactly(
@@ -234,7 +261,7 @@ private static void jdbcTraceAssertion(DbInfo dbInfo, String query) {
                                 DB_CONNECTION_STRING,
                                 emitStableDatabaseSemconv() ? null : dbInfo.getShortUrl()),
                             equalTo(maybeStable(DB_STATEMENT), query),
-                            equalTo(maybeStable(DB_OPERATION), "SELECT"),
+                            equalTo(maybeStable(DB_OPERATION), operation),
                             equalTo(maybeStable(DB_SQL_TABLE), "users"),
                             equalTo(SERVER_ADDRESS, dbInfo.getHost()),
                             equalTo(SERVER_PORT, dbInfo.getPort()))));

instrumentation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/TestCallableStatement.java

@@ -13,6 +13,7 @@
 import java.sql.Blob;
 import java.sql.CallableStatement;
 import java.sql.Clob;
+import java.sql.Connection;
 import java.sql.Date;
 import java.sql.NClob;
 import java.sql.Ref;
@@ -25,6 +26,10 @@
 import java.util.Map;
 
 class TestCallableStatement extends TestPreparedStatement implements CallableStatement {
+  TestCallableStatement(Connection connection) {
+    super(connection);
+  }
+
   @Override
   public Array getArray(int parameterIndex) throws SQLException {
     return null;

instrumentation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/TestConnection.java

@@ -183,20 +183,20 @@ public String nativeSQL(String sql) throws SQLException {
 
   @Override
   public CallableStatement prepareCall(String sql) throws SQLException {
-    return new TestCallableStatement();
+    return new TestCallableStatement(this);
   }
 
   @Override
   public CallableStatement prepareCall(String sql, int resultSetType, int resultSetConcurrency)
       throws SQLException {
-    return new TestCallableStatement();
+    return new TestCallableStatement(this);
   }
 
   @Override
   public CallableStatement prepareCall(
       String sql, int resultSetType, int resultSetConcurrency, int resultSetHoldability)
       throws SQLException {
-    return new TestCallableStatement();
+    return new TestCallableStatement(this);
   }
 
   @Override

instrumentation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/TestDatabaseMetaData.java

@@ -12,7 +12,7 @@
 import java.sql.SQLException;
 
 class TestDatabaseMetaData implements DatabaseMetaData {
-  final String url;
+  private final String url;
 
   TestDatabaseMetaData() {
     this("jdbc:postgresql://127.0.0.1:5432/dbname");

instrumentation/jdbc/testing/src/main/java/io/opentelemetry/instrumentation/jdbc/TestPreparedStatement.java

@@ -28,15 +28,17 @@
 import java.util.Calendar;
 
 class TestPreparedStatement extends TestStatement implements PreparedStatement {
-
-  TestPreparedStatement() {
-    super();
-  }
+  private boolean hasResultSet = true;
 
   TestPreparedStatement(Connection connection) {
     super(connection);
   }
 
+  @Override
+  protected boolean hasResultSet() {
+    return hasResultSet;
+  }
+
   @Override
   public void addBatch() throws SQLException {}
 
@@ -50,11 +52,12 @@ public boolean execute() throws SQLException {
 
   @Override
   public ResultSet executeQuery() throws SQLException {
-    return null;
+    return new TestResultSet(this);
   }
 
   @Override
   public int executeUpdate() throws SQLException {
+    hasResultSet = false;
     return 0;
   }