fix(daemon): handle bool values sent as strings in OTEL processor#166
fix(daemon): handle bool values sent as strings in OTEL processor#166
Conversation
Claude Code sends some boolean values as strings. Added getBoolFromValue helper function to properly parse both bool and string representations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @AnnatarHe, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a data parsing inconsistency within the daemon's OTEL processor, where boolean values, particularly from Claude Code, are sometimes transmitted as strings instead of native booleans. To rectify this, a new utility function, Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a helper function getBoolFromValue to handle boolean values that may be sent as strings from OTEL data sources, which is a good improvement for data parsing robustness. My review includes a critical fix to prevent a potential panic in the new function and a suggestion to improve its clarity using a type switch, which is more idiomatic in Go for this scenario. I've also pointed out some commented-out code that should be removed to keep the codebase clean. While not part of this PR's changes, I noticed that similar helper functions like getIntFromValue and getFloatFromValue might also be vulnerable to a nil pointer panic. It would be beneficial to address this in a follow-up change to improve overall stability.
| func getBoolFromValue(value *commonv1.AnyValue) bool { | ||
| // First try to get as bool | ||
| if boolVal := value.GetBoolValue(); boolVal { | ||
| return boolVal | ||
| } | ||
| // Try to parse from string (Claude Code sends some values as strings) | ||
| if strVal := value.GetStringValue(); strVal != "" { | ||
| if parsed, err := strconv.ParseBool(strVal); err == nil { | ||
| return parsed | ||
| } | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
This function can panic if value is nil. The value comes from attr.GetValue(), which can return nil and would cause a panic on value.GetBoolValue(). You should add a nil check at the beginning of the function.
Additionally, the current implementation is a bit difficult to follow. A type switch on value.Value would be more idiomatic for handling protobuf oneof fields, making the logic clearer and more robust.
| func getBoolFromValue(value *commonv1.AnyValue) bool { | |
| // First try to get as bool | |
| if boolVal := value.GetBoolValue(); boolVal { | |
| return boolVal | |
| } | |
| // Try to parse from string (Claude Code sends some values as strings) | |
| if strVal := value.GetStringValue(); strVal != "" { | |
| if parsed, err := strconv.ParseBool(strVal); err == nil { | |
| return parsed | |
| } | |
| } | |
| return false | |
| } | |
| func getBoolFromValue(value *commonv1.AnyValue) bool { | |
| if value == nil { | |
| return false | |
| } | |
| switch v := value.Value.(type) { | |
| case *commonv1.AnyValue_BoolValue: | |
| return v.BoolValue | |
| case *commonv1.AnyValue_StringValue: | |
| if parsed, err := strconv.ParseBool(v.StringValue); err == nil { | |
| return parsed | |
| } | |
| } | |
| return false | |
| } |
| // case "decision_source": | ||
| // event.DecisionSource = value.GetStringValue() | ||
| // case "decision_type": | ||
| // event.DecisionType = value.GetStringValue() | ||
| // case "tool_result_size_bytes": | ||
| // event.ToolResultSizeBytes = getIntFromValue(value) |
There was a problem hiding this comment.
This commented-out code should be removed. If this functionality is planned for the future, it should be tracked in an issue rather than being left as dead code in the codebase, as it reduces readability.
1 participant
Summary
getBoolFromValuehelper function to parse boolean values from OTEL datasuccess) as strings instead of native boolsTest plan
🤖 Generated with Claude Code