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.

Sign up for GitHub

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

Jump to bottom

Django database transaction duration has 0 duration #1385

Closed
fredgido opened this issue Apr 1, 2022 · 5 comments
Closed

Django database transaction duration has 0 duration #1385

fredgido opened this issue Apr 1, 2022 · 5 comments
Assignees
@sl0thentr0py

Comments

@fredgido
Copy link

fredgido commented Apr 1, 2022 •
edited
Loading

How do you use Sentry?

Sentry Saas (sentry.io)

Version

1.5.8

Steps to Reproduce

  1. Use sentry 1.5.8 with latest django 3.2
  2. Enable django integration and transactions
  3. Create a transaction of one request with database queries

Expected Result

Queries last anything from 1ms to 20ms depending on whatever the query is
This is what happens in 1.5.7

Actual Result

everything takes 0.01ms to 0.02ms
image
@sl0thentr0py
Copy link
Member

@fredgido I'd need some sort of minimal reproduction to take a look at this.

@fredgido
Copy link
Author

fredgido commented Apr 4, 2022 •
edited
Loading

@fredgido I'd need some sort of minimal reproduction to take a look at this.

https://gitlab.com/fred.fernandes/minimal_sentry/-/tree/master/app
Built a simple example here
2022-04-04_14-17
1.5.8
image

1.57

do you need my traces ids?

@sl0thentr0py sl0thentr0py self-assigned this Apr 4, 2022
@sl0thentr0py
Copy link
Member

uhh thanks for the report @fredgido, this is a regression caused by #1368 because we got rid of the yield in the context manager so the time is measured incorrectly now.
will try to fix soon.

sl0thentr0py added a commit that referenced this issue Apr 8, 2022
@sl0thentr0py
Revert "fix(sqlalchemy): Change context manager type to avoid race in…
27abc12 
… threads (#1368)"

This reverts commit de0bc50.
This caused a regression (#1385) since the span finishes immediately in
__enter__ and so all db spans have wrong time durations.
@sl0thentr0py sl0thentr0py mentioned this issue Apr 8, 2022
Merged
@antonpirker
Copy link
Member

Hey @fredgido !
@sl0thentr0py has created a fix for this and we will release this on Monday.
Thanks again for reporting this. If you send me your mail address to anton [dot] pirker [at] sentry {dot} io I can send you a couple of stickers as a small "Thank you" for your contribution!

antonpirker pushed a commit that referenced this issue Apr 11, 2022
@sl0thentr0py
fix(sqlalchemy): Use context instead of connection in sqlalchemy inte…
9a0c133 
…gration (#1388)

* Revert "fix(sqlalchemy): Change context manager type to avoid race in threads (#1368)"

This reverts commit de0bc50.
This caused a regression (#1385) since the span finishes immediately in
__enter__ and so all db spans have wrong time durations.

* Use context instead of conn in sqlalchemy hooks
@sl0thentr0py
Copy link
Member

@fredgido should be fixed in 1.5.9, can you give it a try and reopen if persists?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sl0thentr0py sl0thentr0py

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@antonpirker @sl0thentr0py @stephanie-anderson @fredgido