feat(remix): Rewrite RemixInstrumentation to orchestrion#22244
Draft
mydea wants to merge 2 commits into
Draft
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8d64ae5. Configure here.
| subscribeCallRouteLoader(); | ||
| if (actionFormDataAttributes) { | ||
| subscribeCallRouteAction(actionFormDataAttributes); | ||
| } |
There was a problem hiding this comment.
Action spans gated on form data
High Severity
instrumentRemix only calls subscribeCallRouteAction when actionFormDataAttributes is truthy, so ACTION spans are never created unless form-data capture is configured. The old OTel instrumentation always patched callRouteAction and only used those keys for optional attribute extraction.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 8d64ae5. Configure here.
| MATCH_SERVER_ROUTES: 'orchestrion:@remix-run/server-runtime:matchServerRoutes', | ||
| CALL_ROUTE_LOADER: 'orchestrion:@remix-run/server-runtime:callRouteLoader', | ||
| CALL_ROUTE_ACTION: 'orchestrion:@remix-run/server-runtime:callRouteAction', | ||
| } as const; |
There was a problem hiding this comment.
Duplicated remix channel names
Low Severity
Channel name strings are duplicated locally even though remixChannels already exists in @sentry/server-utils, and NestJS already exports nestjsChannels from @sentry/server-utils/orchestrion for the same purpose. Drift between the two copies would silently stop Remix channel spans from firing.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 8d64ae5. Configure here.
Contributor
size-limit report 📦
|
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
1 participant
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.
Rewrites the vendored
RemixInstrumentation(an OpenTelemetryInstrumentationBasethat patched@remix-run/server-runtime) to a diagnostics-channel listener, with orchestrion injecting the channels into the instrumented module at build time — mirroring the mysql/mysql2/kafkajs ports.SDK changes
@sentry/server-utils— populated the orchestrion config for@remix-run/server-runtime(orchestrion/config/remix.ts): one channel each forrequestHandler,matchServerRoutes,callRouteLoader, andcallRouteAction(the pre-2.9…RRnames collapse onto the same channels), across the CJS and ESM builds.requestHandleris a named function expression returned bycreateRequestHandler, which name-based matching can't target, so it uses theastQueryescape hatch.@sentry/remix— addedremixChannelIntegration, a diagnostics-channel subscriber that reproduces the old instrumentation:requestHandler→ theremix.requesthttp.serverserver span,matchServerRoutes→ enriches the active request span with the matched route (http.route, name,match.route.id) — no span of its own,callRouteLoader/callRouteAction→LOADER/ACTIONspans (the action path awaits form-data extraction from a pre-consumption request clone).remixChannelIntegrationfrom@sentry/remix. The OTelremixIntegrationstays the default; the channel integration replaces it (sameRemixname) when diagnostics-channel injection is enabled.E2E changes
remix-orchestriontest app intocreate-remix-app-v2as anINJECT_ORCHESTRION-gatedsentryTestvariant, so one codebase runs both the OpenTelemetry and orchestrion paths (build-time injection via the orchestrion Vite plugin, DB routes + docker for mysql/ioredis, and a build-injection assertion). The base transaction tests run in both variants as a parity check.@remix-run/server-runtimereachable by the build-time transform,@remix-run/nodeis force-bundled (it re-exports server-runtime, which otherwise stays external and is loaded byremix-serveat runtime, outside the Vite bundle).Fixes #20910