Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

perf(node): Short circuit flushing on Vercel only for Vercel #15734

Merged
merged 3 commits into from
Mar 21, 2025

Conversation

lforst
Copy link
Member

@lforst lforst commented Mar 19, 2025

Only adds the HttpInstrumentation that is supposed to flush for Vercel when we are on Vercel. Should reduce some overhead.

Also adds a bit more explanation because of #15602 (comment)

@lforst lforst requested a review from mydea March 19, 2025 12:52
@lforst lforst marked this pull request as ready for review March 19, 2025 12:52
Copy link
Member

@mydea mydea left a comment

Choose a reason for hiding this comment

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

This is good 👍
Maybe a follow up to this - could we only add this instrumentation if needed (e.g. when process.env.VERCEL is set)? THis would avoid this (slight, but still) overhead for all other platforms 🤔

@lforst
Copy link
Member Author

lforst commented Mar 19, 2025

Maybe a follow up to this - could we only add this instrumentation if needed (e.g. when process.env.VERCEL is set)? THis would avoid this (slight, but still) overhead for all other platforms 🤔

Actually, let me do this in this PR directly 👍 separation of concern wise I don't like it too much but for now it makes sense.

@lforst lforst changed the title docs(node): Add explanation for second http instrumentation perf(node): Short circuit flushing on Vercel only for Vercel Mar 20, 2025
@lforst lforst requested a review from mydea March 20, 2025 09:48
@lforst lforst merged commit f4cd540 into develop Mar 21, 2025
104 checks passed
@lforst lforst deleted the lforst-explanation-for-second-http-instrumentation branch March 21, 2025 08:13
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants