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.

Sign up for GitHub

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

Jump to bottom

[Filebeat] Add Initial Interval for Microsoft Filesets #42309

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[Filebeat] Add Initial Interval for Microsoft Filesets #42309

wants to merge 6 commits into from
+16 −3

Conversation

DumbBoi
Copy link

@DumbBoi DumbBoi commented Jan 14, 2025

Proposed commit message

The initial intervals for microsoft filesets are currently hard coded. Adding initial interval variable.

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.

How to test this PR locally

Will need to test this with varying initial intervals.

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jan 14, 2025
@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 14, 2025

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @DumbBoi? 🙏.
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

@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 14, 2025

backport-8.x has been added to help with the transition to the new branch 8.x.
If you don't need it please use backport-skip label and remove the backport-8.x label.

@mergify mergify bot added the backport-8.x Automated backport to the 8.x branch with mergify label Jan 14, 2025
@DumbBoi DumbBoi marked this pull request as ready for review January 14, 2025 22:18
@DumbBoi DumbBoi requested a review from a team as a code owner January 14, 2025 22:18
@DumbBoi DumbBoi marked this pull request as draft January 15, 2025 08:50
@DumbBoi DumbBoi marked this pull request as ready for review January 15, 2025 08:51
@DumbBoi
Copy link
Author

DumbBoi commented Jan 29, 2025 •
edited
Loading

@elastic-data-integration @elastic-endpoint-team can anyone please review this PR?

@jamiehynds jamiehynds added the Team:Security-Service Integrations Security Service Integrations Team label Feb 6, 2025
@elasticmachine
Copy link
Collaborator

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Feb 6, 2025
@strawgate
Copy link
Contributor

The docs https://github.com/elastic/beats/blob/main/x-pack/filebeat/module/microsoft/_meta/docs.asciidoc will need to be updated to mention the new parameter

@strawgate
Copy link
Contributor

I'm not a required reviewer but this looks to match the integration configuration for m365_defender https://github.com/elastic/integrations/blob/main/packages/m365_defender/data_stream/alert/agent/stream/httpjson.yml.hbs

It doesn't look like we offer initial_interval for the endpoint atp integration https://github.com/elastic/integrations/blob/ea91c5762cb71a0c8ab73979509e219a78223b42/packages/microsoft_defender_endpoint/data_stream/log/agent/stream/httpjson.yml.hbs#L34 so i've filed this issue in that repo elastic/integrations#12912

Seems like a low risk change

@andrewkroh andrewkroh requested a review from a team March 3, 2025 14:23
@efd6
Copy link
Contributor

efd6 commented Mar 4, 2025

/test

efd6
efd6 reviewed Mar 4, 2025
View reviewed changes
Copy link
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

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

Please add a changelog entry to added section for filebeat.

@DumbBoi DumbBoi requested a review from a team as a code owner March 4, 2025 07:05
@DumbBoi
Copy link
Author

DumbBoi commented Mar 4, 2025

@efd6 added. also added initial interval entry to docs and a small logging fix in sqs.

efd6
efd6 reviewed Mar 4, 2025
View reviewed changes
x-pack/filebeat/input/awss3/sqs_s3_event.go
@@ -255,7 +255,7 @@ func (r sqsProcessingResult) Done() {
return
}
p.metrics.sqsMessagesDeletedTotal.Inc()
p.log.Errorf("failed processing SQS message (message was deleted): %w", processingErr)
p.log.Errorf("failed processing SQS message (message was deleted): %v", processingErr.Error())
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
p.log.Errorf("failed processing SQS message (message was deleted): %v", processingErr.Error())
p.log.Errorf("failed processing SQS message (message was deleted): %v", processingErr)

I'm not sure about having this in this PR since it's an unrelated change. See what other reviewers say.

Copy link
Author

Choose a reason for hiding this comment

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

I can remove this if its an issue. also processingErr.Error() is used in line 253 for the same object.

CHANGELOG.next.asciidoc
@@ -431,6 +431,7 @@ otherwise no tag is added. {issue}42208[42208] {pull}42403[42403]
- Add metrics for number of events and pages published by HTTPJSON input. {issue}42340[42340] {pull}42442[42442]
- Add `etw` input fallback to attach an already existing session. {pull}42847[42847]
- Update CEL mito extensions to v1.17.0. {pull}42851[42851]
- Add Initial Interval for Microsoft Filesets (ATP, Defender) {pull}42309[42309]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Add Initial Interval for Microsoft Filesets (ATP, Defender) {pull}42309[42309]
- Add Initial Interval for Microsoft Filesets (ATP, Defender). {pull}42309[42309]

@efd6
Copy link
Contributor

efd6 commented Mar 4, 2025

/test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@efd6 efd6 efd6 left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@DumbBoi DumbBoi

Labels
backport-8.x Automated backport to the 8.x branch with mergify enhancement Filebeat Filebeat Team:Security-Service Integrations Security Service Integrations Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@DumbBoi @elasticmachine @strawgate @efd6 @andrewkroh @jamiehynds