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

Bug: Set netflow event.created to use current timestamp #23094

Merged
merged 2 commits into from
Jan 14, 2021
Merged

Bug: Set netflow event.created to use current timestamp #23094

merged 2 commits into from
Jan 14, 2021

Conversation

shanavas786
Copy link
Contributor

What does this PR do?

Set netflow event.created field to current time instead of using flow.Timestamp

Why is it important?

As per [https://github.com/elastic/ecs/blob/master/code/go/ecs/event.go#L163](the doc), event.created contains the date/time when the event was first read by an agent/pipeline.
But for netflow input type, it is being copied from flow.Timestamp field.

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.

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Dec 12, 2020
@cla-checker-service
Copy link

cla-checker-service bot commented Dec 12, 2020 •
edited
Loading

💚 CLA has been signed

@elasticmachine
Copy link
Collaborator

elasticmachine commented Dec 12, 2020 •
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

Expand to view the summary

Build stats

  • Build Cause: Started by user Adrian Serrano

    • Start Time: 2021-01-14T08:47:31.651+0000

  • Duration: 60 min 17 sec

  • Commit: 056da53

Test stats 🧪

Test Results
Failed 0
Passed 5129
Skipped 574
Total 5703

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 5129
Skipped 574
Total 5703

@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 Dec 13, 2020
andrewkroh
andrewkroh reviewed Jan 13, 2021
View reviewed changes
x-pack/filebeat/input/netflow/convert.go Outdated
@@ -65,7 +65,7 @@ func toBeatEventCommon(flow record.Record) (event beat.Event) {

// ECS Fields -- event
ecsEvent := common.MapStr{
"created": flow.Timestamp,
"created": time.Now(),
Copy link
Member

Choose a reason for hiding this comment

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

@adriansr @ebeahan What do you think about this change? The current flow.Timestamp is the time when the router emitted the record (distinct from the actual flow start/end times).

event.created contains the date/time when the event was first read by an agent, or by your pipeline.

This change would make the data consistent with other sources. On the other hand the time being used is the time when the event was created, except it's by the observer rather than agent.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this makes sense. Its clear that it wasn't following the ECS convention. Also currently we were duplicating this same information in @timestamp.

This field is distinct from @timestamp in that @timestamp typically contain the time extracted from the original event.

In most situations, these two timestamps will be slightly different. The difference can be used to calculate the delay between your source generating an event, and the time when your agent first processed it. This can be used to monitor your agent’s or pipeline’s ability to keep up with your event source.

This change will allow to perform the above calculation, so LGTM.

Copy link
Member

Choose a reason for hiding this comment

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

LGTM as well

@andrewkroh andrewkroh added the Filebeat Filebeat label Jan 13, 2021
@adriansr
Copy link
Contributor

/test

@andrewkroh
Copy link
Member

I pushed an update to remove event.created from the golden files so the tests can pass.

@adriansr
Copy link
Contributor

jenkins test this please

@adriansr
Copy link
Contributor

cla/check

@andrewkroh andrewkroh merged commit 6986c84 into elastic:master Jan 14, 2021
@andrewkroh andrewkroh mentioned this pull request Jan 14, 2021
Merged
6 tasks
andrewkroh pushed a commit to andrewkroh/beats that referenced this pull request Jan 14, 2021
@shanavas786 @andrewkroh
Set netflow event.created to use current timestamp (elastic#23094)
84846d7 
* Update event.created to use current timestamp

* Update golden files, add changelog

Co-authored-by: Andrew Kroh <andrew.kroh@elastic.co>
(cherry picked from commit 6986c84)
andrewkroh added a commit that referenced this pull request Jan 15, 2021
@andrewkroh @shanavas786
Set netflow event.created to use current timestamp (#23094) (#23520)
500e8c2 
* Update event.created to use current timestamp

* Update golden files, add changelog

Co-authored-by: Andrew Kroh <andrew.kroh@elastic.co>
(cherry picked from commit 6986c84)

Co-authored-by: Shanavas M <shanavas.m2@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ebeahan ebeahan ebeahan left review comments

@andrewkroh andrewkroh andrewkroh left review comments

@adriansr adriansr adriansr approved these changes

Assignees
No one assigned
Labels
Filebeat Filebeat v7.12.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@shanavas786 @elasticmachine @adriansr @andrewkroh @ebeahan @andresrc