fix(server-utils): Ensure all orchestrion instrumentation lazy registers#22387
fix(server-utils): Ensure all orchestrion instrumentation lazy registers#22387mydea wants to merge 1 commit into
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
size-limit report 📦
|
d3bf68e to
8e918d3
Compare
timfish
left a comment
timfish
left a comment
There was a problem hiding this comment.
I can't find where invokeOrchestrionInstrumentation is defined 🤔
This is in the other PR in this stack: #22386 |
8e918d3 to
03b2355
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 03b2355. Configure here.
1ba8963 to
3e8c1ea
Compare
03b2355 to
93dba1a
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| setup(client) { | ||
| invokeOrchestrionInstrumentation(client, langchainModuleNames, instrumentLangChain, [options]); | ||
| }, |
There was a problem hiding this comment.
Bug: Calling Sentry.init() multiple times causes subsequent clients to skip instrumentation because invokeOrchestrionInstrumentation deduplicates based on a shared, module-level function reference.
Severity: MEDIUM
Suggested Fix
The deduplication logic in invokeOrchestrionInstrumentation should be client-aware. Instead of marking the function object globally, the check should be scoped per client. For example, maintain a set of instrumented functions on the client object itself, or use a WeakMap with the client as the key to track which instrumentations have been applied for that specific client. This ensures that each client can be instrumented independently, respecting the setup(client) contract.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/server-utils/src/integrations/tracing-channel/langchain.ts#L87-L89
Potential issue: The `invokeOrchestrionInstrumentation` function uses `hasBeenInjected`
to prevent re-instrumentation. However, the deduplication key is the callback function
object itself (e.g., `instrumentLangChain`), which is a module-level singleton. When
`Sentry.init()` is called multiple times, it creates new clients. The first client's
`setup(client)` call successfully instruments and marks the callback function.
Subsequent calls for new clients will see the function is already marked and will skip
instrumentation entirely for that client. This leads to a silent loss of tracing for any
client initialized after the first one.
Also affects:
packages/server-utils/src/integrations/tracing-channel/koa.ts:95~97
Did we get this right? 👍 / 👎 to inform future reviews.
2 participants
Migrates all orchestrion tracing-channel integrations to the
invokeOrchestrionInstrumentationhelper so that every integration lazily registers its channel subscribers only once the corresponding module is actually injected, rather than registering eagerly atSentry.init().Previously each integration wired up its own diagnostics-channel subscription directly, which meant subscribers could be registered before (or without) the target module ever being loaded, and the timing was inconsistent across integrations. Routing everything through the shared helper makes the behavior uniform: subscribers are set up for build-time-injected modules immediately and for runtime-injected modules when injection happens, always before the module publishes, and never more than once.
Alongside the migration, several integrations (hapi, graphql, kafkajs) are restructured into a
<name>/index.ts+<name>/instrumentation.tslayout to match the existing convention and keep the instrumentation wiring separate from the integration definition.Stacked on top of #22386 (ensure injected modules can reliably be picked up).