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

processors/actions/add_fields: Do not panic if event.Fields is nil map #28219

Merged
merged 4 commits into from
Oct 7, 2021

Conversation

bmoylan
Copy link
Contributor

@bmoylan bmoylan commented Oct 1, 2021

What does this PR do?

Avoid panicking when the add_fields processor is run on an event whose Fields map is nil. This can be the case when setting normalize to false in MakeDefaultSupport.

Why is it important?

Disabling event normalization avoids copying every event into a second map (and its memory allocation along with it).

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

  • [ ]

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Oct 1, 2021
@mergify
Copy link
Contributor

mergify bot commented Oct 1, 2021

This pull request does not have a backport label. Could you fix it @bmoylan? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@mergify mergify bot added the backport-skip Skip notification from the automated backport with mergify label Oct 1, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 1, 2021

💚 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: 2021-10-06T17:09:26.156+0000

  • Duration: 166 min 20 sec

  • Commit: c22ff6c

Test stats 🧪

Test Results
Failed 0
Passed 53655
Skipped 5346
Total 59001

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

@bmoylan
Copy link
Contributor Author

bmoylan commented Oct 1, 2021

I don't have permissions to edit labels, but would love it backported as far as possible :) We are currently running 7.14 but will upgrade to 7.15 soon.

@adriansr adriansr added backport-v7.15.0 Automated backport with mergify backport-v7.16.0 Automated backport with mergify backport-v8.0.0 Automated backport with mergify libbeat review labels Oct 6, 2021
@mergify mergify bot removed the backport-skip Skip notification from the automated backport with mergify label Oct 6, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@elasticmachine
Copy link
Collaborator

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

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Oct 6, 2021
@P1llus
Copy link
Member

P1llus commented Oct 6, 2021

/test

Copy link
Contributor

@adriansr adriansr left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing

@bmoylan
Copy link
Contributor Author

bmoylan commented Oct 6, 2021

@P1llus @adriansr thanks for the review. Looks ready to merge!

@bmoylan bmoylan deleted the bm/add_fields_panic branch October 7, 2021 14:20
v1v added a commit to v1v/beats that referenced this pull request Oct 11, 2021
* upstream/master: (73 commits)
  Remove GCP support from Functionbeat (elastic#28253)
  Move labels and annotations under kubernetes.namespace. (elastic#27917)
  Update go release version 1.17.1 (elastic#27543)
  Osquerybeat: Runner and Fetcher unit tests (elastic#28290)
  Osquerybeat: Improve handling of osquery.autoload file, allow customizations (elastic#28289)
  seccomp: allow clone3 syscall for x86 (elastic#28117)
  packetbeat/protos/dns: don't render missing A and AAAA addresses from truncated records (elastic#28297)
  [7.x] [DOCS] Update api_key example on elasticsearch output (elastic#28288)
  [cloud][docker] use the private docker namespace (elastic#28286)
  Update aws-lambda-go library version to 1.13.3 (elastic#28236)
  Deprecate common.Float (elastic#28280)
  Filebeat: Change compatibility test stage to test against previous minor instead of 7.11 (elastic#28274)
  x-pack/filebeat/module/threatintel/misp: add support for secondary object attribute handling (elastic#28124)
  Explicitly pass http config to doppler consumer (elastic#28277)
  processors/actions/add_fields: Do not panic if event.Fields is nil map (elastic#28219)
  Resolved timestamp for defender atp (elastic#28272)
  [Winlogbeat] Tolerate faults when Windows Event Log session is interrupted (elastic#28191)
  [elastic-agent] proxy requests to subprocesses to their metrics endpoints (elastic#28165)
  Build cloud docker images for elastic-agent (elastic#28134)
  Upgrade k8s go-client library (elastic#28228)
  ...
jsoriano pushed a commit that referenced this pull request Oct 26, 2021
jsoriano added a commit that referenced this pull request Oct 29, 2021
…f event.Fields is nil map (#28300)

* processors/actions/add_fields: Do not panic if event.Fields is nil map (#28219)

(cherry picked from commit bef0411)

Co-authored-by: Brad Moylan <[email protected]>
Co-authored-by: Jaime Soriano Pastor <[email protected]>
Icedroid pushed a commit to Icedroid/beats that referenced this pull request Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.15.0 Automated backport with mergify backport-v7.16.0 Automated backport with mergify backport-v8.0.0 Automated backport with mergify libbeat review Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants