Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add hibernate.show_sql to dhis.conf #19873

Closed
wants to merge 1 commit into from

Conversation

vietnguyen
Copy link
Contributor

No description provided.

Copy link

sonarqubecloud bot commented Feb 6, 2025

* Show SQL statements generated by hibernate in the log. Can be 'true', 'false'. (default: false)
* This will also enable hibernate.format_sql and hibernate.highlight_sql
*/
HIBERNATE_SHOW_SQL("hibernate.show_sql", "false", false),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is this different from changing the log4j2 config to enable logging 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<!-- JDBC template queries (SQL) -->
<!--
<Logger name="org.springframework.jdbc.core" level="debug" additivity="false">
<AppenderRef ref="console"/>
</Logger>-->
<!-- Hibernate queries (SQL) -->
<!--<Logger name="org.hibernate.SQL" level="TRACE">
<AppenderRef ref="console"/>
</Logger>-->
<!-- Hibernate binding parameters -->
<!--<Logger name="org.hibernate.type.descriptor.sql.BasicBinder" level="TRACE">
<AppenderRef ref="console"/>
</Logger>-->
<!-- Uncomment if you want to debug our JUnit extension -->
<!--<Logger name="org.hisp.dhis.test.junit" level="debug">
<AppenderRef ref="console"/>
</Logger>-->

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@teleivo Sometimes changing the log4j config doesn't work for me. Have you ever experienced that ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mhm, I mean our log setup is pretty complex 😅 You need to configure the correct file depending on what/how you execute code. Maybe share your use case and we'll figure it out together. I just wonder if adding another way is not more confusing/complex 🤔 also if you want to see jdbc logs you'll need to go to log4j2.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My use case is just to know what queries hibernate are generating in test and also production code.

I think using the log4j config is more complicated, and I also have to remember revert the changed before merging.

Using dhis.conf is straight forward, and i just need to change my local file. Maybe it's easier for debugging deployed instance, you don't need to look for the log4j file in the built folder.

I don't think there is a confusion regarding adding two ways to enable hibernate debug. It just helps to make the debugging job easier.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using dhis.conf is straight forward, and i just need to change my local file. Maybe it's easier for debugging deployed instance, you don't need to look for the log4j file in the built folder.

I think that is due to a lack of understanding logs 😅 Users can provide their config. They do not need to touch the config we ship by default.

I don't think there is a confusion regarding adding two ways to enable hibernate debug. It just helps to make the debugging job easier.

I disagree but maybe others agree. With this you now have 2 options to enable hibernate logging. If you want to enable jdbc logging you are then left wondering why there is no dhis.conf option for that and have to go to log4j2 anyway.

@vietnguyen
Copy link
Contributor Author

Close this for now. If there would be more requests from others then we could reopen.

@vietnguyen vietnguyen closed this Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants