feat(node): Update `httpIntegration` handling of incoming requests #17371

mydea · 2025-08-11T09:00:44Z

This PR updates the handling of the Node SDK of incoming requests. Instead of relying on @opentelemetry/instrumentation-http for this, we now handle this internally, ensuring that we can optimize performance as much as possible and avoid interop problems. This will also allow us to extract this out of an OTEL instrumentation, eventually paving the way for a non-OTEL SDK with basic tracing capabilities.

This change should not affect users, unless they are relying on very in-depth implementation details. Importantly, this also drops the _experimentalConfig option of the integration - this will no longer do anything.
Finally, you can still pass instrumentation.{requestHook,responseHook,applyCustomAttributesOnSpan} options, but they are deprecated and will be removed in v11. Instead, you can use the new incomingRequestSpanHook configuration option if you want to adjust the incoming request span.

I have tried to ensure the spans look as similar as possible as before, this should be reflected in the tests.

github-actions · 2025-08-11T09:10:56Z

size-limit report 📦

Path	Size	% Change	Change
@sentry/browser	24.16 kB	-	-
@sentry/browser - with treeshaking flags	22.73 kB	-	-
@sentry/browser (incl. Tracing)	39.87 kB	-	-
@sentry/browser (incl. Tracing, Replay)	77.99 kB	-	-
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags	67.83 kB	-	-
@sentry/browser (incl. Tracing, Replay with Canvas)	82.69 kB	-	-
@sentry/browser (incl. Tracing, Replay, Feedback)	94.79 kB	-	-
@sentry/browser (incl. Feedback)	40.83 kB	-	-
@sentry/browser (incl. sendFeedback)	28.81 kB	-	-
@sentry/browser (incl. FeedbackAsync)	33.7 kB	-	-
@sentry/react	25.88 kB	-	-
@sentry/react (incl. Tracing)	41.86 kB	-	-
@sentry/vue	28.64 kB	-	-
@sentry/vue (incl. Tracing)	41.69 kB	-	-
@sentry/svelte	24.18 kB	-	-
CDN Bundle	25.66 kB	-	-
CDN Bundle (incl. Tracing)	39.76 kB	-	-
CDN Bundle (incl. Tracing, Replay)	75.82 kB	-	-
CDN Bundle (incl. Tracing, Replay, Feedback)	81.27 kB	-	-
CDN Bundle - uncompressed	74.96 kB	-	-
CDN Bundle (incl. Tracing) - uncompressed	117.59 kB	-	-
CDN Bundle (incl. Tracing, Replay) - uncompressed	231.88 kB	-	-
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed	244.48 kB	-	-
@sentry/nextjs (client)	43.89 kB	-	-
@sentry/sveltekit (client)	40.32 kB	-	-
@sentry/node-core	49.59 kB	+3.35%	+1.6 kB 🔺
@sentry/node	147.99 kB	+0.66%	+961 B 🔺
@sentry/node - without tracing	93.01 kB	+0.84%	+769 B 🔺
@sentry/aws-serverless	104.3 kB	+0.61%	+632 B 🔺

View base workflow run

mydea · 2025-08-11T11:31:20Z

