Repository navigation
Conversation
aikins01
left a comment
aikins01
left a comment
There was a problem hiding this comment.
Thanks, this covers a lot of what the audit found. One of the new tests can't fail yet, details inline.
Part of telemetry-dev#83. A real ai@6 generateText run with a tool call now goes through the ai-v6 dispatcher, where every other ai@6 test sends hand-built events. The media and batch wrappers get tests for the video count, the transcription duration and its fallback, the gateway relabel, the status and cancel batch operations, and returning the provider function unchanged without an API key. On ai@7, executeTool runs the tool inside the span its tool end finishes, a step that ends without onStepStart starts with the trace, and every span relabels the gateway provider. Breaking each of those mechanisms fails a new test.
onStart and onStepEnd ran in the same millisecond, so a step that started at its own end still matched the trace start. The step now ends a second after the trace starts.
488f078 to
c3d81fb
Compare
aikins01
left a comment
aikins01
left a comment
There was a problem hiding this comment.
Thanks for the quick fix, the faked clock catches the endedAt start now. A few of the other new tests still pass with the behavior they name removed, details inline.
|
|
||
| // The other ai@6 tests send hand-built events; this one runs generateText through the real ai@6 | ||
| // dispatcher, so a change in the events it sends or the hooks it calls shows up here. | ||
| test("a real ai@6 generateText run with a tool call records the root, both steps, and the tool", async () => { |
There was a problem hiding this comment.
This test goes through the real dispatcher, but it doesn't check anything ai@6 sends in its events. It still passes if the root's model is dropped, the tool result is read from the wrong field, or input.prompt isn't set, so the comment above overstates it. Please give it the assertions from the hand-built test at L159: root gen_ai.operation.name is invoke_agent, its model, provider, and input messages containing "find otel", and the tool's arguments and result.
| const step = byOperation(spans, "chat").find((s) => s.kind === SpanKind.CLIENT)!; | ||
| const tools = byOperation(spans, "execute_tool"); | ||
| expect(tools).toHaveLength(1); | ||
| expect(spanId(tools[0]!)).toBe(activeSpanId); |
There was a problem hiding this comment.
With span.end(endedAt) removed from onToolExecutionEnd, this still passes, because the tool span is exported without ever ending. Asserting tools[0]!.ended, or the tool span's endTime with a faked Date like the next test, would cover "the tool end finishes that span".
| expect(steps[0]!.attributes["gen_ai.usage.input_tokens"]).toBe(2); | ||
| }); | ||
|
|
||
| test("ai@7 relabels the gateway provider to Vercel AI Gateway on every span", async () => { |
There was a problem hiding this comment.
This covers the generateText spans, but ai@7 relabels the provider separately in onObjectStepStart, onEmbedStart, onRerankStart, and onEvaluateStart. With providerLabel removed from all four, the whole package suite still passes, so "every span" claims more than this test shows. Please make it a table over generateText, generateObject, embed, rerank, and evaluate, each starting with provider: "gateway", and assert the root and step spans by kind instead of length >= 2.
Part of #83.
generateTextrun with a tool call now goes through theai-v6dispatcher. Every other ai@6 test sends hand-built events.videoslist or a singlevideo), the transcription duration and itsdurationfallback, and the gateway relabel.wrapGetBatchStatusandwrapCancelBatchrecord their own operation, the batch id and the status.executeToolruns the tool inside the span that the tool end finishes, a step that ends withoutonStepStartstarts with the trace, and every span relabels the gateway provider.I broke each of these mechanisms on its own, 15 in all, and each one fails a new test. The ai@6 run checks the dispatcher's real events rather than one mechanism, so it has no mutant of its own.
These tests capture spans through
overrideslike the existing ones. If #83 ends up removing it, I'll move them over with the rest.