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

Use journald for system module on Debian 12 #41061

Merged
merged 38 commits into from
Oct 14, 2024

Conversation

belimawr
Copy link
Contributor

@belimawr belimawr commented Oct 1, 2024

Proposed commit message

This commit adds Debian 12 support to our system module, to support
Debian 12 we need to use the journald input to collect the system
logs.

To support it, a new, internal, input system-logsis introduced, it is responsible
for deciding whether the log input or journald must be used. If var.paths is defined
in the module configuration, system-logs looks at the files, if any of the globs resolves
to one or more files the log input is used, otherwise the jouranld input is used.

This behaviour can be overridden by setting var.use_journald or var.use_files,
which will force the use of journald or files.

Other changes:

  • Journald input now support filtering by facilities
  • System tests for modules now support handling journal files
  • The TESTING_FILEBEAT_FILEPATTERN environment variable now is a
    comma separated list of globs, it defaults to .log,*.journal
  • Multiple lint warnings are fixed
  • The documentation has been updated where needed.

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.

Disruptive User Impact

  • Debian 12 is now supported by Filebeat's system module
  • On Debian 12 the system module will use the journald input
  • The ingest pipelines for the system module were modified from 1 per fileset to 3
  • The journald input currently only reads logs from the current boot

## Author's Checklist

How to test this PR locally

Run the tests

mage buildSystemTestBinary
mage docker:ComposeUp
source $(mage PythonVirtualEnv)/bin/activate
INTEGRATION_TESTS=1 BEAT_STRICT_PERMS=false ES_PASS=testing ES_USER=admin TESTING_FILEBEAT_MODULES=system pytest tests/system/test_modules.py

Run the system module

Package Filebeat from this PR.
Start the Debian 12 VM, run Filebeat

vagrant up debian12
vagrant ssh debian12
cp /vagrant/filebeat/build/distributions/filebeat-oss-9.0.0-SNAPSHOT-linux-x86_64.tar.gz ./
tar -xf filebeat-oss-9.0.0-SNAPSHOT-linux-x86_64.tar.gz 
cd filebeat-9.0.0-SNAPSHOT-linux-x86_64/
./filebeat modules enable system
./filebeat setup -e -v
# edit modules.d/system.yml and enable both filesets
# edit filebeat.yml and add the ES output and Kibana URL/credentials
./filebeat -e -v

Ensure data is ingested (datastream filebeat-9.0.0)

Related issues

## Use cases
## Screenshots
## Logs

@belimawr belimawr added the skip-ci Skip the build in the CI but linting label Oct 1, 2024
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Oct 1, 2024
Copy link
Contributor

mergify bot commented Oct 1, 2024

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

Copy link
Contributor

mergify bot commented Oct 1, 2024

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 Oct 1, 2024
@belimawr belimawr added Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team and removed skip-ci Skip the build in the CI but linting labels Oct 1, 2024
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Oct 1, 2024
@belimawr
Copy link
Contributor Author

belimawr commented Oct 2, 2024

buildkite test this

@belimawr belimawr changed the title [WIP] Add Journald support to system module Add Journald support to system module Oct 2, 2024
Copy link
Contributor

mergify bot commented Oct 2, 2024

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 40526-system-integration-journald upstream/40526-system-integration-journald
git merge upstream/main
git push upstream 40526-system-integration-journald

@belimawr belimawr changed the title Add Journald support to system module Use journald for system module on Debian 12 Oct 2, 2024
@belimawr belimawr force-pushed the 40526-system-integration-journald branch from bebe599 to 07582e4 Compare October 2, 2024 19:57
Copy link
Contributor

mergify bot commented Oct 3, 2024

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 40526-system-integration-journald upstream/40526-system-integration-journald
git merge upstream/main
git push upstream 40526-system-integration-journald

@belimawr belimawr marked this pull request as ready for review October 4, 2024 13:15
@belimawr belimawr requested a review from a team as a code owner October 4, 2024 13:15
@elasticmachine
Copy link
Collaborator

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

@pierrehilbert pierrehilbert requested review from rdner and faec and removed request for khushijain21 and VihasMakwana October 4, 2024 15:04
@belimawr belimawr force-pushed the 40526-system-integration-journald branch from a0a9e61 to 1df05cc Compare October 10, 2024 14:01
@belimawr belimawr added the skip-ci Skip the build in the CI but linting label Oct 10, 2024
@pierrehilbert
Copy link
Collaborator

/test

@belimawr
Copy link
Contributor Author

/test

I have the skip-ci tag on it, it's not meant to run the CI tests yet. I'm just pushing some commits to share the progress on this task. I was talking with Julien yesterday and he mentioned he hadn't see any changes in the PR for a while, so I pushed the commits I had made in the past few days, but were not ready to run CI yet.

Copy link
Contributor

@leehinman leehinman left a comment

Choose a reason for hiding this comment

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

Looking good. Couple small things below.

Any chance we can get a "real" integration test? Even if it just tests that the auto detect "detects" files on a non debian 12 system?

Comment on lines 9 to 15
- grok:
description: Grok specific auth messages.
tag: grok-specific-messages
field: message
ignore_missing: true
patterns:
- '^%{DATA:system.auth.ssh.event} %{DATA:system.auth.ssh.method} for (invalid user)?%{DATA:user.name} from %{IPORHOST:source.address} port %{NUMBER:source.port:long} ssh2(: %{GREEDYDATA:system.auth.ssh.signature})?'
- '^%{DATA:system.auth.ssh.event} user %{DATA:user.name} from %{IPORHOST:source.address}'
- '^Did not receive identification string from %{IPORHOST:system.auth.ssh.dropped_ip}'
- '^%{DATA:user.name} :( %{DATA:system.auth.sudo.error} ;)? TTY=%{DATA:system.auth.sudo.tty} ; PWD=%{DATA:system.auth.sudo.pwd} ; USER=%{DATA:system.auth.sudo.user} ; COMMAND=%{GREEDYDATA:system.auth.sudo.command}'
- '^new group: name=%{DATA:group.name}, GID=%{NUMBER:group.id}'
- '^new user: name=%{DATA:user.name}, UID=%{NUMBER:user.id}, GID=%{NUMBER:group.id}, home=%{DATA:system.auth.useradd.home}, shell=%{DATA:system.auth.useradd.shell}$'
ignore_failure: true
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure this is a duplicate of what was in pipeline.yml which is now renamed to files.yml. The grok patterns are hard enough to maintain, we don't want to have to remember to do it in 2 places. Can you move the duplicates into a separate pipeline that both call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I managed to move it to a separated pipeline: 9f57ea8

filebeat/tests/system/test_modules.py Show resolved Hide resolved
@belimawr belimawr force-pushed the 40526-system-integration-journald branch from 9f57ea8 to fbfdbbe Compare October 11, 2024 19:08
@belimawr
Copy link
Contributor Author

/test

@belimawr
Copy link
Contributor Author

@belimawr belimawr merged commit cfd1f1c into elastic:main Oct 14, 2024
140 of 142 checks passed
@belimawr belimawr deleted the 40526-system-integration-journald branch October 14, 2024 16:24
mergify bot pushed a commit that referenced this pull request Oct 14, 2024
This commit adds Debian 12 support to our system module, to support
Debian 12 we need to use the journald input to collect the system
logs.

To support it, a new, internal, input  `system-logs`is introduced, it is responsible
for deciding whether the log input or journald must be used. If `var.paths` is defined
in the module configuration, `system-logs` looks at the files, if any of the globs resolves
to one or more files the `log` input is used, otherwise the `jouranld` input is used.

This behaviour can be overridden by setting `var.use_journald` or `var.use_files`,
which will force the use of journald or files.

Other changes:
 - Journald input now support filtering by facilities
 - System tests for modules now support handling journal files
 - The `TESTING_FILEBEAT_FILEPATTERN` environment variable now is a
 comma separated list of globs, it defaults to `.log,*.journal`
 - Multiple lint warnings are fixed
 - The documentation has been updated where needed.

(cherry picked from commit cfd1f1c)
pierrehilbert pushed a commit that referenced this pull request Oct 15, 2024
This commit adds Debian 12 support to our system module, to support
Debian 12 we need to use the journald input to collect the system
logs.

To support it, a new, internal, input  `system-logs`is introduced, it is responsible
for deciding whether the log input or journald must be used. If `var.paths` is defined
in the module configuration, `system-logs` looks at the files, if any of the globs resolves
to one or more files the `log` input is used, otherwise the `jouranld` input is used.

This behaviour can be overridden by setting `var.use_journald` or `var.use_files`,
which will force the use of journald or files.

Other changes:
 - Journald input now support filtering by facilities
 - System tests for modules now support handling journal files
 - The `TESTING_FILEBEAT_FILEPATTERN` environment variable now is a
 comma separated list of globs, it defaults to `.log,*.journal`
 - Multiple lint warnings are fixed
 - The documentation has been updated where needed.

(cherry picked from commit cfd1f1c)

Co-authored-by: Tiago Queiroz <tiago.queiroz@elastic.co>

// newV1Input checks whether the log input must be created and
// delegates to loginput.NewInput if needed.
func newV1Input(
Copy link
Member

Choose a reason for hiding this comment

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

Does this input not have tests, or am I just missing where they are?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has integration tests, the python system tests.

But I can write some unit/integration (integration in the sense they will rely on journalclt existing on the host) if you prefer.

Copy link
Member

@cmacknz cmacknz Oct 15, 2024

Choose a reason for hiding this comment

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

I would prefer if there were tests that lived with the input, right now they live with the system module which is quite far away from the input. In a year from now someone reading this may have trouble spotting them.

What I see now is this configuration in the system module tests:

        if ".journal" in test_file:
            cmd.remove("-once")
            cmd.append("-M")
            cmd.append("{module}.{fileset}.var.use_journald=true".format(
                module=module, fileset=fileset))
            cmd.append("-M")
            cmd.append("{module}.{fileset}.input.journald.paths=[{test_file}]".format(
                module=module, fileset=fileset, test_file=test_file))
        else:
            cmd.append("-M")
            cmd.append("{module}.{fileset}.var.paths=[{test_file}]".format(
                module=module, fileset=fileset, test_file=test_file))

This tests the use_journald configuration and also the case where paths exists. These are not exhaustive tests :)

I don't see a test for the case where there is a list of file paths, but the paths don't exist in the file system, so we read from journald. This is the most common case we expect to for this to handle.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add some tests for this case on a new PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created a GH issue to help keep track of it: #41259

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

Investigate the best way to decide when to read system logs from files or journald
5 participants