feat: Add event_id(UUID) to events info in agent message to backend#160
feat: Add event_id(UUID) to events info in agent message to backend#160ambermingxin merged 1 commit intomainfrom
Conversation
📝 WalkthroughWalkthroughAdded UUID-based EventID generation and propagation: new Changes
Sequence DiagramsequenceDiagram
participant Client as Collector
participant Store as EventStore
participant Converter as OTLP Converter
participant Exporter as OTLP Exporter
Client->>Client: collectEvents() iterates events
alt event has EventID
Client->>Store: append event with existing EventID
else no EventID
Client->>Client: GenerateEventID() -> UUID
Client->>Store: append event with generated EventID
end
Converter->>Store: retrieve stored event
Converter->>Converter: build OTLP log record (include event_id = Event.EventID)
Converter->>Exporter: export OTLP log record
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
155d5d0 to
56a754a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 155d5d0159
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/exporter/converter/otlp.go (1)
290-295: Consider omittingevent_idwhen empty.Right now blank
event_idvalues are exported for events without IDs. Skipping empty values keeps payload cleaner and avoids low-value attributes.Optional refactor
- { - Key: "event_id", - Value: &commonv1.AnyValue{ - Value: &commonv1.AnyValue_StringValue{StringValue: event.EventID}, - }, - }, + // event_id is added only when presentattributes := []*commonv1.KeyValue{ { Key: "component", @@ { Key: "log_type", @@ }, } + if event.EventID != "" { + attributes = append(attributes, &commonv1.KeyValue{ + Key: "event_id", + Value: &commonv1.AnyValue{ + Value: &commonv1.AnyValue_StringValue{StringValue: event.EventID}, + }, + }) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/exporter/converter/otlp.go` around lines 290 - 295, The code currently always adds an "event_id" attribute with event.EventID even when it's empty; update the builder in internal/exporter/converter/otlp.go so it only appends the attribute when event.EventID is non-empty (e.g., guard the block that creates the commonv1.AnyValue for event_id with if event.EventID != ""), leaving all other attribute creation unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@third_party/fleet-intelligence-sdk/pkg/eventstore/types.go`:
- Around line 30-31: The EventID field on the local Event struct is never
propagated when converting to the API type; update the converter function
(*Event).ToEvent() to carry e.EventID into the outgoing apiv1.Event (either into
an explicit event_id field if present or into a stable metadata/attributes key
such as "event_id") so the identifier is not lost during serialization; locate
the ToEvent method and add logic to set apiv1.Event.Metadata["event_id"] =
e.EventID (or the equivalent field) only when e.EventID is non-empty, ensuring
round-trip preservation until apiv1.Event gains a first-class event_id.
---
Nitpick comments:
In `@internal/exporter/converter/otlp.go`:
- Around line 290-295: The code currently always adds an "event_id" attribute
with event.EventID even when it's empty; update the builder in
internal/exporter/converter/otlp.go so it only appends the attribute when
event.EventID is non-empty (e.g., guard the block that creates the
commonv1.AnyValue for event_id with if event.EventID != ""), leaving all other
attribute creation unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ff900a34-8c75-4894-ac66-b164f2ed8d43
📒 Files selected for processing (5)
internal/exporter/collector/collector.gointernal/exporter/collector/collector_test.gointernal/exporter/converter/otlp.gointernal/exporter/converter/otlp_test.gothird_party/fleet-intelligence-sdk/pkg/eventstore/types.go
Signed-off-by: Amber Xue <ambermingxin@nvidia.com>
56a754a to
3b32cc6
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/exporter/collector/collector.go (1)
249-252: Validate non-empty incomingevent_idbefore forwarding.Right now only empty IDs are replaced. A malformed non-empty ID will still be emitted, which can break the “UUID” contract downstream.
Suggested patch
eventID := event.EventID if eventID == "" { eventID = GenerateEventID() + } else if _, err := uuid.Parse(eventID); err != nil { + log.Logger.Warnw("Invalid event ID from component, regenerating", + "component", componentName, "event_id", eventID, "error", err) + eventID = GenerateEventID() }Also applies to: 256-256
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/exporter/collector/collector.go` around lines 249 - 252, Validate that incoming event.EventID is both non-empty and a well-formed UUID before forwarding: replace the current unconditional assignment block that only checks for empty string (where eventID := event.EventID / if eventID == "" { eventID = GenerateEventID() }) with a validation that attempts to parse event.EventID as a UUID (or use the project’s UUID validation helper) and only accept it if parsing succeeds; otherwise call GenerateEventID() and use that value. Apply the same validation logic to the other occurrence referenced in the comment (the second place around the existing check at the later occurrence).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@internal/exporter/collector/collector.go`:
- Around line 249-252: Validate that incoming event.EventID is both non-empty
and a well-formed UUID before forwarding: replace the current unconditional
assignment block that only checks for empty string (where eventID :=
event.EventID / if eventID == "" { eventID = GenerateEventID() }) with a
validation that attempts to parse event.EventID as a UUID (or use the project’s
UUID validation helper) and only accept it if parsing succeeds; otherwise call
GenerateEventID() and use that value. Apply the same validation logic to the
other occurrence referenced in the comment (the second place around the existing
check at the later occurrence).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c65de2d0-c18c-44b4-8ec0-687e7c45b37d
📒 Files selected for processing (7)
internal/exporter/collector/collector.gointernal/exporter/collector/collector_test.gointernal/exporter/converter/otlp.gointernal/exporter/converter/otlp_test.gothird_party/fleet-intelligence-sdk/api/v1/types.gothird_party/fleet-intelligence-sdk/pkg/eventstore/types.gothird_party/fleet-intelligence-sdk/pkg/eventstore/types_test.go
✅ Files skipped from review due to trivial changes (1)
- third_party/fleet-intelligence-sdk/pkg/eventstore/types_test.go
🚧 Files skipped from review as they are similar to previous changes (4)
- internal/exporter/converter/otlp.go
- internal/exporter/converter/otlp_test.go
- internal/exporter/collector/collector_test.go
- third_party/fleet-intelligence-sdk/pkg/eventstore/types.go
|
Please remove the ticket number from PR title and add a semantic prefix like feat: |
Description
Checklist
Summary by CodeRabbit
New Features
Tests