dev-packages/node-integration-tests/suites/express/tracing/updateName/test.ts

      const runner = createRunner(__dirname, 'server.js')
        .expect({
          transaction: {
-            transaction: 'GET /test/:id/span-updateName-source',
+            transaction: 'new-name',


this is actually... fixed here now? Not quite sure why/how, but apparently some timing stuff is better now?

this indicates that the retroactive span name extraction is no longer use. Which makes sense to me if we now set a good name right from the start!

(I think this is fine btw. Actually leaves less room for user error in case they don't use the Sentry.updateSpanName helper)

…17373) Add tests that definitely show: 1. How get and post spans are captured 2. How URLs are handled with params and fragments This should make #17371 much easier to verify.

mydea · 2025-08-12T14:19:49Z

dev-packages/node-integration-tests/suites/tracing/httpIntegration/test.ts

-              status: 'ok',
-            },
-          },
+    describe('custom server.emit', () => {


these tests have been added to verify that everything still works even if a user (or some other library) also wraps server.emit. This was the case in the nestjs-distributed-tracing E2E test.

mydea · 2025-08-13T09:35:08Z

packages/node-core/src/integrations/http/incoming-requests.ts

+              span.end();
+
+              // Update the transaction name if the route has changed
+              if (newAttributes['http.route']) {


Since we control this now, we can actually do this in a generalized way instead of doing this per-framework, I think most if not all of them (e.g. express etc.) use this mechanism to set the route, so we could eventually remove the per-integration code for this in many places, I believe (and we'll also automatically support other frameworks in the future). cc @Lms24

mydea · 2025-08-13T09:35:34Z

dev-packages/e2e-tests/test-applications/node-express-esm-preload/tests/server.test.ts

-// This fails https://github.com/getsentry/sentry-javascript/pull/12587#issuecomment-2181019422
-// Skipping this for now so we don't block releases
-test.skip('Should record spans from http instrumentation', async ({ request }) => {
+test('Should record spans from http instrumentation', async ({ request }) => {


not sure if this is fixed by this specifically or something else, but this seems to work again now.

Lms24

Nice! Looking forward to basic tracing without always depending on OTel. Had some minor remarks but looks ready to me otherwise!

Lms24 · 2025-08-13T13:30:24Z

.size-limit.js

@@ -224,7 +224,7 @@ module.exports = [
    import: createImport('init'),
    ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
    gzip: true,
-    limit: '116 KB',
+    limit: '51 KB',


I guess we just forgot to adjust this? Would be pretty sweet otherwise xD

I just counted this wrongly 😅

CHANGELOG.md

Lms24 · 2025-08-13T13:34:54Z

dev-packages/node-integration-tests/suites/express/tracing/updateName/test.ts

      const runner = createRunner(__dirname, 'server.js')
        .expect({
          transaction: {
-            transaction: 'GET /test/:id/span-updateName-source',
+            transaction: 'new-name',


this indicates that the retroactive span name extraction is no longer use. Which makes sense to me if we now set a good name right from the start!

(I think this is fine btw. Actually leaves less room for user error in case they don't use the Sentry.updateSpanName helper)

packages/node-core/src/integrations/http/SentryHttpInstrumentation.ts

Lms24 · 2025-08-13T13:40:48Z

packages/node-core/src/integrations/http/incoming-requests.ts

+              span.end();
+
+              // Update the transaction name if the route has changed
+              if (newAttributes['http.route']) {


packages/node-core/src/integrations/http/index.ts

Extracted out of #17371 I noticed that we were not fully consistent in instrumentation IDs for integrations that have multiple instrumentation. The intent is that users can provide the _integration name_ (e.g. `Http`) and it will preload all http instrumentation. To achieve this, I adjusted the preload filter code to look for exact matches as well as `startsWith(`${name}.id`)`. I also adjusted the test to be more declarative and mock/reset stuff properly (this lead to issues in the linked PR, and should generally be a bit cleaner). I also updated all instrumentation IDs to follow this pattern. We should be mindful of following this with new instrumentation we add.

Co-authored-by: Lukas Stracke <[email protected]>

mydea requested review from Lms24, AbhiPrasad and andreiborza August 11, 2025 09:00

mydea self-assigned this Aug 11, 2025

mydea force-pushed the fn/ref-otel-incoming-request branch from 8d7f49c to e798a09 Compare August 11, 2025 10:49

mydea mentioned this pull request Aug 11, 2025

test(node): Add tests for full http.server span attribute coverage #17373

Merged

mydea force-pushed the fn/ref-otel-incoming-request branch from e798a09 to 57f8184 Compare August 11, 2025 11:30

mydea changed the base branch from develop to fn/add-full-attribute-http-span-test August 11, 2025 11:30

mydea commented Aug 11, 2025

View reviewed changes

mydea force-pushed the fn/ref-otel-incoming-request branch 3 times, most recently from 2f90c3c to c120396 Compare August 11, 2025 12:01

Base automatically changed from fn/add-full-attribute-http-span-test to develop August 11, 2025 13:00

mydea force-pushed the fn/ref-otel-incoming-request branch 2 times, most recently from a280e0c to 7b052e3 Compare August 12, 2025 14:15

mydea commented Aug 12, 2025

View reviewed changes

mydea force-pushed the fn/ref-otel-incoming-request branch from 8a65711 to d9e8b1f Compare August 13, 2025 09:32

mydea commented Aug 13, 2025

View reviewed changes

mydea mentioned this pull request Aug 13, 2025

fix(node): Fix preloading of instrumentation #17403

Merged

mydea force-pushed the fn/ref-otel-incoming-request branch from a6259bf to 434adfe Compare August 13, 2025 13:24

mydea marked this pull request as ready for review August 13, 2025 13:24

Lms24 approved these changes Aug 13, 2025

View reviewed changes

mydea force-pushed the fn/ref-otel-incoming-request branch from 78679a0 to b8613b0 Compare August 13, 2025 14:29

mydea added 3 commits August 14, 2025 08:51

feat(node): Update httpIntegration handling of incoming requests

c2353ac

fix nestjs ???

e3e0282

fixes

000a045

mydea and others added 6 commits August 14, 2025 08:51

revert nest.js change

cbeda1f

fixes

7c7447d

fix linting

65709fb

fix preload

b1f0918

Apply suggestion from @Lms24

4b3add7

Co-authored-by: Lukas Stracke <[email protected]>

revert changelog change, oops

ce6aaa1

mydea force-pushed the fn/ref-otel-incoming-request branch from b8613b0 to ce6aaa1 Compare August 14, 2025 06:51

This comment was marked as outdated.

Sign in to view

PR cleanup

ff6cc6d

Uh oh!

feat(node): Update httpIntegration handling of incoming requests #17371

Are you sure you want to change the base?

feat(node): Update httpIntegration handling of incoming requests #17371

Uh oh!

Conversation

mydea commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Lms24 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

feat(node): Update `httpIntegration` handling of incoming requests #17371

feat(node): Update `httpIntegration` handling of incoming requests #17371

mydea commented Aug 11, 2025 •

edited

Loading

github-actions bot commented Aug 11, 2025 •

edited

Loading

Lms24 left a comment •

edited

Loading