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

[Infoblox NIOS] Update timestamp parsing logic #8767

Merged
merged 6 commits into from Dec 21, 2023

Conversation

kcreddy
Copy link
Contributor

@kcreddy kcreddy commented Dec 20, 2023

Proposed commit message

There are 2 style of log formats (regards to how timestamps are handled).

  1. <29>Mar 21 09:53:51 infoblox.localdomain httpd[]: 2022-03-18 13:24:41.705Z [admin]: ........
  2. <30>Mar 27 08:32:59 infoblox.localdomain dhcpd[2567]: ............

Event 1: Contains 2 timestamps inside the log. In current default pipeline, the first timestamp Mar 21 09:53:51 is set as event.created and then individual pipelines sets second timestamp as @timestamp if present.
Event 2: Contains 1 timestamp inside the log. In current default pipeline, the only timestamp Mar 27 08:32:59 is set as event.created and a set processor copies that into @timestamp if @timestamp is not already present from individual pipelines.

But, when Logstash is involved in between Agent and Elasticsearch, it adds @timestamp to the event before ingest pipeline. Hence, the logs format 2, i.e., containing single timestamp, skips the set processor leading to incorrect @timestamp values and also not conforming to ECS norm @timestamp < event.created < event.ingested.

This PR fixes the problem by:

  1. Modifying individual pipelines to not set @timestamp inside them.
  2. Set override: true in the default pipeline, thus overriding the value of @timestamp with event's timestamp.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

How to test this PR locally

elastic-package stack down && elastic-package build && elastic-package stack up --version=8.11.0 -d -v --services=elasticsearch && eval "$(elastic-package stack shellinit)" && elastic-package test pipeline --generate -v

Run pipeline tests for the package
--- Test results for package: infoblox_nios - START ---
╭───────────────┬─────────────┬───────────┬────────────────┬────────┬──────────────╮
│ PACKAGE       │ DATA STREAM │ TEST TYPE │ TEST NAME      │ RESULT │ TIME ELAPSED │
├───────────────┼─────────────┼───────────┼────────────────┼────────┼──────────────┤
│ infoblox_nios │ log         │ pipeline  │ test-audit.log │ PASS   │  12.847542ms │
│ infoblox_nios │ log         │ pipeline  │ test-dhcp.log  │ PASS   │  16.714875ms │
│ infoblox_nios │ log         │ pipeline  │ test-dns.log   │ PASS   │  10.268166ms │
╰───────────────┴─────────────┴───────────┴────────────────┴────────┴──────────────╯
--- Test results for package: infoblox_nios - END   ---
Done

@elasticmachine
Copy link

elasticmachine commented Dec 20, 2023

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

@kcreddy kcreddy marked this pull request as ready for review December 21, 2023 05:45
@kcreddy kcreddy requested a review from a team as a code owner December 21, 2023 05:45
@elasticmachine
Copy link

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

Copy link
Contributor

@efd6 efd6 left a comment

Choose a reason for hiding this comment

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

AFAICS the ordering of the timestamps is coming from the structure of the event. Is this the case? If so, I think it would be helpful to make this clear in the commit message. If not, can you explain the logic more clearly?

Comment on lines 35 to 37
- remove:
field: event.created
ignore_missing: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this deletion also happen in the next date processor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've made logic bit clearer and this is irrelevant now.

@kcreddy
Copy link
Contributor Author

kcreddy commented Dec 21, 2023

AFAICS the ordering of the timestamps is coming from the structure of the event. Is this the case? If so, I think it would be helpful to make this clear in the commit message. If not, can you explain the logic more clearly?

I've made commit message more clearer. Let me know if that explains the issue. Thanks!

@kcreddy kcreddy merged commit 8ca582e into elastic:main Dec 21, 2023
3 checks passed
@elasticmachine
Copy link

Package infoblox_nios - 1.19.3 containing this change is available at https://epr.elastic.co/search?package=infoblox_nios

v1v added a commit that referenced this pull request Dec 21, 2023
* upstream/main: (117 commits)
  [TI MISP] Add IOC expiration support (#8639)
  Add CSPM Rules 6.2, 6.3 and 6.4 (#8778)
  [Infoblox NIOS] Update timestamp parsing logic (#8767)
  [Rapid7 InsightVM] Split vulnerability categories into array (#8768)
  [Exchange Online Message Trace] Add Additional Look-back Time & Fix Cursor Value (#8717)
  [Buildkite] Update bucket settings (#8765)
  Remove Jenkins .ci folder (#8766)
  First part of removal of Jenkins jobs (#8763)
  misp: parse URIs for URI type threats (#8760)
  [amazon_security_lake] Added support for all the OCSF Classes (#8579)
  [Buildkite] Update settings for integrations pipeline (#8758)
  [TI ThreatQ] Add IOC expiration support (#8691)
  [ti_opencti] Support OpenCTI 5.12 by removing filters parameter (#8744)
  [Cribl] Updating setup guidance for Cribl field (#8746)
  crowdstrike: add userinfo enrichment support and map fields to ECS (#8742)
  [etcd] Enable TSDB for metrics datastream (#8649)
  Bump golang.org/x/crypto from 0.16.0 to 0.17.0 (#8749)
  auditd: relax field_split pattern and handle AVC header (#8748)
  Update cloud packages codeowner (#8672)
  [O11Y] [AWS Billing] Convert "Total Estimated Charges" visualization to new metric (#8509)
  ...
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.

None yet

3 participants