Fix system module with both filesets enabled #41381

belimawr · 2024-10-22T20:48:06Z

Proposed commit message

The system module did not define an ID at the root of the config, that made the V2 input loader only start the first journald input it saw because they both ended up with the same identifier (type, ID and path). This is fixed by defining an ID at the root of the configuration templates.

The journald input now also adds the input_id key to its loggers and a non-fatal error is now logged at debug level.

The system-logs input is now marked as experimental instead of stable.

Fix lint warnings by moving toJournalConfig to input_linux.go

Move TestSystemLogsCanUseLogInput to a file without the
linux build constraint so it can run on all OSes supported by the
system integration.

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

How to test this PR locally

Run Filebeat with the following filebeat.yml and modules.d/system.yml (adjust credentials/addresses as necessary)

filebeat.yml

filebeat.inputs:
  - type: journald
    id: my-journald-normal-input
    tags:
       - journald-input
  - type: filestream
    id: my-filestream-id
    paths:
      - /tmp/flog.log

filebeat.config.modules:
  path: ${path.config}/modules.d/*.yml
  reload.enabled: false
  reload.period: 1s

setup.template:
  settings:
    index.number_of_shards: 1

setup.kibana:
  host: "http://kibana:5601"
  username: admin
  password: testing
  ssl.verification_mode: none

output.elasticsearch:
  hosts: ["http://elasticsearch:9200"]
  preset: latency
  protocol: "http"

  username: admin
  password: testing
  ssl.verification_mode: none

modules.d/system.yml

- module: system
  syslog:
    enabled: true
    var.use_journald: true
    input:
      tags:
        - from-journald

  auth:
    enabled: true
    var.use_journald: true
    var.tags:
      - from-journald

Go to Discover in Kibana, filter by tags: from-journald
Look at fileset.name from the events, make sure auth and syslog are there

Related issues

Closes [System module] Only one instance of Journald runs when both syslog and auth filesets are enabled #41378

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

elasticmachine · 2024-10-22T20:48:09Z

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

filebeat/input/journald/pkg/journalctl/journalctl.go

filebeat/tests/integration/systemlogs_test.go

filebeat/tests/integration/testdata/filebeat_system_module.yml

filebeat/tests/integration/systemlogs_test.go

mauri870

I don't see the linter change nor the comment for the journald stdout, did you forgot to push?

mergify · 2024-10-23T19:23:44Z

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 41378-fix-system-module-only-starts-one-input upstream/41378-fix-system-module-only-starts-one-input
git merge upstream/main
git push upstream 41378-fix-system-module-only-starts-one-input

belimawr · 2024-10-24T14:24:36Z

Some CI failures seem to be unrelated:

<html>
<body>
<!--StartFragment-->
=== Failed
--
  | === FAIL: filebeat/input/filestream/internal/task TestGroup_Go/workloads_wait_for_available_worker (0.20s)
  | group_test.go:129: f2 started
  | group_test.go:116: f1 started
  | group_test.go:135: f2 done
  | group_test.go:148: f3 started
  | group_test.go:182: waiting the worker pool to finish all workloads
  | group_test.go:118: f1 done
  | group_test.go:150: f3 done
  | group_test.go:185: worker pool to finished all workloads
  | group_test.go:187:
  | Error Trace:	C:/buildkite-agent/builds/bk-agent-prod-gcp-1729720802417888488/elastic/filebeat/filebeat/input/filestream/internal/task/group_test.go:187
  | Error:      	Condition never satisfied
  | Test:       	TestGroup_Go/workloads_wait_for_available_worker
  | Messages:   	not all goroutines finished
  |  
  | === FAIL: filebeat/input/filestream/internal/task TestGroup_Go (0.70s)

<!--EndFragment-->
</body>
</html>

BK link: https://buildkite.com/elastic/filebeat/builds/10493#0192bb64-7d3a-4e8a-9879-e4528b7076ab/154-388

I tried to reproduce it on Win and did not succeed. I'll just re-run, if it fails again I'll open a FlakyTest issue.

mauri870 · 2024-10-24T15:36:04Z

@belimawr This test looks suspiciously like the other ones I fixed recently #41230. I wouldn't be surprised if it is flaky.

Fix lint warnings and add Linux build tag in the files that were missing it.

Skip TestSystemLogsCanUseJournaldInput if on BuildKite and not running as root, running commands as root is also fixed.

Journald input only works on Linux because Systemd is only available for Linux, so remove any file from the journald input that are built on OSes other than Linux. All integration test helpers from system-logs input are moved to a file that is build on all platforms.

filebeat/integ-test-buildkite.sh

filebeat/tests/integration/systemlogs_linux_test.go

rowlandgeoff

BK pipeline change looks good

The system module did not define an ID at the root of the config, that made the V2 input loader only start the first journald input it saw because they both ended up with the same identifier (type, ID and path). This is fixed by defining an ID at the root of the configuration templates. The journald input now also adds the input_id key to its loggers and a non-fatal error is now logged at debug level. The system-logs input is now marked as experimental instead of stable. Fix lint warnings by moving toJournalConfig to input_linux.go Move TestSystemLogsCanUseLogInput to a file without the linux build constraint so it can run on all OSes supported by the system integration. (cherry picked from commit b1c7478) # Conflicts: # .buildkite/filebeat/filebeat-pipeline.yml

…41488) Even though the System module is not using the system-logs input any more, this commit brings improvements to the Journald input and to the, currently skipped, system-logs + system module tests. Original commit message: The system module did not define an ID at the root of the config, that made the V2 input loader only start the first journald input it saw because they both ended up with the same identifier (type, ID and path). This is fixed by defining an ID at the root of the configuration templates. The journald input now also adds the input_id key to its loggers and a non-fatal error is now logged at debug level. The system-logs input is now marked as experimental instead of stable. Fix lint warnings by moving toJournalConfig to input_linux.go Move TestSystemLogsCanUseLogInput to a file without the linux build constraint so it can run on all OSes supported by the system integration. (cherry picked from commit b1c7478) # Conflicts: # .buildkite/filebeat/filebeat-pipeline.yml * Fix filebeat-pipeline.yml merge conflict * skip system-logs test --------- Co-authored-by: Tiago Queiroz <tiago.queiroz@elastic.co> Co-authored-by: Geoff Rowland <70981735+rowlandgeoff@users.noreply.github.com>

…41487) Even though the System module is not using the system-logs input any more, this commit brings improvements to the Journald input and to the, currently skipped, system-logs + system module tests. Original commit message: The system module did not define an ID at the root of the config, that made the V2 input loader only start the first journald input it saw because they both ended up with the same identifier (type, ID and path). This is fixed by defining an ID at the root of the configuration templates. The journald input now also adds the input_id key to its loggers and a non-fatal error is now logged at debug level. The system-logs input is now marked as experimental instead of stable. Fix lint warnings by moving toJournalConfig to input_linux.go Move TestSystemLogsCanUseLogInput to a file without the linux build constraint so it can run on all OSes supported by the system integration. (cherry picked from commit b1c7478) # Conflicts: # .buildkite/filebeat/filebeat-pipeline.yml * Resolve filebeat-pipeline.yml merge conflicts * skip system-logs test --------- Co-authored-by: Tiago Queiroz <tiago.queiroz@elastic.co> Co-authored-by: Geoff Rowland <70981735+rowlandgeoff@users.noreply.github.com>

belimawr added bug Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team backport-8.x Automated backport to the 8.x branch with mergify backport-8.16 Automated backport with mergify labels Oct 22, 2024

belimawr self-assigned this Oct 22, 2024

belimawr requested a review from a team as a code owner October 22, 2024 20:48

belimawr requested review from khushijain21 and VihasMakwana October 22, 2024 20:48

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 22, 2024

belimawr force-pushed the 41378-fix-system-module-only-starts-one-input branch from 0765233 to 0501772 Compare October 22, 2024 22:35

belimawr changed the title ~~Fix system module with both filesets enables~~ Fix system module with both filesets enabled Oct 22, 2024

belimawr force-pushed the 41378-fix-system-module-only-starts-one-input branch from 0501772 to 59b7cb6 Compare October 22, 2024 23:37

pierrehilbert requested review from rdner and mauri870 and removed request for khushijain21 and VihasMakwana October 23, 2024 08:42

mauri870 reviewed Oct 23, 2024

View reviewed changes

filebeat/input/journald/pkg/journalctl/journalctl.go Outdated Show resolved Hide resolved

mauri870 reviewed Oct 23, 2024

View reviewed changes

filebeat/tests/integration/systemlogs_test.go Outdated Show resolved Hide resolved

mauri870 reviewed Oct 23, 2024

View reviewed changes

filebeat/tests/integration/testdata/filebeat_system_module.yml Outdated Show resolved Hide resolved

rdner reviewed Oct 23, 2024

View reviewed changes

filebeat/tests/integration/systemlogs_test.go Outdated Show resolved Hide resolved

mauri870 reviewed Oct 23, 2024

View reviewed changes

belimawr force-pushed the 41378-fix-system-module-only-starts-one-input branch 2 times, most recently from 59974d2 to d25f496 Compare October 23, 2024 21:40

mauri870 approved these changes Oct 24, 2024

View reviewed changes

belimawr requested a review from a team as a code owner October 24, 2024 15:42

belimawr added 12 commits October 29, 2024 14:47

Add a debug test and remove the pipeline steps

c53c109

Try root again

796edaa

Address lint warnings

a644b5f

Fix lint warnings and add Linux build tag in the files that were missing it.

Fix running tests as root and skip TestSystemLogsCanUseJournaldInput

a5a17a6

Skip TestSystemLogsCanUseJournaldInput if on BuildKite and not running as root, running commands as root is also fixed.

Fix lint warings: remove un-used field

e931e9c

Fix integration tests as root step

847719a

Add nolint directives

b768575

Try sudo su

b04e427

try using systemd-journal group

d34d078

escape env var

0eb87ab

add use to 'systemd-journal' group before running integration tests

49e5076

belimawr force-pushed the 41378-fix-system-module-only-starts-one-input branch from 9759879 to 49e5076 Compare October 29, 2024 18:52

belimawr added 5 commits October 29, 2024 15:44

Export files from the debug test

3ad61c8

try with sudo su user and more debug

9857943

try with tee instead of redirecting output

f276e04

Try everything with sudo su $$USER

3f976af

put everything on a script

d867b33

alexsapran mentioned this pull request Oct 30, 2024

[DO-NOT-MERGE] testing #41479

Closed

alexsapran reviewed Oct 30, 2024

View reviewed changes

filebeat/integ-test-buildkite.sh Outdated Show resolved Hide resolved

filebeat/tests/integration/systemlogs_linux_test.go Show resolved Hide resolved

belimawr added 2 commits October 30, 2024 09:28

Remove debug messages and sudo stuff

cba5cfe

Remove debugging code

ee50da4

rowlandgeoff approved these changes Oct 30, 2024

View reviewed changes

belimawr merged commit b1c7478 into elastic:main Oct 30, 2024
141 checks passed

mergify bot mentioned this pull request Oct 30, 2024

[8.16](backport #41381) Fix system module with both filesets enabled #41487

Merged

3 tasks

mergify bot mentioned this pull request Oct 30, 2024

[8.x](backport #41381) Fix system module with both filesets enabled #41488

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix system module with both filesets enabled #41381

Fix system module with both filesets enabled #41381

belimawr commented Oct 22, 2024 •

edited

Loading

elasticmachine commented Oct 22, 2024

mauri870 left a comment

mergify bot commented Oct 23, 2024

belimawr commented Oct 24, 2024

mauri870 commented Oct 24, 2024

rowlandgeoff left a comment

Fix system module with both filesets enabled #41381

Fix system module with both filesets enabled #41381

Conversation

belimawr commented Oct 22, 2024 • edited Loading

Proposed commit message

Checklist

How to test this PR locally

Related issues

elasticmachine commented Oct 22, 2024

mauri870 left a comment

Choose a reason for hiding this comment

mergify bot commented Oct 23, 2024

belimawr commented Oct 24, 2024

mauri870 commented Oct 24, 2024

rowlandgeoff left a comment

Choose a reason for hiding this comment

belimawr commented Oct 22, 2024 •

edited

Loading