Skip to content

feat(core): Set custom transaction source for event processors #5722

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

Merged
merged 2 commits into from
Sep 13, 2022

Conversation

AbhiPrasad
Copy link
Member

Part of #5679

Dependent on #5714 merging

This PR updates BaseClient class to set a custom transaction source if event processors change the transaction name. It also adds metadata around a transaction name change when this is done.

@AbhiPrasad AbhiPrasad mentioned this pull request Sep 12, 2022
10 tasks
@github-actions
Copy link
Contributor

github-actions bot commented Sep 12, 2022

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 19.53 KB (+0.37% 🔺)
@sentry/browser - ES5 CDN Bundle (minified) 60.3 KB (+0.33% 🔺)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 18.12 KB (+0.41% 🔺)
@sentry/browser - ES6 CDN Bundle (minified) 53.22 KB (+0.36% 🔺)
@sentry/browser - Webpack (gzipped + minified) 19.91 KB (+0.37% 🔺)
@sentry/browser - Webpack (minified) 64.61 KB (+0.31% 🔺)
@sentry/react - Webpack (gzipped + minified) 19.93 KB (+0.38% 🔺)
@sentry/nextjs Client - Webpack (gzipped + minified) 44.98 KB (+0.15% 🔺)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 26.09 KB (+0.25% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 24.47 KB (+0.24% 🔺)

Base automatically changed from abhi-transaction-name-change-interface to master September 12, 2022 13:10
Comment on lines +663 to +667
if (isTransaction && transactionInfo && processedEvent.transaction !== event.transaction) {
const source = 'custom';
Copy link
Member

Choose a reason for hiding this comment

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

When I first saw this, my gut reaction was to worry that it was going to break the RequestData integration, until I realized that the integration shouldn't ever change a transaction event's transaction, only an error event's transaction. So we're good there, but still, I wonder if it might not be worth putting a note here basically saying "we know none of our stuff does this, so if it happened, it must be because of some custom event processor added by the user."

@AbhiPrasad AbhiPrasad force-pushed the abhi-baseclient-name-changes branch 2 times, most recently from 113d518 to 5b360ef Compare September 13, 2022 08:56
@AbhiPrasad AbhiPrasad enabled auto-merge (squash) September 13, 2022 08:56
@AbhiPrasad AbhiPrasad force-pushed the abhi-baseclient-name-changes branch from 5b360ef to 6d15645 Compare September 13, 2022 09:16
@AbhiPrasad AbhiPrasad merged commit ff4f97f into master Sep 13, 2022
@AbhiPrasad AbhiPrasad deleted the abhi-baseclient-name-changes branch September 13, 2022 09:36
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