[filebeat] Fix ingest pipeline overwriting module field values

[crespocarlos]

What does this PR do?

This PR fixes a problem with the ingest pipeline not fully considering the fields included in the module configuration.

Notes

According to Filebeat doc, users can add new fields to the output, but doesn't mention anything about overwriting log entry's existing field values

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.

How to test this PR locally

• Start local Kibana and add logging config to kibana.yml

logging:
  appenders:
    console:
      type: console
      layout:
        type: pattern
        highlight: true
    file:
      type: file
      fileName: ./logs/kibana-json.log
      layout:
        type: json
  root:
    appenders: [default, console, file]
    level: debug

Pull this branch and start filebeat from the source https://github.com/elastic/kibana/blob/main/x-pack/plugins/monitoring/dev_docs/how_to/running_components_from_source.md#filebeat

• On filebeat.yml, enable Kibana module.

- module: kibana
    log:
      enabled: true
      var.paths:
        - PATH_TO_KIBANA_LOG
      input:
        fields:
          ecs.version: "9.0.0" # existing field
          log.level: "TEST" # existing field
          service.name: "Kibana" # new field
          cloud.availability_zone: "danger-zone" # new field
        fields_under_root: true

Note that if fields_under_root is omitted or false, these custom fields will appear on the log as fields.ecs.version, fields.kibana.service and etc. When true, besides being included in the root, they can overwrite existing log entry fields.

cloud.availability_zone and service.name will be added to the ingested log, service.* won't be overwritten by service.name and ecs.version and log.level will remain with their original value

Related issues

Closes #32665

[mergify]

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @crespocarlos? 🙏.
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-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

[elasticmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2022-10-04T09:52:05.256+0000

• Duration: 70 min 52 sec

Test stats 🧪

Test	Results
Failed	0
Passed	6899
Skipped	737
Total	7636

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

• /test : Re-trigger the build.

• /package : Generate the packages and run the E2E tests.

• /beats-tester : Run the installation tests with beats-tester.

• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

[matschaffer]

it also drops support for non-ecs compliant logs

Did you see a way to avoid doing this? Not sure but I'm guessing if we drop support we can't consider this a minor change.

[crespocarlos]

it also drops support for non-ecs compliant logs

Did you see a way to avoid doing this? Not sure but I'm guessing if we drop support we can't consider this a minor change.

Yeah. This makes sense. Better address that in another ticket.

[crespocarlos]

When this is set to true, Filebeat overwrites all fields correctly, but it also replaces the log entry @timestamp with Filebeat's. And that would make the ingested data inconsistent

[crespocarlos]

Consistent with integration packages. These lines are responsible for making this fix work:

add_to_root: true
add_to_root_conflict_strategy: merge

[matschaffer]

LGTM, will approve after some exploratory testing.

I was wondering if maybe there's a way to override the configuration used in testing so we could inject some override fields. This would help protect against future regression. But if there's nothing we can easily use, I think it'd be beyond the scope of this PR to add it.

[matschaffer]

Cool. I was able to see the new fields. I was intuitively expecting the fields to override things originally found in the logs, but it doesn't look like that's what's being asked for in the original issue.

[crespocarlos]

Cool. I was able to see the new fields. I was intuitively expecting the fields to override things originally found in the logs, but it doesn't look like that's what's being asked for in the original issue.

That is possible using decode_json_fields processor. It's the easiest way to both add new fields and override original stuff from the logs. But since the doc doesn't mention anything about the latter, I decided not to enable this behaviour