-
-
Notifications
You must be signed in to change notification settings - Fork 446
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
Tracing support #1053
base: master
Are you sure you want to change the base?
Tracing support #1053
Conversation
What are some examples of other database drivers that have adopted these logging conventions? |
@sfackler I did some digging, here is a list at the OTEL registry. A specific example in code in the first item of using the OTEL semantic conventions, go-pg. |
This looks fantastic! I was actually just looking at implementing this exact same thing, and you've done it better than I would have. I will apply this patch and try it out in production for an app that I work on. |
This works great, I ended up making two changes:
|
I merged in that commit and bumped all the trace levels as well. Thanks! |
From what I can tell most or all of the libraries at that OTEL reference are external libraires that you can use with a database driver to apply tracing conventions. I am hesitant to land this for a couple of reasons:
I think you'd probably be better served by following what those other libraries do and making a separate library that can wrap tokio-postgres and apply the appropriate tracing logic. |
I think a separate library would be fine, though it seems like a hook system would be necessary to ergonomically log everything we have in this PR. For example, there is currently no good way for a separate library to properly trace connection attempts with all of the relevant metadata, especially with timings for each phase. |
That approach seems like it could work in principle - I'd be interested in what the hook trait would look like. |
This PR:
DEBUG
orTRACE
level.Example trace overview on real application (vaultwarden):
Breaking down to find a slow query:
Note that this PR does not add distributed tracing support into the database, as it is (AFAIK) experimental and not well supported -- it would also add some extra dependencies.
Fixes #642