gavlyukovskiy / spring-boot-data-source-decorator

Spring Boot integration with p6spy, datasource-proxy, flexy-pool and spring-cloud-sleuth
Apache License 2.0
870 stars 83 forks source link

Add a new configuration that will format the SQL #89

Closed rvullriede closed 1 year ago

rvullriede commented 1 year ago

This PR introduces a new configuration property decorator.datasource.datasource-proxy.format-sql that leverages the new ProxyDataSourceBuilder.FormatQueryCallback() from datasource-proxy-1.9 to enable to log formatted output. It uses Hibernate's formatter if present on the class path (which is the case for 99% of the user due to it being the default implementation used by Spring Boot). If somebody opted in for a different JPA provider, https://github.com/vertical-blank/sql-formatter can be added as a runtime dependency to the app to enable this.

See https://github.com/jdbc-observations/datasource-proxy/pull/94 for additional information.

rvullriede commented 1 year ago

@gavlyukovskiy would you mind having a look?

Thanks :-) Raphael

gavlyukovskiy commented 1 year ago

Hey @rvullriede, sorry for delayed response. I like the idea of formatter, but I'd like to implement it a bit differently - instead of using classExists I'd rather have autoconfigurations depending on the formatter class that would create appropriate FormatQueryCallback:

    @Configuration(proxyBeanMethods = false)
    @ConditionalOnClass(BasicFormatterImpl.class)
    @ConditionalOnProperty(name = "decorator.datasource.datasource-proxy.format-sql", havingValue = "true", matchIfMissing = true)
    static class HibernateFormatterConfiguration {

        @Bean
        @ConditionalOnMissingBean // let users define their own
        public FormatQueryCallback hibernateFormatQueryCallback() {
            BasicFormatterImpl hibernateFormatter = new BasicFormatterImpl();
            return hibernateFormatter::format;
        }
    }

    @Configuration(proxyBeanMethods = false)
    @ConditionalOnClass(SqlFormatter.class)
    @ConditionalOnProperty(name = "decorator.datasource.datasource-proxy.format-sql", havingValue = "true", matchIfMissing = true)
    static class SqlFormatterConfiguration {

        @Bean
        @ConditionalOnMissingBean // let users define their own
        public FormatQueryCallback sqlFormatterFormatQueryCallback() {
            return SqlFormatter::format;
        }
    }

then in the builder you can inject this optional bean and if exists - configure to use it.

It is a bit more complicated, but this way it would be more extensible in the future if there are other formatter dependencies we'd like to add, or, if users have their own preferred formatter they could always simply define a bean to override default formatter.

rvullriede commented 1 year ago

Hi, @gavlyukovskiy

Thanks for your feedback. Your suggestion is certainly more the Spring way, I'll rework the PR accordingly.

rvullriede commented 1 year ago

PR has been updated! Since the log output changes quite a bit I didn't use matchIfMissing = true to keep the default behaviour unchanged.

gavlyukovskiy commented 1 year ago

Thank you for the PR @rvullriede!

gavlyukovskiy commented 1 year ago

Since the default is now not changed, I have updated your changes with an exception instead of a warning.