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

Enable debugging tracing of SQL queries #2467

Merged
merged 8 commits into from
Apr 21, 2023
Merged

Enable debugging tracing of SQL queries #2467

merged 8 commits into from
Apr 21, 2023

Conversation

craigpastro
Copy link
Contributor

Description

Closes #2389.

I had a quick go at trying to resolve #2389. Is this the approach you would like to take? I can see the spans in Jaeger, but, afaict, they are not part of anything else.

Screenshot 2023-04-18 at 10 10 22

Readiness checklist

  • I added/updated unit tests.
  • I added/updated integration/compatibility tests.
  • I added/updated comments and checked rendering.
  • I made spot refactorings.
  • I updated user documentation.
  • I ran task all, and it passed.
  • I ensured that PR title is good enough for the changelog.
  • (for maintainers only) I set Reviewers (@FerretDB/core), Labels, Project and project's Sprint fields.
  • I marked all done items in this checklist.

@codecov
Copy link

codecov bot commented Apr 18, 2023

Codecov Report

Merging #2467 (0a7aa45) into main (2cddf07) will decrease coverage by 38.16%.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2467       +/-   ##
===========================================
- Coverage   64.69%   26.53%   -38.16%     
===========================================
  Files         398      399        +1     
  Lines       19681    19714       +33     
===========================================
- Hits        12733     5232     -7501     
- Misses       6033    13925     +7892     
+ Partials      915      557      -358     
Impacted Files Coverage Δ
internal/handlers/pg/pgdb/pool.go 61.01% <100.00%> (+3.21%) ⬆️
internal/handlers/pg/pgdb/trace.go 100.00% <100.00%> (ø)

... and 128 files with indirect coverage changes

Flag Coverage Δ
integration 5.03% <0.00%> (-52.85%) ⬇️
mongodb 5.03% <0.00%> (-0.01%) ⬇️
pg ?
unit 25.17% <100.00%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@craigpastro craigpastro marked this pull request as ready for review April 20, 2023 02:40
@craigpastro craigpastro requested review from AlekSi and a team as code owners April 20, 2023 02:40
@craigpastro craigpastro requested a review from w84thesun April 20, 2023 02:40
@AlekSi AlekSi added the code/chore Code maintenance improvements label Apr 20, 2023
@AlekSi AlekSi added this to the v1.1.0 milestone Apr 20, 2023
@AlekSi AlekSi enabled auto-merge (squash) April 20, 2023 05:47
@AlekSi AlekSi requested review from rumyantseva and removed request for w84thesun April 20, 2023 05:50
Copy link
Contributor

@rumyantseva rumyantseva left a comment

Choose a reason for hiding this comment

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

Thank you very much for sending the PR!

Yes, this approach is something we want to start from. Of course, we want to add an ability to trace requests, so those pgdb spans will become parts of some other spans. But it's for a separate issue. For now, we just want to start seeing at least some data :)

I added a couple of comments about TraceQueryStart and TraceQueryEnd implementation - let's add some data as span attributes, so we can see some actual info about the queries in Jaeger.

internal/handlers/pg/pgdb/trace.go Outdated Show resolved Hide resolved
internal/handlers/pg/pgdb/trace.go Show resolved Hide resolved
auto-merge was automatically disabled April 20, 2023 17:37

Head branch was pushed to by a user without write access

Copy link
Contributor

@rumyantseva rumyantseva left a comment

Choose a reason for hiding this comment

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

Thanks for contributing! I tried everything locally, I think it's a great way to start collecting traces.

@w84thesun w84thesun enabled auto-merge (squash) April 21, 2023 15:12
@w84thesun w84thesun merged commit a7e36aa into FerretDB:main Apr 21, 2023
@craigpastro craigpastro deleted the resolve-2389-debug-traces branch April 21, 2023 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code/chore Code maintenance improvements
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Enable debugging tracing of SQL queries
4 participants