feat(eino): add OpenTelemetry handler for eino callbacks #1170
Conversation
- Implement an OpenTelemetry handler for eino callbacks - Add options to configure the handler, including custom tracer provider - Implement context propagation for OpenTelemetry spans - Update statshandler_test.go to use lowercase 'span' in comments
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CallbackExecutor
participant OTELHandler
participant OpenTelemetry
User->>CallbackExecutor: Initiate Callback
CallbackExecutor->>OTELHandler: OnStart(ctx, info, input)
OTELHandler->>OpenTelemetry: Start Span
OTELHandler->>OTELHandler: Annotate Span (run info, input)
OTELHandler-->>CallbackExecutor: ctx (with span)
CallbackExecutor->>OTELHandler: OnError(ctx, info, err) / OnEnd(ctx, info, output)
OTELHandler->>OpenTelemetry: Record error / End Span
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
- Changed 'span' to 'Span' in the comment for consistency with the code
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
eino/callbacks/otel/utils.go (1)
25-30: Consider edge cases in span name generation.The
getNamefunction could produce empty or unexpected span names in edge cases:
- If
info.Typeis empty, the span name would only be theComponent- If both
info.Typeandinfo.Componentare empty, the span name would be emptyConsider adding validation or fallback logic to ensure meaningful span names.
func getName(info *callbacks.RunInfo) string { if len(info.Name) != 0 { return info.Name } - return info.Type + string(info.Component) + name := info.Type + string(info.Component) + if name == "" { + return "unknown" + } + return name }eino/callbacks/otel/handler.go (1)
40-54: Consider enhancing span observability.The implementation is solid with proper defensive programming. Consider these optional improvements:
- Add span attributes from the callback info for better observability
- The span kind is hardcoded to
SpanKindInternal- consider making it configurable based on the callback typeExample enhancement:
func (h *Handler) OnStart(ctx context.Context, info *callbacks.RunInfo, input callbacks.CallbackInput) context.Context { if info == nil { return ctx } spanName := getName(info) ctx, span := h.tracer.Start(ctx, spanName, trace.WithSpanKind(trace.SpanKindInternal), + trace.WithAttributes( + attribute.String("callback.name", info.Name), + attribute.String("callback.type", string(info.Type)), + ), )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
eino/callbacks/otel/go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
eino/callbacks/otel/go.mod(1 hunks)eino/callbacks/otel/handler.go(1 hunks)eino/callbacks/otel/options.go(1 hunks)eino/callbacks/otel/utils.go(1 hunks)kratos/middleware/tracing/statshandler_test.go(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
eino/callbacks/otel/options.go (1)
eino/callbacks/otel/handler.go (1)
Handler(16-20)
eino/callbacks/otel/utils.go (1)
support/contexts/values.go (1)
Value(5-8)
eino/callbacks/otel/handler.go (2)
eino/callbacks/otel/options.go (1)
Option(5-7)version.go (1)
Version(3-5)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: lint
- GitHub Check: go test (1.24.x, ubuntu-latest, mysql:5.7)
- GitHub Check: go test (1.23.x, ubuntu-latest, mysql:latest)
- GitHub Check: go test (1.24.x, ubuntu-latest, mysql:latest)
- GitHub Check: go test (1.23.x, ubuntu-latest, mysql:5.7)
- GitHub Check: Analyze (go)
🔇 Additional comments (10)
eino/callbacks/otel/go.mod (1)
3-3:✅ Verification successful
Verify Go version requirement is not too restrictive.
Go 1.23.0 is quite recent and may cause compatibility issues for users on older Go versions. Consider using a more conservative version requirement unless specific 1.23.0 features are required.
🌐 Web query:
What Go versions are currently supported and widely used? Is Go 1.23.0 requirement too restrictive for a library?💡 Result:
As of May 25, 2025, the Go programming language supports the following versions:
Go 1.24.x: The latest stable release, with the most recent update being Go 1.24.3, released on May 6, 2025. (go.dev)
Go 1.23.x: The previous stable release, with the latest update being Go 1.23.9, released on May 6, 2025. (go.dev)
Go 1.22 and earlier versions have reached their end of support. (endoflife.date)
Regarding your question about setting Go 1.23.0 as a requirement for a library: Go 1.23.0 was released on August 13, 2024. (go.dev) By May 2025, Go 1.23.0 is still within its support period, and many developers are likely using it. Therefore, requiring Go 1.23.0 for your library is not overly restrictive and aligns with current usage patterns.
Citations:
- 1: https://go.dev/doc/devel/release?utm_source=openai
- 2: https://go.dev/doc/devel/release?utm_source=openai
- 3: https://endoflife.date/go?utm_source=openai
- 4: https://go.dev/blog/go1.23?utm_source=openai
Go 1.23.0 requirement is acceptable
As of May 2025, Go 1.23.x is still supported and widely used. Requiring Go 1.23.0 for this library aligns with current usage and is not overly restrictive.kratos/middleware/tracing/statshandler_test.go (1)
69-69: LGTM: Consistent comment styling.Good stylistic improvement - using lowercase 'span' in comments is more consistent with Go conventions unless referring to the specific
Spantype.eino/callbacks/otel/options.go (1)
5-19: LGTM: Clean functional options implementation.The functional options pattern is implemented correctly following Go best practices. The
Optioninterface,optionFunctype, andWithTracerProviderfunction provide a clean and extensible way to configure theHandler.eino/callbacks/otel/utils.go (1)
10-23: LGTM: Well-implemented context utilities.The context utilities follow Go best practices:
- Using an unexported struct type as context key prevents collisions
- Safe type assertion with boolean return in
fromOTelState- Clean separation of concerns with the
otelStatewrappereino/callbacks/otel/handler.go (6)
1-12: LGTM!The package declaration and imports are well-structured and include all necessary dependencies for OpenTelemetry integration.
14-22: LGTM!The scope name follows OpenTelemetry naming conventions, and the Handler struct design is clean with appropriate interface compliance verification.
24-38: LGTM!The constructor follows best practices with functional options pattern and properly initializes the tracer with appropriate scope name and instrumentation version.
56-73: LGTM!The implementation correctly handles span lifecycle with proper defensive checks and uses defer to ensure spans are properly ended.
75-95: LGTM!The error handling correctly follows OpenTelemetry conventions by recording errors and setting appropriate span status with proper defensive programming.
97-105:✅ Verification successful
Verify that placeholder implementations are acceptable.
The streaming methods are not implemented and contain TODO comments. Ensure that:
- The callbacks.Handler interface allows placeholder implementations
- Existing code doesn't rely on these methods being functional
- There's a plan to implement these methods if streaming functionality is needed
🏁 Script executed:
#!/bin/bash # Description: Check if streaming methods are used elsewhere in the codebase # Expected: Find any usage of OnStartWithStreamInput or OnEndWithStreamOutput echo "Searching for usage of streaming callback methods..." rg -A 3 "OnStartWithStreamInput|OnEndWithStreamOutput" echo "Checking if streaming is mentioned in tests..." rg -A 5 -B 5 "stream.*callback|callback.*stream" --type goLength of output: 975
Confirm No-Op Streaming Callbacks
Searches show that neither OnStartWithStreamInput nor OnEndWithStreamOutput are ever invoked (in code or tests), so these stub implementations satisfy the interface without impacting existing functionality. No immediate fixes are required—just ensure there’s a plan (e.g., an issue or roadmap entry) to replace the
// TODO implement mestubs when streaming support is added.
…ributes - Add more attributes to the span, including run info and model callback input - Create separate functions for adding attributes to improve code organization - Remove redundant context key struct and simplify context handling - Update naming convention for consistency with OpenTelemetry guidelines
…ributes - Add more attributes to the span, including run info and model callback input - Create separate functions for adding attributes to improve code organization - Remove redundant context key struct and simplify context handling - Update naming convention for consistency with OpenTelemetry guidelines
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
eino/callbacks/otel/utils.go (1)
22-31: Consider using the existing generic context utility.The implementation is correct, but you could leverage the existing generic
Valuefunction fromsupport/contexts/values.gofor better type safety and consistency.+import "github.com/go-fries/fries/support/contexts" // fromOTelState retrieves the OpenTelemetry state from the context. func fromOTelState(ctx context.Context) (*otelState, bool) { - state, ok := ctx.Value(otelStateKey{}).(*otelState) - return state, ok + return contexts.Value[*otelState](ctx, otelStateKey{}) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
eino/callbacks/otel/handler.go(1 hunks)eino/callbacks/otel/utils.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- eino/callbacks/otel/handler.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
eino/callbacks/otel/utils.go (1)
support/contexts/values.go (1)
Value(5-8)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: lint
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
eino/callbacks/otel/utils.go (4)
1-12: LGTM! Clean package structure and appropriate imports.The package declaration and imports are well-organized and include the necessary OpenTelemetry dependencies for tracing functionality.
14-20: LGTM! Proper context key design pattern.The
otelStateKeystruct{} follows Go best practices for context keys, and theotelStatestruct appropriately encapsulates the trace span.
41-47: LGTM! Consistent attribute naming.The span annotation follows a clear naming convention with the "eino.runinfo." prefix for all attributes.
33-39:❓ Verification inconclusive
Verify Component type conversion safety.
The string conversion
string(info.Component)assumes Component can be safely converted to string. Please verify this is always valid.
🏁 Script executed:
#!/bin/bash # Description: Check the type of Component field in callbacks.RunInfo # Expected: Component should be a type that can be converted to string ast-grep --pattern 'type RunInfo struct { $$$ Component $_ $$$ }'Length of output: 73
🏁 Script executed:
#!/bin/bash # Locate the RunInfo struct definition and inspect the Component field rg -n -C3 'type RunInfo struct' --include '*.go'Length of output: 492
#!/bin/bash # Search for the RunInfo struct definition in the callbacks package rg -n -C3 'type RunInfo struct' eino/callbacks
#!/bin/bash # Search for the RunInfo struct definition and its Component field type grep -R -n -C3 "type RunInfo struct" eino/callbacks grep -R -n -C2 "Component" eino/callbacks
Unable to locate RunInfo definition—please verify Component conversion safety
- We couldn’t find the
RunInfostruct (and itsComponentfield) in the local codebase.- Confirm that
Componentis either astringalias or has an underlying type safe forstring(info.Component)conversion (e.g.type Component string).- If
Componentis numeric or otherwise non-string, casting produces a Unicode code point. In that case, consider usingfmt.Sprint(info.Component)or providing aString() stringmethod onComponent.
eino/callbacks/otel/utils.go
Outdated
| func spanWithModelCallbackInput(span trace.Span, input callbacks.CallbackInput) { | ||
| callbackInput := model.ConvCallbackInput(input) | ||
| if callbackInput == nil { | ||
| return | ||
| } | ||
|
|
||
| if config := callbackInput.Config; config != nil { | ||
| span.SetAttributes( | ||
| semconv.GenAIRequestModel(config.Model), | ||
| semconv.GenAIRequestMaxTokens(config.MaxTokens), | ||
| semconv.GenAIRequestTemperature(float64(config.Temperature)), | ||
| semconv.GenAIRequestTopP(float64(config.TopP)), | ||
| ) | ||
| } | ||
|
|
||
| if len(callbackInput.Messages) > 0 { | ||
| for i, msg := range callbackInput.Messages { | ||
| span.AddEvent( | ||
| fmt.Sprintf("input.message.%d", i+1), | ||
| trace.WithAttributes( | ||
| attribute.String("gen_ai.request.role", string(msg.Role)), | ||
| attribute.String("gen_ai.request.content", msg.Content), | ||
| // Add more attributes as needed | ||
| ), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| if extra := callbackInput.Extra; len(extra) > 0 { | ||
| attrs := make([]attribute.KeyValue, 0, len(extra)) | ||
| for k, v := range extra { | ||
| attrs = append(attrs, attribute.String("callback.input.extra."+k, fmt.Sprintf("%v", v))) | ||
| } | ||
| span.SetAttributes(attrs...) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve semantic convention usage and error handling.
Several improvements needed for robustness and standards compliance:
- Use semantic convention constants for message attributes
- Add nil checks for config fields
- Handle potential type conversion errors
func spanWithModelCallbackInput(span trace.Span, input callbacks.CallbackInput) {
callbackInput := model.ConvCallbackInput(input)
if callbackInput == nil {
return
}
if config := callbackInput.Config; config != nil {
- span.SetAttributes(
- semconv.GenAIRequestModel(config.Model),
- semconv.GenAIRequestMaxTokens(config.MaxTokens),
- semconv.GenAIRequestTemperature(float64(config.Temperature)),
- semconv.GenAIRequestTopP(float64(config.TopP)),
- )
+ attrs := []attribute.KeyValue{}
+ if config.Model != "" {
+ attrs = append(attrs, semconv.GenAIRequestModel(config.Model))
+ }
+ if config.MaxTokens > 0 {
+ attrs = append(attrs, semconv.GenAIRequestMaxTokens(config.MaxTokens))
+ }
+ if config.Temperature != 0 {
+ attrs = append(attrs, semconv.GenAIRequestTemperature(float64(config.Temperature)))
+ }
+ if config.TopP != 0 {
+ attrs = append(attrs, semconv.GenAIRequestTopP(float64(config.TopP)))
+ }
+ if len(attrs) > 0 {
+ span.SetAttributes(attrs...)
+ }
}
if len(callbackInput.Messages) > 0 {
for i, msg := range callbackInput.Messages {
span.AddEvent(
fmt.Sprintf("input.message.%d", i+1),
trace.WithAttributes(
- attribute.String("gen_ai.request.role", string(msg.Role)),
- attribute.String("gen_ai.request.content", msg.Content),
- // Add more attributes as needed
+ semconv.GenAIPromptRole(string(msg.Role)),
+ semconv.GenAIPromptContent(msg.Content),
),
)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func spanWithModelCallbackInput(span trace.Span, input callbacks.CallbackInput) { | |
| callbackInput := model.ConvCallbackInput(input) | |
| if callbackInput == nil { | |
| return | |
| } | |
| if config := callbackInput.Config; config != nil { | |
| span.SetAttributes( | |
| semconv.GenAIRequestModel(config.Model), | |
| semconv.GenAIRequestMaxTokens(config.MaxTokens), | |
| semconv.GenAIRequestTemperature(float64(config.Temperature)), | |
| semconv.GenAIRequestTopP(float64(config.TopP)), | |
| ) | |
| } | |
| if len(callbackInput.Messages) > 0 { | |
| for i, msg := range callbackInput.Messages { | |
| span.AddEvent( | |
| fmt.Sprintf("input.message.%d", i+1), | |
| trace.WithAttributes( | |
| attribute.String("gen_ai.request.role", string(msg.Role)), | |
| attribute.String("gen_ai.request.content", msg.Content), | |
| // Add more attributes as needed | |
| ), | |
| ) | |
| } | |
| } | |
| if extra := callbackInput.Extra; len(extra) > 0 { | |
| attrs := make([]attribute.KeyValue, 0, len(extra)) | |
| for k, v := range extra { | |
| attrs = append(attrs, attribute.String("callback.input.extra."+k, fmt.Sprintf("%v", v))) | |
| } | |
| span.SetAttributes(attrs...) | |
| } | |
| } | |
| func spanWithModelCallbackInput(span trace.Span, input callbacks.CallbackInput) { | |
| callbackInput := model.ConvCallbackInput(input) | |
| if callbackInput == nil { | |
| return | |
| } | |
| if config := callbackInput.Config; config != nil { | |
| attrs := []attribute.KeyValue{} | |
| if config.Model != "" { | |
| attrs = append(attrs, semconv.GenAIRequestModel(config.Model)) | |
| } | |
| if config.MaxTokens > 0 { | |
| attrs = append(attrs, semconv.GenAIRequestMaxTokens(config.MaxTokens)) | |
| } | |
| if config.Temperature != 0 { | |
| attrs = append(attrs, semconv.GenAIRequestTemperature(float64(config.Temperature))) | |
| } | |
| if config.TopP != 0 { | |
| attrs = append(attrs, semconv.GenAIRequestTopP(float64(config.TopP))) | |
| } | |
| if len(attrs) > 0 { | |
| span.SetAttributes(attrs...) | |
| } | |
| } | |
| if len(callbackInput.Messages) > 0 { | |
| for i, msg := range callbackInput.Messages { | |
| span.AddEvent( | |
| fmt.Sprintf("input.message.%d", i+1), | |
| trace.WithAttributes( | |
| semconv.GenAIPromptRole(string(msg.Role)), | |
| semconv.GenAIPromptContent(msg.Content), | |
| ), | |
| ) | |
| } | |
| } | |
| if extra := callbackInput.Extra; len(extra) > 0 { | |
| attrs := make([]attribute.KeyValue, 0, len(extra)) | |
| for k, v := range extra { | |
| attrs = append(attrs, attribute.String("callback.input.extra."+k, fmt.Sprintf("%v", v))) | |
| } | |
| span.SetAttributes(attrs...) | |
| } | |
| } |
🤖 Prompt for AI Agents
In eino/callbacks/otel/utils.go lines 49 to 84, improve semantic convention
usage by replacing hardcoded attribute keys for message roles and content with
appropriate semantic convention constants. Add nil checks for config fields like
Model, MaxTokens, Temperature, and TopP before setting attributes to avoid nil
pointer dereferences. For type conversions such as float64 conversions of
Temperature and TopP, ensure safe handling or validation to prevent runtime
errors. Implement these changes to enhance robustness and standards compliance.
….3.0 - Upgrade github.com/go-fries/fries/v3 from v3.0.0 to v3.3.0 in eino/callbacks/otel
- Update eino version to v0.3.41 - Remove replace directive for github.com/go-fries/fries/v3 - Add testify as a new dependency - Refactor Handler initialization and error handling - Rename utils.go to state.go for better naming convention - Add version.go and version_test.go for version management
…k lifecycle- Add tracing for OnStartWithStreamInput and OnEndWithStreamOutput- Implement span recording for callback input and output - Refactor state handling to improve trace context management - Move span attribute setting to separate functions for better organization
- Update go.opentelemetry.io/otel from v1.36.0 to v1.37.0 - Update go.opentelemetry.io/otel/trace from v1.36.0 to v1.37.0 - Update go.opentelemetry.io/otel/metric from v1.36.0 to v1.37.0 - Update github.com/go-logr/logr from v1.4.2 to v1.4.3
Co-Authored-By: Flc゛ <four_leaf_clover@foxmail.com>
- Complete TODO in OnEndWithStreamOutput following existing patterns - Add proper span state handling and cleanup - Maintain consistency with OnEnd method implementation Co-Authored-By: Flc゛ <four_leaf_clover@foxmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 3.x #1170 +/- ##
=======================================
- Coverage 65.8% 65.7% -0.1%
=======================================
Files 137 137
Lines 5670 5677 +7
=======================================
Hits 3733 3733
- Misses 1860 1867 +7
Partials 77 77 🚀 New features to boost your workflow:
|
feat(eino/callbacks/otel): implement OnEndWithStreamOutput method
Summary
Completes the implementation of the OpenTelemetry handler for eino callbacks by implementing the previously stubbed
OnEndWithStreamOutputmethod. This method handles the end of component execution when the output is a stream rather than a regular value.Key changes:
OnEndWithStreamOutputineino/callbacks/otel/handler.gofollowing the existingOnEndpatternThe implementation follows the same span lifecycle management as the existing
OnEndmethod: validates input parameters, retrieves OpenTelemetry state from context, checks span validity, and ensures proper span cleanup via defer.Review & Testing Checklist for Human
output *schema.StreamReader[callbacks.CallbackOutput]parameter entirely. Review if this is the intended behavior or if stream-specific processing is needed.OnEndmethod processes its output parameter, but this implementation doesn't touch the stream. Verify if this behavioral difference is intentional.callbacks.Handlerinterface requirements and matches expected usage patterns in the eino ecosystem.Recommended test plan:
Diagram
%%{ init : { "theme" : "default" }}%% graph TD CallbacksHandler["callbacks.Handler<br/>(interface)"]:::context OtelHandler["eino/callbacks/otel/<br/>handler.go"]:::major-edit OnEnd["OnEnd()<br/>(existing method)"]:::context OnEndStream["OnEndWithStreamOutput()<br/>(new implementation)"]:::major-edit OtelState["eino/callbacks/otel/<br/>state.go"]:::context GoSum["eino/callbacks/otel/<br/>go.sum"]:::minor-edit CallbacksHandler --> OtelHandler OtelHandler --> OnEnd OtelHandler --> OnEndStream OtelHandler --> OtelState OtelHandler --> GoSum OnEnd -.->|"pattern followed"| OnEndStream OtelState -->|"span management"| OnEndStream subgraph Legend L1[Major Edit]:::major-edit L2[Minor Edit]:::minor-edit L3[Context/No Edit]:::context end classDef major-edit fill:#90EE90 classDef minor-edit fill:#87CEEB classDef context fill:#FFFFFFNotes