feat(server-runtimes): 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}
| 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
|
Should we try using #21641 here? |
sure, we can wait for this! in the meanwhile, I added truncation support here as well. |
| cleanupChildProcesses(); | ||
| }); | ||
|
|
||
| createEsmAndCjsTests( |
There was a problem hiding this comment.
There was a problem hiding this comment.
changed it to only run a single thing in cjs, and guarding this for node 18 where it does not work. other tests only run in esm not for simplicity.
| export type { MetricOptions } from './metrics/public-api'; | ||
| export { createConsolaReporter } from './integrations/consola'; | ||
| export { addVercelAiProcessors } from './tracing/vercel-ai'; | ||
| export { getTruncatedJsonString, shouldEnableTruncation } from './tracing/ai/utils'; |
There was a problem hiding this comment.
are we turning on sendGenAiSpans by default now? in that case we might be fine without adding truncation saving us a breaking change since we will need to remove these exports again. alternatively we could only do the media truncation part (which will stay) and just not doing text truncation
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 3b3a6b1. Configure here.
| // This is a no-op on versions that don't publish to the channel, so it is always safe to call. | ||
| // The factory needs the Sentry OTel context manager, which `initOpenTelemetry()` registers after `setupOnce`, so defer a tick. | ||
| // Options are passed in here rather than read back off the integration per event. | ||
| void Promise.resolve().then(() => subscribeVercelAiTracingChannel(dc.tracingChannel, options)); |
There was a problem hiding this comment.
Bug: The type of dc.tracingChannel is not correctly narrowed within the Promise.then() callback, creating a potential type-safety issue that relies solely on a runtime guard.
Severity: LOW
Suggested Fix
To ensure type safety and align with patterns elsewhere in the codebase, either store dc.tracingChannel in a local variable before the promise chain or add an explicit type assertion, like subscribeVercelAiTracingChannel(dc.tracingChannel as VercelAiTracingChannelFactory, options).
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/index.ts#L40
Potential issue: In Node.js versions where `diagnostics_channel.tracingChannel` is
undefined (e.g., Node 18), TypeScript's type narrowing does not persist across
asynchronous boundaries. A truthiness check is performed on `dc.tracingChannel`, but it
is then used within a `Promise.resolve().then()` callback. The TypeScript compiler may
not be able to infer that `dc.tracingChannel` is still defined inside the callback,
potentially leading to a type error. While a runtime check currently prevents issues,
the code is not type-safe, unlike similar integrations in the codebase that use explicit
type assertions to resolve this ambiguity for the compiler.
better wrapping as integration
5 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