Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Tracing support#1053

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
Protryon wants to merge4 commits intorust-postgres:master
base:master
Choose a base branch
Loading
fromProtryon:master
Open

Conversation

Protryon
Copy link

@ProtryonProtryon commentedJul 24, 2023
edited
Loading

This PR:

  • Adds tracing support followingOpenTelemetry database semantic conventions
  • Adds no new dependencies (tracing was already added transitively by other dependencies)
  • Will have no performance impact if no tracer or logger is setup at aDEBUG orTRACE level.

Example trace overview on real application (vaultwarden):

direct

Breaking down to find a slow query:

direct

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

LeoniePhiline reacted with heart emojiLeoniePhiline reacted with rocket emojirubdos and LeoniePhiline reacted with eyes emoji
@sfackler
Copy link
Collaborator

What are some examples of other database drivers that have adopted these logging conventions?

@Protryon
Copy link
Author

@sfackler I did some digging, here is a list at theOTEL registry.

A specific example in code in the first item of using the OTEL semantic conventions,go-pg.

@titanous
Copy link

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.

Protryon reacted with heart emoji

@titanous
Copy link

This works great, I ended up making two changes:

  1. I set all of the spans to INFO because that is the level that we send to Honeycomb. I think at the very least prepare should be bumped to DEBUG, as trace is almost always too noisy to be useful and prepare spans are useful to me (I try to have a span for every external network call).
  2. I added a span for connect, which delivered value as soon as I deployed it. Feel free to pull in this commit and/or modify it. I have already found the sub-span timings that I added to be useful as well.titanous@7e2e3a2

@Protryon
Copy link
Author

This works great, I ended up making two changes:

  1. I set all of the spans to INFO because that is the level that we send to Honeycomb. I think at the very least prepare should be bumped to DEBUG, as trace is almost always too noisy to be useful and prepare spans are useful to me (I try to have a span for every external network call).
  2. I added a span for connect, which delivered value as soon as I deployed it. Feel free to pull in this commit and/or modify it. I have already found the sub-span timings that I added to be useful as well.titanous@7e2e3a2

I merged in that commit and bumped all the trace levels as well. Thanks!

@sfackler
Copy link
Collaborator

sfackler commentedJul 30, 2023
edited
Loading

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:

  1. People can be using this library as a tracing appender, and this change can potentially cause some undesired infinite logging recursion. cfFeature Request: Optionally disable use of the log crate #215
  2. This seems like a convention that your particular organization wants to use, e.g. "I set all of the spans to INFO because that is the level that we send to Honeycomb", not something that is uniformly applicable.

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.

@titanous
Copy link

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.

@sfackler
Copy link
Collaborator

That approach seems like it could work in principle - I'd be interested in what the hook trait would look like.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

built-in tracing support
3 participants
@Protryon@sfackler@titanous

[8]ページ先頭

©2009-2025 Movatter.jp