[AWSX] fix(logs fowarder): Skip header line on VPC flow logs parsing#1044
[AWSX] fix(logs fowarder): Skip header line on VPC flow logs parsing#1044
Conversation
|
|
||
|
|
||
| def is_vpc_flowlog(key): | ||
| return "vpcflowlogs" in key |
There was a problem hiding this comment.
💭 thought: I'm not fan of this as it puts back some source identification in the forwarder and also some business logic.
While this check is the current method for S3-based VPC Flow Log, we also support CloudWatch logs for VPC Flow Logs (they seems to not face the issue). Is there we can implement this on backend side so that if we update the source identification we don't have two places to maintain it?
There was a problem hiding this comment.
I think this is doable from the backend, but requires a change in the logic as we won't know which line comes first on the intake side. For that we need to perform a keyword check maybe ? For the file key, it's already included in the log payload so we could use that instead of doing the check here.
The only downside is that we're going to still send the log and filter it out in the backend.
Regarding the source identification I agree that this brings back some business logic, but it doesn't add any extra fields to the log itself. We're still going to keep some business logic eventually (same goes for cloudtrail), but I think it should be fine if we don't use it amending payload to the log itself.
I'm fine with either choices by the way, this one seemed more logical as we trim sending the log at the source and not filter it out on the back-end side.
There was a problem hiding this comment.
You got me at the "we don't know the order on backend side" so Ok for this.
What does this PR do?
Motivation
See #1043
Testing Guidelines
Additional Notes
Types of changes
Check all that apply