feat: add interception tracing #63
Conversation
3fb2522 to
af4a1fd
Compare
|
Hey @pawbana, have you tested it with Jaeger tracing involved as we chatted on the weekly call? If so, perhaps update the PR description with any findings or graphs. |
| resp, err := http.DefaultClient.Do(req) | ||
| t.Cleanup(func() { _ = resp.Body.Close() }) | ||
| require.NoError(t, err) | ||
| bridgeSrv.Close() |
There was a problem hiding this comment.
dannykopping
left a comment
There was a problem hiding this comment.
Did a quick skim review, spotted a few things which need attention.
I think there a few too many drive-by changes in this PR which I want to understand better.
dannykopping
left a comment
There was a problem hiding this comment.
Almost there! 🎉 A few minor questions & nits
| return attrs | ||
| } | ||
|
|
||
| func EndSpanErr(span trace.Span, err *error) { |
There was a problem hiding this comment.
Might want to explain the *error in a comment for the confused reader later 😆
bridge_integration_test.go
Outdated
|
|
||
| // Setup MCP tools. | ||
| tools := setupMCPServerProxiesForTest(t) | ||
| tools := setupMCPServerProxiesForTest(t, testTracer) |
There was a problem hiding this comment.
Nit: it's confusing to name this tools, since it's not actually a list of tools.
| _ = json.NewDecoder(&buf).Decode(&args) | ||
| res, err := tool.Call(ctx, args) | ||
|
|
||
| args := i.unmarshalArgs(tc.Function.Arguments) |
There was a problem hiding this comment.
unmarshalArgs returns a ToolArgs struct, which is a type alias for any.
You're subtly changing behaviour now. Previously this was being decoded to a map[string]string.
Why was this change necessary?
There was a problem hiding this comment.
Without the change TestOpenAIInjectedToolsTrace test was failing due to ToolCall span input attribute being empty.
I tested current code manually and tool.Call gets empty input in openAI blocking case.
There was a problem hiding this comment.
So are you saying blocking openai requests fail to invoke tools correctly?
| github.com/openai/openai-go/v2 v2.7.0 | ||
| ) | ||
|
|
||
| require ( |
There was a problem hiding this comment.
| require ( | |
| // Tracing-related libs. | |
| require ( |
Also, thanks for getting rid of go-cmp 👍
Adds tracing for AIBridge. Updates github.com/coder/aibridge version from `v0.2.2` to `v0.3.0` Depends on: coder/aibridge#63 Fixes: coder/aibridge#26 --------- Co-authored-by: Danny Kopping <danny@coder.com>
Adds tracing for interceptions, tool calls and recording.
Example interception trace: