Opentracing-jdbc does not work with quarkus 2.0.0.CR3 #18033
Comments
|
/cc @Ladicek, @evanchooly, @kenfinnigan |
|
Are you running in the JVM, as this looks like an error from running in Native with GraalVM instead? When it worked in 1.13, was that on JVM or Native? |
|
@lenalebt I think a reproducer would be welcome. Thanks! |
|
I added a reproducer here: https://github.com/lenalebt/quarkus-ticket-18033-reproducer The initial commit is a working example with 1.13.7.Final, latest commit is the update to quarkus 2.0.0.CR3 from 1.13.7.Final. It works in JVM mode for both 1.13.7.Final and 2.0.0.CR3, but it does not in native mode for 2.0.0.CR3 (I used native mode). JVM works for 2.0.0.CR3. There is a This is the error message from the reproducer application: If there is anything more I can provide, I'll do that :). |
|
It definitely looks like some classes were removed from the Native Image that shouldn't have. However, I'm not sure why it's happening. |
|
@kenfinnigan it's not complaining about a missing class AFAICS but about a missing GraalVM proxy definition (i.e. with I think we will need to add some additional magic to the Agroal extension. I'll have a look but it's too late for 2.0.0.Final. |
|
Ah yes, you're right. |
|
This mainly happened because there was a version bump in the |
|
@luneo7 this change appears to have been added to 0.2.8, can you try with 0.2.7 to see if it works ? We don't have a native test with the opentracing contrib JDBC tracer so we must be careful when upgrading it (and test it manually)! |
|
I've done some tests... to make it work in native mode I had to do all possible permutations of the interfaces for the PostgreSQL classes (PgConnection, PgStatemet, PgPreparedStatement, PgCallableStatement) to build a proxy config for GraalVM.... Using the If you change the gradle dependency config to: It will definitely work... so this is a workaround for Quarkus 2.0.0 for now.... |
|
Thanks for the investigations @luneo7, appreciated! |
|
So I think we need to downgrade to As I understand you need to configure all possible interfaces combinations, which can be very challenging as we also need to include the driver custom interfaces (Postgres interfaces in your case). |
|
Same issue here. A downgrade of |
|
As it's not the first time we had an issue, I'll try to add the usage of the tracing driver in an integration-test to avoid such issue in the future, @kenfinnigan WDYT ? I can work on it tomorrow. |
|
I provided a PR to downgrade the driver to a working version, see #18128 I'll open an issue and write something in the migration guide later this day or tomorrow |
|
Upstream issue: opentracing-contrib/java-jdbc#116 |
loicmathieu
added
the
triage/upstream
|
Wow, I really appreciate this being taken care of so quickly! I can live with that workaround for now. Thank you all <3! |
|
So I also opened up an upstream PR opentracing-contrib/java-common#2 so we have a predictable interfaces array to configure the proxy, after this one is merged, we have to bump the common lib version in the jdbc lib... and just after that we have to go through Quarkus's JDBC libs adding the proxy configuration if it is a native build and has the OpenTracing feature enabled.... and we should be good to go with the newer versions 🎉 |
|
@luneo7 thanks for making an upsteam improvement by using an ordered collection. This will make things easier and we could add to the documentation how the user of the tracing driver can do it. However, the best is to avoid proxy creation, if possible, as, at Quarkus side, we cannot deal with all the possible databases interface list easily. |
|
@luneo7 the funny part will be to get the specific classes involved for each supported JDBC driver :). |
|
I added a section on the 2.0.0 migration guide: https://github.com/quarkusio/quarkus/wiki/Migration-Guide-2.0#opentracing-jdbc-instrumentation @gsmet maybe you want to have a look for typos ;) |
|
I created a PR to add an integration test so we might be able to catch those issue before releasing an RC: #18169 |
|
@gsmet it is fun, but when you use https://github.com/ronmamo/reflections is quite easy 😃 import io.opentracing.contrib.common.Classes;
import org.reflections.Reflections;
import java.lang.reflect.Modifier;
import java.sql.Connection;
import java.sql.Statement;
import java.util.Arrays;
import java.util.Comparator;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
public class ProxyConfig {
private static String getProxyConfig(String driverPackage) {
Reflections reflections = new Reflections(driverPackage);
StringBuilder stringBuilder = new StringBuilder("[\n");
appendInterfaces(reflections.getSubTypesOf(Connection.class), stringBuilder);
appendInterfaces(reflections.getSubTypesOf(Statement.class), stringBuilder);
stringBuilder.append("\n]");
return stringBuilder.toString();
}
private static <T> void appendInterfaces(Set<Class<? extends T>> classes, StringBuilder stringBuilder) {
classes.stream()
.filter(clazz -> !Modifier.isAbstract(clazz.getModifiers()) && !Modifier.isInterface(clazz.getModifiers()))
.sorted(Comparator.comparing(Class::getName))
.map(clazz -> "\t[" + Arrays.stream(Classes.getAllInterfaces(clazz))
.map(c -> "\"" + c.getName() + "\"")
.collect(Collectors.joining(",")) + "]")
.collect(Collectors.toCollection(LinkedHashSet::new))
.forEach(item -> stringBuilder.append(stringBuilder.length() == 2 ? item : ",\n" + item));
}
public static void main(String[] args) {
List<String> driverPackagePrefixes = Arrays.asList("org.postgresql",
"com.mysql",
"org.mariadb.jdbc",
"com.microsoft.sqlserver.jdbc",
"oracle",
"org.h2",
"org.apache.derby",
"com.ibm.db2");
driverPackagePrefixes.stream()
.collect(Collectors.toMap(k -> k, ProxyConfig::getProxyConfig))
.forEach((driverPackage, proxyConfig) -> {
System.out.println("----");
System.out.println("Driver package " + driverPackage + " proxy config:\n");
System.out.println(proxyConfig);
});
}
}So if the PR is merged upstream and we were to do for the JDBC drivers supported by Quarkus the proxy config for each driver would be as in this gist: https://gist.github.com/luneo7/8d4b6fbebc3d16576f1e56e31c3a7050 The opentracing jdbc lib just does proxy for the I've built this gist using the drivers from Quarkus BOM =] There are ways of doing this discovery and proxy config creation automatically in the opentracing deployment lib or somewhere else, and there is also the way of adding that to the deployment lib of each supported JDBC driver, or creating aa new lib for opentracing jdbc (quarkiverse?) for it... hahaha there are multiple ways of handling it =] |
|
@kenfinnigan @gsmet I can create one extension in Quarkiverse for OpenTracing JDBC contrib lib that does the native configuration for it automatically and also does the class substitution so the method behaves in a predictable manner and we don't wouldn't need to wait for the upstream merge to have it working, what do you guys think? |
|
I think that's reasonable |
|
Maybe wait for some feedback from the upstream issue I created. If we can avoid the creation of a new extension it would be better. |
|
Upstream PR never replied, indicating that OpenTracing is abandoned. Closing this because we are no longer supporting OpenTracing. |
Describe the bug
When using the dependencies
with the configuration
I get the following error logs in the console, and the application does not work:
It does work in 1.13.7.Final.
Expected behavior
The application should start up and work as expected, no error logs.
Actual behavior
See above logs :).
To Reproduce
I don't have a reproducer yet, maybe you already know what to do? If not, I can provide one. I guess it's just some class registration missing in graalvm for for the case I do have at hand here, which I think should be done by the dependencies already?
Configuration
Environment:
Output of
uname -aorverLinux lena-pc 5.11.0-18-generic #19-Ubuntu SMP Fri May 7 14:22:03 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Output of
java -versionopenjdk version "11.0.10" 2021-01-19
OpenJDK Runtime Environment GraalVM CE 21.0.0.2 (build 11.0.10+8-jvmci-21.0-b06)
OpenJDK 64-Bit Server VM GraalVM CE 21.0.0.2 (build 11.0.10+8-jvmci-21.0-b06, mixed mode, sharing)
Quarkus version or git rev
2.0.0-CR3
Build tool (ie. output of
mvnw --versionorgradlew --version)Gradle 7.0
Build time: 2021-04-09 22:27:31 UTC
Revision: d5661e3f0e07a8caff705f1badf79fb5df8022c4
Kotlin: 1.4.31
Groovy: 3.0.7
Ant: Apache Ant(TM) version 1.10.9 compiled on September 27 2020
JVM: 11.0.10 (GraalVM Community 11.0.10+8-jvmci-21.0-b06)
OS: Linux 5.11.0-18-generic amd64
Additional context
When I remove the dependencies and do not use the tracing, the application does start up and work as expected.
The text was updated successfully, but these errors were encountered: