Skip to content

Commit 42a171e

Browse files
CodeBlackwellclaude
andcommitted
[extension/opampextension] fix self-reported status events being dropped by non-Reporter hosts
opampAgent.Start wired its internal reportFunc to call componentstatus.ReportStatus(host, event) directly. That helper silently no-ops when the component.Host passed to Start does not implement componentstatus.Reporter, so self-reported status changes -- most notably the fatal error monitorPPID emits when the collector's parent process disappears -- never reached the extension's own status aggregator. The collector could keep reporting healthy despite being orphaned. Track the extension's own component.ID (added extensionID field, set from extension.Settings.ID in newOpampAgent) and route self-reported events through the extension's own ComponentStatusChanged handler instead, which always forwards to the status aggregator regardless of what the host implements. Adds TestReportFuncReachesStatusAggregatorRegardlessOfHostReporter, which starts the agent against componenttest.NewNopHost() (which intentionally does not implement componentstatus.Reporter) and asserts a fatal error event reported via o.reportFunc still reaches the mock status aggregator. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5f5ec54 commit 42a171e

3 files changed

Lines changed: 71 additions & 1 deletion

File tree

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
2+
change_type: bug_fix
3+
4+
# The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog)
5+
component: extension/opamp
6+
7+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
8+
note: "Fix opampextension self-reported status events (e.g. the fatal error emitted when the parent process disappears) being silently dropped when the component.Host does not implement componentstatus.Reporter"
9+
10+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
11+
issues: [49412]
12+
13+
# (Optional) One or more lines of additional information to render under the primary note.
14+
subtext: |
15+
Previously, opampAgent.Start wired its internal reportFunc to call
16+
componentstatus.ReportStatus(host, event) directly. That helper is a no-op
17+
when the given host does not implement componentstatus.Reporter, so
18+
self-reported status changes -- most notably the fatal error monitorPPID
19+
emits when the collector's parent process disappears -- were silently
20+
discarded and never reached the extension's status aggregator, leaving the
21+
collector reporting healthy despite being orphaned.
22+
23+
The extension now tracks its own component.ID and routes self-reported
24+
events through its own ComponentStatusChanged handler
25+
(o.ComponentStatusChanged(selfInstanceID, event)), which always forwards to
26+
the status aggregator regardless of whether the host implements
27+
componentstatus.Reporter.
28+
29+
change_logs: [user]

extension/opampextension/opamp_agent.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ type opampAgent struct {
6262
resourceAttrs map[string]string
6363

6464
instanceUID uuid.UUID
65+
extensionID component.ID
6566

6667
eclk sync.RWMutex
6768
effectiveConfig *confmap.Conf
@@ -106,8 +107,9 @@ var (
106107
)
107108

108109
func (o *opampAgent) Start(ctx context.Context, host component.Host) error {
110+
selfInstanceID := componentstatus.NewInstanceID(o.extensionID, component.KindExtension)
109111
o.reportFunc = func(event *componentstatus.Event) {
110-
componentstatus.ReportStatus(host, event)
112+
o.ComponentStatusChanged(selfInstanceID, event)
111113
}
112114

113115
header := http.Header{}
@@ -332,6 +334,7 @@ func newOpampAgent(cfg *Config, set extension.Settings) (*opampAgent, error) {
332334
serviceVersion: serviceVersion,
333335
serviceInstanceID: serviceInstanceID,
334336
instanceUID: uid,
337+
extensionID: set.ID,
335338
capabilities: cfg.Capabilities,
336339
opampClient: opampClient,
337340
resourceAttrs: resourceAttrs,

extension/opampextension/opamp_agent_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,44 @@ func TestStartAvailableComponents(t *testing.T) {
267267
assert.NoError(t, o.Shutdown(t.Context()))
268268
}
269269

270+
// TestReportFuncReachesStatusAggregatorRegardlessOfHostReporter verifies that a
271+
// status reported through o.reportFunc (e.g. the fatal error monitorPPID emits when
272+
// the parent process disappears) always reaches the status aggregator, even when the
273+
// component.Host passed to Start does not implement componentstatus.Reporter. Without
274+
// this, PPID-orphan detection silently fails to mark the collector unhealthy.
275+
func TestReportFuncReachesStatusAggregatorRegardlessOfHostReporter(t *testing.T) {
276+
cfg := createDefaultConfig().(*Config)
277+
set := extensiontest.NewNopSettings(extensiontest.NopType)
278+
279+
sa := &mockStatusAggregator{mtx: &sync.RWMutex{}}
280+
281+
o := newTestOpampAgent(cfg, set, &mockOpAMPClient{
282+
setHealthFunc: func(_ *protobufs.ComponentHealth) error { return nil },
283+
}, sa)
284+
285+
o.initHealthReporting()
286+
287+
// componenttest.NewNopHost() intentionally does not implement componentstatus.Reporter.
288+
require.NoError(t, o.Start(t.Context(), componenttest.NewNopHost()))
289+
require.NoError(t, o.Ready())
290+
291+
fatalErr := errors.New("collector was orphaned")
292+
o.reportFunc(componentstatus.NewFatalErrorEvent(fatalErr))
293+
294+
require.Eventually(t, func() bool {
295+
sa.mtx.RLock()
296+
defer sa.mtx.RUnlock()
297+
return len(sa.receivedEvents) == 1
298+
}, 5*time.Second, 100*time.Millisecond)
299+
300+
sa.mtx.RLock()
301+
defer sa.mtx.RUnlock()
302+
require.Equal(t, componentstatus.StatusFatalError, sa.receivedEvents[0].event.Status())
303+
require.Equal(t, fatalErr, sa.receivedEvents[0].event.Err())
304+
305+
require.NoError(t, o.Shutdown(t.Context()))
306+
}
307+
270308
// availableComponentsHost mocks a receiver.ReceiverHost for test purposes.
271309
type availableComponentsHost struct {
272310
t *testing.T

0 commit comments

Comments
 (0)