getsentry / sentry-python

The official Python SDK for Sentry.io
https://sentry.io/for/python/
MIT License
1.93k stars 509 forks source link

Exception attributed to previous transaction #2064

Open sentrivana opened 1 year ago

sentrivana commented 1 year ago

How do you use Sentry?

Sentry Saas (sentry.io)

Version

1.21.1

Steps to Reproduce

  1. Create a virtual env, activate it, and install sentry_sdk.
  2. Create a minimal app.py with this:
import sentry_sdk
from sentry_sdk import start_transaction

sentry_sdk.init(
    dsn="<add_me>",
    debug=True,
    traces_sample_rate=1.0,
)

with start_transaction(name="my_transaction"):
    pass

raise ValueError('Oh no!')
  1. Run app.py.
  2. Find the event in Sentry.

Expected Result

The Sentry error event has no transaction tag since my_transaction has already concluded by the time the exception occurred.

Actual Result

The Sentry error event has a transaction tag with my_transaction attached.

Screenshot 2023-05-03 at 09 32 52

StabbarN commented 2 months ago

Why was this closed? We are seeting this and are confused.

When it has exited the with block the transaction should have finished. The transaction's name should't show in Sentry issue when some error was raised later and outside the with start_transaction block.

https://github.com/getsentry/sentry-python/blob/master/sentry_sdk/api.py#L368-L369

sentrivana commented 2 months ago

Hey @StabbarN, agree with you there. I think the SDK makes the assumption that as long as you have tracing enabled, there is always an active transaction. This is what most of our auto instrumentations are doing. So basically we don't clean up properly because we expect the transaction to simply be replaced by another. (Just an assumption from glancing at the code -- will investigate this properly further.) If that's the case, we just need to make sure the transaction is properly unset at the end of the with block.

Can you tell me a bit more about your setup if you're seeing this in the wild? I assume you're using custom instrumentation?

StabbarN commented 2 months ago

Sure, we use Sentry with Django. We have a background service that execute queued jobs and for that we use custom instrumentation:

from django.db import transaction
while True:
    try:
        job = claim_job(...)
    except:
        continue
    if job:
        with start_transaction(name=job.task_name):
            job.execute(...)
    else:
        sleep(1)

If claim_job(...) raises an exception then a Sentry issue/event with Sentry transaction named job.task_name will be seen on sentry.io, but that's the previous job.

We have resolved our issue by wrapping claim_job in another start_transaction but it would be nicer if the transaction was properly cleaned up.

sentrivana commented 2 months ago

Gotcha, thanks for the example! Yes, we should fix this. We're also working on replacing our tracing with OpenTelemetry in the near future so if we don't get around to fixing this soon, I imagine it'll go away with the switch to OTel. Keeping this open until we fix it or we've verified it's gone.