NETOBSERV-1688: relax ebpf drop alert #674
Conversation
|
@jotak: This pull request references NETOBSERV-1688 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 bug to target the "4.17.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. |
|
@msherif1234 @memodi please let me know also if the new doc & alert text is better like that. The changes I'm making are to convey:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #674 +/- ##
==========================================
+ Coverage 66.60% 66.78% +0.18%
==========================================
Files 70 69 -1
Lines 8115 8093 -22
==========================================
Hits 5405 5405
+ Misses 2315 2293 -22
Partials 395 395
Flags with carried forward coverage won't be shown. Click here to find out more.
|
controllers/ebpf/agent-metrics.go
Outdated
| }, | ||
| Expr: intstr.FromString("sum(rate(netobserv_agent_dropped_flows_total[1m])) > 0"), | ||
| Expr: intstr.FromString("sum(rate(netobserv_agent_dropped_flows_total[1m])) > 100"), |
There was a problem hiding this comment.
just put this 100 limit in the constants
| @@ -124,10 +124,10 @@ func (c *AgentController) agentPrometheusRule(target *flowslatest.FlowCollectorE | |||
| rules = append(rules, monitoringv1.Rule{ | |||
| Alert: string(flowslatest.AlertDroppedFlows), | |||
| Annotations: map[string]string{ | |||
| "description": "NetObserv eBPF agent is not able to process new flows. Possible reasons are the BPF hashmap being full, or the capacity limiter being triggered. Both causes can be worked around by increasing cacheMaxFlows value in Flowcollector resource.", | |||
| "summary": "NetObserv eBPF is not able to process any new flows", | |||
| "description": "NetObserv eBPF agent is missing packets or flows. The metric netobserv_agent_dropped_flows_total provides more information on the cause. Possible reasons are the BPF hashmap being busy or full, or the capacity limiter being triggered. This may be worked around by increasing cacheMaxFlows value in Flowcollector resource.", | |||
There was a problem hiding this comment.
by saying "full", doesn't that mean capacity ( of hashmap) is reached? if it's same thing than we can update text busy or full, or the capacity limiter being triggered since its a repetition.
There was a problem hiding this comment.
the capacity limiter is something different than the hashmap capacity , it's something that comes later in the process (just before flows are about to be sent over the network to kafka or FLP)
There was a problem hiding this comment.
I am fine with new description
There was a problem hiding this comment.
I see, so increasing the cacheMaxFlows value will help when flows/packets are dropped due to hashmap being full but it wouldn't really help with if the capacity limit is triggered, correct?
There was a problem hiding this comment.
Actually, no. The capacity limiter also obeys to cacheMaxFlows setting, so it will also be a solution in that case.
The case where cacheMaxFlows won't be a solution is the very one we've seen here: when packets are missed due to hashmap being "busy", but not full. Unfortunately we haven't found so far a way to distinguish cases between this sort of "busy" error versus map being full, that would be easily visible to users. One thing they might do is correlating this error with other metrics that provide the current size usage of the map, and see whether or not it's close to its max capacity.
There's certainly more that we could do here, in follow-ups; like creating another alert precisely for when the map size comes close to its capacity.
| bpfTraceMountPath = "/sys/kernel/debug" | ||
| bpfNetNSMountName = "var-run-netns" | ||
| bpfNetNSMountPath = "/var/run/netns" | ||
| droppedFlowsAlertThreshold = 100 |
There was a problem hiding this comment.
the threshold of 100 , is it based on some testing or an arbitrary value that we think is good (not too high or not too low)?
There was a problem hiding this comment.
That's arbitrary, but I don't see how to not make it arbitrary, unless by making it a configurable threshold. People who would like to set another limit can still copy/paste the rule to a new one, and turn off this alert in netobserv. The rule is still useful in that sense.
Still, I did test it on my cluster.
With my "medium" hey-ho test intensity (which actually is quite high already), I'm between 10 and 15 drops/sec. With my "high" intensity I'm around 25-30.
Bigger clusters would likely see more drops/sec under stress.
|
/ok-to-test |
openshift-ci
bot
added
the
ok-to-test
|
New images:
They will expire after two weeks. To deploy this build: # Direct deployment, from operator repo
IMAGE=quay.io/netobserv/network-observability-operator:cb21b5e make deploy
# Or using operator-sdk
operator-sdk run bundle quay.io/netobserv/network-observability-operator-bundle:v0.0.0-cb21b5eOr as a Catalog Source: apiVersion: operators.coreos.com/v1alpha1
kind: CatalogSource
metadata:
name: netobserv-dev
namespace: openshift-marketplace
spec:
sourceType: grpc
image: quay.io/netobserv/network-observability-operator-catalog:v0.0.0-cb21b5e
displayName: NetObserv development catalog
publisher: Me
updateStrategy:
registryPoll:
interval: 1m |
| @@ -1,56 +0,0 @@ | |||
| package ebpf | |||
There was a problem hiding this comment.
any reason for deleting this unit-test ?
There was a problem hiding this comment.
Yes I explained it in the commit message :)
dc257fb
|
@jotak: This pull request references NETOBSERV-1688 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 bug to target the "4.17.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. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jotak The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
reformulate some of the text for ebpf drop alert, and relax threshold a little bit.
On removing test: agent-metrics-test.go did never run; it should have been named "agent_metrics_test.go" to run (_ instead of -). When renamed, it actually fails. Given how little value these tests are adding, I'm removing them rather than spending time to fix them. (they only check that the objects created names match with the expected constant) The controller integration tests cover more and are much more relevant (and they run)
github-actions
bot
removed
the
ok-to-test
|
/lgtm |
* NETOBSERV-1688: relax ebpf drop alert reformulate some of the text for ebpf drop alert, and relax threshold a little bit. * Use const for alert threshold; remove unused test On removing test: agent-metrics-test.go did never run; it should have been named "agent_metrics_test.go" to run (_ instead of -). When renamed, it actually fails. Given how little value these tests are adding, I'm removing them rather than spending time to fix them. (they only check that the objects created names match with the expected constant) The controller integration tests cover more and are much more relevant (and they run)
reformulate some of the text for ebpf drop alert, and relax threshold a little bit.
Description
Dependencies
n/a
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.