Skip to content
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.

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

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

Merged
merged 6 commits into from
Jul 10, 2024

Conversation

VihasMakwana
Copy link
Contributor

@VihasMakwana VihasMakwana commented Jul 5, 2024

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
Copy link
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
@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
Collaborator

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
CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
@VihasMakwana VihasMakwana merged commit 0e9c9de into elastic:main Jul 10, 2024
19 checks passed
mergify bot pushed a commit that referenced this pull request Jul 10, 2024
…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)
oakrizan pushed a commit to oakrizan/beats that referenced this pull request Jul 11, 2024
…lastic#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
@@ -163,7 +164,11 @@ func (inp *filestream) Run(
})
defer streamCancel()

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
Member

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
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 I understand your concern.
I did some digging and turns out that readFromSource only throws an error when it calls Publish()

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

The above error is returned when the context is cancelled.
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
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
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