Skip to content

Conversation

@leehinman
Copy link
Contributor

What does this PR do?

Fixes bug in Third Party REST API ingest pipelines where a rename of
host.name would fail because it was already set. Also moves third
party api processing to a separate pipeline.

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.
    - [ ] If I'm introducing a new feature, I have modified the Kibana version constraint in my package's manifest.yml file to point to the latest Elastic stack release (e.g. ^7.13.0).

How to test this PR locally

need to install Splunk & ingest data into that, then configure these
integrations. Bug isn't visible with pipeline tests.

Related issues

@elasticmachine
Copy link

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

@leehinman leehinman added the bug Something isn't working, use only for issues label Jun 23, 2021
@leehinman leehinman force-pushed the 1146_splunk_pipeline branch from 2a8af38 to 2d1dcf5 Compare June 23, 2021 18:21
@elasticmachine
Copy link

elasticmachine commented Jun 23, 2021

💚 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: Pull request #1201 updated

  • Start Time: 2021-06-28T15:11:45.324+0000

  • Duration: 67 min 19 sec

  • Commit: b05f9c4

Test stats 🧪

Test Results
Failed 0
Passed 521
Skipped 0
Total 521

Trends 🧪

Image of Build Times

Image of Tests

Copy link
Member

@P1llus P1llus left a comment

Choose a reason for hiding this comment

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

LGTM, thanks a lot for this work, it cleans up some of the technical debt from my earlier changes.
Initially this was planned once a package could share pipelines, but I think it was good to do it already now!

Copy link
Member

Choose a reason for hiding this comment

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

Just a general question, I am quite sure that set always correctly overwrites fields right? Since event.original is already set at this point?

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct, set overwrites any previous value

- Apache - Zeek - Nginx - CloudTrail - move third party api processing to separate pipeline - convert rename to set so value can be overwritten Fixes elastic#1146
@leehinman leehinman force-pushed the 1146_splunk_pipeline branch from 2d1dcf5 to b05f9c4 Compare June 28, 2021 15:11
@leehinman leehinman merged commit 388cf4b into elastic:master Jun 28, 2021
@leehinman leehinman deleted the 1146_splunk_pipeline branch September 28, 2021 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working, use only for issues

4 participants