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

Remove TRANSACTION_SOURCE_UNKNOWN and default to CUSTOM #1558

Merged
merged 3 commits into from
Aug 11, 2022

Conversation

sl0thentr0py
Copy link
Member

@sl0thentr0py sl0thentr0py commented Aug 11, 2022

Fixes #1557
see getsentry/develop#667

unknown is only supposed to be inferred by relay as a default and not
set by any SDKs.
Additionally, fix some of the other cases where start_transaction was
being called without a source in integrations.

…USTOM

`unknown` is only supposed to be inferred by relay as a default and not
set by any SDKs.
Additionally, fix some of the other cases where start_transaction was
begin called without a source in integrations.

Partially verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

LGTM! Just a question: Are we sure that whenever we set TRANSACTION_SOURCE_ROUTE, the transaction name is actually a route and not some sort of unparameterized URL? Just asking because I'm probably not up to date w/ the latest status of high-cardinality transaction names in Pythonland.

@sl0thentr0py
Copy link
Member Author

@Lms24 yea I manually checked all the cases, route is here only for the Generic WSGI.. stuff which is also not exactly a route but just the default name.
Later in other integrations, we do the actual resolving and set the correct source. Examples

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@sl0thentr0py sl0thentr0py enabled auto-merge (squash) August 11, 2022 11:43
@Lms24
Copy link
Member

Lms24 commented Aug 11, 2022

@sl0thentr0py perfect, thanks!

@sl0thentr0py sl0thentr0py merged commit cf9c2d8 into master Aug 11, 2022
@sl0thentr0py sl0thentr0py deleted the neel/fix-source-unknown branch August 11, 2022 11:58
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.

Transaction source custom semantics not correct
2 participants