NETOBSERV-2186: set sampling field even without filters for consistency #691
Conversation
|
@jotak: This pull request references NETOBSERV-2186 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
jotak
added
the
ok-to-test
|
New images: These will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=445d0bd make set-agent-image |
bpf/flows.c
Outdated
| @@ -167,6 +169,7 @@ static inline int flow_monitor(struct __sk_buff *skb, u8 direction) { | |||
| do_sampling = 0; | |||
| return TC_ACT_OK; | |||
| } | |||
| flow_sampling = filter_sampling; | |||
There was a problem hiding this comment.
we probably can remove filter_sampling var
and just use
flow_sampling = sampling; like you did above
There was a problem hiding this comment.
Yeah, I was unsure, since filter_sampling is passed down to check_and_do_flow_filtering and the result is used to determine if a random sampling check is needed... I think it will work with a single var but didn't want to mess it up especially as there's no unit test
github-actions
bot
removed
the
ok-to-test
|
/lgtm |
|
you forget to rebuild ebpf code :) pls make sure flow filter with sampling is still working on ur cluster |
|
/ok-to-test |
openshift-ci
bot
added
the
ok-to-test
|
New images: These will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=8c85dc4 make set-agent-image |
|
/label qe-approved |
|
@jotak: This pull request references NETOBSERV-2186 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.20.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
github-actions
bot
removed
the
ok-to-test
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
|
/ok-to-test |
openshift-ci
bot
added
the
ok-to-test
|
New images: These will expire after two weeks. To deploy this build, run from the operator repo, assuming the operator is running: USER=netobserv VERSION=936aac1 make set-agent-image |
|
Retested post commit, works as expected! |
|
@jotak: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
tested flow filtering with and without sampling on my side too |
Description
Since sampling has to be set per-flow and can be modified from FLP, we need now to also include the global sampling, not only the filters sampling
Dependencies
Related:
Checklist
If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.