Conversation
…cs/events
- Remove CCOtelSession struct entirely
- Add CCOtelResourceAttributes helper for extracting resource-level attributes
- Embed all resource attributes directly in CCOtelMetric and CCOtelEvent:
- Standard: sessionId, userAccountUuid, organizationId, terminalType, appVersion, osType, osVersion, hostArch
- Additional: userId, userEmail
- Custom (OTEL_RESOURCE_ATTRIBUTES): userName, machineName, teamId
- Add EventTimestamp field for ISO 8601 timestamp
- Add getIntFromValue/getFloatFromValue helpers to handle string-encoded numeric values
- Update CCOtelRequest to flat structure: { host, project, metrics[], events[] }
- Remove SessionID from CCOtelResponse
Based on Claude Code monitoring documentation which specifies attributes
should be on individual metrics/events, not in a separate session entity.
🤖 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 significantly refactors the OpenTelemetry (OTEL) data processing pipeline by moving away from a distinct session concept. Instead of grouping metrics and events under a Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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 refactors the OTEL data processing by removing the session concept and embedding resource attributes directly into metrics and events. This flattens the data structure, which is a good simplification. The changes are well-executed, but I have identified a few areas for improvement.
My review includes:
- A high-severity issue regarding a potential regression where session IDs are no longer generated if missing.
- A high-severity correctness issue in the new helper functions for parsing numeric values, with a suggestion for a more robust implementation.
- A medium-severity suggestion to reduce code duplication in the data model structs for better long-term maintainability.
Overall, this is a solid refactoring, and addressing these points will make it even better.
| func extractResourceAttributes(resource *resourcev1.Resource) *model.CCOtelResourceAttributes { | ||
| attrs := &model.CCOtelResourceAttributes{} | ||
|
|
||
| if resource == nil { | ||
| return session | ||
| return attrs | ||
| } | ||
|
|
||
| for _, attr := range resource.GetAttributes() { | ||
| key := attr.GetKey() | ||
| value := attr.GetValue() | ||
|
|
||
| switch key { | ||
| // Standard resource attributes | ||
| case "session.id": | ||
| session.SessionID = value.GetStringValue() | ||
| attrs.SessionID = value.GetStringValue() | ||
| case "app.version": | ||
| session.AppVersion = value.GetStringValue() | ||
| attrs.AppVersion = value.GetStringValue() | ||
| case "organization.id": | ||
| session.OrganizationID = value.GetStringValue() | ||
| attrs.OrganizationID = value.GetStringValue() | ||
| case "user.account_uuid": | ||
| session.UserAccountUUID = value.GetStringValue() | ||
| attrs.UserAccountUUID = value.GetStringValue() | ||
| case "terminal.type": | ||
| session.TerminalType = value.GetStringValue() | ||
| attrs.TerminalType = value.GetStringValue() | ||
| case "service.version": | ||
| session.ServiceVersion = value.GetStringValue() | ||
| attrs.ServiceVersion = value.GetStringValue() | ||
| case "os.type": | ||
| session.OSType = value.GetStringValue() | ||
| attrs.OSType = value.GetStringValue() | ||
| case "os.version": | ||
| session.OSVersion = value.GetStringValue() | ||
| attrs.OSVersion = value.GetStringValue() | ||
| case "host.arch": | ||
| session.HostArch = value.GetStringValue() | ||
| attrs.HostArch = value.GetStringValue() | ||
| case "wsl.version": | ||
| session.WSLVersion = value.GetStringValue() | ||
| attrs.WSLVersion = value.GetStringValue() | ||
| // Additional identifiers | ||
| case "user.id": | ||
| attrs.UserID = value.GetStringValue() | ||
| case "user.email": | ||
| attrs.UserEmail = value.GetStringValue() | ||
| // Custom resource attributes (from OTEL_RESOURCE_ATTRIBUTES) | ||
| case "user.name": | ||
| attrs.UserName = value.GetStringValue() | ||
| case "machine.name": | ||
| attrs.MachineName = value.GetStringValue() | ||
| case "team.id": | ||
| attrs.TeamID = value.GetStringValue() | ||
| } | ||
| } | ||
|
|
||
| // Generate session ID if not present | ||
| if session.SessionID == "" { | ||
| session.SessionID = uuid.New().String() | ||
| } | ||
| return attrs | ||
| } |
There was a problem hiding this comment.
The previous extractSessionFromResource function would generate a new UUID for the session ID if one wasn't provided in the resource attributes. This behavior seems to have been lost in the refactoring. If a sessionId is expected to be present on all events and metrics, you might want to reintroduce this fallback logic to prevent potential issues downstream. For example, by adding if attrs.SessionID == "" { attrs.SessionID = uuid.New().String() } before returning from this function.
| func getIntFromValue(value *commonv1.AnyValue) int { | ||
| // First try to get as int | ||
| if intVal := value.GetIntValue(); intVal != 0 { | ||
| return int(intVal) | ||
| } | ||
| // Try to parse from string (Claude Code sends some values as strings) | ||
| if strVal := value.GetStringValue(); strVal != "" { | ||
| if parsed, err := strconv.Atoi(strVal); err == nil { | ||
| return parsed | ||
| } | ||
| } | ||
| return 0 | ||
| } |
There was a problem hiding this comment.
The current implementation of getIntFromValue relies on checking for a non-zero value from value.GetIntValue(), which is not a robust way to determine the type of a commonv1.AnyValue. This can be ambiguous if 0 is a valid integer value. The idiomatic and safer approach for handling protobuf oneof fields is to use a type switch on the Value field. This makes the code cleaner, more readable, and less prone to breaking with future library changes. A similar improvement can be applied to getFloatFromValue.
| func getIntFromValue(value *commonv1.AnyValue) int { | |
| // First try to get as int | |
| if intVal := value.GetIntValue(); intVal != 0 { | |
| return int(intVal) | |
| } | |
| // Try to parse from string (Claude Code sends some values as strings) | |
| if strVal := value.GetStringValue(); strVal != "" { | |
| if parsed, err := strconv.Atoi(strVal); err == nil { | |
| return parsed | |
| } | |
| } | |
| return 0 | |
| } | |
| func getIntFromValue(value *commonv1.AnyValue) int { | |
| if value == nil { | |
| return 0 | |
| } | |
| switch v := value.Value.(type) { | |
| case *commonv1.AnyValue_IntValue: | |
| return int(v.IntValue) | |
| case *commonv1.AnyValue_StringValue: | |
| if parsed, err := strconv.Atoi(v.StringValue); err == nil { | |
| return parsed | |
| } | |
| } | |
| return 0 | |
| } |
| // Embedded resource attributes (previously in session) | ||
| SessionID string `json:"sessionId,omitempty"` | ||
| UserAccountUUID string `json:"userAccountUuid,omitempty"` | ||
| OrganizationID string `json:"organizationId,omitempty"` | ||
| TerminalType string `json:"terminalType,omitempty"` | ||
| AppVersion string `json:"appVersion,omitempty"` | ||
| OSType string `json:"osType,omitempty"` | ||
| OSVersion string `json:"osVersion,omitempty"` | ||
| HostArch string `json:"hostArch,omitempty"` | ||
|
|
||
| // Additional identifiers | ||
| UserID string `json:"userId,omitempty"` | ||
| UserEmail string `json:"userEmail,omitempty"` | ||
|
|
||
| // Custom resource attributes | ||
| UserName string `json:"userName,omitempty"` | ||
| MachineName string `json:"machineName,omitempty"` | ||
| TeamID string `json:"teamId,omitempty"` |
There was a problem hiding this comment.
These resource attribute fields are duplicated in CCOtelMetric. To improve maintainability, you could extract them into a shared struct and embed it in both CCOtelEvent and CCOtelMetric.
For example:
// CCOtelSharedAttributes contains common attributes embedded in events and metrics.
type CCOtelSharedAttributes struct {
SessionID string `json:"sessionId,omitempty"`
UserAccountUUID string `json:"userAccountUuid,omitempty"`
// ... other shared fields
}
type CCOtelEvent struct {
// ... other event fields
CCOtelSharedAttributes
}
type CCOtelMetric struct {
// ... other metric fields
CCOtelSharedAttributes
}This would also allow you to simplify the applyResourceAttributesToMetric and applyResourceAttributesToEvent functions in ccotel_processor.go into a single function.
Add pwd field extraction from resource attributes and propagate it to both metrics and events for better context tracking. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
CCOtelSessionstruct - no separate session entityCCOtelResourceAttributeshelper for extracting resource-level attributesCCOtelMetricandCCOtelEvent:sessionId,userAccountUuid,organizationId,terminalType,appVersion,osType,osVersion,hostArchuserId,userEmailuserName,machineName,teamIdEventTimestampfield for ISO 8601 timestampgetIntFromValue/getFloatFromValuehelpers to handle string-encoded numeric values (critical fix for Claude Code OTEL output)CCOtelRequestto flat structure:{ host, project, metrics[], events[] }API Request Structure
Before:
{ "host": "machine", "project": "/path", "session": { "sessionId": "...", "userAccountUuid": "...", ... }, "metrics": [{ "metricId": "...", "value": 100 }] }After:
{ "host": "machine", "project": "/path", "metrics": [{ "metricId": "...", "value": 100, "sessionId": "...", "userAccountUuid": "...", "userId": "...", "userEmail": "...", "userName": "annatarhe", "machineName": "MacBook-Pro.local", "teamId": "shelltime" }] }Test plan
go build ./model/... ./daemon/...go test ./daemon/... ./model/...🤖 Generated with Claude Code