Skip to content

ref(node): Add propagations to http integration #5720

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 1 commit into from
Sep 13, 2022

Conversation

AbhiPrasad
Copy link
Member

Part of #5679

Dependent on #5714 merging

This PR updates the propagations metadata field in the node SDK for the HTTP integration.

@AbhiPrasad AbhiPrasad added this to the Dynamic Sampling milestone Sep 12, 2022
@AbhiPrasad AbhiPrasad self-assigned this Sep 12, 2022
@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.46 KB (+0.01% 🔺)
@sentry/browser - ES5 CDN Bundle (minified) 60.11 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified) 18.04 KB (-0.02% 🔽)
@sentry/browser - ES6 CDN Bundle (minified) 53.03 KB (0%)
@sentry/browser - Webpack (gzipped + minified) 19.83 KB (0%)
@sentry/browser - Webpack (minified) 64.42 KB (0%)
@sentry/react - Webpack (gzipped + minified) 19.85 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified) 44.91 KB (+0.04% 🔺)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified) 26.03 KB (+0.05% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified) 24.42 KB (+0.06% 🔺)

Base automatically changed from abhi-transaction-name-change-interface to master September 12, 2022 13:10
Copy link
Member

@lobsterkatie lobsterkatie left a comment

Choose a reason for hiding this comment

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

LGTM. Do we not need to do it in the https integration also, though?

@AbhiPrasad
Copy link
Member Author

Patching the Http integration should cover both the http and https integration, as per:

fill(httpsModule, 'get', wrappedHandlerMaker);
fill(httpsModule, 'request', wrappedHandlerMaker);

@AbhiPrasad AbhiPrasad force-pushed the abhi-node-sdk-propagations branch from 675f790 to e55496e Compare September 13, 2022 08:50
fix tests

rename name_changes -> changes

remaining nameChanges

ref(node): Add propagations to http integration

fix nextjs tests
@AbhiPrasad AbhiPrasad force-pushed the abhi-node-sdk-propagations branch from e55496e to 2bf9a72 Compare September 13, 2022 08:51
@AbhiPrasad AbhiPrasad enabled auto-merge (squash) September 13, 2022 08:51
@AbhiPrasad AbhiPrasad merged commit 8cff038 into master Sep 13, 2022
@AbhiPrasad AbhiPrasad deleted the abhi-node-sdk-propagations branch September 13, 2022 09:08
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