-
Notifications
You must be signed in to change notification settings - Fork 213
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
feat(conntrack): conntrack integration with packetparser #624
Conversation
Signed-off-by: Quang Nguyen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left some nits, lgtm
@@ -31,22 +31,15 @@ const ( | |||
// 2 is from host to network and 3 is from network to host. | |||
// ts is the timestamp in nanoseconds. | |||
func ToFlow( | |||
l *log.ZapLogger, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there any extra reasoning to passing logger here besides warn? this was pretty concise, requires refactoring usage downstream just for a warn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no reason to pass the logger here. From some of the pprof we collected, this actually incurred quite a bit of memory usage.
Description
Part 2 of #610
Related Issue
If this pull request is related to any issue, please mention it here. Additionally, make sure that the issue is assigned to you before submitting this pull request.
Checklist
git commit -S -s ...
). See this documentation on signing commits.Screenshots (if applicable) or Testing Completed
Output from debug CLI tool:
Hubble flow logs:
Additional Notes
Add any additional notes or context about the pull request here.
Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.