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

test(browser): Add integration test for changing transaction name in beforeSendTransaction #14495

Merged
merged 3 commits into from
Nov 27, 2024

Conversation

Lms24
Copy link
Member

@Lms24 Lms24 commented Nov 26, 2024

This came up in an internal conversation: Some users need to update the transcation name in beforeSendTransaction. While we strictly recommend changing the root span/transaction name as early as possible, it should nevertheless be possible at the end of the root span lifecycle, too.

This test demonstrates that this works but it also shows "unexpected" behaviour: Despite users setting an explicit source in beforeSendTransaction, we overwrite the source to 'custom'. There's good reason for this in general: Changing the name of something the SDK previously identified as source url or route should be reflected by the source. But we don't account for the case where users explicitly set the source.

For the moment though, this not an actual problem because sources route and custom are treated equally by Relay.

@Lms24 Lms24 self-assigned this Nov 26, 2024
Copy link

codecov bot commented Nov 26, 2024

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
663 1 662 24
View the top 1 failed tests by shortest run time
tracing/browserTracingIntegration/pageload-update-txn-name/test.ts sets the source to custom when updating the transaction name
Stack Traces | 30s run time
test.ts:13:11 sets the source to custom when updating the transaction name

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@Lms24 Lms24 requested review from a team, lforst, chargome and s1gr1d and removed request for a team November 26, 2024 19:08
@Lms24 Lms24 enabled auto-merge (squash) November 27, 2024 12:38
@Lms24 Lms24 merged commit c3c3910 into develop Nov 27, 2024
153 checks passed
@Lms24 Lms24 deleted the lms/test-browser-beforeSendTransaction branch November 27, 2024 12:55
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.

3 participants