feat(cloudflare): Auto-instrument Workers AI binding via env instrumentation#22126
feat(cloudflare): Auto-instrument Workers AI binding via env instrumentation#22126JPeer264 wants to merge 2 commits into
Conversation
size-limit report 📦
|
c626710 to
a8b1d8b
Compare
0aa52a5 to
8011b0b
Compare
…ntation Detect the Workers AI binding (env.AI) in instrumentEnv via duck-typing (run + gateway + toMarkdown) and wrap it automatically, matching how D1, R2, and Queue bindings are instrumented. Manual wrapping via instrumentWorkersAiClient remains available for custom options and is now guarded against double-wrapping. Also removes the unused WORKERS_AI_INTEGRATION_NAME constant and aligns the integration test wrangler config with sibling suites (nodejs_als). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
a8b1d8b to
cd876d4
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 cd876d4. Configure here.
| const instrumented = instrumentWorkersAiClient(item); | ||
| instrumentedBindings.set(item, instrumented); | ||
| return instrumented; | ||
| } |
There was a problem hiding this comment.
Double-wrap of AI bindings
Medium Severity
instrumentEnv always wraps AI bindings with instrumentWorkersAiClient, but that helper has no double-wrap guard (unlike D1’s ensureInstrumented). Calling instrumentWorkersAiClient(env.AI, options) for custom options — the path the API docs call out — nests a second proxy and emits duplicate gen_ai spans.
Reviewed by Cursor Bugbot for commit cd876d4. Configure here.
| if (isAiBinding(item)) { | ||
| const instrumented = instrumentWorkersAiClient(item); | ||
| instrumentedBindings.set(item, instrumented); | ||
| return instrumented; | ||
| } |
There was a problem hiding this comment.
Bug: Manually instrumenting an AI client with instrumentWorkersAiClient and then passing it to instrumentEnv causes double-wrapping and duplicate spans, as the existing instrumentation is not detected.
Severity: MEDIUM
Suggested Fix
Add a guard to instrumentWorkersAiClient to prevent re-instrumenting an already-wrapped client. This could be achieved by using a mechanism similar to the ensureInstrumented() helper used for D1 bindings, which checks for a unique symbol or property on the object before applying instrumentation.
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/cloudflare/src/instrumentations/worker/instrumentEnv.ts#L81-L85
Potential issue: The `instrumentWorkersAiClient` function lacks a guard to prevent
double instrumentation. If a user manually instruments an AI client and then also uses
the automatic `instrumentEnv` on an environment containing this already-instrumented
client, the client will be wrapped a second time. This happens because the caching in
`instrumentEnv` keys on the original object reference, which is lost after manual
wrapping. The already-instrumented proxy still passes the `isAiBinding` check, leading
to re-instrumentation and resulting in duplicate spans.
Did we get this right? 👍 / 👎 to inform future reviews.


This auto-wraps workers-ai bindings via
instrumentEnv. Unfortunately locally there is no AI binding available and would require an actual LLM on Cloudflare, so in the integration tests there is only a MockAi. However, the autobinding is tested via unit tests and have the same pattern as the other instrumentations.