Skip to content

[filebeat][fix] propagate Start() errors from beat receivers #49757

Open
barkhayot wants to merge 5 commits intoelastic:mainfrom
barkhayot:fix/receiver-start-error-propagation
Open

[filebeat][fix] propagate Start() errors from beat receivers #49757
barkhayot wants to merge 5 commits intoelastic:mainfrom
barkhayot:fix/receiver-start-error-propagation

Conversation

@barkhayot
Copy link
Copy Markdown

  • When storage is configured for the Filebeat OTel receiver but the corresponding extension is not present on the host, the startup failure was silently swallowed. Both filebeatReceiver and metricbeatReceiver called BeatReceiver.Start() inside a goroutine, so any error (including the missing extension error) was only logged and never propagated — causing Start() to always return nil and making a misconfigured deployment appear healthy at startup [bug-hunter] Filebeat receiver Start returns nil when configured storage extension is missing #49617
  • Split BeatReceiver.Start() into two methods:
    • Setup(host): runs all initialization synchronously (diagnostic hooks, storage extension lookup, metric reporter, stop callback) and returns errors immediately
    • Run(): calls beater.Run() and is meant to be executed in a goroutine
  • Both filebeatReceiver and metricbeatReceiver now call Setup() synchronously before spawning the goroutine for Run(), ensuring startup errors are surfaced to the OTel collector as hard failures.
  • Add suggested test from issue.

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Mar 28, 2026
@cla-checker-service
Copy link
Copy Markdown

cla-checker-service bot commented Mar 28, 2026

💚 CLA has been signed

@github-actions
Copy link
Copy Markdown
Contributor

🤖 GitHub comments

Just comment with:

  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Mar 28, 2026

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @barkhayot? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit
  • backport-active-all is the label that automatically backports to all active branches.
  • backport-active-8 is the label that automatically backports to all active minor branches for the 8 major.
  • backport-active-9 is the label that automatically backports to all active minor branches for the 9 major.

@barkhayot barkhayot marked this pull request as ready for review March 28, 2026 02:47
@barkhayot barkhayot requested a review from a team as a code owner March 28, 2026 02:47
@barkhayot barkhayot requested review from leehinman and mauri870 March 28, 2026 02:47
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 03cd23a8-011c-4ec5-8368-f68a7a972878

📥 Commits

Reviewing files that changed from the base of the PR and between 9fe11f5 and e5f8a6d.

📒 Files selected for processing (1)
  • x-pack/libbeat/cmd/instance/receiver.go

📝 Walkthrough

Walkthrough

The BeatReceiver lifecycle was split: host-dependent initialization moved into Setup(host) which returns initialization errors immediately, and the main execution moved into Run(). Filebeat and Metricbeat receivers now call Setup before spawning their goroutines and call Run inside the goroutine; they no longer start the goroutine when Setup fails. libbeat's BeatReceiver replaced Start with Setup and Run and added a public groupReporter field. A test was added that verifies Start fails when a required storage extension is missing.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • 🛠️ Update Documentation: Commit on current branch
  • 🛠️ Update Documentation: Create PR

Comment @coderabbitai help to get the list of available commands and usage tips.

@barkhayot barkhayot force-pushed the fix/receiver-start-error-propagation branch from 6e1d893 to 9fe11f5 Compare March 28, 2026 03:12
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@x-pack/libbeat/cmd/instance/receiver.go`:
- Around line 121-125: The code unconditionally dereferences br.groupReporter in
the Run() error path; update the Run error handling to guard against nil by
checking br.groupReporter != nil before calling br.groupReporter.UpdateStatus
(or any method on it) so beat receivers that don't implement
cfgfile.WithOtelFactoryWrapper won't panic; locate the Run error handling in the
same receiver (method Run on BeatReceiver) and wrap the UpdateStatus call with a
nil-check (or alternative safe path such as logging the failure) to avoid
panics.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 96dd7d48-5cac-43fd-a977-457e0ef34e9e

📥 Commits

Reviewing files that changed from the base of the PR and between 6e1d893 and 9fe11f5.

📒 Files selected for processing (4)
  • x-pack/filebeat/fbreceiver/receiver.go
  • x-pack/filebeat/fbreceiver/receiver_test.go
  • x-pack/libbeat/cmd/instance/receiver.go
  • x-pack/metricbeat/mbreceiver/receiver.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • x-pack/filebeat/fbreceiver/receiver.go

@pierrehilbert pierrehilbert added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Mar 28, 2026
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Mar 28, 2026
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@cmacknz cmacknz requested a review from VihasMakwana March 30, 2026 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants