fix(deno): Avoid inferring invalid span op from Deno tracer#20128
fix(deno): Avoid inferring invalid span op from Deno tracer#20128
Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Core
Deps
Other
Bug Fixes 🐛Core
Other
Internal Changes 🔧Core
Deps
Other
🤖 This preview updates automatically when you update the PR. |
size-limit report 📦
|
![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 00ff5b5. Configure here.
| span.op === 'gen_ai.invoke_agent' || | ||
| span.op === 'gen_ai.generate_content' || | ||
| span.op === 'otel.span' || | ||
| !span.op || |
There was a problem hiding this comment.
Overly broad filter makes AI test trivially pass
Medium Severity
The !span.op condition in the AI span filter is far too broad — it matches any span without an op, not just AI-related OTel spans. The previous condition span.op === 'otel.span' was specific to spans coming from the custom OTel tracer, but !span.op will match any span in the transaction that happens to lack an op. Combined with the assertion expect(aiSpans.length).toBeGreaterThanOrEqual(1), this means the test can trivially pass even if no AI spans are actually created.
Reviewed by Cursor Bugbot for commit 00ff5b5. Configure here.
| expect.arrayContaining([ | ||
| expect.objectContaining({ | ||
| description: 'test-otel-span', | ||
| op: 'otel.span', |
There was a problem hiding this comment.
Tests don't assert absence of op field
Low Severity
The expect.objectContaining assertions were updated to remove the op: 'otel.span' check, but no assertion was added to verify that op is actually absent or undefined. The whole point of this fix is that op is no longer set for unknown span kinds, but the tests would still pass even if the old op: 'otel.span' value came back. This is flagged because the review rules require flagging relaxed expect.objectContaining assertions where something is expected NOT to be included but isn't asserted.
Additional Locations (2)
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 00ff5b5. Configure here.
2 participants
Looks like an invalid span op snuck into the Deno SDK when we pick up spans from Deno's tracer. We have a
_mapSpanKindToOphelper in there which as a default value returnedotel.span. This value does not align with our definition of span ops. Instead, if we can't infer a span op, we should just not set one in the first place.(side-note: Long-term, we might need something more sophisticated here like
inferSpanDatain the Node SDK. But for now, I'd rather remove the invalid op and move on. Only came across this while working on #20127)