Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: parse_aws_vpc_flow_log types account-id as int64 instead of string #263

Closed
timcosta opened this issue Jun 2, 2023 · 2 comments
Closed
Labels
type: bug A code related bug vrl: stdlib Changes to the standard library

Comments

@timcosta
Copy link

timcosta commented Jun 2, 2023

Hi all! I've been parsing some VPC Flow Logs with VRL, and noticed that a chunk of them are being sidelined due to errors. All of the sidelined logs have unknown as the value for the account-id column, which is listed as possible in the AWS docs.

VRL appears to type account-id as an int64, which makes it fail to parse any VPC Flow logs where the account id is unknown. I believe parsing as int64 will also drop any leading 0 characters, which would make the output incorrect for any account id that has one or more leading zeroes.

Sample log with ENI and IPs obfuscated:

2 unknown eni-070b1422222222222 192.168.1.1 10.0.0.2 55828 60002 6 1 44 1685727399 1685727446 ACCEPT OK

Let me know if you need any additional information!

@neuronull neuronull added the type: bug A code related bug label Jun 5, 2023
@thhwalker
Copy link
Contributor

Just thinking out loud here, but would it be feasible to mark the account ID as a string in this instance?
AWS have specified the key as a string type, and at face value I don't see any specific need for this to be an integer for the sake of manipulating the value.

That being said, I'm cognisant of the fact that there may be users that are handling these values specifically as integers. I'm not sure what they would be doing with this information, but I feel it's something to be mindful of.

I could just be overthinking this one.

I'm more than happy to handle a PR for this if changing Kind::integer to Kind::bytes is the agreed solution.

@jszwedko
Copy link
Member

Yeah, agreed, this seems like it should be a string rather than an int64.

@jszwedko jszwedko added the vrl: stdlib Changes to the standard library label Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A code related bug vrl: stdlib Changes to the standard library
Projects
None yet
Development

No branches or pull requests

5 participants