feat!: Use handler span op for terminal request handlers - #22871
Conversation
size-limit report 📦
|
function span op for terminal request handlersfunction span op for terminal request handlers
nicohrubec
left a comment
There was a problem hiding this comment.
looks like the PR needs to be rebased because it looks like the diff still includes the changes from #22852
isaacs
left a comment
There was a problem hiding this comment.
Some minor notes/questions, nothing worth gating, imo. Looks good!
At some point when these are all finished, we're going to likely want to add a note in MIGRATION.md to the effect that ignoreSpans: [{ op: 'express.router.middleware' }] or whatever will no longer work, because the op field is getting blunter.
|
|
||
| const attributes: Record<string, string> = { | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, | ||
| [SENTRY_OP]: REQUEST_HANDLER_OP, |
There was a problem hiding this comment.
If I'm reading this right, this puts a op: 'function' span as the parent of another op: 'function' span (on line 356), which seems a bit odd? Do we need an op here?
There was a problem hiding this comment.
it's indeed a bit odd, will take a look in a follow-up 👍
| /** | ||
| * Patches `app.request()` on a Hono instance so that each internal dispatch | ||
| * is traced as a `hono.request` span — child of whatever span is active at | ||
| * is traced as an `http.server` span — child of whatever span is active at |
There was a problem hiding this comment.
I had remembered seeing cases where http.server was assumed to be on root spans, and went looking for cases where it might be used as an indicator of root-ness. But they do not. Every one of them already holds the root before testing the op. So, I think this is safe.
|
|
||
| function startMetadataSpan(metadata: SpanMetadata, original: () => unknown): unknown { | ||
| const hapiType = metadata.attributes[AttributeNames.HAPI_TYPE]; | ||
| const op = hapiType === HapiLayerType.PLUGIN ? WEB_SERVER_FUNCTION_SPAN_OP : `${hapiType}.hapi`; |
There was a problem hiding this comment.
So, we still have op fields with .hapi? I thought that we were trying to make them all known conventional types? (If this is coming in a subsequent PR, ignore, it's fine to do these piecemeal, of course.)
There was a problem hiding this comment.
yes this (and koa, express) will be updated to router which needs to land in conventions first
There was a problem hiding this comment.
Same comment here as the .hapi op above
fa5721f to
d78a45b
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.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d78a45b. Configure here.
yup i was planning on doing the MIGRATION note all at once at the end 👍 |
d78a45b to
d09a042
Compare
function span op for terminal request handlershandler span op for terminal request handlers
|
Discussed with @Lms24 that we'll add a new |
Migrate the terminal request-handler span ops across the server integrations to the cross-framework `function` op, and trace Hono's internal `app.request()` dispatch as an `http.server` span: - express: `request_handler.express` -> `function` - fastify: `request_handler.fastify` -> `function` - elysia: `request_handler.elysia` -> `function` - nestjs: `handler.nestjs` -> `function` - hono: `hono.request` -> `http.server` Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A plugin-registered hapi route runs the user's request handler, so it is a terminal handler like the express/fastify/hono routes in this migration. Map `plugin.hapi` to the cross-framework `function` op; `router.hapi` and `server.ext.hapi` (framework routing/extension lifecycle) keep their ops. Op is set via the `sentry.op` attribute only; `hapi.type` is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
d09a042 to
9eb802c
Compare
## pnpm-workspace.yaml (default) ## Dependency Updates | Package | From | To | Type | | --- | --- | --- | --- | | `@sentry/cloudflare` | 10.75.0 | 11.0.0 | major | | `@sentry/react` | 10.75.0 | 11.0.0 | major | ## Release Notes <details> <summary><b>@<!---->sentry/cloudflare</b> (10.75.0 → 11.0.0) — 4 releases</summary> <details> <summary><b>11.0.0</b></summary> Version `11.0.0` marks a major release of the Sentry JavaScript SDKs containing breaking changes. The goal of this release is to be better compatible with OpenTelemetry, make our integrations work across Node.js, Cloudflare, Bun and Deno through run-time and build-time instrumentation, and make span streaming and more permissive data collection the default. ### How To Upgrade Please carefully read through the migration guide in the Sentry docs on how to upgrade from version 10 to version 11. Make sure to select your specific platform/framework in the top left corner: https://docs.sentry.io/platforms/javascript/migration/v10-to-v11/ A comprehensive migration guide outlining all changes can be found within the Sentry JavaScript SDK Repository: https://github.com/getsentry/sentry-javascript/blob/develop/MIGRATION.md ### Breaking Changes #### All SDKs - feat: Remove support for initialising via `--require` ([#22513](getsentry/sentry-javascript#22513)) - feat!: Rename deprecated `http.*` span attributes ([#23574](getsentry/sentry-javascript#23574)) - feat!: Rename deprecated `net.` span attributes ([#23301](getsentry/sentry-javascript#23301)) - feat!: Replace `skipOpenTelemetrySetup` with `enableOpenTelemetrySetup` ([#23199](getsentry/sentry-javascript#23199)) - feat!: Replace the deprecated `http.target` span attribute ([#23575](getsentry/sentry-javascript#23575)) - feat!: Require Node `>=20.19.0` as minimum supported version ([#22558](getsentry/sentry-javascript#22558)) - feat!: Use `handler` span op for terminal request handlers ([#22871](getsentry/sentry-javascript#22871)) - feat!: Use `middleware` span op for web-server middleware ([#22852](getsentry/sentry-javascript#22852)) - feat(frameworks)!: Use `function` op for framework functions ([#23047](https://github.com/getse …[full notes](https://github.com/getsentry/sentry-javascript/releases/tag/11.0.0) </details> <details> <summary><b>10.75.3</b></summary> - fix(v10/tanstackstart-react): Reject non-POST requests to the managed tunnel route ([#24617](getsentry/sentry-javascript#24617)) <details> <summary><strong>Internal Changes</strong></summary> - chore(v10/bundler-plugins): move traces sample rate from 1.0 to 0.3 ([#24646](getsentry/sentry-javascript#24646)) - chore(v10/publish): Tag all packages as v10 ([#24619](getsentry/sentry-javascript#24619)) </details> ## Bundle size 📦 | Path | Size | | -------------------------------------------------------------------------- | ----------------- | | @<!---->sentry/browser | 27.56 KB | | @<!---->sentry/browser - with treeshaking flags | 26.04 KB | | @<!---->sentry/browser (incl. Tracing) | 46.02 KB | | @<!---->sentry/browser (incl. Tracing + Span Streaming) | 47.77 KB | | @<!---->sentry/browser (incl. Tracing, Profiling) | 50.67 KB | | @<!---->sentry/browser (incl. Tracing, Replay) | 84.38 KB | | @<!---->sentry/browser (incl. Tracing, Replay) - with treeshaking flags | 74.28 KB | | @<!---->sentry/browser (incl. Tracing, Replay with Canvas) | 89 KB | | @<!---->sentry/browser (incl. Tracing, Replay, Feedback) | 101.33 KB | | @<!---->sentry/browser (incl. Feedback) | 44.33 KB | | @<!---->sentry/browser (incl. sendFeedback) | 32.25 KB | | @<!---->sentry/browser (incl. FeedbackAsync) | 37.27 KB | | @<!---->sentry/browser (incl. Metrics) | 28.63 KB | | @<!---->sentry/browser (incl. Logs) | 28.84 KB | | @<!---->sentry/bro …[full notes](https://github.com/getsentry/sentry-javascript/releases/tag/10.75.3) </details> <p><i>…and 2 more release(s) not shown</i></p> </details> <details> <summary><b>@<!---->sentry/react</b> (10.75.0 → 11.0.0) — 4 releases</summary> <details> <summary><b>11.0.0</b></summary> Version `11.0.0` marks a major release of the Sentry JavaScript SDKs containing breaking changes. The goal of this release is to be better compatible with OpenTelemetry, make our integrations work across Node.js, Cloudflare, Bun and Deno through run-time and build-time instrumentation, and make span streaming and more permissive data collection the default. ### How To Upgrade Please carefully read through the migration guide in the Sentry docs on how to upgrade from version 10 to version 11. Make sure to select your specific platform/framework in the top left corner: https://docs.sentry.io/platforms/javascript/migration/v10-to-v11/ A comprehensive migration guide outlining all changes can be found within the Sentry JavaScript SDK Repository: https://github.com/getsentry/sentry-javascript/blob/develop/MIGRATION.md ### Breaking Changes #### All SDKs - feat: Remove support for initialising via `--require` ([#22513](getsentry/sentry-javascript#22513)) - feat!: Rename deprecated `http.*` span attributes ([#23574](getsentry/sentry-javascript#23574)) - feat!: Rename deprecated `net.` span attributes ([#23301](getsentry/sentry-javascript#23301)) - feat!: Replace `skipOpenTelemetrySetup` with `enableOpenTelemetrySetup` ([#23199](getsentry/sentry-javascript#23199)) - feat!: Replace the deprecated `http.target` span attribute ([#23575](getsentry/sentry-javascript#23575)) - feat!: Require Node `>=20.19.0` as minimum supported version ([#22558](getsentry/sentry-javascript#22558)) - feat!: Use `handler` span op for terminal request handlers ([#22871](getsentry/sentry-javascript#22871)) - feat!: Use `middleware` span op for web-server middleware ([#22852](getsentry/sentry-javascript#22852)) - feat(frameworks)!: Use `function` op for framework functions ([#23047](https://github.com/getse …[full notes](https://github.com/getsentry/sentry-javascript/releases/tag/11.0.0) </details> <details> <summary><b>10.75.3</b></summary> - fix(v10/tanstackstart-react): Reject non-POST requests to the managed tunnel route ([#24617](getsentry/sentry-javascript#24617)) <details> <summary><strong>Internal Changes</strong></summary> - chore(v10/bundler-plugins): move traces sample rate from 1.0 to 0.3 ([#24646](getsentry/sentry-javascript#24646)) - chore(v10/publish): Tag all packages as v10 ([#24619](getsentry/sentry-javascript#24619)) </details> ## Bundle size 📦 | Path | Size | | -------------------------------------------------------------------------- | ----------------- | | @<!---->sentry/browser | 27.56 KB | | @<!---->sentry/browser - with treeshaking flags | 26.04 KB | | @<!---->sentry/browser (incl. Tracing) | 46.02 KB | | @<!---->sentry/browser (incl. Tracing + Span Streaming) | 47.77 KB | | @<!---->sentry/browser (incl. Tracing, Profiling) | 50.67 KB | | @<!---->sentry/browser (incl. Tracing, Replay) | 84.38 KB | | @<!---->sentry/browser (incl. Tracing, Replay) - with treeshaking flags | 74.28 KB | | @<!---->sentry/browser (incl. Tracing, Replay with Canvas) | 89 KB | | @<!---->sentry/browser (incl. Tracing, Replay, Feedback) | 101.33 KB | | @<!---->sentry/browser (incl. Feedback) | 44.33 KB | | @<!---->sentry/browser (incl. sendFeedback) | 32.25 KB | | @<!---->sentry/browser (incl. FeedbackAsync) | 37.27 KB | | @<!---->sentry/browser (incl. Metrics) | 28.63 KB | | @<!---->sentry/browser (incl. Logs) | 28.84 KB | | @<!---->sentry/bro …[full notes](https://github.com/getsentry/sentry-javascript/releases/tag/10.75.3) </details> <p><i>…and 2 more release(s) not shown</i></p> </details> --- *This PR was auto-generated by [catalog-update-action](https://github.com/brandhaug/catalog-update-action).* Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

Collapses the terminal request-handler span ops across the server frameworks to the cross-framework
handlerop, so every "this is where the user's route handler runs" span shares one op regardless of framework.Hono’s app.request() is an internal HTTP dispatch (one sub-app calling another in-process), so it is semantically an inbound server request and is now traced as
http.serverPart of #22446