Skip to content

ref(nestjs): extract shared span helpers#22175

Open
isaacs wants to merge 1 commit into
isaacs/node-move-default-integration-swapfrom
isaacs/nestjs-extract-helpers
Open

ref(nestjs): extract shared span helpers#22175
isaacs wants to merge 1 commit into
isaacs/node-move-default-integration-swapfrom
isaacs/nestjs-extract-helpers

Conversation

@isaacs

@isaacs isaacs commented Jul 10, 2026

Copy link
Copy Markdown
Member

Pull the span-emitting/patching logic out of the OTel InstrumentationBase classes into wrap-components, wrap-handlers, and wrap-route so it can be reused by the diagnostics-channel integration.

Refactor only. There is still only the OTel implementation, which still emits the same spans as before. However, this will be needed when the next implementation is added.

@isaacs isaacs requested a review from a team as a code owner July 10, 2026 03:34
@isaacs isaacs requested review from chargome, mydea and nicohrubec and removed request for a team July 10, 2026 03:34
@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

size-limit report 📦

Path Size % Change Change
@sentry/browser 27.6 kB - -
@sentry/browser - with treeshaking flags 26.04 kB - -
@sentry/browser (incl. Tracing) 46.35 kB - -
@sentry/browser (incl. Tracing + Span Streaming) 48.14 kB - -
@sentry/browser (incl. Tracing, Profiling) 51.13 kB - -
@sentry/browser (incl. Tracing, Replay) 85.62 kB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 75.26 kB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 90.33 kB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 102.99 kB - -
@sentry/browser (incl. Feedback) 44.78 kB - -
@sentry/browser (incl. sendFeedback) 32.4 kB - -
@sentry/browser (incl. FeedbackAsync) 37.53 kB - -
@sentry/browser (incl. Metrics) 28.68 kB - -
@sentry/browser (incl. Logs) 28.93 kB - -
@sentry/browser (incl. Metrics & Logs) 29.61 kB - -
@sentry/react 29.39 kB - -
@sentry/react (incl. Tracing) 48.62 kB - -
@sentry/vue 33.03 kB - -
@sentry/vue (incl. Tracing) 48.33 kB - -
@sentry/svelte 27.63 kB - -
CDN Bundle 30 kB - -
CDN Bundle (incl. Tracing) 48.33 kB - -
CDN Bundle (incl. Logs, Metrics) 31.58 kB - -
CDN Bundle (incl. Tracing, Logs, Metrics) 49.65 kB - -
CDN Bundle (incl. Replay, Logs, Metrics) 70.82 kB - -
CDN Bundle (incl. Tracing, Replay) 85.84 kB - -
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) 87.16 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 91.64 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) 92.92 kB - -
CDN Bundle - uncompressed 89.33 kB - -
CDN Bundle (incl. Tracing) - uncompressed 146.07 kB - -
CDN Bundle (incl. Logs, Metrics) - uncompressed 94.03 kB - -
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed 150.05 kB - -
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed 218.76 kB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 265.28 kB - -
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed 269.24 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 278.98 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed 282.93 kB - -
@sentry/nextjs (client) 51.17 kB - -
@sentry/sveltekit (client) 46.8 kB - -
@sentry/core/server 78.42 kB - -
@sentry/core/browser 64.77 kB - -
@sentry/node-core 62.73 kB - -
@sentry/node 125.41 kB +0.05% +59 B 🔺
@sentry/node (incl. diagnostics channel injection) 139.66 kB +0.02% +25 B 🔺
@sentry/node/import (ESM hook with diagnostics-channel injection) 69.96 kB - -
@sentry/node/light 50.73 kB - -
@sentry/node - without tracing 74.1 kB +0.09% +61 B 🔺
@sentry/aws-serverless 83.32 kB +0.08% +64 B 🔺
@sentry/cloudflare (withSentry) - minified 181.47 kB - -
@sentry/cloudflare (withSentry) 448.98 kB - -

View base workflow run

@nicohrubec nicohrubec left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

overall lgtm, I just have two small questions/remarks

Comment thread packages/nestjs/src/integrations/helpers.ts
Comment thread packages/nestjs/src/integrations/sentry-nest-bullmq-instrumentation.ts Outdated
@isaacs isaacs force-pushed the isaacs/node-move-default-integration-swap branch from 7b12bae to 63a580f Compare July 10, 2026 14:15
@isaacs isaacs requested a review from a team as a code owner July 10, 2026 14:15
@isaacs isaacs requested review from JPeer264 and andreiborza and removed request for a team July 10, 2026 14:15
@isaacs isaacs force-pushed the isaacs/nestjs-extract-helpers branch from 1eb7633 to fa0de85 Compare July 10, 2026 14:16
@isaacs isaacs force-pushed the isaacs/node-move-default-integration-swap branch from 63a580f to 89c3b4c Compare July 10, 2026 16:30
@isaacs isaacs force-pushed the isaacs/nestjs-extract-helpers branch from fa0de85 to dd92103 Compare July 10, 2026 16:30
@isaacs isaacs force-pushed the isaacs/node-move-default-integration-swap branch from 89c3b4c to 1d877df Compare July 10, 2026 16:53
@isaacs isaacs force-pushed the isaacs/nestjs-extract-helpers branch from dd92103 to b74dcbd Compare July 10, 2026 16:53
@isaacs isaacs force-pushed the isaacs/node-move-default-integration-swap branch from 1d877df to 5d4d988 Compare July 10, 2026 17:16
@isaacs isaacs force-pushed the isaacs/nestjs-extract-helpers branch from b74dcbd to 9315716 Compare July 10, 2026 17:16
Comment thread packages/nestjs/src/integrations/wrap-handlers.ts
Pull the span-emitting/patching logic out of the OTel
`InstrumentationBase` classes into `wrap-components`, `wrap-handlers`,
and `wrap-route` so it can be reused by the diagnostics-channel
integration.

Refactor only. There is still only the OTel implementation, which still
emits the same spans as before. However, this will be needed when the
next implementation is added.
@isaacs isaacs force-pushed the isaacs/node-move-default-integration-swap branch from 5d4d988 to b2a369d Compare July 10, 2026 17:36
@isaacs isaacs force-pushed the isaacs/nestjs-extract-helpers branch from 9315716 to 28b5141 Compare July 10, 2026 17:36
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.

2 participants