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

eventcachemetrics: fix eventcache and add error metrics #291

Merged
merged 1 commit into from
Aug 8, 2022

Conversation

willfindlay
Copy link
Contributor

Minor fixes to event cache handlers so that they deal with missing pod info separately
from missing process info.

Also update the event cache to increment pod and process info errors by event type when we
hit our cache strikes limit.

Signed-off-by: William Findlay [email protected]

@willfindlay willfindlay requested a review from a team as a code owner August 5, 2022 19:15
@willfindlay willfindlay force-pushed the pr/willfindlay/more-metrics branch from 17feb06 to 7681eaa Compare August 5, 2022 19:21
@tpapagian tpapagian force-pushed the pr/apapag/cache_missing_exec branch 8 times, most recently from 4dfab55 to 2557de2 Compare August 6, 2022 10:25
@tpapagian tpapagian force-pushed the pr/willfindlay/more-metrics branch from 7681eaa to e26abfd Compare August 6, 2022 11:21
pkg/reader/notify/notify.go Show resolved Hide resolved
@willfindlay
Copy link
Contributor Author

willfindlay commented Aug 8, 2022

@tpapagian just curious because I remember you mentioned you were going to look into it. Does this actually resolve the kprobes issue you were looking into or did we just get lucky with the CI?

@tpapagian
Copy link
Member

@tpapagian just curious because I remember you mentioned you were going to look into it. Does this actually resolve the kprobes issue you were looking into or did we just get lucky with the CI?

@willfindlay Sadly, no. I rerun this PR and the same errors appeared.

On the other hand, I think I have found the issue. I hope that 2557de2 and 3ac2672 (part of #283) should solve all kprobe flakes!

@tpapagian
Copy link
Member

I have also rebased this PR to the changes that I have done in #283.

@willfindlay
Copy link
Contributor Author

Nice. Anyway let's get the metrics in regardless. I guess we should wait for an ack from @jrfastab but I don't think I have anything controversial here so maybe we can just merge this into your branch.

Base automatically changed from pr/apapag/cache_missing_exec to main August 8, 2022 15:52
Copy link
Contributor

@michi-covalent michi-covalent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

Minor fixes to event cache handlers so that they deal with missing pod info separately
from missing process info. This enables us to set process information even if we are missing
pod information with k8sAPIEnabled == true.

Also update the event cache to increment pod and process info errors by event type when we
hit our cache strikes limit.

Signed-off-by: William Findlay <[email protected]>
@willfindlay willfindlay force-pushed the pr/willfindlay/more-metrics branch from e26abfd to 8fc76ff Compare August 8, 2022 17:13
@willfindlay
Copy link
Contributor Author

@jrfastab I'm gonna ship it

@willfindlay willfindlay merged commit fa6448d into main Aug 8, 2022
@willfindlay willfindlay deleted the pr/willfindlay/more-metrics branch August 8, 2022 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants