Skip to content

[filebeat][filestream] Enable status reporter for filestream input#40121

Merged
VihasMakwana merged 6 commits intoelastic:mainfrom
VihasMakwana:status-reporter-filestream
Jul 10, 2024
Merged

[filebeat][filestream] Enable status reporter for filestream input#40121
VihasMakwana merged 6 commits intoelastic:mainfrom
VihasMakwana:status-reporter-filestream

Conversation

@VihasMakwana
Copy link
Copy Markdown
Contributor

@VihasMakwana VihasMakwana commented Jul 5, 2024 •
edited
Loading

Proposed commit message

Enable StatusReporter for filestream input which was introduced in #39209

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Related issues

Use cases

This PR allows the filestream to report its status to elastic-agent. This would keep the status of the unit up-to-date and would help with diagnostics.

Screenshots

Logs

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jul 5, 2024
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jul 5, 2024

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @VihasMakwana? 🙏.
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-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

@VihasMakwana VihasMakwana added Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team and removed needs_team Indicates that the issue/PR needs a Team:* label labels Jul 5, 2024
@pierrehilbert pierrehilbert linked an issue Jul 8, 2024 that may be closed by this pull request
[Elastic Agent] The filestream input should report itself as Degraded when it encounters a permissions error #39734
Closed
@VihasMakwana VihasMakwana changed the title [DRAFT] filestream status reporter [filebeat][filestream] Enable status reporter for filestream input Jul 9, 2024
@VihasMakwana VihasMakwana marked this pull request as ready for review July 9, 2024 06:49
@VihasMakwana VihasMakwana requested a review from a team as a code owner July 9, 2024 06:49
@elasticmachine
Copy link
Copy Markdown
Contributor

elasticmachine commented Jul 9, 2024

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

@pierrehilbert pierrehilbert requested review from belimawr and rdner and removed request for AndersonQ and leehinman July 9, 2024 07:29
@pierrehilbert pierrehilbert added the backport-8.15 Automated backport to the 8.15 branch with mergify label Jul 9, 2024
@VihasMakwana VihasMakwana mentioned this pull request Jul 9, 2024
Closed
belimawr
belimawr reviewed Jul 9, 2024
View reviewed changes
belimawr
belimawr requested changes Jul 9, 2024
View reviewed changes
@VihasMakwana VihasMakwana requested a review from belimawr July 10, 2024 10:02
@VihasMakwana VihasMakwana merged commit 0e9c9de into elastic:main Jul 10, 2024
mergify bot pushed a commit that referenced this pull request Jul 10, 2024
@VihasMakwana @mergify
[filebeat][filestream] Enable status reporter for filestream input (#…
0e73241 
…40121)

* initial commit filestream status

* fix: test cleanup

* fix: move the statusReporter to correct place

* fix: remove test cases for now

* chore: add changelog

* fix: address review comments

(cherry picked from commit 0e9c9de)
@mergify mergify bot mentioned this pull request Jul 10, 2024
Closed
6 tasks
cmacknz
cmacknz reviewed Jul 15, 2024
View reviewed changes
filebeat/input/filestream/input.go

return inp.readFromSource(ctx, log, r, fs.newPath, state, publisher, metrics)
if err := inp.readFromSource(ctx, log, r, fs.newPath, state, publisher, metrics); err != nil {
ctx.UpdateStatus(status.Degraded, fmt.Sprintf("error while reading from source: %v", err))
Copy link
Copy Markdown
Member

@cmacknz cmacknz Jul 15, 2024

Choose a reason for hiding this comment

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

Sorry for the late comment, if this exits with a transient error, what is setting the input state back to healthy?

In general the thing I am watching out for in the status reporting PRs situations where we permanently mark inputs as degraded or failed after the error condition has cleared.

Copy link
Copy Markdown
Contributor Author

@VihasMakwana VihasMakwana Jul 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@cmacknz I understand your concern.
I did some digging and turns out that readFromSource only throws an error when it calls Publish()

beats/filebeat/input/filestream/input.go

Lines 394 to 397 in d87d15b

if err := p.Publish(message.ToEvent(), s); err != nil {
metrics.ProcessingErrors.Inc()
return err
}

The above error is returned when the context is cancelled.

beats/filebeat/input/filestream/internal/input-logfile/publish.go

Lines 91 to 97 in d87d15b

func (c *cursorPublisher) forward(event beat.Event) error {
c.client.Publish(event)
if c.canceler == nil {
return nil
}
return c.canceler.Err()
}

For other cases (EOF, reader was closed etc.), we just log the error and return nil. So, we will not mark the input as degraded, as they are transient in nature.

Copy link
Copy Markdown
Contributor Author

@VihasMakwana VihasMakwana Jul 15, 2024

Choose a reason for hiding this comment

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

@cmacknz maybe we don't need this particular ctx.UpdateStatus? I don't think we need to mark input as degraded when the context is cancelled (which should ideally happen during beat shutdown).

I overlooked this particular ctx.UpdateStatus.

@pierrehilbert pierrehilbert mentioned this pull request Jul 16, 2024
Merged
6 tasks
@cmacknz cmacknz mentioned this pull request Jul 23, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cmacknz cmacknz cmacknz left review comments

@belimawr belimawr belimawr approved these changes

@rdner rdner Awaiting requested review from rdner

Assignees

@VihasMakwana VihasMakwana

Labels

backport-8.15 Automated backport to the 8.15 branch with mergify Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Elastic Agent] The filestream input should report itself as Degraded when it encounters a permissions error

5 participants

@VihasMakwana @elasticmachine @belimawr @cmacknz @pierrehilbert