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

Add more filtering options to journald input #29294

Merged
merged 29 commits into from
Jan 4, 2022

Conversation

kvch
Copy link
Contributor

@kvch kvch commented Dec 6, 2021

What does this PR do?

This PR adds support for unit, transports and syslog_identifiers options for filtering.

This PR also introduces a breaking change to include_matches option. From now on it does not accept a list of expressions. Now both conjunction (AND) and disjunctions (OR) are supported when applying matches to journals.

Collecting entries of two different units:

- type: journald
  include_matches.or:
  - equals:
    - _SYSTEMD_UNIT=my_unit
    - _SYSTEMD_UNIT=my_other_unit

Collecting entries using syslog transport for a unit

- type: journald
  include_matches.and:
  - equals:
    - _SYSTEMD_UNIT=my_unit
    - _TRANSPORT=syslog

Although the configuration lets you write complex expressions, systemd does not provide full logical expression support.

Why is it important?

When this change merged, journald input can be marked either beta or GA. Furthermore, now it provides similar filtering capabilities as the good old community Journalbeat did.

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

Supersedes #10985

@kvch kvch mentioned this pull request Dec 6, 2021
1 task
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Dec 6, 2021
@kvch kvch added Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team v8.1.0 labels Dec 6, 2021
@elasticmachine
Copy link
Collaborator

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

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Dec 6, 2021
@mergify
Copy link
Contributor

mergify bot commented Dec 6, 2021

This pull request does not have a backport label. Could you fix it @kvch? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@mergify mergify bot added the backport-skip Skip notification from the automated backport with mergify label Dec 6, 2021
@kvch kvch added backport-v8.1.0 Automated backport with mergify needs_team Indicates that the issue/PR needs a Team:* label and removed backport-skip Skip notification from the automated backport with mergify labels Dec 6, 2021
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Dec 6, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Dec 6, 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 preview

Expand to view the summary

Build stats

  • Start Time: 2022-01-04T11:01:34.855+0000

  • Duration: 101 min 39 sec

  • Commit: 8dae41b

Test stats 🧪

Test Results
Failed 0
Passed 9642
Skipped 1275
Total 10917

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

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

@kvch kvch requested review from belimawr and andrewkroh December 8, 2021 13:03
@kvch kvch changed the title Add journal field matching to journald input Add more filtering options to journald input Dec 8, 2021
kvch and others added 7 commits December 13, 2021 13:15
Co-authored-by: Tiago Queiroz <contato@tiago.eti.br>
Co-authored-by: Tiago Queiroz <contato@tiago.eti.br>
Co-authored-by: Tiago Queiroz <contato@tiago.eti.br>
Co-authored-by: Andrew Kroh <andrew.kroh@elastic.co>
@mergify

This comment has been minimized.

@andrewkroh
Copy link
Member

WDYT about doing this in a way to allows the old configs to continue to work:

I'm wondering if there's a way to implement it without breaking earlier configs though. Like if a users had include_matches: [_TRANSPORT=audit] we just re-write it to include_matches.equals: [_TRANSPORT=audit] and log a deprecation warning.

Like if include_matches is a []string then automatically convert it to the proper format.

@mergify
Copy link
Contributor

mergify bot commented Dec 16, 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 feature-filebeat-journald-filtering upstream/feature-filebeat-journald-filtering
git merge upstream/master
git push upstream feature-filebeat-journald-filtering

@kvch kvch requested a review from belimawr December 17, 2021 09:49
CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved
filebeat/input/journald/pkg/journalfield/matcher.go Outdated Show resolved Hide resolved
Copy link
Contributor

@belimawr belimawr left a comment

Choose a reason for hiding this comment

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

Oops, I selected the wrong option

@mergify
Copy link
Contributor

mergify bot commented Dec 20, 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 feature-filebeat-journald-filtering upstream/feature-filebeat-journald-filtering
git merge upstream/master
git push upstream feature-filebeat-journald-filtering

Copy link
Contributor

@belimawr belimawr left a comment

Choose a reason for hiding this comment

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

It looks great, just need to solve the merge conflicts.

Comment on lines 55 to 57
coreDumpMsgID = MustBuildMatcher("message_id=fc2e22bc6ee647b6b90729ab34a250b1")
journaldUID = MustBuildMatcher("journald.uid=0")
journaldPID = MustBuildMatcher("journald.pid=1")
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the comment you added to ApplyUnitMatchers, however I think it could also be added here, so they don't look like magic strings to the reader.

Anyway, it's not a blocker ;)

@dikshachauhan-qasource
Copy link

Hi @kvch

We need details around Acceptance Criteria here, so that we can validate same.

As of now, I Have gone through Filebeat.yml file and could not find any updates or reference entries regarding journald been included under Filebeat from 8.0. Could you please help us with more details here.

Thanks
QAS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v8.1.0 Automated backport with mergify breaking change release-note:breaking The content should be included as a breaking change Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants