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

Add stored procedure parameters to debug logging #979

Merged
merged 1 commit into from
Feb 10, 2020
Merged

Add stored procedure parameters to debug logging #979

merged 1 commit into from
Feb 10, 2020

Conversation

eemelisoderholm
Copy link
Contributor

While the debug package can be used to see all queries and stored procedure executions, debugging proc-heavy applications is difficult since the log doesn’t contain any parameters.

AFAIK there’s no easy way to add global parameter logging without changing this package, correct me if I'm wrong.

This PR changes the stored procedure debug line to also contain all the provided parameters. Main drawback is additional noise in debug logs, but long strings in parameters are cut short to compensate.

Copy link
Collaborator

@dhensby dhensby left a comment

Choose a reason for hiding this comment

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

This is a cool idea, I like it - my only hesitation is that this may disclose sensitive information to logs, but on reflection the debug logs should only be used for debugging and not on production systems, so this should be safe enough.

@eemelisoderholm
Copy link
Contributor Author

While this definitely adds sensitive information to logs whenever the debug environment variable is set, I'd argue it already contains lots of sensitive information since regular queries are also printed.

Normally I'd approach this by just creating a wrapper function to serve the additional logging needs during development, but due to the chaining API, this seems non-trivial. If you've got any ideas about making this easier (log all execute calls with parameters) while not changing current behaviour, I'm open to helping with that too.

@dhensby
Copy link
Collaborator

dhensby commented Jan 22, 2020

I think this is an acceptable solution for now - my longer term goal is to add a generic "reporter" to the library and not have debug be a production requirement (see #840) but I don't have the time for that at the moment, so it's not going to happen for a while

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.

None yet

2 participants