feat(core, node): portable Express integration#19928
Merged
Conversation
Contributor
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
size-limit report 📦
|
Contributor
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
isaacs
force-pushed
the
isaacschlueter/portable-express-integration
branch
from
3cddcee to
dc1b9cf
Compare
isaacs
force-pushed
the
isaacschlueter/portable-express-integration
branch
5 times, most recently
from
64c45e7 to
fd4fefe
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
isaacs
force-pushed
the
isaacschlueter/portable-express-integration
branch
from
7e0d74f to
db667fc
Compare
isaacs
force-pushed
the
isaacschlueter/portable-express-integration
branch
2 times, most recently
from
b2d4e3c to
d5d4e9b
Compare
24 tasks
isaacs
force-pushed
the
isaacschlueter/portable-express-integration
branch
2 times, most recently
from
f694a6b to
23f24a1
Compare
isaacs
force-pushed
the
isaacschlueter/portable-express-integration
branch
from
23f24a1 to
37d4883
Compare
isaacs
force-pushed
the
isaacschlueter/portable-express-integration
branch
2 times, most recently
from
ff8586c to
c78e960
Compare
isaacs
force-pushed
the
isaacschlueter/portable-express-integration
branch
from
c78e960 to
3c1ebe0
Compare
isaacs
force-pushed
the
isaacschlueter/portable-express-integration
branch
from
2d55c15 to
4adacd5
Compare
isaacs
force-pushed
the
isaacschlueter/portable-express-integration
branch
from
fe09c38 to
f87db82
Compare
isaacs
force-pushed
the
isaacschlueter/portable-express-integration
branch
from
f87db82 to
ad38395
Compare
isaacs
force-pushed
the
isaacschlueter/portable-express-integration
branch
from
ad38395 to
15a772d
Compare
isaacs
force-pushed
the
isaacschlueter/portable-express-integration
branch
2 times, most recently
from
4c52729 to
f266a09
Compare
Lms24
approved these changes
Mar 31, 2026
Member
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
Thanks for addressing all the comments in my previous review! I had some minor suggestions but this looks good to me now!
packages/core/src/integrations/express/set-sdk-processing-metadata.ts
Outdated
Show resolved
Hide resolved
isaacs
force-pushed
the
isaacschlueter/portable-express-integration
branch
from
f266a09 to
6334480
Compare
This extracts the functionality from the OTel Express intstrumentation,
replacing it with a portable standalone integration in `@sentry/core`,
which can be extended and applied to patch any Express module import in
whatever way makes sense for the platform in question.
Currently in node, that is still an OpenTelemetry intstrumentation, but
just handling the automatic module load instrumentation, not the entire
tracing integration.
This is somewhat a proof of concept, to see what it takes to port a
fairly invovled OTel integration into a state where it can support all
of the platforms that we care about, but it does impose a bit less of a
translation layer between OTel and Sentry semantics (for example, no
need to use the no-op `span.recordException()`).
User-visible changes (beyond the added export in `@sentry/core`):
- Express router spans have an origin of `auto.http.express` rather than
`auto.http.otel.express`, since it's no longer technically an otel
integration.
- The empty `measurements: {}` object is no longer attached to span
data, as that was an artifact of otel's `span.recordError`, which is a
no-op anyway, and no longer executed.
Obviously this is not a full clean-room reimplementation, and relies on
the fact that the opentelemetry-js-contrib project is Apache 2.0
licensed. I included the link to the upstream license in the index file
for the Express integration, but there may be a more appropriate way to
ensure that the license is respected properly. It was arguably a
derivative work already, but simple redistribution is somewhat different
than re-implementation with subtly different context.
This reduces the node overhead and makes the Express instrumentation
portable to other SDKs, but it of course *increases* the bundle size of
`@sentry/core` considerably. It would be a good idea to explore
splitting out integrations from core, so that they're bundled and
analyzed separately, rather than shipping to all SDKs that extend core.
isaacs
force-pushed
the
isaacschlueter/portable-express-integration
branch
from
6334480 to
a6a6e90
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This extracts the functionality from the OTel Express intstrumentation,
replacing it with a portable standalone integration in
@sentry/core,which can be extended and applied to patch any Express module import in
whatever way makes sense for the platform in question.
Currently in node, that is still an OpenTelemetry intstrumentation, but
just handling the automatic module load instrumentation, not the entire
tracing integration.
This is somewhat a proof of concept, to see what it takes to port a
fairly invovled OTel integration into a state where it can support all
of the platforms that we care about, but it does impose a bit less of a
translation layer between OTel and Sentry semantics (for example, no
need to use the no-op
span.recordException()).User-visible changes (beyond the added export in
@sentry/core):auto.http.expressrather thanauto.http.otel.express, since it's no longer technically an otelintegration.
measurements: {}object is no longer attached to spandata, as that was an artifact of otel's
span.recordError, which is ano-op anyway, and no longer executed.
Obviously this is not a full clean-room reimplementation, and relies on
the fact that the opentelemetry-js-contrib project is Apache 2.0
licensed. I included the link to the upstream license in the index file
for the Express integration, but there may be a more appropriate way to
ensure that the license is respected properly. It was arguably a
derivative work already, but simple redistribution is somewhat different
than re-implementation with subtly different context.
This reduces the node overhead and makes the Express instrumentation
portable to other SDKs, but it of course increases the bundle size of
@sentry/coreconsiderably. It would be a good idea to exploresplitting out integrations from core, so that they're bundled and
analyzed separately, rather than shipping to all SDKs that extend core.
Closes JS-1982 (added automatically)