Skip to content

Conversation

pando85
Copy link
Contributor

@pando85 pando85 commented Dec 1, 2024

Motivation

The current implementation of the event recorder just creates new events which is not enough for my event handling expectations.

Solution

I added a recorder implementation more similar to the one in client-go library. This implementation caches events in local and groups isomorphic events to increment series count on similar events.

@pando85 pando85 force-pushed the feat/add-series-implementation-for-events-recorder branch 2 times, most recently from adeabe4 to e09e462 Compare December 3, 2024 23:58
Copy link
Member

@clux clux left a comment

Choose a reason for hiding this comment

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

hey, thanks for this! looks largely sensible to me, gave a quick go over with some quick questions and minor nits, lmk if my thinking makes sense.

@clux clux added the changelog-change changelog change category for prs label Dec 4, 2024
@clux clux added this to the 0.98.0 milestone Dec 4, 2024
@pando85 pando85 force-pushed the feat/add-series-implementation-for-events-recorder branch 2 times, most recently from 79074f2 to e8e4b54 Compare December 5, 2024 16:32
Copy link

codecov bot commented Dec 6, 2024

Codecov Report

Attention: Patch coverage is 98.47328% with 2 lines in your changes missing coverage. Please review.

Project coverage is 75.8%. Comparing base (419442b) to head (cb50617).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
kube-runtime/src/events.rs 98.5% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1655     +/-   ##
=======================================
+ Coverage   75.6%   75.8%   +0.3%     
=======================================
  Files         82      82             
  Lines       7430    7513     +83     
=======================================
+ Hits        5611    5693     +82     
- Misses      1819    1820      +1     
Files with missing lines Coverage Δ
kube-runtime/src/events.rs 98.1% <98.5%> (+0.9%) ⬆️

@pando85 pando85 force-pushed the feat/add-series-implementation-for-events-recorder branch 4 times, most recently from 96240f4 to dd650b2 Compare December 6, 2024 12:06
@pando85
Copy link
Contributor Author

pando85 commented Dec 6, 2024

Now all tests were passed but I forgot about DCO again...

You can review it when you have time and if more improvements are needed I will work on them. If not I have to squash all those commits and we can merge.

clux added a commit to kube-rs/controller-rs that referenced this pull request Dec 9, 2024
for kube-rs/kube#1655

Signed-off-by: clux <sszynrae@gmail.com>
Copy link
Member

@clux clux left a comment

Choose a reason for hiding this comment

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

thanks for the cleanups and sorry for the delay, have had a busy week. a few more small comments.

@pando85 pando85 force-pushed the feat/add-series-implementation-for-events-recorder branch from 269eed5 to 12ab345 Compare December 11, 2024 17:37
@clux clux linked an issue Dec 12, 2024 that may be closed by this pull request
Signed-off-by: Alexander Gil <pando855@gmail.com>
@pando85 pando85 force-pushed the feat/add-series-implementation-for-events-recorder branch from 12ab345 to cb50617 Compare December 12, 2024 08:53
@pando85
Copy link
Contributor Author

pando85 commented Dec 12, 2024

I fixed the issues with the tests and squashed all commits.

@clux clux merged commit 36882b6 into kube-rs:main Dec 12, 2024
17 checks passed
@pando85 pando85 deleted the feat/add-series-implementation-for-events-recorder branch December 12, 2024 09:49
Danil-Grigorev pushed a commit to Danil-Grigorev/kube that referenced this pull request Dec 18, 2024
clux added a commit to kube-rs/controller-rs that referenced this pull request Dec 23, 2024
* test out new event recorder interface

for kube-rs/kube#1655

Signed-off-by: clux <sszynrae@gmail.com>

* cache recorder in Context to use the cache correctly

Signed-off-by: clux <sszynrae@gmail.com>

* event by ref

Signed-off-by: clux <sszynrae@gmail.com>

* use official now that it's released

Signed-off-by: clux <sszynrae@gmail.com>

* merge conflicts

Signed-off-by: clux <sszynrae@gmail.com>

* fix test compile

Signed-off-by: clux <sszynrae@gmail.com>

* gen

Signed-off-by: clux <sszynrae@gmail.com>

---------

Signed-off-by: clux <sszynrae@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-change changelog change category for prs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A way to aggregate repeated events with the recorder.
2 participants