Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip PreparedStatement wrappers #9399

Merged
merged 2 commits into from
Sep 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.jdbc.internal.DbRequest;
import io.opentelemetry.instrumentation.jdbc.internal.JdbcData;
import io.opentelemetry.javaagent.bootstrap.CallDepth;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
Expand Down Expand Up @@ -55,6 +56,12 @@ public static void onEnter(
@Advice.Local("otelRequest") DbRequest request,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
// skip prepared statements without attached sql, probably a wrapper around the actual
// prepared statement
if (JdbcData.preparedStatement.get(statement) == null) {
return;
}

// 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -861,4 +861,36 @@ class JdbcInstrumentationTest extends AgentInstrumentationSpecification {
statement.close()
connection.close()
}

// regression test for https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/9359
def "test proxy prepared statement"() {
def connection = new Driver().connect(jdbcUrls.get("h2"), null)
PreparedStatement statement = connection.prepareStatement("SELECT 3")
PreparedStatement proxyStatement = ProxyStatementFactory.proxyPreparedStatement(statement)
ResultSet resultSet = runWithSpan("parent") {
return proxyStatement.executeQuery()
}

expect:
resultSet.next()
resultSet.getInt(1) == 3
assertTraces(1) {
trace(0, 2) {
span(0) {
name "parent"
kind SpanKind.INTERNAL
hasNoParent()
}
span(1) {
name "SELECT $dbNameLower"
kind CLIENT
childOf span(0)
}
}
}

cleanup:
statement.close()
connection.close()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Proxy;
import java.sql.PreparedStatement;
import java.sql.Statement;

public final class ProxyStatementFactory {
Expand Down Expand Up @@ -34,5 +35,28 @@ public static Statement proxyStatement(Statement statement) throws Exception {
return proxyStatement;
}

public static PreparedStatement proxyPreparedStatement(PreparedStatement statement) {
InvocationHandler invocationHandler = (proxy, method, args) -> method.invoke(statement, args);
PreparedStatement proxyStatement =
(PreparedStatement)
Proxy.newProxyInstance(
ProxyStatementFactory.class.getClassLoader(),
new Class<?>[] {PreparedStatement.class, TestInterface.class},
invocationHandler);

// adding package private interface TestInterface to jdk proxy forces defining the proxy class
// in the same package as the package private interface
// by default we ignore jdk proxies, having the proxy in a different package ensures it gets
// instrumented
if (!proxyStatement
.getClass()
.getName()
.startsWith("io.opentelemetry.javaagent.instrumentation.jdbc.test")) {
throw new IllegalStateException("proxy statement is in wrong package");
}

return proxyStatement;
}

private ProxyStatementFactory() {}
}