Skip to content

Remove TRANSACTION_SOURCE_UNKNOWN and default to CUSTOM#1558

Merged
sl0thentr0py merged 3 commits intomasterfrom
neel/fix-source-unknown
Aug 11, 2022
Merged

Remove TRANSACTION_SOURCE_UNKNOWN and default to CUSTOM#1558
sl0thentr0py merged 3 commits intomasterfrom
neel/fix-source-unknown

Conversation

@sl0thentr0py
Copy link
Copy Markdown
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.
Copy link
Copy Markdown
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
Copy Markdown
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

@sl0thentr0py sl0thentr0py enabled auto-merge (squash) August 11, 2022 11:43
@Lms24
Copy link
Copy Markdown
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