quarkus icon indicating copy to clipboard operation
quarkus copied to clipboard

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

Open GeauxEric opened this issue 1 year ago • 12 comments

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

GeauxEric avatar Jan 20 '24 00:01 GeauxEric

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

quarkus-bot[bot] avatar Jan 20 '24 00:01 quarkus-bot[bot]

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 avatar Jan 22 '24 07:01 geoand

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

brunobat avatar Jan 22 '24 09:01 brunobat

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 avatar Jan 22 '24 17:01 GeauxEric

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.

GeauxEric avatar Jan 22 '24 17:01 GeauxEric

but there are inconsistencies here and there

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

geoand avatar Jan 23 '24 07:01 geoand

@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.,

geoand avatar Jan 23 '24 07:01 geoand

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

brunobat avatar Jan 23 '24 09:01 brunobat

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?

GeauxEric avatar Jan 24 '24 00:01 GeauxEric

There is an open issue about such support

geoand avatar Jan 24 '24 04:01 geoand

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?

GeauxEric avatar Jan 24 '24 22:01 GeauxEric

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

geoand avatar Jan 25 '24 07:01 geoand

Enabling tracing really requires a dependency on io.opentracing.contrib.jdbc.TracingDriver, which is not really documented, plus the Datasource tracing guide recommends using quarkus.datasource.jdbc.telemetry=true instead.

I created the following issue asking to deprecate the quarkus.datasource.jdbc.tracing setting and have it removed in a future version (or have it better documented):

  • https://github.com/quarkusio/quarkus/issues/42563

I'm going to close this issue for now, feel free to reopen if you feel we need to take another look at this.

Thanks for reporting this!

gastaldi avatar Aug 15 '24 03:08 gastaldi

The Flyway extension should be using the quarkus.datasource.jdbc.telemetry property instead.

brunobat avatar Aug 16 '24 11:08 brunobat