asyncer-io / r2dbc-mysql

Reactive Relational Database Connectivity for MySQL. The official successor to mirromutth/r2dbc-mysql(dev.miku:r2dbc-mysql).
https://r2dbc.io
Apache License 2.0
195 stars 21 forks source link

[feature] Support Logging Query Parameters #138

Closed jchrys closed 6 months ago

jchrys commented 1 year ago

Is your feature request related to a problem? Please describe. Currently, r2dbc-mysql supports logging the query sentence but does not include query parameters in the logs. This limitation makes debugging more challenging when dealing with parameterized queries.

Describe the solution you'd like I would like to request support for logging query parameters in r2dbc-mysql. When executing parameterized queries, the query parameters should be included in the logs alongside the query sentence. This addition would greatly aid developers in debugging and understanding the exact parameter values used during query execution.

Additional context As a developer, having access to the logged query parameters would be immensely helpful in troubleshooting issues related to database interactions. It would provide a comprehensive view of the queries being executed and the corresponding parameter values. This feature would significantly enhance the usability and debugging capabilities of r2dbc-mysql.

For instance, with the incorporation of this feature, resolving this issue would become effortless: https://github.com/asyncer-io/r2dbc-mysql/issues/129

PFCJeong commented 1 year ago

hi @jchrys. Thank you for your effort on maintaining this project.

Always thought this feature needed to be implemented. As far as i know, some other drivers have supported this feature for a long time.

I am thinking of referring to jasync and postgresql. They log pair of (index, parameter) in Statement.bind functions. (jasync, postgresql)

And, maybe we can add additional information like mysql data type as hibernate does.

MySqlParameter.getType() can be used for that, but as i remember, it didn't strictly follow actual server data type (Correct me if i am wrong.)

2023-08-30T10:10:56.795+09:00 TRACE 40886 --- [           main] org.hibernate.orm.jdbc.bind              : binding parameter [1] as [BIGINT] - [123]
2023-08-30T10:10:56.796+09:00 TRACE 40886 --- [           main] org.hibernate.orm.jdbc.bind              : binding parameter [2] as [VARCHAR] - [abcd]
2023-08-30T10:10:56.796+09:00 TRACE 40886 --- [           main] org.hibernate.orm.jdbc.bind              : binding parameter [3] as [TIMESTAMP_WITH_TIMEZONE] - [2023-08-30T10:10:56.637315+09:00[Asia/Seoul]]

I'll come back to you again with draft PR. Any guidance would be greatly appreciated.

Thanks!

jchrys commented 1 year ago

Hi, @PFCJeong. Thanks your interest in this project.

MySqlParameter.getType() can be used for that, but as i remember, it didn't strictly follow actual server data type (Correct me if i am wrong.)

Unfortunately, following the server's data type exactly during the query or query building phase won't be feasible. To log the target table's column type, we would need to maintain the table's schema in locally or fire additional query, which aren't advisable in a low-level driver like this one.

Please ping me once you create a PR. :D

mirromutth commented 8 months ago

We can make a simple log first, just print MySqlParameter[] when they're converting to a message object.

Currently, all MySqlParameter.toString will return REDACTED, which may not be a good idea. The consideration at the time was to prevent users from using lower log levels (such as DEBUG), thereby causing the driver to output sensitive data.

It should be easy to have MySqlParameter.toString return the internal value. And we should warn users in the README that it is best not to use a log level lower than INFO in an environment with real-world data.

jchrys commented 6 months ago

Resolved by #165