quarkusio / quarkus

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

how can I tell what database I'm connecting to? #40922

Open gavinking opened 3 months ago

gavinking commented 3 months ago

Description [EDITED]

After a fairly recent change to log levels in Hibernate, I get no confirmation at all about what database Hibernate is connecting to without turning on DEBUG logging using quarkus.log.category."org.hibernate".level=DEBUG, which fills my log with hundreds of useless messages which I have to hunt through to find what I'm looking for.

This results in a surprisingly poor experience at development time. It's really hard to see, for example, whether setting:

quarkus.datasource.db-kind = postgresql
quarkus.datasource.username = hreact
quarkus.datasource.password = hreact
quarkus.datasource.jdbc.url = jdbc:postgresql://localhost:5432/hreact

actually had any effect at all, or whether I'm still just connecting to h2.

Implementation ideas

No response

gavinking commented 3 months ago

Quarkus reroutes all Hibernate's INFO-level logging to DEBUG level.

Hahaha, no, sorry, my bad. What actually happened is that fairly recently @Sanne changed a very useful message which has always been logged at INFO level to DEBUG (in this commit https://github.com/hibernate/hibernate-orm/commit/af86f96f274608f912479c7d9f53c88d6a157649).

I have never really noticed the change, because in my projects I have a standard log4j.properties file which sets:

# Print the selected dialect information
logger.hibernate-dialect.name = org.hibernate.orm.dialect
logger.hibernate-dialect.level = debug

which apparently is there precisely to undo the effect of that change.

Now, usually when I'm developing Hibernate, I would not even care much about this particular log message because there are a couple of other messages coming from DriverManagerConnectionProvider which log the JDBC URL and JDBC driver class, so I'm always sure exactly what database I'm connecting to.

But Agroal is much less noisy, logging nothing at all. So I don't see:

And I don't even know what database I'm connected to.

quarkus-bot[bot] commented 3 months ago

/cc @DavideD (hibernate-reactive), @gsmet (hibernate-orm), @mswatosh (db2), @yrodiere (hibernate-orm)

DavideD commented 3 months ago

Maybe it's not related to this issue, but Quarkus seems to only be able to select the correct dialect only for supported databases. It would be nice if quarkus would let us know when it's using the default dialect version because the database is not supported, and mention the property quarkus.datasource.db-version to override it.

yrodiere commented 3 months ago

You can certainly alter the extension to log a line looking something like:

Persistence unit '<default>' will connect to datasource 'something': jdbc:postgresql://localhost:5432/hreact. Assuming database version 14.0.

And append something like this if db-version is not set explicitly:

Persistence unit '<default>' will connect to datasource 'something': jdbc:postgresql://localhost:5432/hreact. Assuming database version 14.0. The database version defaults to the oldest supported value; if your database is more recent, set the build-time property `quarkus.datasource.db-version` to benefit from more features.

That being said, people in this project have strong opinions about logs on startup and have been actively hunting them down. I personally don't mind.

DavideD commented 3 months ago

It would be nice if quarkus would let us know when it's using the default dialect version because the database is not supported, and mention the property quarkus.datasource.db-version to override it.

I see now that Quarkus cannot select on its own the correct dialect + version. I'm not sure why these properties aren't mandatory (at least for unsupported databases), but I guess adding a message specifying this on each start up could be redundant.

yrodiere commented 3 months ago

I'm not sure why these properties aren't mandatory (at least for unsupported databases),

I'm pretty sure the dialect is mandatory for unsupported databases.

The version is always optional, and the reason is part upstream (ORM has default versions for all dialects anyway), part history (the concept of DB version only appeared in ORM 6) and part Quarkus "philosophy" ("thou shall not require any configuration for Quarkus to start").

To be clear, I also think it only makes sense to force people to explicitly select a DB version. I agree with you. We mandate it for Elasticsearch in Hibernate Search, by the way. In practice though:

  1. The default behavior of using the oldest supported version kind of works, at least as long as you're not relying on new-ish features.
  2. Requiring people to set the DB version explicitly in Quarkus would be a breaking change affecting lots of applications.

... so I don't see it happening anytime soon.

Displaying the defaulted version on startup could be a nice first step; at least people would be able to notice there is something wrong in their config.

DavideD commented 3 months ago

Thanks for the clarification.

Displaying the defaulted version on startup could be a nice first step; at least people would be able to notice there is something wrong in their config

Yes, I think this would be really helpful

gavinking commented 3 months ago

You can certainly alter the extension to log a line looking something like:

Persistence unit '<default>' will connect to datasource 'something': jdbc:postgresql://localhost:5432/hreact. Assuming database version 14.0.

And append something like this if db-version is not set explicitly:

Persistence unit '<default>' will connect to datasource 'something': jdbc:postgresql://localhost:5432/hreact. Assuming database version 14.0. The database version defaults to the oldest supported value; if your database is more recent, set the build-time property `quarkus.datasource.db-version` to benefit from more features.

Yes this is all reasonable. I would also log the product name of the database FWIW.

Sanne commented 3 months ago

+1 to improve the logged details and docs.

I think the wording should focus on stating what's the "minimal database version we'll be compatible with" though, that should make it clearer to users that there's some usefulness in this.

There's often a need to build an application which is compatible with a certain range of DB versions rather than what one is happening to connect to today, or developing with in their dev environment; if our users can see that they would understand that this inconvenience of needing to specify an additional property is actually a quite reasonable feature.

gavinking commented 3 months ago

So what we still need to establish is, exactly who is responsible for logging what:

I would propose that we add String getLoggableDescrption() or something to ConnectionProvider, so that the pool can give us a description of what it's connected to.

But then:

yrodiere commented 3 months ago

Does Hibernate itself log that? This is my preference

Note this will prevent:

So even if Hibernate ORM does it, we might want to override what's done and do it again in Quarkus with more context.

I'm not sure how we control whether this message will be logged at build vs run time in Quarkus

It basically depends where you do it in the build startup process. The later, the better as far as Quarkus is concerned. JdbcEnvironmentInitiator is not a good place as it happens before metadata creation (done at build time, or at least static init, in Quarkus); something that happens during SessionFactory creation would be a good place.

and we would need to ensure that Quarkus is not hiding Hibernate's log messages.

Quarkus only hides very specific messages, so it's unlikely it will hide a new log.

gavinking commented 3 months ago

https://hibernate.atlassian.net/browse/HHH-18224

gavinking commented 3 months ago

@yrodiere so it sounds like "both" is viable...