[cef] Update Beats config to ignore empty fields #13110
Conversation
| @P1llus could you check this for me, please? |
P1llus left a comment
P1llus left a comment There was a problem hiding this comment.
Except the note the rest seems correct :)
🚀 Benchmarks reportTo see the full report comment with |
packages/cef/data_stream/log/_dev/test/system/test-logfile-config.yml Outdated Show resolved Hide resolved
andrewkroh added the Team:Security-Deployment and Devices 
| Pinging @elastic/sec-deployment-and-devices (Team:Security-Deployment and Devices) |
andrewkroh left a comment
andrewkroh left a comment There was a problem hiding this comment.
The test changes look good now.
I assume the intention behind using YAML anchors is to reduce duplication, but I have concerns about whether the added complexity and loss of readability make it worthwhile. It's not a blocker from me, but I would seek out a few more opinions.
i can revert those. now that i know they work, i'm excited about the possibility of using them in pipelines |
I have mixed feelings about them. On one hand, it's really nice to avoid code duplication, but it does reduce readability, at least at first. I think with time I could get used to it. |
|
💚 Build Succeeded
History
|
| @@ -0,0 +1,3 @@ | |||
| <163>Apr 1 05:14:15 192.0.2.1 CEF:0|Elastic|Vaporware|1.0.0-alpha|18|Web request|low|cs1= msg=rfc3164 | |||
| Apr 1 05:14:15 192.0.2.1 CEF:0|Elastic|Vaporware|1.0.0-alpha|18|Web request|low|msg=rfc3164 cs2= | |||
| 2021-04-01T05:14:15.000003Z 192.0.2.1 rfc5424App 8710 - - CEF:0|Elastic|Vaporware|1.0.0-alpha|18|Web request|low|cs4= msg=rfc5424 cs3= cs1= cs2= eventId=22 | |||
There was a problem hiding this comment.
Interested to see how the ingest document looks like if ignore_empty_values is false. Is it something like this? Or does it fail?
cef: { cs1: null, cs2: null, message: rfc5424, ......... 
There was a problem hiding this comment.
Looking at the associated SDH, it looks like it was producing empty strings, which was throwing off another processor. Unfortunately, since this involves a beats processor, we don't have a sample event we can look at like we can with a pipeline test.
@jrmolin, could you fill out the PR description explaining why this change is needed? Doesn't need to be extensive, just something brief. Helps reviewers know why this change is going in.
There was a problem hiding this comment.
@taylor-swanson Running elastic-package system test , can we not see what event.original / message field looks like in the pipeline? It should be the one processed from beats processors? Or do we bypass the agent config?
There was a problem hiding this comment.
Oh yes, you could do that, it's just a manual process rather than looking at the expected file in github.
There was a problem hiding this comment.
But yeah, I was just curious. But this is not a blocker comment @jrmolin .. But as Taylor mentioned please add some description about the change.
| Package cef - 2.21.1 containing this change is available at https://epr.elastic.co/package/cef/2.21.1/ |
* When the pipeline hits empty fields, it errors out * An update in the cef_processor allows for empty fields to be ignored, but this is not configurable in the settings * This updates the settings to add a configuration value to turn on/off the ability to ignore empty fields * A new test was added * No test regressions
* When the pipeline hits empty fields, it errors out * An update in the cef_processor allows for empty fields to be ignored, but this is not configurable in the settings * This updates the settings to add a configuration value to turn on/off the ability to ignore empty fields * A new test was added * No test regressions
* When the pipeline hits empty fields, it errors out * An update in the cef_processor allows for empty fields to be ignored, but this is not configurable in the settings * This updates the settings to add a configuration value to turn on/off the ability to ignore empty fields * A new test was added * No test regressions
* When the pipeline hits empty fields, it errors out * An update in the cef_processor allows for empty fields to be ignored, but this is not configurable in the settings * This updates the settings to add a configuration value to turn on/off the ability to ignore empty fields * A new test was added * No test regressions
6 participants
A user is noticing parsing failures with the cef processor that can be attributed to empty fields not being properly ignored.
Proposed commit message
[cef] Update the settings to ignore empty fields
Checklist
changelog.ymlfile.Author's Checklist
How to test this PR locally
Related issues
Screenshots