Use journald for system module on Debian 12#41061
Conversation
|
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
|
|
|
buildkite test this |
|
This pull request is now in conflicts. Could you fix it? 🙏 |
bebe599 to
07582e4
Compare
|
This pull request is now in conflicts. Could you fix it? 🙏 |
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
a0a9e61 to
1df05cc
Compare
|
/test |
I have the |
leehinman
left a comment
There was a problem hiding this comment.
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?
| - 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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
I managed to move it to a separated pipeline: 9f57ea8
9f57ea8 to
fbfdbbe
Compare
|
/test |
|
The failing tests seem to be flaky tests, I created issues for them: |
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)
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>
filebeat/module/system/syslog/test/darwin-syslog-sample.log-expected.json
Show resolved
Hide resolved
|
|
||
| // newV1Input checks whether the log input must be created and | ||
| // delegates to loginput.NewInput if needed. | ||
| func newV1Input( |
There was a problem hiding this comment.
Does this input not have tests, or am I just missing where they are?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I'll add some tests for this case on a new PR.
There was a problem hiding this comment.
I created a GH issue to help keep track of it: #41259
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 responsiblefor deciding whether the log input or journald must be used. If
var.pathsis definedin the module configuration,
system-logslooks at the files, if any of the globs resolvesto one or more files the
loginput is used, otherwise thejouranldinput is used.This behaviour can be overridden by setting
var.use_journaldorvar.use_files,which will force the use of journald or files.
Other changes:
TESTING_FILEBEAT_FILEPATTERNenvironment variable now is acomma separated list of globs, it defaults to
.log,*.journalChecklist
CHANGELOG.next.asciidocorCHANGELOG-developer.next.asciidoc.Disruptive User Impact
## Author's ChecklistHow to test this PR locally
Run the tests
Run the system module
Package Filebeat from this PR.
Start the Debian 12 VM, run Filebeat
Ensure data is ingested (datastream
filebeat-9.0.0)Related issues
## Use cases## Screenshots## Logs