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

Warn when transaction entered without calling start_transaction #3003

Merged

Conversation

szokeasaurusrex
Copy link
Member

This change should prevent confusion like what led to #2990 being opened.

Copy link
Member

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

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

Looks good, just one question.

sentry_sdk/tracing.py Outdated Show resolved Hide resolved
sentry_sdk/tracing.py Outdated Show resolved Hide resolved
sentry_sdk/tracing.py Show resolved Hide resolved
tests/tracing/test_misc.py Outdated Show resolved Hide resolved
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/transaction-not-started-warning branch from 00c2bc7 to 5cd3884 Compare April 23, 2024 09:17
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/transaction-sampled-debug-message branch from a090a5c to f8586eb Compare April 25, 2024 12:58
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/transaction-not-started-warning branch from 5cd3884 to f1346b0 Compare April 25, 2024 13:02
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/transaction-sampled-debug-message branch from f8586eb to d0fc544 Compare April 25, 2024 13:11
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/transaction-not-started-warning branch from f1346b0 to 84ab96a Compare April 25, 2024 13:12
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/transaction-sampled-debug-message branch from d0fc544 to 534a301 Compare April 25, 2024 13:26
Base automatically changed from szokeasaurusrex/transaction-sampled-debug-message to master April 25, 2024 13:40
sentry_sdk/tracing.py Outdated Show resolved Hide resolved
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/transaction-not-started-warning branch from 84ab96a to 314ef01 Compare May 28, 2024 15:44
@szokeasaurusrex
Copy link
Member Author

Made the requested change @antonpirker

Copy link
Member

@antonpirker antonpirker left a comment

Choose a reason for hiding this comment

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

Great, thanks for the change!

…ransaction`

Users who enter a transaction without calling `start_transaction` likely intended to start the transaction, since without a call to `start_transaction`, their transaction will not get sent to Sentry. This warning message clarifies this behavior, and could help avoid the confusion that led to issue #2990.

Also, add tests to ensure the message is logged.
@szokeasaurusrex szokeasaurusrex force-pushed the szokeasaurusrex/transaction-not-started-warning branch from ea19248 to 3fd09e5 Compare June 6, 2024 14:17
@szokeasaurusrex szokeasaurusrex enabled auto-merge (squash) June 6, 2024 14:17
@szokeasaurusrex szokeasaurusrex enabled auto-merge (squash) June 6, 2024 14:20
@szokeasaurusrex szokeasaurusrex merged commit c2af1b0 into master Jun 6, 2024
110 checks passed
@szokeasaurusrex szokeasaurusrex deleted the szokeasaurusrex/transaction-not-started-warning branch June 6, 2024 14:24
arjennienhuis pushed a commit to arjennienhuis/sentry-python that referenced this pull request Sep 30, 2024
Users who enter a transaction without calling `start_transaction` likely intended to start the transaction, since without a call to `start_transaction`, their transaction will not get sent to Sentry. This warning message clarifies this behavior, and could help avoid the confusion that led to issue getsentry#2990.

Also, add tests to ensure the message is logged.
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