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

support_flow_sample_expanded_and_counter_sample_expanded #170

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rachelScout
Copy link

support flow sample expanded and counter sample expanded

@dbardbar
Copy link
Contributor

dbardbar commented Jan 2, 2023

I think this change is problematic.
It defines a new type called FlowSampleExpand, which has different fields than the regular FlowSample. The expanded format is just a different binary representation of the same data, so it is not reasonable to have a different JSON representation of them. As vflow aims to give a standard representation of the data on wire, regardless of minor differences in encoding, this doesn't sound like the way to go.

Also, there's a lot of code duplication here, while actually the different in compact vs. expanded boils down to just a few differences.

See #179 for my detailed suggestion on how to solve this properly.

@fthrslntgy
Copy link

fthrslntgy commented Aug 16, 2023

THIS PR IS LIFE-SAVING. GOD BLESS YOU BRO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants