From e733761c8f3268326ab116759066b32a3512a439 Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Tue, 10 Dec 2024 10:40:57 +0100 Subject: [PATCH 01/11] opamp agent shutdown: do not wait for componentStatusLoop to end Signed-off-by: Florian Bacher --- extension/opampextension/opamp_agent.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/extension/opampextension/opamp_agent.go b/extension/opampextension/opamp_agent.go index c638e8727b05..faff6cdfaf0d 100644 --- a/extension/opampextension/opamp_agent.go +++ b/extension/opampextension/opamp_agent.go @@ -74,7 +74,6 @@ type opampAgent struct { statusAggregator statusAggregator statusSubscriptionWg *sync.WaitGroup - componentHealthWg *sync.WaitGroup startTimeUnixNano uint64 componentStatusCh chan *eventSourcePair readyCh chan struct{} @@ -161,7 +160,6 @@ func (o *opampAgent) Shutdown(ctx context.Context) error { } o.statusSubscriptionWg.Wait() - o.componentHealthWg.Wait() o.logger.Debug("OpAMP agent shutting down...") if o.opampClient == nil { @@ -289,7 +287,6 @@ func newOpampAgent(cfg *Config, set extension.Settings) (*opampAgent, error) { capabilities: cfg.Capabilities, opampClient: opampClient, statusSubscriptionWg: &sync.WaitGroup{}, - componentHealthWg: &sync.WaitGroup{}, readyCh: make(chan struct{}), customCapabilityRegistry: newCustomCapabilityRegistry(set.Logger, opampClient), } @@ -465,7 +462,6 @@ func (o *opampAgent) initHealthReporting() { // Start processing events in the background so that our status watcher doesn't // block others before the extension starts. o.componentStatusCh = make(chan *eventSourcePair) - o.componentHealthWg.Add(1) go o.componentHealthEventLoop() } @@ -476,7 +472,6 @@ func (o *opampAgent) componentHealthEventLoop() { // individually by the service. var eventQueue []*eventSourcePair - defer o.componentHealthWg.Done() for loop := true; loop; { select { case esp, ok := <-o.componentStatusCh: From defddc6a56c45997bd402d2b182715a5b6911958 Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Tue, 10 Dec 2024 10:52:20 +0100 Subject: [PATCH 02/11] fix linting Signed-off-by: Florian Bacher --- extension/opampextension/opamp_agent_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/extension/opampextension/opamp_agent_test.go b/extension/opampextension/opamp_agent_test.go index 7921bd767470..efaa284d8328 100644 --- a/extension/opampextension/opamp_agent_test.go +++ b/extension/opampextension/opamp_agent_test.go @@ -687,7 +687,6 @@ func newTestOpampAgent(cfg *Config, set extension.Settings, mockOpampClient *moc capabilities: cfg.Capabilities, opampClient: mockOpampClient, statusSubscriptionWg: &sync.WaitGroup{}, - componentHealthWg: &sync.WaitGroup{}, readyCh: make(chan struct{}), customCapabilityRegistry: newCustomCapabilityRegistry(set.Logger, mockOpampClient), statusAggregator: sa, From 7d19bb88aaf1728974ef0bb26c26240f3eef6cbb Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Tue, 10 Dec 2024 11:03:28 +0100 Subject: [PATCH 03/11] trigger CI Signed-off-by: Florian Bacher From 15bdf2754d0ed94147129a6f8f114e5558187187 Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Tue, 10 Dec 2024 11:14:46 +0100 Subject: [PATCH 04/11] trigger CI Signed-off-by: Florian Bacher From 4aca1cbf65688c692296a6c41b62bd67d61ca1fe Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Tue, 10 Dec 2024 11:30:13 +0100 Subject: [PATCH 05/11] trigger CI Signed-off-by: Florian Bacher From 030771be0ec39da75152d45efecd0ed0fe14e9db Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Tue, 10 Dec 2024 12:37:48 +0100 Subject: [PATCH 06/11] trigger CI Signed-off-by: Florian Bacher From ee3a475e750a7a4a12c95d0eec68f3864009fd24 Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Tue, 10 Dec 2024 13:09:36 +0100 Subject: [PATCH 07/11] trigger CI Signed-off-by: Florian Bacher From 49dc478e43123eb03a0b1b464bfabc89440a54f7 Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Tue, 10 Dec 2024 13:19:04 +0100 Subject: [PATCH 08/11] trigger CI Signed-off-by: Florian Bacher From c511562506b3b38bdc0cb50214a05fc801e3dceb Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Wed, 11 Dec 2024 07:54:16 +0100 Subject: [PATCH 09/11] Revert "opamp agent shutdown: do not wait for componentStatusLoop to end" This reverts commit e733761c8f3268326ab116759066b32a3512a439. --- extension/opampextension/opamp_agent.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/extension/opampextension/opamp_agent.go b/extension/opampextension/opamp_agent.go index faff6cdfaf0d..c638e8727b05 100644 --- a/extension/opampextension/opamp_agent.go +++ b/extension/opampextension/opamp_agent.go @@ -74,6 +74,7 @@ type opampAgent struct { statusAggregator statusAggregator statusSubscriptionWg *sync.WaitGroup + componentHealthWg *sync.WaitGroup startTimeUnixNano uint64 componentStatusCh chan *eventSourcePair readyCh chan struct{} @@ -160,6 +161,7 @@ func (o *opampAgent) Shutdown(ctx context.Context) error { } o.statusSubscriptionWg.Wait() + o.componentHealthWg.Wait() o.logger.Debug("OpAMP agent shutting down...") if o.opampClient == nil { @@ -287,6 +289,7 @@ func newOpampAgent(cfg *Config, set extension.Settings) (*opampAgent, error) { capabilities: cfg.Capabilities, opampClient: opampClient, statusSubscriptionWg: &sync.WaitGroup{}, + componentHealthWg: &sync.WaitGroup{}, readyCh: make(chan struct{}), customCapabilityRegistry: newCustomCapabilityRegistry(set.Logger, opampClient), } @@ -462,6 +465,7 @@ func (o *opampAgent) initHealthReporting() { // Start processing events in the background so that our status watcher doesn't // block others before the extension starts. o.componentStatusCh = make(chan *eventSourcePair) + o.componentHealthWg.Add(1) go o.componentHealthEventLoop() } @@ -472,6 +476,7 @@ func (o *opampAgent) componentHealthEventLoop() { // individually by the service. var eventQueue []*eventSourcePair + defer o.componentHealthWg.Done() for loop := true; loop; { select { case esp, ok := <-o.componentStatusCh: From da5f1592e732031145cad8658e7f51fb25b3e8cc Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Wed, 11 Dec 2024 07:59:16 +0100 Subject: [PATCH 10/11] close the channel for component status updates to avoid blocking Signed-off-by: Florian Bacher --- extension/opampextension/opamp_agent.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/extension/opampextension/opamp_agent.go b/extension/opampextension/opamp_agent.go index c638e8727b05..3e26d407fd80 100644 --- a/extension/opampextension/opamp_agent.go +++ b/extension/opampextension/opamp_agent.go @@ -162,6 +162,9 @@ func (o *opampAgent) Shutdown(ctx context.Context) error { o.statusSubscriptionWg.Wait() o.componentHealthWg.Wait() + if o.componentStatusCh != nil { + close(o.componentStatusCh) + } o.logger.Debug("OpAMP agent shutting down...") if o.opampClient == nil { From bf4823567df0b599a8143eb58f6f712cc7bde806 Mon Sep 17 00:00:00 2001 From: Florian Bacher Date: Wed, 11 Dec 2024 08:09:55 +0100 Subject: [PATCH 11/11] add changelog entry, fix unit test Signed-off-by: Florian Bacher --- .chloggen/opamp-extension-shutdown.yaml | 27 ++++++++++++++++++++ extension/opampextension/opamp_agent_test.go | 1 + 2 files changed, 28 insertions(+) create mode 100644 .chloggen/opamp-extension-shutdown.yaml diff --git a/.chloggen/opamp-extension-shutdown.yaml b/.chloggen/opamp-extension-shutdown.yaml new file mode 100644 index 000000000000..370386f84889 --- /dev/null +++ b/.chloggen/opamp-extension-shutdown.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: opampextension + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Fix blocking agent shutdown due to unclosed channel + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [36764] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] diff --git a/extension/opampextension/opamp_agent_test.go b/extension/opampextension/opamp_agent_test.go index efaa284d8328..7921bd767470 100644 --- a/extension/opampextension/opamp_agent_test.go +++ b/extension/opampextension/opamp_agent_test.go @@ -687,6 +687,7 @@ func newTestOpampAgent(cfg *Config, set extension.Settings, mockOpampClient *moc capabilities: cfg.Capabilities, opampClient: mockOpampClient, statusSubscriptionWg: &sync.WaitGroup{}, + componentHealthWg: &sync.WaitGroup{}, readyCh: make(chan struct{}), customCapabilityRegistry: newCustomCapabilityRegistry(set.Logger, mockOpampClient), statusAggregator: sa,