feat(node): Add v7 support for vercelAiIntegration#21613
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
size-limit report 📦
|
mydea
requested review from
JPeer264 and
andreiborza
and removed request for
a team
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
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.
Reviewed by Cursor Bugbot for commit f8e0817. Configure here.
| if (type === 'step' || !event || typeof event !== 'object') { | ||
| data._sentrySkip = true; | ||
| // The active span is the enclosing invoke_agent span; reusing it leaves the tree unchanged. | ||
| return getActiveSpan() as Span; |
There was a problem hiding this comment.
Bug: The result of getActiveSpan() is unsafely cast to Span, but it can be undefined. This can lead to trace.setSpan being called with undefined, causing undefined behavior.
Severity: LOW
Suggested Fix
Remove the unsafe as Span cast. Before using the result of getActiveSpan(), add a check to ensure it is not undefined. If it is undefined, handle it gracefully instead of passing it to trace.setSpan. This will enforce type safety and remove the reliance on implicit assumptions about the execution context.
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/vercel-ai/vercel-ai-dc-subscriber.ts#L216-L219
Potential issue: In the Vercel AI data-consistency subscriber, the result of
`getActiveSpan()` is unsafely cast to `Span`, even though the function can return
`undefined`. If `getActiveSpan()` returns `undefined`, this leads to
`trace.setSpan(context.active(), undefined)` being called in `tracingChannel.ts`.
According to OpenTelemetry documentation, passing `undefined` to this function results
in undefined behavior. While this scenario is unlikely in normal operation because
`step` events are expected to occur within an active span, the code violates type safety
and relies on implicit assumptions about event ordering.
Also affects:
packages/server-utils/src/vercel-ai/vercel-ai-dc-subscriber.ts:248~251
| const INTEGRATION_NAME = 'VercelAI'; | ||
|
|
||
| // `@sentry/conventions` does not expose these yet, so we keep the literals here. | ||
| const GEN_AI_TOOL_CALL_ID_ATTRIBUTE = 'gen_ai.tool.call.id'; |
There was a problem hiding this comment.
l: We already have an export for this:
| code: SPAN_STATUS_ERROR, | ||
| message: data.error instanceof Error ? data.error.message : 'unknown_error', | ||
| }); | ||
| span.end(); |
There was a problem hiding this comment.
m: According to @logaretm we should not call span.end() in the error channel. See notion doc.
| if (messages === undefined) { | ||
| return {}; | ||
| } | ||
| return { [GEN_AI_INPUT_MESSAGES]: safeStringify(messages) }; |
There was a problem hiding this comment.
m: While I know truncation is going away soon(tm), we should make sure we pass this through truncation logic today. The helpers are currently not exported from @sentry/core, so I think we should inline them to avoid exposing new api from core that goes way with the next major anyway.
And also once we do that, we should also record the original length of the message via GEN_AI_INPUT_MESSAGES_ORIGINAL_LENGTH_ATTRIBUTE.
There was a problem hiding this comment.
hm not sure if worth it to add this today? What if we say for v7 we simply have no truncation? 🤔
There was a problem hiding this comment.
I think it's fairly straight forward to add and we wouldn't have to document any differences.
There was a problem hiding this comment.
I think we still need truncation until we enable streamGenAiSpans by default, else we'll have users running into dropped spans/transactions again
| }); | ||
| } | ||
|
|
||
| function getRecordingOptions(event: Record<string, unknown>): { recordInputs: boolean; recordOutputs: boolean } { |
There was a problem hiding this comment.
l: Maybe we can reword this away from "recording options" and expand this to also get the enableTruncation option so we can truncate input messages.
3 participants
In the latest beta release of v7 of the ai package, native tracing channel events are now emitted. This PR adds support for this and thus for instrumentation in this package.
For now, this is only implemented in node, but can also be ported to deno/bun.
A big part of this PR was making sure that the v6 tests also run for v7 to make sure this is compatible. It may need some cleanup but tests pass now. The tests are identical for v6 and v7 (just fixed some formatting stuff, moved folder, added a describe.each to run it multiple times), git just could not fully keep up with the changes so it appears bigger than it is.
Supersedes #21584