Skip to content

Conversation

@JonasBa
Copy link
Member

@JonasBa JonasBa commented Nov 8, 2022

We want to reduce the network pressure during profiling and reuse the connection if possible so that we dont block test execution when profiling tests. This change allows us to leverage http connection keepalive by overriding the default SDK value (false). Since the tests that we profile run on node 18, we should not see any memory leaks and it is probably a good practice to allow an override here anyways.

@JonasBa JonasBa requested review from a team and AbhiPrasad November 8, 2022 18:30
@AbhiPrasad AbhiPrasad added the Package: node Issues related to the Sentry Node SDK label Nov 8, 2022
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

How hard could it be to write a test? If it's too long, not worth the effort.

I also took the time to open a docs PR for this: getsentry/sentry-docs#5740

Once lint passes we can :shipit:

@JonasBa
Copy link
Member Author

JonasBa commented Nov 9, 2022

@AbhiPrasad thank you for the docs 🙏🏼 I added a test to check if connection: 'keep-alive' header is present on request.

@getsentry getsentry deleted a comment from github-actions bot Nov 9, 2022
@AbhiPrasad AbhiPrasad merged commit c59c70b into master Nov 9, 2022
@AbhiPrasad AbhiPrasad deleted the jb/feat/node-keepalive branch November 9, 2022 16:14
@AbhiPrasad
Copy link
Member

This change was part of the latest release https://github.com/getsentry/sentry-javascript/releases/tag/7.19.0!

docs merged also: getsentry/sentry-docs#5740

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Package: node Issues related to the Sentry Node SDK

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants