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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 4 commits into from
Oct 7, 2021
Merged

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

merged 4 commits into from
Oct 7, 2021

Conversation

bmoylan
Copy link
Contributor

@bmoylan bmoylan commented Oct 1, 2021 •
edited
Loading

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 •
edited by jenkins-beats-ci bot
Loading

💚 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

adriansr
adriansr approved these changes Oct 6, 2021
View reviewed changes
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!

@adriansr adriansr merged commit bef0411 into elastic:master Oct 7, 2021
mergify bot pushed a commit that referenced this pull request Oct 7, 2021
@bmoylan
processors/actions/add_fields: Do not panic if event.Fields is nil map (
c28e0c4 
#28219)

(cherry picked from commit bef0411)
@mergify mergify bot mentioned this pull request Oct 7, 2021
Closed
mergify bot pushed a commit that referenced this pull request Oct 7, 2021
@bmoylan
processors/actions/add_fields: Do not panic if event.Fields is nil map (
e917e41 
#28219)

(cherry picked from commit bef0411)
@mergify mergify bot mentioned this pull request Oct 7, 2021
Merged
mergify bot pushed a commit that referenced this pull request Oct 7, 2021
@bmoylan
processors/actions/add_fields: Do not panic if event.Fields is nil map (
60b5585 
#28219)

(cherry picked from commit bef0411)
@mergify mergify bot mentioned this pull request Oct 7, 2021
Merged
@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
@v1v
Merge remote-tracking branch 'upstream/master' into feature/e2e-fleet
86c1cf8 
* 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
@mergify @bmoylan
processors/actions/add_fields: Do not panic if event.Fields is nil map (
5ad13fd 
#28219) (#28299)

(cherry picked from commit bef0411)

Co-authored-by: Brad Moylan <bmoylan@palantir.com>
jsoriano added a commit that referenced this pull request Oct 29, 2021
@mergify @bmoylan @jsoriano
[7.15](backport #28219) processors/actions/add_fields: Do not panic i…
e0d2587 
…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 <bmoylan@palantir.com>
Co-authored-by: Jaime Soriano Pastor <jaime.soriano@elastic.co>
Icedroid pushed a commit to Icedroid/beats that referenced this pull request Nov 1, 2021
@bmoylan
processors/actions/add_fields: Do not panic if event.Fields is nil map (
da56e35 
elastic#28219)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adriansr adriansr adriansr approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bmoylan @elasticmachine @P1llus @adriansr