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

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

Open
wants to merge 4 commits into
base: 8.16
Choose a base branch
from

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Oct 30, 2024

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

  1. 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

  2. Go to Discover in Kibana, filter by tags: from-journald

  3. Look at fileset.name from the events, make sure auth and syslog are there

Related issues

## Use cases
## Screenshots
## Logs


This is an automatic backport of pull request #41381 done by Mergify.

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
@mergify mergify bot requested a review from a team as a code owner October 30, 2024 19:29
@mergify mergify bot added backport conflicts There is a conflict in the backported pull request labels Oct 30, 2024
@mergify mergify bot requested a review from a team as a code owner October 30, 2024 19:29
@mergify mergify bot requested review from faec and VihasMakwana and removed request for a team October 30, 2024 19:29
Copy link
Contributor Author

mergify bot commented Oct 30, 2024

Cherry-pick of b1c7478 has failed:

On branch mergify/bp/8.16/pr-41381
Your branch is up to date with 'origin/8.16'.

You are currently cherry-picking commit b1c7478458.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   filebeat/input/journald/config.go
	modified:   filebeat/input/journald/input.go
	modified:   filebeat/input/journald/pkg/journalctl/jctlmock_test.go
	modified:   filebeat/input/journald/pkg/journalctl/journalctl.go
	modified:   filebeat/input/journald/pkg/journalctl/mode.go
	modified:   filebeat/input/journald/pkg/journalctl/mode_test.go
	modified:   filebeat/input/journald/pkg/journalctl/reader.go
	modified:   filebeat/input/journald/pkg/journalctl/reader_test.go
	modified:   filebeat/input/journald/pkg/journalfield/conv.go
	deleted:    filebeat/input/journald/pkg/journalfield/default_other.go
	modified:   filebeat/input/journald/pkg/journalfield/matcher.go
	modified:   filebeat/input/systemlogs/input.go
	modified:   filebeat/input/systemlogs/input_linux.go
	modified:   filebeat/module/system/auth/config/auth.yml
	modified:   filebeat/module/system/syslog/config/syslog.yml
	new file:   filebeat/tests/integration/systemlogs_all_test.go
	renamed:    filebeat/tests/integration/systemlogs_test.go -> filebeat/tests/integration/systemlogs_linux_test.go
	new file:   filebeat/tests/integration/systemlogs_other_test.go
	modified:   filebeat/tests/integration/testdata/filebeat_system_module.yml
	modified:   libbeat/tests/integration/framework.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   .buildkite/filebeat/filebeat-pipeline.yml

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Oct 30, 2024
@belimawr
Copy link
Contributor

Because we decided to revert the changes in the system module (see #41489), this PR should not be merged yet.

Alongside with the fixes in the system module, the original PR also brought some improvements in the journald input that would be nice to have in 8.16. So let's wait for #41489 to be merged, rebase onto 8.16 then merge this PR, this will avoid conflicts with #41489 that is more important.

@belimawr belimawr changed the title [8.16](backport #41381) Fix system module with both filesets enabled [DO NOT MERGE] [8.16](backport #41381) Fix system module with both filesets enabled Oct 30, 2024
Copy link
Contributor Author

mergify bot commented Oct 31, 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 mergify/bp/8.16/pr-41381 upstream/mergify/bp/8.16/pr-41381
git merge upstream/8.16
git push upstream mergify/bp/8.16/pr-41381

@pierrehilbert pierrehilbert added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Oct 31, 2024
@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 Oct 31, 2024
@belimawr belimawr changed the title [DO NOT MERGE] [8.16](backport #41381) Fix system module with both filesets enabled [8.16](backport #41381) Fix system module with both filesets enabled Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport conflicts There is a conflict in the backported pull request 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.

4 participants