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

Fallback on LevelRaw If the Level is not in the RenderingInfo section of the event #4257

Merged
merged 3 commits into from
May 10, 2017

Conversation

LucasArona
Copy link

@elasticmachine
Copy link
Collaborator

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

1 similar comment
@elasticmachine
Copy link
Collaborator

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

@ruflin ruflin requested a review from andrewkroh May 9, 2017 09:54
@ruflin ruflin added the needs_backport PR is waiting to be backported to other branches. label May 9, 2017
Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

Thanks for contributing! The changes look good. I left some minor comments.

@@ -216,6 +217,11 @@ func (l *winEventLog) buildRecordFromXML(x []byte, recoveredErr error) (Record,
e.RenderErr = recoveredErr.Error()
}

if e.Level == "" {
//Let's fallback on LevelRaw if the level is not set in the RenderingInfo
Copy link
Member

Choose a reason for hiding this comment

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

Could you please change this to

// Fallback on LevelRaw if the Level is not set in the RenderingInfo.

@@ -11,6 +11,7 @@ import (
"github.com/elastic/beats/libbeat/common"
"github.com/elastic/beats/libbeat/logp"
"github.com/elastic/beats/winlogbeat/sys"
"github.com/elastic/beats/winlogbeat/sys/wineventlog"
Copy link
Member

Choose a reason for hiding this comment

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

For consistency, could you use the "aliased" import below for this package. It's just aliased to win.

@andrewkroh andrewkroh merged commit c18adce into elastic:5.4 May 10, 2017
andrewkroh pushed a commit to andrewkroh/beats that referenced this pull request Jul 19, 2017
…event (elastic#4257)

If the Level is not in the RenderingInfo section of the event, fallback on the raw level. Applies to Windows Vista and above only.

https://discuss.elastic.co/t/event-fields-missing-if-renderinginfo-is-empty/84709

(cherry picked from commit c18adce)
@andrewkroh andrewkroh removed the needs_backport PR is waiting to be backported to other branches. label Jul 19, 2017
andrewkroh pushed a commit to andrewkroh/beats that referenced this pull request Jul 19, 2017
…event (elastic#4257)

If the Level is not in the RenderingInfo section of the event, fallback on the raw level. Applies to Windows Vista and above only.

https://discuss.elastic.co/t/event-fields-missing-if-renderinginfo-is-empty/84709

(cherry picked from commit c18adce)
andrewkroh pushed a commit to andrewkroh/beats that referenced this pull request Jul 19, 2017
…event (elastic#4257)

If the Level is not in the RenderingInfo section of the event, fallback on the raw level. Applies to Windows Vista and above only.

https://discuss.elastic.co/t/event-fields-missing-if-renderinginfo-is-empty/84709

(cherry picked from commit c18adce)
tsg pushed a commit that referenced this pull request Jul 20, 2017
…event (#4257) (#4713)

If the Level is not in the RenderingInfo section of the event, fallback on the raw level. Applies to Windows Vista and above only.

https://discuss.elastic.co/t/event-fields-missing-if-renderinginfo-is-empty/84709

(cherry picked from commit c18adce)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants