Allow customizing spam filtering in event client library #103918
Conversation
k8s-ci-robot
added
release-note
|
Hi @olagacek. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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/test-infra repository. |
k8s-ci-robot
added
the
sig/api-machinery
| duplicateEvent := makeEvent("duplicate", "me again", makeObjectReference("Pod", "my-pod", "my-ns")) | ||
| similarEvent := makeEvent("first", "me again", makeObjectReference("Pod", "my-pod", "my-ns")) | ||
| testCases := map[string]struct { | ||
| previousEvents []v1.Event |
There was a problem hiding this comment.
Nit, looks like previous event is always the same, it might be more readable to not include previousEvents field in the test case
There was a problem hiding this comment.
Removed previousEvents field
| newEvent v1.Event | ||
| expectedEvent v1.Event | ||
| intervalSeconds int | ||
| burstSize int |
There was a problem hiding this comment.
ditto for burstSize and expected Skip. Not need to include in test case if they don't change between tests.
There was a problem hiding this comment.
burstSize is not included in test case anymore, but expectedSkip actually changes between the test cases :)
| "") | ||
| } | ||
| firstEvent := makeEvent("first", "i am first", makeObjectReference("Pod", "my-pod", "my-ns")) | ||
| duplicateEvent := makeEvent("duplicate", "me again", makeObjectReference("Pod", "my-pod", "my-ns")) |
There was a problem hiding this comment.
For me those names are reversed. Duplicate event should be exactly the same as the original and similar can differ. Can we make it more clearer in names or in test arguments what are differences?
There was a problem hiding this comment.
Renamed the variables, hope they are more descriptive now
|
/assign @serathius |
k8s-ci-robot
added
triage/accepted
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: olagacek, serathius 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 |
k8s-ci-robot
added
the
approved
What type of PR is this?
/kind feature
What this PR does / why we need it:
Current implementation
Event client library has aggregation and spam filtering implemented. While aggregation can be fully customized, spam filtering functionality doesn't allow overriding spamKey func - function that is responsible for creating a key (based on an event) which is used to detect "spam" events. Currently used
getSpamKey()function takes into accountSourceandInvolvedObjectof an event. Per each such keyBurstSizeevents can be sent.Changes in PR
Allow customizing spamKey func in
CorrelatorOptionsofEventCorrelator.Why allow customizing spamKey func?
For some use cases it can be useful to also include for example
Reasonfield of an event in spamKey. For example, if we havennormal events for a specific Object with the same reason andn+1-th event is e.g warning with a different reason we would like to have it emitted, while filtering n-1 normal events.This PR does not change default behavior and doesn't introduce any breaking change. Current users of client library won't experience any change, unless they explicitly modify
CorrelatorOptions.Which issue(s) this PR fixes:
None
Does this PR introduce a user-facing change?
/cc @serathius @dashpole @logicalhan
/sig instrumentation