[Filebeat][Fortinet] Remove pre populated event.timezone #20273

marc-gr · 2020-07-28T13:46:28Z

What does this PR do?

Removes the pre populated event.timezone field.

Why is it important?

Some fortinet logs do not have a tz field to set as event.timezone, for this reason, when this happens, the event.timezone was incorrectly set to the system default instead of UTC or none (which represents UTC).

With this change event.timezone will only be set when the log has a timezone itself.

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.

How to test this PR locally

cd x-pack/filebeat
TESTING_FILEBEAT_MODULES=fortinet mage pythonIntegTest

elasticmachine · 2020-07-28T13:46:31Z

Pinging @elastic/siem (Team:SIEM)

elasticmachine · 2020-07-28T13:55:15Z

💚 Build Succeeded

Expand to view the summary

Build stats

Build Cause: [Pull request #20273 updated]
Start Time: 2020-07-29T11:17:01.239+0000
Duration: 50 min 56 sec

Test stats 🧪

Test	Results
Failed	0
Passed	2434
Skipped	385
Total	2819

andrewkroh

I think event.timezone is being populated by the module with the beats add_locale processor. So if the field needs to be removed then we should remove this processor so that the field is never set with the wrong value.

marc-gr · 2020-07-29T11:12:42Z

I think event.timezone is being populated by the module with the beats add_locale processor. So if the field needs to be removed then we should remove this processor so that the field is never set with the wrong value.

Good catch, will change it, thanks!

adriansr · 2020-07-30T08:18:45Z

So if I understand this correctly, Fortinet logs may or may not have a tz. If the tz is there, ingested dates will be relative to that. If it isn't, with this change, we'll assume dates are GMT. Before this change, dates were interpreted as local to the system running Filebeat.

I don't think this is a good or bad idea, just that we might find customers with different needs.

At some point we need to review how the modules behave regarding timezones and possibly add a common setting (var.tz or similar) to control how dates are interpreted when tz is not known.

marc-gr · 2020-07-30T08:56:46Z

So if I understand this correctly, Fortinet logs may or may not have a tz. If the tz is there, ingested dates will be relative to that. If it isn't, with this change, we'll assume dates are GMT. Before this change, dates were interpreted as local to the system running Filebeat.

Exactly that

I don't think this is a good or bad idea, just that we might find customers with different needs.

At some point we need to review how the modules behave regarding timezones and possibly add a common setting (var.tz or similar) to control how dates are interpreted when tz is not known.

I agree, there is this new ticket to add this possibility to fortinet #20300, maybe could be changed to extend this option more generally.

* Remove pre populated event.timezone * Add changelog entry * Remove processor instead of the field (cherry picked from commit b1b7860)

…0348) * Remove pre populated event.timezone * Add changelog entry * Remove processor instead of the field (cherry picked from commit b1b7860)

…0347) * Remove pre populated event.timezone * Add changelog entry * Remove processor instead of the field (cherry picked from commit b1b7860)

…ne-2.0 * upstream/master: [Elastic Agent] Add skeleton for client/server for agent control protocol (elastic#20163) Auditbeat: Allow multiple instances by grouping kprobes by PID (elastic#20325) [Filebeat][Fortinet] Remove pre populated event.timezone (elastic#20273)

…allation * upstream/master: Check expand_event_list_from_field when json in map[string]interface{} format (elastic#20370) [docs] Remove deprecated security roles (elastic#20162) Modify doc in app_insights metricset (elastic#20185) [Elastic Agent] Add skeleton for client/server for agent control protocol (elastic#20163) Auditbeat: Allow multiple instances by grouping kprobes by PID (elastic#20325) [Filebeat][Fortinet] Remove pre populated event.timezone (elastic#20273) Add an explicit system test for processes on unix systems (elastic#20320)

* Remove pre populated event.timezone * Add changelog entry * Remove processor instead of the field

…) (elastic#20347) * Remove pre populated event.timezone * Add changelog entry * Remove processor instead of the field (cherry picked from commit 61b0730)

marc-gr added bug review Team:SIEM labels Jul 28, 2020

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 28, 2020

marc-gr force-pushed the fix_fortinet_timezone branch from cd22e5a to 76b774a Compare July 28, 2020 14:18

andrewkroh requested changes Jul 29, 2020

View reviewed changes

marc-gr added 3 commits July 29, 2020 13:15

Remove pre populated event.timezone

8f1dc4c

Add changelog entry

538865c

Remove processor instead of the field

cae41b1

marc-gr force-pushed the fix_fortinet_timezone branch from 76b774a to cae41b1 Compare July 29, 2020 11:15

marc-gr requested a review from andrewkroh July 29, 2020 11:15

adriansr approved these changes Jul 30, 2020

View reviewed changes

andrewkroh approved these changes Jul 30, 2020

View reviewed changes

marc-gr merged commit b1b7860 into elastic:master Jul 30, 2020

marc-gr deleted the fix_fortinet_timezone branch July 30, 2020 12:31

marc-gr mentioned this pull request Jul 30, 2020

Cherry-pick #20273 to 7.9: [Filebeat][Fortinet] Remove pre populated event.timezone #20347

Merged

2 tasks

marc-gr added the v7.9.0 label Jul 30, 2020

marc-gr mentioned this pull request Jul 30, 2020

Cherry-pick #20273 to 7.x: [Filebeat][Fortinet] Remove pre populated event.timezone #20348

Merged

2 tasks

marc-gr added the v7.10.0 label Jul 30, 2020

melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020

[Filebeat][Fortinet] Remove pre populated event.timezone (elastic#20273)

6e1f18d

* Remove pre populated event.timezone * Add changelog entry * Remove processor instead of the field

111andre111 mentioned this pull request Nov 16, 2020

[Filebeat][Fortinet] Add the ability to set a default timezone in fortinet config #20300

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Filebeat][Fortinet] Remove pre populated event.timezone #20273

[Filebeat][Fortinet] Remove pre populated event.timezone #20273

marc-gr commented Jul 28, 2020 •

edited

Loading

elasticmachine commented Jul 28, 2020

elasticmachine commented Jul 28, 2020 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

andrewkroh left a comment

marc-gr commented Jul 29, 2020

adriansr commented Jul 30, 2020

marc-gr commented Jul 30, 2020

[Filebeat][Fortinet] Remove pre populated event.timezone #20273

[Filebeat][Fortinet] Remove pre populated event.timezone #20273

Conversation

marc-gr commented Jul 28, 2020 • edited Loading

What does this PR do?

Why is it important?

Checklist

How to test this PR locally

elasticmachine commented Jul 28, 2020

elasticmachine commented Jul 28, 2020 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

andrewkroh left a comment

Choose a reason for hiding this comment

marc-gr commented Jul 29, 2020

adriansr commented Jul 30, 2020

marc-gr commented Jul 30, 2020

marc-gr commented Jul 28, 2020 •

edited

Loading

elasticmachine commented Jul 28, 2020 •

edited by jenkins-beats-ci bot

Loading