Skip to content

Exclude jdbc wrappers from instrumentation #14760

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Sep 26, 2025
Merged

Exclude jdbc wrappers from instrumentation #14760

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
558a7bf
Exclude jdbc wrappers from instrumentation
laurit Sep 23, 2025
108047e
merge
laurit Sep 25, 2025
ccb9505
spotless
laurit Sep 25, 2025
ba15c66
add test
laurit Sep 25, 2025
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
8 changes: 8 additions & 0 deletions .../main/java/io/opentelemetry/javaagent/instrumentation/jdbc/ConnectionInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ public static class PrepareAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void addDbInfo(
@Advice.Argument(0) String sql, @Advice.Return PreparedStatement statement) {
if (JdbcSingletons.isWrapper(statement, PreparedStatement.class)) {
return;
}

JdbcData.preparedStatement.set(statement, sql);
}
}
Expand All @@ -74,6 +78,10 @@ public static void onEnter(
@Advice.Origin("#m") String methodName,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
if (JdbcSingletons.isWrapper(connection, Connection.class)) {
return;
}

Context parentContext = currentContext();
DbRequest request =
DbRequest.createTransaction(connection, methodName.toUpperCase(Locale.ROOT));
Expand Down
29 changes: 29 additions & 0 deletions ...vaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/JdbcSingletons.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,15 @@
import io.opentelemetry.instrumentation.api.incubator.semconv.net.PeerServiceAttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.AttributesExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.api.internal.cache.Cache;
import io.opentelemetry.instrumentation.jdbc.internal.DbRequest;
import io.opentelemetry.instrumentation.jdbc.internal.JdbcInstrumenterFactory;
import io.opentelemetry.instrumentation.jdbc.internal.JdbcNetworkAttributesGetter;
import io.opentelemetry.javaagent.bootstrap.internal.AgentCommonConfig;
import io.opentelemetry.javaagent.bootstrap.internal.AgentInstrumentationConfig;
import io.opentelemetry.javaagent.bootstrap.jdbc.DbInfo;
import java.sql.SQLException;
import java.sql.Wrapper;
import java.util.Collections;
import javax.sql.DataSource;

Expand Down Expand Up @@ -68,5 +71,31 @@ public static Instrumenter<DataSource, DbInfo> dataSourceInstrumenter() {
return DATASOURCE_INSTRUMENTER;
}

private static final Cache<Class<?>, Boolean> wrapperClassCache = Cache.weak();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does ClassValue work here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't work in the standard way because we need the actual object to decide whether it can be unwrapped. I think the most straightforward way we could use class value would be to use

  private static final ClassValue<IsWrapper> wrapperClassCache =
      new ClassValue<IsWrapper>() {
        @Override
        protected IsWrapper computeValue(Class<?> type) {
          return new IsWrapper();
        }
      };
  
  private static class IsWrapper {
    volatile boolean valueSet;
    volatile boolean value;
    
    boolean getValue(Wrapper object, Class<? extends Wrapper> clazz) {
      if (valueSet) {
        return value;
      }
      value = isWrapperInternal(object, clazz);
      valueSet = true;
      return true;
    }
  }

  /**
   * Returns true if the given object is a wrapper and shouldn't be instrumented. We'll instrument
   * the underlying object called by the wrapper instead.
   */
  public static <T extends Wrapper> boolean isWrapper(T object, Class<T> clazz) {
    IsWrapper isWrapper = wrapperClassCache.get(clazz);
    return isWrapper.getValue(object, clazz);
  }

  private static boolean isWrapperInternal(Wrapper object, Class<? extends Wrapper> clazz) {
    try {
      // we are dealing with a wrapper when the object unwraps to a different instance
      if (object.isWrapperFor(clazz)) {
        Wrapper unwrapped = object.unwrap(clazz);
        if (object != unwrapped) {
          return true;
        }
      }
    } catch (SQLException | AbstractMethodError e) {
      // ignore
    }
    return false;
  }

or

  private static final int UNDEFINED = 0;
  private static final int WRAPPER = 1;
  private static final int NOT_WRAPPER = -1;
  private static final ClassValue<AtomicInteger> wrapperClassCache =
      new ClassValue<AtomicInteger>() {
        @Override
        protected AtomicInteger computeValue(Class<?> type) {
          return new AtomicInteger();
        }
      };

  /**
   * Returns true if the given object is a wrapper and shouldn't be instrumented. We'll instrument
   * the underlying object called by the wrapper instead.
   */
  public static <T extends Wrapper> boolean isWrapper(T object, Class<T> clazz) {
    AtomicInteger isWrapper = wrapperClassCache.get(clazz);
    int value = isWrapper.get();
    if (value != UNDEFINED) {
      return value == WRAPPER;
    }
    boolean result = isWrapperInternal(object, clazz);
    isWrapper.set(result ? WRAPPER : NOT_WRAPPER);
    return result;
  }

  private static <T extends Wrapper> boolean isWrapperInternal(T object, Class<T> clazz) {
    try {
      // we are dealing with a wrapper when the object unwraps to a different instance
      if (object.isWrapperFor(clazz)) {
        Wrapper unwrapped = object.unwrap(clazz);
        if (object != unwrapped) {
          return true;
        }
      }
    } catch (SQLException | AbstractMethodError e) {
      // ignore
    }
    return false;
  }

which is slightly more code than the weak cache, but not too bad really.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't work in the standard way because we need the actual object to decide whether it can be unwrapped

ah, makes sense

I like the IsWrapper version better than the AtomicInteger version, but I also like your existing implementation given that it's not really a standard usage of ClassValue anyways


/**
* Returns true if the given object is a wrapper and shouldn't be instrumented. We'll instrument
* the underlying object called by the wrapper instead.
*/
public static <T extends Wrapper> boolean isWrapper(T object, Class<T> clazz) {
return wrapperClassCache.computeIfAbsent(
object.getClass(), key -> isWrapperInternal(object, clazz));
}

private static <T extends Wrapper> boolean isWrapperInternal(T object, Class<T> clazz) {
try {
// we are dealing with a wrapper when the object unwraps to a different instance
if (object.isWrapperFor(clazz)) {
T unwrapped = object.unwrap(clazz);
if (object != unwrapped) {
return true;
}
}
} catch (SQLException | AbstractMethodError e) {
// ignore
}
return false;
}

private JdbcSingletons() {}
}
17 changes: 17 additions & 0 deletions ...ava/io/opentelemetry/javaagent/instrumentation/jdbc/PreparedStatementInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ public static void onEnter(
return;
}

