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: provide pgx logging wrapper #18

Closed
wants to merge 1 commit into from
Closed

feat: provide pgx logging wrapper #18

wants to merge 1 commit into from

Conversation

lzap
Copy link
Collaborator

@lzap lzap commented Mar 18, 2025

This PR provides a function which returns a wrapper that can be used for pgx SQL driver logging:

pgxConfig.Logger = sinit.PgxLogger()

Most projects do use pgx driver so I thought it will be useful to have it in the library. Unfortunately pulls few dependencies in as the interface is designed in a way it requires a constant from the pgx package so it cannot be copied.

@lzap lzap requested review from ezr-ondrej and schuellerf March 18, 2025 13:01
Copy link

@ezr-ondrej ezr-ondrej left a comment

Choose a reason for hiding this comment

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

I don't think it's a big deal to have the dependency, it's a bit annoying, but configuring pgx logging is really nice benefit to get.

@@ -137,6 +137,9 @@ var handlerMulti *strc.MultiHandler
var handlerSplunk *splunk.SplunkHandler
var handlerCloudWatch *cloudwatchwriter2.Handler

// The main logger instance. Use InitializeLogging to create it.
var logger *slog.Logger

Choose a reason for hiding this comment

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

do we need to have global logger instance?
Sounds like it's just for pgx, could we avoid it ?

@lzap
Copy link
Collaborator Author

lzap commented Mar 20, 2025

I just realized that PGX provides tracing capabilities and logging is subset of tracing, so this is actually not needed.

@lzap lzap closed this Mar 20, 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