Skip to content

fix(tracing): Add sentry-trace header to outgoing http(s) requests in node #3053

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 4 commits into from
Dec 2, 2020

Conversation

lobsterkatie
Copy link
Member

@lobsterkatie lobsterkatie commented Nov 17, 2020

This brings @sentry/node in line with @sentry/browser, in that any outgoing request recorded as a span will now correctly include the sentry-trace header.

Main Changes:

  • Split integrations/http utils off into their own file (things were getting unwieldy). urlToOptions and normalizeRequestArgs are the only actual additions there.

  • Handle all allowable inputs to http(s).request() and http(s).get (we were missing the option of passing a URL object) by normalizing those inputs to one of two options, down from ten.

  • Fix the types in various places where we had them backwards (requests typed as responses and vice-versa).

  • Move our node types to version 10 as a) until two days ago, that version included method in the http.ClientRequest type whereas version 11 didn't for some reason, and b) even now that that's moot, if we supposedly support node versions back to 6, we shouldn't be using types which didn't exist back then. v10 is the earliest node version supported by @types/node, and is still being actively maintained, so it seemed like a good compromise.

  • (The whole point) Add sentry-trace header to outgoing requests.

@lobsterkatie lobsterkatie force-pushed the kmclb-node-http-sentry-trace-header branch from 103e760 to 1eaa6af Compare November 17, 2020 19:37
@lobsterkatie lobsterkatie changed the title [WIP] fix(tracing): Add sentry-trace header to outgoing http(s) requests in node fix(tracing): Add sentry-trace header to outgoing http(s) requests in node Nov 17, 2020
@lobsterkatie lobsterkatie marked this pull request as ready for review November 17, 2020 19:38
@lobsterkatie lobsterkatie requested a review from a team November 17, 2020 19:38
@github-actions
Copy link
Contributor

github-actions bot commented Nov 17, 2020

size-limit report

Path Size
@sentry/browser - CDN Bundle (gzipped) 19.73 KB (-0.01% 🔽)
@sentry/browser - Webpack 20.6 KB (0%)
@sentry/react - Webpack 20.6 KB (0%)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped) 26.88 KB (0%)

@lobsterkatie lobsterkatie force-pushed the kmclb-node-http-sentry-trace-header branch 10 times, most recently from 80e172b to ec39c29 Compare November 24, 2020 02:01
@lobsterkatie lobsterkatie force-pushed the kmclb-node-http-sentry-trace-header branch from ec39c29 to 6d34c9f Compare December 1, 2020 06:42
@lobsterkatie lobsterkatie force-pushed the kmclb-node-http-sentry-trace-header branch from 6d34c9f to 4d76edd Compare December 1, 2020 18:43
@lobsterkatie lobsterkatie force-pushed the kmclb-node-http-sentry-trace-header branch from 4d76edd to dcee804 Compare December 1, 2020 18:49
/**
* Normalize inputs to `http(s).request()` and `http(s).get()`.
*
* Legal inputs to `http(s).request()` and `http(s).get()` can take one of ten forms:
Copy link
Contributor

Choose a reason for hiding this comment

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

ten forms

😢

@lobsterkatie lobsterkatie merged commit 5b67fc9 into master Dec 2, 2020
@lobsterkatie lobsterkatie deleted the kmclb-node-http-sentry-trace-header branch December 2, 2020 18:47
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