quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.38k stars 2.56k forks source link

flyway plugin combined with tracing flag requires extra 3rd party dpendency #38314

Open GeauxEric opened 5 months ago

GeauxEric commented 5 months ago

Describe the bug

If one uses the flyway extention, and also turn on the quarkus.datasource.jdbc.tracing=true flag, the service will fail to start with an error "Caused by: io.quarkus.runtime.configuration.ConfigurationException: Unable to load the tracing driver io.opentracing.contrib.jdbc.TracingDriver for the default datasource"

If one does not introduce the flyway extention, and turn on the quarkus.datasource.jdbc.tracing=true flag, it gives the warning

2024-01-19 16:42:28,740 WARN  [io.qua.config] (Quarkus Main Thread) Unrecognized configuration key "quarkus.datasource.jdbc.tracing" was provided; it will be ignored; verify that the dependency extension for this configuration is set or that you did not make a typo

Expected behavior

It is very surprising the quarkus.datasource.jdbc.tracing flag is associated with flyway plugin.

From the doc, the flag belongs to https://quarkus.io/guides/datasource. It leaves an impression that one enable tracing by just turning the flag, and no 3rd party dependency is needed. The document needs clarification.

Besides, I suspect that under the hood, combing flyway and quarkus.datasource.jdbc.tracing=true secretly set the driver for the default datasource, which may affect the the reactive sql client, and leads to another confusion as I reported here: https://stackoverflow.com/questions/77849325/does-quarkus-reactive-sql-client-support-tracing

Actual behavior

No response

How to Reproduce?

check out https://github.com/GeauxEric/flyway-tracing-bug

If one uncomment # quarkus.datasource.jdbc.tracing=true, the server will fail to start.

Output of uname -a or ver

Linux dell-lt 5.15.0-91-generic

Output of java -version

openjdk version "17.0.9" 2023-10-17

Quarkus version or git rev

3.6.6

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

quarkus-bot[bot] commented 5 months ago

/cc @brunobat (tracing), @cristhiank (flyway), @gastaldi (flyway), @geoand (flyway), @gsmet (flyway), @radcortez (tracing)

geoand commented 5 months ago

Your sample is using both the JDBC driver and reactive driver for PostgreSQL... Is there a reason for that or is is just an oversight?

brunobat commented 5 months ago

@GeauxEric, What tracing framework are you declaring in the dependencies?

GeauxEric commented 5 months ago

I did not declare any dependencies of tracing framework, and it is unclear to the developer whether they should declare any dependency or not in order to enable jdbc tracing. The doc (https://quarkus.io/guides/datasource) mentions it as a flag, and there is no corresponding flag for the reactive client.

GeauxEric commented 5 months ago

Your sample is using both the JDBC driver and reactive driver for PostgreSQL... Is there a reason for that or is is just an oversight?

@geoand jdbc driver was added in order to use flyway, as I was following the document here: https://quarkus.io/guides/flyway#developing-with-flyway

I did not know that quarkus-reactive-pg-client also contains a standalone driver.. if I removed jdbc driver from the build, then it gave the error: Caused by: io.quarkus.runtime.configuration.ConfigurationException: Unable to find a JDBC driver corresponding to the database kind 'postgresql' for the default datasource (available: ''). Check if it's a typo, otherwise provide a suitable JDBC driver extension, define the driver manually, or disable the JDBC datasource by adding 'quarkus.datasource.jdbc=false' to your configuration if you don't need it.

If quarkus.datasource.jdbc=false is added, then the flyway migration won't even run. and I guess that also makes quarkus.datasource.jdbc.tracing invalid?

As a quarkus framework user, I think the overall documentation is very good, but there are inconsistencies here and there, due to the inter-dependencies between the extensions.

geoand commented 5 months ago

but there are inconsistencies here and there

Unfortunately this is true and we try to address them promptly when discovered.

geoand commented 5 months ago

@GeauxEric is your goal simply to use a relational database with Flyway? In that case, I strongly suggest using the JDBC driver and not the reactive one.,

brunobat commented 5 months ago

@GeauxEric created this PR to clarify the Datasource tracing in the docs: https://github.com/quarkusio/quarkus/pull/38342

GeauxEric commented 5 months ago

is your goal simply to use a relational database with Flyway? In that case, I strongly suggest using the JDBC driver and not the reactive one.,

@geoand, do you mean reactive sql client does not work with Flyway extension? Then how are the users of reactive sql client supposed to manage the DB schema then?

geoand commented 5 months ago

There is an open issue about such support

GeauxEric commented 5 months ago

There is an open issue about such support

this is quite a surprising news to us developers (because that means reactive sql client is barely usable) ... is there a issue ID I can follow?

geoand commented 5 months ago

https://github.com/quarkusio/quarkus/issues/14682 is the one for Liquibase but it's essentially the same