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

ref(tracing): Add transaction source to default router #5386

Merged
merged 5 commits into from
Jul 7, 2022

Conversation

AbhiPrasad
Copy link
Member

ref: #5345

@AbhiPrasad AbhiPrasad requested review from a team, lforst and Lms24 and removed request for a team July 7, 2022 16:28
@AbhiPrasad AbhiPrasad mentioned this pull request Jul 7, 2022
15 tasks
@AbhiPrasad AbhiPrasad self-assigned this Jul 7, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2022

size-limit report 📦

Path Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified) 19.34 KB (+0.01% 🔺)
@sentry/browser - ES5 CDN Bundle (minified) 59.86 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 17.94 KB (+0.01% 🔺)
@sentry/browser - ES6 CDN Bundle (minified) 52.78 KB (0%)
@sentry/browser - Webpack (gzipped + minified) 19.71 KB (0%)
@sentry/browser - Webpack (minified) 64.15 KB (0%)
@sentry/react - Webpack (gzipped + minified) 19.73 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified) 43.93 KB (+0.05% 🔺)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 25.7 KB (+0.1% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 23.98 KB (+0.12% 🔺)

@Lms24
Copy link
Member

Lms24 commented Jul 7, 2022

Do we have integration tests for the default router? If yes, we could try to add a test that the source really ends up in the event payload. (Optional, can also be done in a separate PR)

@AbhiPrasad
Copy link
Member Author

Yes! We can take a look with integration tests. Will add.

Comment on lines +211 to +216
...(parentContextFromHeader && {
metadata: {
...context.metadata,
...parentContextFromHeader.metadata,
},
}),
Copy link
Member Author

Choose a reason for hiding this comment

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

Had to add this to make the integration tests pass. TODO on coming back to it

@AbhiPrasad AbhiPrasad enabled auto-merge (squash) July 7, 2022 18:41
@AbhiPrasad AbhiPrasad merged commit 537527e into master Jul 7, 2022
@AbhiPrasad AbhiPrasad deleted the abhi-default-router-source branch July 7, 2022 18:42
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.

None yet

3 participants