if (JdbcSingletons.isWrapper(statement, PreparedStatement.class)) {
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 Expand Up @@ -165,6 +169,10 @@ public static class AddBatchAdvice {

@Advice.OnMethodExit(suppress = Throwable.class)
public static void addBatch(@Advice.This PreparedStatement statement) {
if (JdbcSingletons.isWrapper(statement, Statement.class)) {
return;
}

JdbcData.addPreparedStatementBatch(statement);
}
}
Expand All @@ -179,6 +187,9 @@ public static void onExit(
if (!CAPTURE_QUERY_PARAMETERS) {
return;
}
if (JdbcSingletons.isWrapper(statement, PreparedStatement.class)) {
return;
}

String str = null;

Expand Down Expand Up @@ -211,6 +222,9 @@ public static void onExit(
if (!CAPTURE_QUERY_PARAMETERS) {
return;
}
if (JdbcSingletons.isWrapper(statement, PreparedStatement.class)) {
return;
}

String str = null;

Expand Down Expand Up @@ -243,6 +257,9 @@ public static void onExit(
if (!CAPTURE_QUERY_PARAMETERS) {
return;
}
if (JdbcSingletons.isWrapper(statement, PreparedStatement.class)) {
return;
}

String str = null;

Expand Down
16 changes: 14 additions & 2 deletions ...c/main/java/io/opentelemetry/javaagent/instrumentation/jdbc/StatementInstrumentation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ public static void onEnter(
@Advice.Local("otelRequest") DbRequest request,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
if (JdbcSingletons.isWrapper(statement, Statement.class)) {
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 Expand Up @@ -102,7 +106,7 @@ public static void stopSpan(
@Advice.Local("otelRequest") DbRequest request,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
if (callDepth.decrementAndGet() > 0) {
if (callDepth == null || callDepth.decrementAndGet() > 0) {
return;
}

Expand All @@ -121,6 +125,10 @@ public static void addBatch(@Advice.This Statement statement, @Advice.Argument(0
if (statement instanceof PreparedStatement) {
return;
}
if (JdbcSingletons.isWrapper(statement, Statement.class)) {
return;
}

JdbcData.addStatementBatch(statement, sql);
}
}
Expand All @@ -144,6 +152,10 @@ public static void onEnter(
@Advice.Local("otelRequest") DbRequest request,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
if (JdbcSingletons.isWrapper(statement, Statement.class)) {
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 Expand Up @@ -190,7 +202,7 @@ public static void stopSpan(
@Advice.Local("otelRequest") DbRequest request,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
if (callDepth.decrementAndGet() > 0) {
if (callDepth == null || callDepth.decrementAndGet() > 0) {
return;
}

Expand Down