Navigation Menu

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

fix: log trace should not log sql values #5288

Closed
wants to merge 2 commits into from

Conversation

ssoroka
Copy link

@ssoroka ssoroka commented Apr 22, 2022

What did this pull request do?

Log parameterized queries instead of full sql queries that may contain sensitive information

User Case Description

See #5287

Closes #5287

@jinzhu
Copy link
Member

jinzhu commented Apr 24, 2022

Hello @ssoroka

Thank you for your PR, but I think we can change log's callback method to

func(... logger.Config) (string, int64) and use the Config param to control returns parameterized or full SQL.

Can you make the change?

@ssoroka
Copy link
Author

ssoroka commented Apr 25, 2022

That sounds like it would be a breaking change for anyone who has a custom logger already. Is there another place to put the config that wouldn't be a breaking change? And if we do that, can we default to the secure option?

Edit: oh, I think I see what you're saying.

@dorsha
Copy link

dorsha commented Jul 26, 2022

Hi, any update with this one?
It is really important for us (and I think any consumer), since PII is getting leaked when enabling sql logs.
Any help needed to close this one?

@finnnark
Copy link

+1

@ssoroka
Copy link
Author

ssoroka commented Aug 19, 2022

PR Updated. let me know if this is what you were thinking. Test failures don't seem related to my change.

@dageev-hs
Copy link

Hi @ssoroka @demoManito
Any updates on this PR?

@thundering-herd
Copy link

Hej

Is there any way to support this to be integrated into gorm? @ssoroka @demoManito

@jinzhu jinzhu closed this in ddd3cc2 Dec 25, 2022
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.

Log only parameterized SQL statements without any values
6 participants