-
Notifications
You must be signed in to change notification settings - Fork 262
feat(packetparser): Allow sampling of packets #1767
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
base: main
Are you sure you want to change the base?
Conversation
ebac9e3 to
b2a5ab5
Compare
|
@mmckeen I will try to make a pass this week |
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.
I think we should revisit the syntax of DataSamplingRate. For example, since I know the code context, I understand that setting DataSamplingRate to 1 means every packet will be sampled. But for someone new to Retina, this behavior may not be obvious.
|
Also, I'm all for ditching the reporting period entirely if we can make packet sampling works as it is a lot more flexible than the currently hardcoding value that we are using for reporting period 🙂 cc @anubhabMajumdar @SRodi So what I'm thinking of is either we report a packet because it carries a new flag or because it was randomly sampled. Some more advanced behavior that I could think of is that we can monitor cpu usage and dynamically adjust the sampling rate and recompile the packet parser program accordingly. |
The reporting interval might still be valid to enable us to make sure we report eventually for every 5-tuple. For example, we wouldn't want to sample and then never report the counters for a particular connection. The reporting interval being something smallish also ensures Prometheus |
|
This PR will be closed in 7 days due to inactivity. |
|
Apologies, been a while for me to circle back on this. I'll try and address the comments this week 🙇 |
4466a52 to
5ddf47d
Compare
Signed-off-by: Matthew McKeen <[email protected]>
5ddf47d to
49ed563
Compare
|
@nddq updated this with docs, the sampling calculation still looks accurate to me but happy for feedback 🙇 |
|
@mmckeen thanks for the docs! After re-reading it, the sampling calculation makes more sense to me now (I thought by default, the |
Description
This PR allows for optional sampling of packet reporting when in high data aggregation level for
packetparser.By default, all packets are reported but optionally
1 out of npackets are sampled by random chance with the exception of certain important control flags or when hitting the reporting interval.This allows Retina to scale to high network volume environments at the trade-off of some reporting granularity.
The performance impact of this is mostly for workloads with lots of new connections, connections already tracked in the conntrack table rely on #1665 for scalability.
The behavior added in #1665 allows for accurate reporting of metrics despite sampling being in place.
Related Issue
#1760
Checklist
git commit -S -s ...). See this documentation on signing commits.Screenshots (if applicable) or Testing Completed
Main
After the change (with default sampling rate of 1)
After the change (with sampling rate of 1000)
Please refer to the CONTRIBUTING.md file for more information on how to contribute to this project.