Fix ClassCastException in JDBC instrumentation (#6088)
* Move DbInfo to boot loader * add comment * add test * spelling
This commit is contained in:
Lauri Tulmin
GitHub
parent
fae88de680
commit
b4a1e2e9ee
|
|
@ -0,0 +1,21 @@
|
|||
plugins {
|
||||
id("otel.javaagent-bootstrap")
|
||||
}
|
||||
|
||||
/*
|
||||
JDDC instrumentation uses VirtualField<Connection, DbInfo>. Add DbInfo, that is used as the value of
|
||||
VirtualField, to boot loader. We do this because when JDBC instrumentation is started in multiple
|
||||
class loaders in the same hierarchy, each would define their own version of DbInfo. It is possible
|
||||
that the value read from virtual field would be from the wrong class loader and could produce a
|
||||
ClassCastException. Having a single copy of DbInfo that is in boot loader avoids this issue.
|
||||
*/
|
||||
|
||||
sourceSets {
|
||||
main {
|
||||
val shadedDep = project(":instrumentation:jdbc:library")
|
||||
output.dir(
|
||||
shadedDep.file("build/extracted/shadow-bootstrap"),
|
||||
"builtBy" to ":instrumentation:jdbc:library:extractShadowJarBootstrap"
|
||||
)
|
||||
}
|
||||
}
|
||||
|
|
@ -9,8 +9,13 @@ muzzle {
|
|||
}
|
||||
|
||||
dependencies {
|
||||
implementation(project(":instrumentation:jdbc:library"))
|
||||
|
||||
bootstrap(project(":instrumentation:jdbc:bootstrap"))
|
||||
compileOnly(
|
||||
project(
|
||||
path = ":instrumentation:jdbc:library",
|
||||
configuration = "shadow"
|
||||
)
|
||||
)
|
||||
compileOnly("com.google.auto.value:auto-value-annotations")
|
||||
annotationProcessor("com.google.auto.value:auto-value")
|
||||
|
||||
|
|
@ -31,6 +36,16 @@ dependencies {
|
|||
testImplementation(project(":instrumentation:jdbc:testing"))
|
||||
}
|
||||
|
||||
sourceSets {
|
||||
main {
|
||||
val shadedDep = project(":instrumentation:jdbc:library")
|
||||
output.dir(
|
||||
shadedDep.file("build/extracted/shadow-javaagent"),
|
||||
"builtBy" to ":instrumentation:jdbc:library:extractShadowJarJavaagent"
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
tasks.withType<Test>().configureEach {
|
||||
jvmArgs("-Dotel.instrumentation.jdbc-datasource.enabled=true")
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,9 +12,9 @@ import static net.bytebuddy.matcher.ElementMatchers.named;
|
|||
import static net.bytebuddy.matcher.ElementMatchers.returns;
|
||||
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
|
||||
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.DbInfo;
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.JdbcConnectionUrlParser;
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.JdbcData;
|
||||
import io.opentelemetry.javaagent.bootstrap.jdbc.DbInfo;
|
||||
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
|
||||
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
|
||||
import java.sql.Connection;
|
||||
|
|
|
|||
|
|
@ -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.test.ProxyStatementFactory
|
||||
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes
|
||||
import org.apache.derby.jdbc.EmbeddedDataSource
|
||||
import org.apache.derby.jdbc.EmbeddedDriver
|
||||
|
|
@ -820,4 +821,36 @@ class JdbcInstrumentationTest extends AgentInstrumentationSpecification {
|
|||
return super.getMetaData()
|
||||
}
|
||||
}
|
||||
|
||||
// regression test for https://github.com/open-telemetry/opentelemetry-java-instrumentation/issues/6015
|
||||
def "test proxy statement"() {
|
||||
def connection = new Driver().connect(jdbcUrls.get("h2"), null)
|
||||
Statement statement = connection.createStatement()
|
||||
Statement proxyStatement = ProxyStatementFactory.proxyStatement(statement)
|
||||
ResultSet resultSet = runWithSpan("parent") {
|
||||
return proxyStatement.executeQuery("SELECT 3")
|
||||
}
|
||||
|
||||
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()
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,36 @@
|
|||
/*
|
||||
* Copyright The OpenTelemetry Authors
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
package io.opentelemetry.javaagent.instrumentation.jdbc.test;
|
||||
|
||||
import java.lang.reflect.InvocationHandler;
|
||||
import java.lang.reflect.Proxy;
|
||||
import java.sql.Statement;
|
||||
|
||||
public class ProxyStatementFactory {
|
||||
|
||||
public static Statement proxyStatement(Statement statement) throws Exception {
|
||||
TestClassLoader classLoader = new TestClassLoader(ProxyStatementFactory.class.getClassLoader());
|
||||
Class<?> testInterface = classLoader.loadClass(TestInterface.class.getName());
|
||||
if (testInterface.getClassLoader() != classLoader) {
|
||||
throw new IllegalStateException("wrong class loader");
|
||||
}
|
||||
InvocationHandler invocationHandler = (proxy, method, args) -> method.invoke(statement, args);
|
||||
Statement proxyStatement =
|
||||
(Statement)
|
||||
Proxy.newProxyInstance(
|
||||
classLoader, new Class<?>[] {Statement.class, testInterface}, invocationHandler);
|
||||
// adding package private interface TestInterface to jdk proxy forces defining the proxy class
|
||||
// in the same package as the package private interface
|
||||
if (!proxyStatement
|
||||
.getClass()
|
||||
.getName()
|
||||
.startsWith("io.opentelemetry.javaagent.instrumentation.jdbc.test")) {
|
||||
throw new IllegalStateException("proxy statement is in wrong package");
|
||||
}
|
||||
|
||||
return proxyStatement;
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,35 @@
|
|||
/*
|
||||
* Copyright The OpenTelemetry Authors
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
package io.opentelemetry.javaagent.instrumentation.jdbc.test;
|
||||
|
||||
import java.net.URL;
|
||||
import java.net.URLClassLoader;
|
||||
|
||||
public class TestClassLoader extends URLClassLoader {
|
||||
|
||||
public TestClassLoader(ClassLoader parent) {
|
||||
super(
|
||||
new URL[] {TestClassLoader.class.getProtectionDomain().getCodeSource().getLocation()},
|
||||
parent);
|
||||
}
|
||||
|
||||
@Override
|
||||
protected synchronized Class<?> loadClass(String name, boolean resolve)
|
||||
throws ClassNotFoundException {
|
||||
Class<?> clazz = findLoadedClass(name);
|
||||
if (clazz != null) {
|
||||
return clazz;
|
||||
}
|
||||
if (name.startsWith("io.opentelemetry.javaagent.instrumentation.jdbc.test")) {
|
||||
try {
|
||||
return findClass(name);
|
||||
} catch (ClassNotFoundException exception) {
|
||||
// ignore
|
||||
}
|
||||
}
|
||||
return super.loadClass(name, resolve);
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,12 @@
|
|||
/*
|
||||
* Copyright The OpenTelemetry Authors
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
package io.opentelemetry.javaagent.instrumentation.jdbc.test;
|
||||
|
||||
// Adding a package private interface to jdk proxy forces defining the proxy class in the package
|
||||
// of the package private class. Usually proxy classes are defined in a package that we exclude from
|
||||
// instrumentation. We use this class to force proxy into a different package so it would get
|
||||
// instrumented.
|
||||
interface TestInterface {}
|
||||
|
|
@ -4,6 +4,7 @@
|
|||
*/
|
||||
|
||||
plugins {
|
||||
id("com.github.johnrengelman.shadow")
|
||||
id("otel.library-instrumentation")
|
||||
}
|
||||
|
||||
|
|
@ -13,3 +14,31 @@ dependencies {
|
|||
|
||||
testImplementation(project(":instrumentation:jdbc:testing"))
|
||||
}
|
||||
|
||||
tasks {
|
||||
shadowJar {
|
||||
dependencies {
|
||||
// including only current module excludes its transitive dependencies
|
||||
include(project(":instrumentation:jdbc:library"))
|
||||
}
|
||||
// rename classes that are included in :instrumentation:jdbc:bootstrap
|
||||
relocate("io.opentelemetry.instrumentation.jdbc.internal.dbinfo", "io.opentelemetry.javaagent.bootstrap.jdbc")
|
||||
}
|
||||
|
||||
// this will be included in javaagent module
|
||||
val extractShadowJarJavaagent by registering(Copy::class) {
|
||||
dependsOn(shadowJar)
|
||||
from(zipTree(shadowJar.get().archiveFile))
|
||||
into("build/extracted/shadow-javaagent")
|
||||
exclude("META-INF/**")
|
||||
exclude("io/opentelemetry/javaagent/bootstrap/**")
|
||||
}
|
||||
|
||||
// this will be included in bootstrap module
|
||||
val extractShadowJarBootstrap by registering(Copy::class) {
|
||||
dependsOn(shadowJar)
|
||||
from(zipTree(shadowJar.get().archiveFile))
|
||||
into("build/extracted/shadow-bootstrap")
|
||||
include("io/opentelemetry/javaagent/bootstrap/**")
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -23,9 +23,9 @@ package io.opentelemetry.instrumentation.jdbc;
|
|||
import static io.opentelemetry.instrumentation.jdbc.internal.JdbcSingletons.INSTRUMENTATION_NAME;
|
||||
|
||||
import io.opentelemetry.instrumentation.api.internal.EmbeddedInstrumentationProperties;
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.DbInfo;
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.JdbcConnectionUrlParser;
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.OpenTelemetryConnection;
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo;
|
||||
import java.sql.Connection;
|
||||
import java.sql.Driver;
|
||||
import java.sql.DriverManager;
|
||||
|
|
|
|||
|
|
@ -26,9 +26,9 @@ import static io.opentelemetry.instrumentation.jdbc.internal.JdbcUtils.computeDb
|
|||
import io.opentelemetry.api.trace.Span;
|
||||
import io.opentelemetry.context.Context;
|
||||
import io.opentelemetry.context.Scope;
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.DbInfo;
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.OpenTelemetryConnection;
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.ThrowingSupplier;
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo;
|
||||
import java.io.PrintWriter;
|
||||
import java.sql.Connection;
|
||||
import java.sql.SQLException;
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ import static io.opentelemetry.instrumentation.jdbc.internal.JdbcUtils.connectio
|
|||
import static io.opentelemetry.instrumentation.jdbc.internal.JdbcUtils.extractDbInfo;
|
||||
|
||||
import com.google.auto.value.AutoValue;
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo;
|
||||
import java.sql.Connection;
|
||||
import java.sql.PreparedStatement;
|
||||
import java.sql.Statement;
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@
|
|||
package io.opentelemetry.instrumentation.jdbc.internal;
|
||||
|
||||
import io.opentelemetry.instrumentation.api.instrumenter.db.SqlClientAttributesGetter;
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo;
|
||||
import javax.annotation.Nullable;
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -5,10 +5,11 @@
|
|||
|
||||
package io.opentelemetry.instrumentation.jdbc.internal;
|
||||
|
||||
import static io.opentelemetry.instrumentation.jdbc.internal.DbInfo.DEFAULT;
|
||||
import static io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo.DEFAULT;
|
||||
import static java.util.logging.Level.FINE;
|
||||
import static java.util.regex.Pattern.CASE_INSENSITIVE;
|
||||
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo;
|
||||
import io.opentelemetry.semconv.trace.attributes.SemanticAttributes.DbSystemValues;
|
||||
import java.io.UnsupportedEncodingException;
|
||||
import java.net.URI;
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@
|
|||
package io.opentelemetry.instrumentation.jdbc.internal;
|
||||
|
||||
import io.opentelemetry.instrumentation.api.util.VirtualField;
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo;
|
||||
import java.lang.ref.WeakReference;
|
||||
import java.sql.Connection;
|
||||
import java.sql.PreparedStatement;
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ package io.opentelemetry.instrumentation.jdbc.internal;
|
|||
|
||||
import static java.util.logging.Level.FINE;
|
||||
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo;
|
||||
import java.lang.reflect.Field;
|
||||
import java.sql.Connection;
|
||||
import java.sql.DatabaseMetaData;
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@
|
|||
|
||||
package io.opentelemetry.instrumentation.jdbc.internal;
|
||||
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo;
|
||||
import java.io.InputStream;
|
||||
import java.io.Reader;
|
||||
import java.math.BigDecimal;
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@
|
|||
|
||||
package io.opentelemetry.instrumentation.jdbc.internal;
|
||||
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo;
|
||||
import java.sql.Array;
|
||||
import java.sql.Blob;
|
||||
import java.sql.CallableStatement;
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@
|
|||
|
||||
package io.opentelemetry.instrumentation.jdbc.internal;
|
||||
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo;
|
||||
import java.io.InputStream;
|
||||
import java.io.Reader;
|
||||
import java.math.BigDecimal;
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ import static io.opentelemetry.instrumentation.jdbc.internal.JdbcSingletons.inst
|
|||
|
||||
import io.opentelemetry.context.Context;
|
||||
import io.opentelemetry.context.Scope;
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo;
|
||||
import java.sql.Connection;
|
||||
import java.sql.ResultSet;
|
||||
import java.sql.SQLException;
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@
|
|||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
package io.opentelemetry.instrumentation.jdbc.internal;
|
||||
package io.opentelemetry.instrumentation.jdbc.internal.dbinfo;
|
||||
|
||||
import com.google.auto.value.AutoValue;
|
||||
import javax.annotation.Nullable;
|
||||
|
|
@ -6,7 +6,7 @@
|
|||
package io.opentelemetry.instrumentation.jdbc
|
||||
|
||||
import io.opentelemetry.api.trace.SpanKind
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.DbInfo
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.OpenTelemetryCallableStatement
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.OpenTelemetryConnection
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.OpenTelemetryPreparedStatement
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@
|
|||
|
||||
package io.opentelemetry.instrumentation.jdbc.internal
|
||||
|
||||
import io.opentelemetry.instrumentation.jdbc.internal.dbinfo.DbInfo
|
||||
import spock.lang.Shared
|
||||
import spock.lang.Specification
|
||||
|
||||
|
|
|
|||
|
|
@ -273,6 +273,7 @@ include(":instrumentation:jaxws:jaxws-common:library")
|
|||
include(":instrumentation:jaxws:jaxws-jws-api-1.1:javaagent")
|
||||
include(":instrumentation:jboss-logmanager:jboss-logmanager-appender-1.1:javaagent")
|
||||
include(":instrumentation:jboss-logmanager:jboss-logmanager-mdc-1.1:javaagent")
|
||||
include(":instrumentation:jdbc:bootstrap")
|
||||
include(":instrumentation:jdbc:javaagent")
|
||||
include(":instrumentation:jdbc:library")
|
||||
include(":instrumentation:jdbc:testing")
|
||||
|
|
|
|||
Loading…
Reference in New Issue