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

Log query execution time. #268

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

maciejtrybilo
Copy link

I've added logging for query execution time.

This is quite useful when monitoring performance of endpoints that issue more than one database query.

@maciejtrybilo maciejtrybilo requested a review from gwynne as a code owner July 5, 2024 10:07
@MahdiBM
Copy link
Contributor

MahdiBM commented Jul 5, 2024

Seems like you're looking for Tracing.
I think you're better off creating a centralized function for now in your own project, that logs any info that you'd like to log.

@maciejtrybilo
Copy link
Author

Tracing is fairly complicated to set up. IMO this solution gives quite a lot for free.

I'm not sure how I could set something like this up on my side without having to instrument each call to Fluent or SQLKit?

@MahdiBM
Copy link
Contributor

MahdiBM commented Jul 5, 2024

Hmm I know @gwynne is currently busy and might take a bit till she can answer, but i'd want to let her decide what we should do.

gives quite a lot for free

The logging does have additional costs though. It's worth it to do if you need the info, but one might not.

I'm not sure how I could set something like this up on my side without having to instrument each call to Fluent or SQLKit?

My Fluent knowledge is getting rusty ... but if you need such logging for Fluent queries or SQLKit queries, it might be better to have the log statements there, not in PostgresKit.

It also looks fine to me to have your own functions which instrument each call.

Again, I'll let Gwynne comment as she'd know better.

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