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

x-pack/filebeat/module/checkpoint: fix handling of R81 fields #32458

Merged
merged 1 commit into from
Jul 26, 2022

Conversation

efd6
Copy link
Contributor

@efd6 efd6 commented Jul 22, 2022

What does this PR do?

Fixes handling of numeric and duplicated fields and removes undocumented fields that appear to be new in R81.

Why is it important?

Relates to a user issue; duplicated fields cause a pipeline failure and non-ingestion due to absence of @timestamp. The other changes keep the pipeline in sync with the integration package pipeline where the changes are required due to field type and documentation checks.

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.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

@efd6 efd6 added Filebeat Filebeat Team:Security-External Integrations backport-7.17 Automated backport to the 7.17 branch with mergify 8.4-candidate labels Jul 22, 2022
@efd6 efd6 self-assigned this Jul 22, 2022
@botelastic 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 Jul 22, 2022
@efd6 efd6 force-pushed the 32380-checkpoint branch from 6df2f21 to c37a52c Compare July 22, 2022 02:30
@efd6 efd6 marked this pull request as ready for review July 22, 2022 02:43
@efd6 efd6 requested a review from a team as a code owner July 22, 2022 02:43
@elasticmachine
Copy link
Collaborator

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-07-22T02:30:46.290+0000

  • Duration: 76 min 11 sec

Test stats 🧪

Test Results
Failed 0
Passed 2174
Skipped 166
Total 2340

💚 Flaky test report

Tests succeeded.

🤖 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!)

Copy link
Contributor

@r00tu53r r00tu53r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +437 to 443
- convert:
field: checkpoint.bytes
type: long
ignore_missing: true
- rename:
field: checkpoint.bytes
target_field: network.bytes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be merged into a single step ? - elastic/integrations#3800 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That leaves the value in checkpoint.bytes.

Comment on lines +478 to 484
- convert:
field: checkpoint.packets
type: long
ignore_missing: true
- rename:
field: checkpoint.packets
target_field: network.packets
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be merged into a single step - elastic/integrations#3800 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

@efd6 efd6 merged commit c580fdb into elastic:main Jul 26, 2022
mergify bot pushed a commit that referenced this pull request Jul 26, 2022
(cherry picked from commit c580fdb)

# Conflicts:
#	x-pack/filebeat/module/checkpoint/firewall/ingest/pipeline.yml
efd6 added a commit that referenced this pull request Jul 26, 2022
…ng of R81 fields (#32494)

* x-pack/filebeat/module/checkpoint: fix handling of R81 fields (#32458)

(cherry picked from commit c580fdb)

# Conflicts:
#	x-pack/filebeat/module/checkpoint/firewall/ingest/pipeline.yml

* fix conflicts

Co-authored-by: Dan Kortschak <90160302+efd6@users.noreply.github.com>
Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.4-candidate backport-7.17 Automated backport to the 7.17 branch with mergify Filebeat Filebeat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Filebeat][Checkpoint module] data stream timestamp field [@timestamp] is missing
3 participants