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}
| 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.
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 63b6d82. Configure here.
| code: SPAN_STATUS_ERROR, | ||
| message: data.error instanceof Error ? data.error.message : 'unknown_error', | ||
| }); | ||
| }, |
There was a problem hiding this comment.
Failed spans never ended
High Severity
On the diagnostics-channel error path, spans get an error status but are never finished. asyncEnd skips ending when data.error is set, so rejected AI operations leave invoke_agent, generate_content, and tool spans open in traces.
Reviewed by Cursor Bugbot for commit 63b6d82. Configure here.
| } catch { | ||
| return '[unserializable]'; | ||
| } | ||
| } |
There was a problem hiding this comment.
v7 channel skips message truncation
Medium Severity
The v7 tracing-channel path records gen AI inputs and outputs via safeStringify with no truncation or GEN_AI_INPUT_MESSAGES_ORIGINAL_LENGTH_ATTRIBUTE, unlike the v6 OTel path that uses getTruncatedJsonString and default enableTruncation.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 63b6d82. Configure here.
| // `callId` → operationId entry. | ||
| clearOperationId(data); | ||
| const span = data._sentrySpan; | ||
| if (!span) { | ||
| return; | ||
| } | ||
| span.setStatus({ | ||
| code: SPAN_STATUS_ERROR, | ||
| message: data.error instanceof Error ? data.error.message : 'unknown_error', | ||
| }); | ||
| }, |
There was a problem hiding this comment.
Bug: The error handler for the Vercel AI subscriber sets the span status to error but fails to call span.end(), leaving the span open indefinitely.
Severity: HIGH
Suggested Fix
Add a span.end() call within the error handler in vercel-ai-dc-subscriber.ts after the span's status has been set. This will ensure that spans for failed operations are properly closed and exported.
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#L234-L249
Potential issue: When an AI operation fails, the `error` event handler in the Vercel AI
subscriber is invoked. This handler correctly sets the span's status to error but omits
the necessary `span.end()` call. Subsequently, the `asyncEnd` handler also exits early
because `data.error` is present, preventing it from closing the span. As a result, the
span is never finalized or exported to Sentry. This will cause failed AI operations to
be invisible in Sentry traces and may lead to a memory leak due to unclosed spans
accumulating in memory.
|
Should we try using #21641 here? |
4 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