Read journal entries from all boots #41244

belimawr · 2024-10-15T19:11:44Z

Proposed commit message

Some versions of journalctl will only return messages from the current boot when --follow is passed, it will even ignore the cursor or date arguments.

This commit reads messages from all boots by first calling journalctl without the --follow flag, reading all entries and once it successfully exits, then we restart journalctl with the cursor and the --follow flag.

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~~

Author's Checklist

Ensure TestInputParsers from filebeat/input/journald/input_parsers_test.go is not flaky
Add tests to ensure messages from all boots are read

How to test this PR locally

1. Run the tests

cd filebeat/input/journald
go test -run=TestInputCanReadAllBoots

2. Run Filebeat reading `filebeat/input/journald/testdata/multiple-boots.journal`

There must be 6 entries, you can see the plaintext entries by looking at filebeat/input/journald/testdata/multiple-boots.export or by running:

journalctl --file filebeat/input/journald/testdata/multiple-boots.export

3. Fully manual test

Run the journald input on a machine (or a journal file) that has got messages from more than one boot
Ensure all messages in the journal are correctly ingested.

Related issues

Closes Journald input only ingests events from the current boot #41083

~~## Use cases~~
~~## Screenshots~~
~~## Logs~~

mergify · 2024-10-15T19:12:28Z

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 41083-journald-input-all-boots upstream/41083-journald-input-all-boots
git merge upstream/main
git push upstream 41083-journald-input-all-boots

mergify · 2024-10-15T19:12:29Z

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

mergify · 2024-10-15T19:12:29Z

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 · 2024-10-18T21:16:21Z

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 41083-journald-input-all-boots upstream/41083-journald-input-all-boots
git merge upstream/main
git push upstream 41083-journald-input-all-boots

Some versions of journalctl will only return messages from the current boot when --follow is passed, it will even ignore the cursor or date arguments. This commit reads messages from all boots by first calling journalctl without the --follow flag, reading all entries and once it successfully exits, then we restart journalctl with the cursor and the --follow flag.

This commit updates the parse test to use ndjson parser instead of multiline because the multiline parser can have issues when journald input is reading from files. There is a corner case where the journalctl exits successfully and the reader goroutine gets an error, this makes Next to return early, making the multiline to also return early. So far I have only seen this happening when reading from file and at the very end of the file, hence it does not seem to be a critical bug.

elasticmachine · 2024-10-18T21:21:02Z

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

mauri870 · 2024-10-21T11:52:18Z

filebeat/input/journald/input_parsers_test.go

+ // The JSON in the test journal is: '{"foo": "bar", "answer":42}'
+ expectedFoo := "bar"
+ expectedAnswer := int64(42)
+ if foo != expectedFoo {
+ t.Errorf("expecting foo to be '%s' got '%s' instead", expectedFoo, foo)
+ }
+ if answer != expectedAnswer {
+ t.Errorf("expecting foo to be '%d' got '%d' instead", expectedAnswer, answer)
+ }


NIT: You might be able to simplify this by using assert.JSONEq

mauri870 · 2024-10-21T12:08:17Z

filebeat/input/journald/testdata/multiple-boots.journal

8 megabytes is quite huge to put under version control, is that standard behavior for test files?

I guess the alternative is to only run the test on journald-enabled systems and compare with the live output.

belimawr added bug skip-ci Skip the build in the CI but linting Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team labels Oct 15, 2024

belimawr self-assigned this Oct 15, 2024

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 Oct 15, 2024

mergify bot added the backport-8.x Automated backport to the 8.x branch with mergify label Oct 15, 2024

belimawr force-pushed the 41083-journald-input-all-boots branch from c6cee3e to 2c39ac0 Compare October 15, 2024 19:15

belimawr removed the skip-ci Skip the build in the CI but linting label Oct 15, 2024

belimawr force-pushed the 41083-journald-input-all-boots branch from 21360eb to 30c8299 Compare October 18, 2024 20:53

belimawr added 5 commits October 18, 2024 17:20

Add tests for reading from all boots

dfa16c4

Fix comments and string manipulation

e4e1c62

add changelog and improve messages

1a91677

belimawr force-pushed the 41083-journald-input-all-boots branch from 6012cb8 to 1a91677 Compare October 18, 2024 21:20

belimawr marked this pull request as ready for review October 18, 2024 21:21

belimawr requested a review from a team as a code owner October 18, 2024 21:21

belimawr requested review from rdner and khushijain21 October 18, 2024 21:21

pierrehilbert requested review from mauri870 and removed request for khushijain21 October 20, 2024 16:24

mauri870 reviewed Oct 21, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Read journal entries from all boots #41244

Read journal entries from all boots #41244

belimawr commented Oct 15, 2024 •

edited

Loading

mergify bot commented Oct 15, 2024

mergify bot commented Oct 15, 2024

mergify bot commented Oct 15, 2024

mergify bot commented Oct 18, 2024

elasticmachine commented Oct 18, 2024

mauri870 Oct 21, 2024

mauri870 Oct 21, 2024

mauri870 Oct 21, 2024

Read journal entries from all boots #41244

Are you sure you want to change the base?

Read journal entries from all boots #41244

Conversation

belimawr commented Oct 15, 2024 • edited Loading

Proposed commit message

Checklist

Author's Checklist

How to test this PR locally

1. Run the tests

2. Run Filebeat reading filebeat/input/journald/testdata/multiple-boots.journal

3. Fully manual test

Related issues

mergify bot commented Oct 15, 2024

mergify bot commented Oct 15, 2024

mergify bot commented Oct 15, 2024

mergify bot commented Oct 18, 2024

elasticmachine commented Oct 18, 2024

mauri870 Oct 21, 2024

Choose a reason for hiding this comment

mauri870 Oct 21, 2024

Choose a reason for hiding this comment

mauri870 Oct 21, 2024

Choose a reason for hiding this comment

belimawr commented Oct 15, 2024 •

edited

Loading

2. Run Filebeat reading `filebeat/input/journald/testdata/multiple-boots.journal`