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

Sampling connection-oriented weird-types that happen only once per connection #623

Closed
ckreibich opened this issue Oct 4, 2019 · 5 comments · Fixed by #1154
Closed

Sampling connection-oriented weird-types that happen only once per connection #623

ckreibich opened this issue Oct 4, 2019 · 5 comments · Fixed by #1154

Comments

@ckreibich
Copy link
Member

Sampling of connection-oriented weird-types that happen only once per connection (such as data_before_established) doesn't work well right now: you either get each of those weirds, or none of them. The logic to tweak here is in Reporter::PermitFlowWeird().

I believe that's a known problem, but figured I'd create a ticket in case it's not tracked. I also don't know whether 3.0 has changed anything in this regard, but if so, that'd be great to know.

@jsiwek
Copy link
Contributor

jsiwek commented Oct 4, 2019

The sampling state for connection-oriented weirds is stored per-connection, but what you expect/want would be for sampling state to be tracked globally per-weird-name?

If so, think it's not hard to add that as an option, but the current default behavior was mostly about being conservative: "people are less likely to miss redundant data we automatically throw out (or offer "compressed" stats for), and more likely to expect that we report all novel information by default".

@jsiwek jsiwek added this to Unassigned / Todo in Release 3.1.0 via automation Oct 4, 2019
@jsiwek jsiwek added this to the 3.1.0 milestone Oct 4, 2019
@ckreibich
Copy link
Member Author

Exactly — some level of global control would be ideal. I believe I'm missing some history here from the updates to earlier versions of the sampling code (2.5, iirc), so others may have additional context.

The problem with the current approach is that in environments where data_before_established comes up a lot it effectively bypasses sampling and blows up the weird-log.

@rsmmr rsmmr removed this from the 3.1.0 milestone Jan 9, 2020
@rsmmr rsmmr removed this from Unassigned / Todo in Release 3.1.0 Jan 14, 2020
@rsmmr rsmmr self-assigned this Aug 31, 2020
@rsmmr
Copy link
Member

rsmmr commented Aug 31, 2020

I'm planning to add an additional config table that puts weirds on a list of samplings that are to be done globally, in contrast to their standard granularity (conn/flow).

rsmmr added a commit that referenced this issue Sep 2, 2020
The new set "sampling_global_list" lists weirds that are to be
rate-limited globally instead of per connection/flow.

Closes #623.
@rsmmr
Copy link
Member

rsmmr commented Sep 2, 2020

@ckreibich how does #1154 look?

@ckreibich
Copy link
Member Author

@rsmmr nice, yeah. Thanks for putting this together. I was wondering whether it would make sense to put together "candidate weirds" for use in the global list, but it seems that's really up to the user's sampling needs. So I think we're all set.

rsmmr added a commit that referenced this issue Sep 5, 2020
The new set "sampling_global_list" lists weirds to rate-limite
globally instead of per connection/flow.

Closes #623.
@jsiwek jsiwek closed this as completed in 5fa9497 Sep 9, 2020
jsiwek added a commit that referenced this issue Sep 9, 2020
- Merge adjustments:
  - Minor whitespace/style tweaks
  - Fixed portability of the btest due to differences in `uniq -c`
    output format

* origin/topic/robin/gh-623-sampling:
  Extend weird sampling with option to track selected weirds globally.
@jsiwek jsiwek added this to Unassigned / Todo in Release 4.0.0 via automation Sep 9, 2020
@jsiwek jsiwek added this to the 4.0.0 milestone Sep 9, 2020
@jsiwek jsiwek moved this from Unassigned / Todo to Done in Release 4.0.0 Sep 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Release 4.0.0
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants