Skip to content

feat(server-utils): Implement orchestrion-based express integration#21889

Draft
mydea wants to merge 7 commits into
developfrom
fn/express-orchestrion
Draft

feat(server-utils): Implement orchestrion-based express integration#21889
mydea wants to merge 7 commits into
developfrom
fn/express-orchestrion

Conversation

@mydea

@mydea mydea commented Jul 1, 2026

Copy link
Copy Markdown
Member

Closes #20917

This adds a new expressChannelIntegration based on orchestrion. It should keep feature parity — we have a lot of Express tests, and the core tracing tests now run against this integration (via the orchestrion variant), which gives good coverage for now.

A follow up would be to also handle the error handler automatically where possible.

Route resolution: metadata is only set for genuinely matched routes

The OpenTelemetry Express integration patches every layer's handle, reconstructs the route from the layers that ran (getConstructedRoute), and validates it against the request URL (getActualMatchedRoute) to discard paths that didn't truly match.

This integration instead hooks Express's per-layer Layer.prototype.handle_request (v4) / handleRequest (v5, the router package). Express only calls that method for a layer whose path and method matched, so a request_handler span only ever exists for a route that genuinely matched. Route metadata — http.route, the root http.server span's route attribute, and the transaction name — is therefore taken directly from the matched route, with no separate reconstruct/validate step.

The observable consequence, which is intentional:

  • 404 / no route matched — no route-dispatch layer runs, so no request_handler span is created and no route (or route-based transaction name) is set.
  • Only middleware ran (no route matched) — middleware produces middleware-type spans, which never set route attributes.

This is a different mechanism from the OTel path, but it lands on the same net result: getActualMatchedRoute returns undefined in exactly these cases, so the OTel integration also emits no route there. We get it structurally, from the injection point, rather than by validating after the fact.

@mydea mydea changed the title Fn/express orchestrion feat(server-utils): Implement orchestrion-based express integration Jul 1, 2026

// Honor `ignoreLayers`/`ignoreLayersType`: skip the span for matching layers.
if (isLayerIgnored(name, type, options)) {
return undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Transaction name before ignore check

Medium Severity

For ignored request-handler layers, the orchestrion path still calls setTransactionName before isLayerIgnored returns. The OTel patchLayer path sets the transaction name only after the ignore check, so ignoreLayers / ignoreLayersType can leave a transaction renamed without the matching span.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b086dd5. Configure here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this is more correct now IMHO

mysqlChannelIntegration(),
lruMemoizerChannelIntegration(),
expressChannelIntegration(),
] as const;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Express options not wired

Medium Severity

Orchestrion mode registers expressChannelIntegration() with no options while replacing the default OTel Express integration. User ignoreLayers and ignoreLayersType settings on expressIntegration never reach the channel integration, so layer filtering lacks parity with the OTel path.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b086dd5. Configure here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

this is normal, if you pass in the express channel intergation manually it will overwrite

@mydea mydea force-pushed the fn/express-orchestrion branch from b086dd5 to 9b43cbb Compare July 1, 2026 14:06

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9b43cbb. Configure here.

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.

Rewrite ExpressInstrumentation to orchestrion

1 participant