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] change multiline configuration in awss3 input to parsers #25873

Merged
merged 2 commits into from
Jun 29, 2021

Conversation

leehinman
Copy link
Contributor

@leehinman leehinman commented May 26, 2021

What does this PR do?

Changes configuration of multiline in awss3 input to parsers to match multiline configuration in filestream input.

Why is it important?

This is the new paradigm for configuring multiline and we want consistency between inputs.

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

  • Setup S3 & SQS in AWS
  • Set QUEUE_URL shell variable
  • Set AWS_PROFILE_NAME shell variable
  • Set S3_BUCKET_NAME shell variable
  • Set S3_BUCKET_REGION shell variable
  • Run go test --tags=integration,aws

Related issues

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels May 26, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented May 26, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #25873 updated

  • Start Time: 2021-06-29T17:58:27.277+0000

  • Duration: 111 min 31 sec

  • Commit: 8c88824

Test stats 🧪

Test Results
Failed 0
Passed 7402
Skipped 1201
Total 8603

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 7402
Skipped 1201
Total 8603

@leehinman leehinman force-pushed the 25249_awss3_add_parsers branch 3 times, most recently from 3381e44 to 8bf160f Compare May 28, 2021 16:17
@leehinman leehinman marked this pull request as ready for review May 28, 2021 18:26
@elasticmachine
Copy link
Collaborator

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@leehinman leehinman requested a review from kvch May 28, 2021 18:26
lineTerminator readfile.LineTerminator
}

func newParsers(in reader.Reader, pCfg parserConfig, c []common.ConfigNamespace) (parser, error) {
Copy link

Choose a reason for hiding this comment

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

@kvch this looks like we have to duplicate parser setup from the filestream input here. This is somewhat unfortunate, especially if we want to add similar functionality to other inputs. How about providing a common package for parser setup?

Inputs that want to make use of parsers should be able to do it more or less like this:


type Config struct {
  ...

  Parser inpparser.Settings
  ...
}

...

parser, err := config.Parser.Create(reader)
...

@mergify
Copy link
Contributor

mergify bot commented Jun 14, 2021

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b 25249_awss3_add_parsers upstream/25249_awss3_add_parsers
git merge upstream/master
git push upstream 25249_awss3_add_parsers

@andrewkroh
Copy link
Member

andrewkroh commented Jun 23, 2021

Just noting that if this doesn't merge for 7.14 that it will need to be updated to reflect that this is a breaking config change since the multiline option is already present (new for 7.14).

@leehinman
Copy link
Contributor Author

needs to be rebased onto #26541 when that is merged

- switches multiline configuration to parsers
- JSON parsing is independent

Closes elastic#25249
Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

LGTM

@leehinman leehinman merged commit beaa972 into elastic:master Jun 29, 2021
mergify bot pushed a commit that referenced this pull request Jun 29, 2021
…25873)

* change multiline configuration in awss3 input to parsers

- switches multiline configuration to parsers
- JSON parsing is independent

Closes #25249

(cherry picked from commit beaa972)
leehinman added a commit that referenced this pull request Jun 29, 2021
…25873) (#26586)

* change multiline configuration in awss3 input to parsers

- switches multiline configuration to parsers
- JSON parsing is independent

Closes #25249

(cherry picked from commit beaa972)

Co-authored-by: Lee Hinman <57081003+leehinman@users.noreply.github.com>
mdelapenya added a commit to mdelapenya/beats that referenced this pull request Jun 30, 2021
* master: (25 commits)
  fix: Force PLATFORMS environment variable when we build Elastic Agent dependencies on arm64 (elastic#26415)
  macos for metricbeat to run in the extended meta-stage (elastic#26573)
  Packaging: add arm7 platform in the main pipeline (elastic#26575)
  [Heartbeat] Skip flakey timer queue test (elastic#26592)
  Update to "read_pipeline" permission (elastic#26465) (elastic#26580)
  API keys do not reflect the need for read_pipeline (elastic#26466) (elastic#26582)
  Add Fleet agent.id to Agent monitoring data (elastic#26548)
  Add kinesis metricset (elastic#25989)
  Refactor of system/memory metricset (elastic#26334)
  Introduce httpcommon package in libbeat (add support for Proxy) (elastic#25219)
  [Filebeat] change multiline configuration in awss3 input to parsers (elastic#25873)
  docs: Hint for the error "Error extracting container id" (elastic#25824)
  [Docs] Fixed metricbeat redis exported field CPU descriptions (elastic#25846) (elastic#26496)
  Update urllib to 1.26.5. (elastic#26380)
  Update golang.org/x/crypto (elastic#26448)
  [Filebeat] Update Fortinet Ingest Pipeline (elastic#24816)
  Move parsers outside of filestream input so others can use them as well (elastic#26541)
  [Filebeat] Fix `threatintel.indicator.url.full` field not populating (elastic#26508)
  [Filebeat] Add network direction processor to Zeek and Suricata modules (elastic#24620)
  Logging code cleanup related to Nomad auto-discovery (elastic#26498)
  ...
@kvch kvch mentioned this pull request Jun 30, 2021
21 tasks
match: after
----

See the available parser settings in detail below.
Copy link
Member

Choose a reason for hiding this comment

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

@kvch would it be possible/make sense to have a shared asciidoc that can be included in all the inputs with parsers?

@leehinman leehinman deleted the 25249_awss3_add_parsers branch August 17, 2021 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.14.0 Automated backport with mergify cleanup Filebeat Filebeat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Filebeat] Support multiline options in aws s3 input
5 participants