Skip to content

fix(k8sobjectsreceiver): restart watch on unexpected channel close #43974

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+10 −1
Open

fix(k8sobjectsreceiver): restart watch on unexpected channel close #43974

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fc6917e
fix(k8sobjectsreceiver): restart watch on unexpected channel close
Utkarsh9571 Nov 3, 2025
d806a39
Merge branch 'main' into fix-watch-restart
Utkarsh9571 Nov 3, 2025
0314b23
Merge branch 'main' into fix-watch-restart
Utkarsh9571 Nov 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion receiver/k8sobjectsreceiver/receiver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,11 @@
return
}

if !done {
time.Sleep(2 * time.Second)

Check failure on line 308 in receiver/k8sobjectsreceiver/receiver.go

View workflow job for this annotation

GitHub Actions / lint-matrix (windows, receiver-1) 

File is not properly formatted (gci)

Check failure on line 308 in receiver/k8sobjectsreceiver/receiver.go

View workflow job for this annotation

GitHub Actions / lint-matrix (linux, receiver-1) 

File is not properly formatted (gci)

Check failure on line 308 in receiver/k8sobjectsreceiver/receiver.go

View workflow job for this annotation

GitHub Actions / scoped-tests-matrix (ubuntu-latest) 

File is not properly formatted (gci)
}

Comment on lines +307 to +310
Copy link
Contributor

@VihasMakwana VihasMakwana Nov 6, 2025

Choose a reason for hiding this comment

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

@Utkarsh9571 Why are we sleeping here?

Copy link
Contributor

@VihasMakwana VihasMakwana Nov 6, 2025

Choose a reason for hiding this comment

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

nevermind, got it.

Copy link
Member

@ChrsMark ChrsMark Nov 6, 2025

Choose a reason for hiding this comment

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

May ask that too :)? Is this supposed to give time to the watcher to start? I don't see why is needed.

Copy link
Contributor

@VihasMakwana VihasMakwana Nov 6, 2025

Choose a reason for hiding this comment

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

@ChrsMark looking at PR description, it looks like this is adding some sort of backoff before retrying immediately.

@Utkarsh9571's comment:

The sleep is there to avoid tight restart loops when the channel closes silently. It gives the receiver a moment to reset before retrying.

Copy link
Contributor

@VihasMakwana VihasMakwana Nov 6, 2025

Choose a reason for hiding this comment

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

i think we will be okay without it, but I'll let @Utkarsh9571 reply to your question before proceeding.

Copy link
Member

@ChrsMark ChrsMark Nov 6, 2025

Choose a reason for hiding this comment

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

I see, thank's for elaborating! My question then is if that's actually needed. It looks a bit arbitrary tbh and I'm not sure if it solves an existing problem.

Copy link
Author

@Utkarsh9571 Utkarsh9571 Nov 6, 2025

Choose a reason for hiding this comment

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

@ChrsMark @VihasMakwana I'm happy to clarify.

The time.Sleep(2 * time.Second) is a minimal backoff to prevent tight restart loops when the channel closes silently. Without it, the receiver can enter a rapid retry cycle that floods logs and consumes CPU unnecessarily, especially in edge cases where the watcher fails to initialize cleanly.

This sleep gives the receiver a moment to reset before retrying. It’s not about waiting for the watcher to start — it’s about avoiding aggressive retries when done is false due to a silent failure.

I added it, the time.Sleep(2 * time.Second) was my interpretation of how to avoid tight restart loops after a silent channel close. It wasn’t directly requested by @atoulme, but I added it based on observed behavior during testing, where the receiver would retry rapidly without delay.

Let me know your preference and I’ll update the patch accordingly.


// need to restart with a fresh resource version
cfgCopy.ResourceVersion = ""
}, 0)
Expand Down Expand Up @@ -394,7 +399,11 @@
if !ok {
kr.setting.Logger.Warn("Watch channel closed unexpectedly",
zap.String("resource", config.gvr.String()))
return true

kr.setting.Logger.Info("Restarting watch after unexpected channel close",
zap.String("resource", config.gvr.String()))

return false
}

if config.exclude[data.Type] {
Expand Down
Loading