Fix GetGid/GetUid not implemented for event type. eventType: symlink#728
Merged
Fix GetGid/GetUid not implemented for event type. eventType: symlink#728
Conversation
…and improve logging Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
…ing consistency Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
📝 WalkthroughWalkthroughRefactored event accessor methods in two core files to use generalized field access with error handling instead of hard-coded reads. Updated Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
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 unit tests (beta)
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. Comment |
![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 (3)
pkg/utils/datasource_event.go (2)
561-568:GetFlagsRawreadsInt32but returnsuint32— potential sign truncation.If
flags_rawis stored as a signedint32with the high bit set (e.g.,O_NOFOLLOW=0x20000), the castuint32(flags)is fine in Go (bit-preserving). However, usingUint32directly would be more semantically correct and avoid the indirection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/utils/datasource_event.go` around lines 561 - 568, The GetFlagsRaw function currently reads flags_raw via Int32 then casts to uint32, which is semantically off and can be clearer by reading it as an unsigned value; update GetFlagsRaw to use the field accessor's Uint32 (or equivalent unsigned) method instead of Int32, handle the error the same way (logging via logger.L().Warning with helpers.String("eventType", string(e.EventType)) and returning 0 on error), and return the unsigned result directly so the code in DatasourceEvent.GetFlagsRaw and its call to e.getFieldAccessor("flags_raw") consistently treats the field as uint32.
418-515: Consider extracting a helper to reduce ECS getter boilerplate.All 11
GetEcs*methods follow the identical pattern:getFieldAccessor("ecs.X").String(e.Data)→ warn on error → return empty string. This is ~100 lines of near-identical code.♻️ Possible helper to reduce duplication
func (e *DatasourceEvent) getEcsString(field, method string) string { val, err := e.getFieldAccessor(field).String(e.Data) if err != nil { logger.L().Warning(method+" - "+field+" field not found in event type", helpers.String("eventType", string(e.EventType))) return "" } return val }Each getter then becomes a one-liner:
func (e *DatasourceEvent) GetEcsClusterARN() string { return e.getEcsString("ecs.clusterARN", "GetEcsClusterARN") }This pattern could also be generalized to non-ECS string getters.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/utils/datasource_event.go` around lines 418 - 515, The 11 GetEcs* methods in DatasourceEvent are duplicate boilerplate calling getFieldAccessor(...).String(e.Data) and logging on error; extract a helper (e.g., DatasourceEvent.getEcsString(field, methodName) or a more general getString(field, methodName)) that performs the accessor call, error logging using logger.L().Warning with helpers.String("eventType", string(e.EventType)), and returns the string or "" on error, then replace each GetEcsClusterARN/GetEcsClusterName/GetEcsContainerARN/GetEcsContainerInstance/GetEcsContainerName/GetEcsAvailabilityZone/GetEcsLaunchType/GetEcsServiceName/GetEcsTaskARN/GetEcsTaskDefinitionARN/GetEcsTaskFamily with a one-liner that returns e.getEcsString("ecs.<field>", "GetEcs<...>") (or e.getString for the generalized helper).pkg/utils/struct_event.go (1)
173-215: ECS stub methods return empty strings for interface compliance.These are no-op stubs since
StructEventdoesn't carry ECS metadata. This is fine for satisfying the interface contract. Consider whether a shared no-op mixin could reduce boilerplate if this pattern grows further.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/utils/struct_event.go` around lines 173 - 215, The file contains many identical no-op ECS accessor methods on StructEvent (GetEcsAvailabilityZone, GetEcsClusterARN, GetEcsClusterName, GetEcsContainerARN, GetEcsContainerInstance, GetEcsContainerName, GetEcsLaunchType, GetEcsServiceName, GetEcsTaskARN, GetEcsTaskDefinitionARN, GetEcsTaskFamily); refactor by extracting these into a single reusable no-op mixin type (e.g., type ecsNoop struct with the same method set returning ""), then embed that mixin in StructEvent (or have StructEvent alias/compose it) and remove the duplicated methods from StructEvent so interface compliance is preserved but boilerplate is eliminated. Ensure the mixin defines all listed method names exactly to satisfy the interface.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/utils/datasource_event.go`:
- Around line 316-369: GetDstEndpoint currently returns empty L4Endpoint if any
sub-field is missing; change it so core network fields (addr via
"endpoint.addr_raw.v4", version "endpoint.version", port "endpoint.port", and
proto "endpoint.proto_raw") still cause an early return on error but make K8s
enrichment fields ("endpoint.k8s.kind", "endpoint.k8s.name",
"endpoint.k8s.namespace", "endpoint.k8s.labels") best-effort: call
getFieldAccessor for each enrichment field, and if it errors log a warning and
continue with zero-value/defaults (e.g., empty strings or empty map from
parseStringToMap) instead of returning; ensure the final return constructs
types.L4Endpoint using rawIPv4ToString(addr), parseStringToMap(podLabels) and
types.EndpointKind(kind) so addr/port/proto are preserved even if enrichment is
missing.
---
Nitpick comments:
In `@pkg/utils/datasource_event.go`:
- Around line 561-568: The GetFlagsRaw function currently reads flags_raw via
Int32 then casts to uint32, which is semantically off and can be clearer by
reading it as an unsigned value; update GetFlagsRaw to use the field accessor's
Uint32 (or equivalent unsigned) method instead of Int32, handle the error the
same way (logging via logger.L().Warning with helpers.String("eventType",
string(e.EventType)) and returning 0 on error), and return the unsigned result
directly so the code in DatasourceEvent.GetFlagsRaw and its call to
e.getFieldAccessor("flags_raw") consistently treats the field as uint32.
- Around line 418-515: The 11 GetEcs* methods in DatasourceEvent are duplicate
boilerplate calling getFieldAccessor(...).String(e.Data) and logging on error;
extract a helper (e.g., DatasourceEvent.getEcsString(field, methodName) or a
more general getString(field, methodName)) that performs the accessor call,
error logging using logger.L().Warning with helpers.String("eventType",
string(e.EventType)), and returns the string or "" on error, then replace each
GetEcsClusterARN/GetEcsClusterName/GetEcsContainerARN/GetEcsContainerInstance/GetEcsContainerName/GetEcsAvailabilityZone/GetEcsLaunchType/GetEcsServiceName/GetEcsTaskARN/GetEcsTaskDefinitionARN/GetEcsTaskFamily
with a one-liner that returns e.getEcsString("ecs.<field>", "GetEcs<...>") (or
e.getString for the generalized helper).
In `@pkg/utils/struct_event.go`:
- Around line 173-215: The file contains many identical no-op ECS accessor
methods on StructEvent (GetEcsAvailabilityZone, GetEcsClusterARN,
GetEcsClusterName, GetEcsContainerARN, GetEcsContainerInstance,
GetEcsContainerName, GetEcsLaunchType, GetEcsServiceName, GetEcsTaskARN,
GetEcsTaskDefinitionARN, GetEcsTaskFamily); refactor by extracting these into a
single reusable no-op mixin type (e.g., type ecsNoop struct with the same method
set returning ""), then embed that mixin in StructEvent (or have StructEvent
alias/compose it) and remove the duplicated methods from StructEvent so
interface compliance is preserved but boilerplate is eliminated. Ensure the
mixin defines all listed method names exactly to satisfy the interface.
YakirOren
approved these changes
Feb 26, 2026
This was referenced Feb 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor