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

[Filebeat][httpjson] httpjson chain calls #29816

Merged
merged 33 commits into from
Mar 29, 2022

Conversation

kush-elastic
Copy link
Collaborator

@kush-elastic kush-elastic commented Jan 12, 2022

  • Enhancement

What does this PR do?

  • This PR contains code for extending httpjson to run chain calls and collect information from multiple urls.
  • This httpjson code enables users to perform chains in a single manner.

Why is it important?

  • Current filebeat httpjson input doesn't support chain calls.
    For Example:
    Salesforce: To collect eventLogFile from salesforce, we first need to collect logfile ids from Salesforce REST API and then use those logfile ids to collect more information from Salesforce.

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.

Author's Checklist

  • Check with Salesforce REST API to collect EventLogFile.

Related issues

Use cases

  • Configure chain parameters with each chain in step parameter.

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jan 12, 2022
@mergify
Copy link
Contributor

mergify bot commented Jan 12, 2022

This pull request does not have a backport label. Could you fix it @kush-elastic? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@kush-elastic kush-elastic requested a review from mtojek January 12, 2022 13:34
@mergify mergify bot added the backport-skip Skip notification from the automated backport with mergify label Jan 12, 2022
@kush-elastic kush-elastic requested a review from belimawr January 12, 2022 13:35
@kush-elastic kush-elastic self-assigned this Jan 12, 2022
@kush-elastic kush-elastic requested a review from marc-gr January 12, 2022 13:39
@elasticmachine
Copy link
Collaborator

elasticmachine commented Jan 12, 2022

💚 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 preview

Expand to view the summary

Build stats

  • Start Time: 2022-03-29T09:43:29.351+0000

  • Duration: 59 min 4 sec

Test stats 🧪

Test Results
Failed 0
Passed 2044
Skipped 159
Total 2203

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@mergify
Copy link
Contributor

mergify bot commented Jan 12, 2022

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b salesforce_httpjson_chain_calls upstream/salesforce_httpjson_chain_calls
git merge upstream/master
git push upstream salesforce_httpjson_chain_calls

@mtojek mtojek added the Team:Integrations Label for the Integrations team label Jan 12, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations (Team:Integrations)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jan 12, 2022
@mtojek mtojek added needs_team Indicates that the issue/PR needs a Team:* label Team:Security-External Integrations labels Jan 12, 2022
@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 Jan 12, 2022
@botelastic
Copy link

botelastic bot commented Jan 12, 2022

This pull request doesn't have a Team:<team> label.

@kush-elastic kush-elastic changed the title Salesforce httpjson chain calls [Filebeat][httpjson] httpjson chain calls Jan 18, 2022
@mtojek mtojek added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Jan 18, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@kush-elastic kush-elastic requested a review from a team as a code owner March 29, 2022 08:03
@kush-elastic kush-elastic requested review from efd6 and removed request for a team March 29, 2022 08:04
@mtojek mtojek requested a review from ruflin March 29, 2022 08:41
@@ -976,9 +1117,18 @@ image:images/input-httpjson-lifecycle.png[Request lifecycle]
. The resulting transformed request is executed.
. The server responds (here is where any retry or rate limit policy takes place when configured).
. The response is transformed using the configured `response.transforms` and `response.split`.
. If chain step is configured. step will generate new requests based on collected ids from responses. the requests will be transformed using configured `request.transforms`. resulting generated transformed requests will be executed. this process will happen for all the steps mentioned in chain.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
. If chain step is configured. step will generate new requests based on collected ids from responses. the requests will be transformed using configured `request.transforms`. resulting generated transformed requests will be executed. this process will happen for all the steps mentioned in chain.
. If a chain step is configured. Each step will generate new requests based on collected IDs from responses. The requests will be transformed using configured `request.transforms` and the resulting generated transformed requests will be executed. This process will happen for all the steps mentioned in the chain.

Comment on lines 1129 to 1130
. go back to step-2 for the next step.
. publish collected responses from the last chain step.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
. go back to step-2 for the next step.
. publish collected responses from the last chain step.
. Go back to step-2 for the next step.
. Publish collected responses from the last chain step.

setupServer: newChainTestServer(httptest.NewServer),
baseConfig: map[string]interface{}{
"interval": 10,
"request.method": "GET",
Copy link
Contributor

Choose a reason for hiding this comment

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

These instances of "GET" should also be changed to http.MethodGet (throughout this file).

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.

LGTM, but please wait for others.

@marc-gr
Copy link
Contributor

marc-gr commented Mar 29, 2022

/test

@mtojek mtojek merged commit 2ec07f5 into elastic:main Mar 29, 2022
mergify bot pushed a commit that referenced this pull request Mar 29, 2022
* [filebeat][httpjson] Extend filebeat httpjson input for chained calls

* Updates based on comments

* Update on httpjson and nits

* Resolve comments from Tiago

* address comments and improved json_parser

* Add entry to CHANGELOG.next.asciidoc

* mage update and mage check for ci linting

* address comments

* Addressed comments

* remove else as we have continue, close resp.Body in id collection mechanism and update json_parser to convert any type values to string

* nits

* mage check

* Changes requested from elastic team

* requested changes on logging level

* requested changes for ids collection

* Requested changes on input-httpjson.asciidoc and added tests for chain in input_test.go

* Added regular tests with chain

* Changes based on CI

* Add New diagram for chain feature and updates based on CI changes

* CI linter problem

* removed unnecessary ci changes

* golangci eventsCh->events

* ignore few CI errors

* update GET to http.MethodGet

* Review changes- GET->http.MethodGet and nits

(cherry picked from commit 2ec07f5)

# Conflicts:
#	x-pack/filebeat/input/httpjson/pagination.go
#	x-pack/filebeat/input/httpjson/request.go
#	x-pack/filebeat/input/httpjson/response.go
@rdner
Copy link
Member

rdner commented Mar 29, 2022

@mtojek merging this PR might have broken the documentation, see https://elasticsearch-ci.elastic.co/blue/organizations/jenkins/elastic%2Bbeats%2Bpull-request%2Bbuild-docs/detail/elastic%20beats%20pull-request%20build-docs/13929/pipeline/

it's hard to tell the exact cause but at least I see these warnings

INFO:build_docs:asciidoctor: WARNING: ../../x-pack/filebeat/docs/inputs/input-httpjson.asciidoc: line 1042: id assigned to block already in use: response-json1
INFO:build_docs:asciidoctor: WARNING: invalid reference: response-json2

@szabosteve
Copy link
Contributor

szabosteve commented Mar 29, 2022

Hi @mtojek, @kush-elastic,

The first issue is caused by this line: https://github.com/elastic/beats/pull/29816/files#diff-531162d76f10990e57691d2298a46f2aa2a4e576a8ec0d17dd1a37c35303983fR1040 (as the ID is already in use).
The second one is caused by this: https://github.com/elastic/beats/pull/29816/files#diff-531162d76f10990e57691d2298a46f2aa2a4e576a8ec0d17dd1a37c35303983fR977 (response-json2 is an invalid reference)

@mtojek mtojek mentioned this pull request Mar 29, 2022
@mtojek mtojek added backport-v8.2.0 Automated backport with mergify and removed backport-v8.1.0 Automated backport with mergify labels Mar 29, 2022
@ruflin
Copy link
Contributor

ruflin commented Mar 30, 2022

@mtojek Why is this planned to be backported to 8.2? This is not a bug fix.

@ruflin ruflin removed the backport-v8.2.0 Automated backport with mergify label Mar 30, 2022
@mtojek
Copy link
Contributor

mtojek commented Mar 30, 2022

@ruflin isn't after the feature freeze now?

@dedemorton
Copy link
Contributor

dedemorton commented Mar 31, 2022

Sorry, I missed the review because I was on PTO. :/ Let's not worry too much about the documentation structure right now. We'll be moving to a new system and need to do some refactoring/cleanup as part of the migration, so let's handle layout issues then.

emilioalvap pushed a commit to emilioalvap/beats that referenced this pull request Apr 6, 2022
* [filebeat][httpjson] Extend filebeat httpjson input for chained calls

* Updates based on comments

* Update on httpjson and nits

* Resolve comments from Tiago

* address comments and improved json_parser

* Add entry to CHANGELOG.next.asciidoc

* mage update and mage check for ci linting

* address comments

* Addressed comments

* remove else as we have continue, close resp.Body in id collection mechanism and update json_parser to convert any type values to string

* nits

* mage check

* Changes requested from elastic team

* requested changes on logging level

* requested changes for ids collection

* Requested changes on input-httpjson.asciidoc and added tests for chain in input_test.go

* Added regular tests with chain

* Changes based on CI

* Add New diagram for chain feature and updates based on CI changes

* CI linter problem

* removed unnecessary ci changes

* golangci eventsCh->events

* ignore few CI errors

* update GET to http.MethodGet

* Review changes- GET->http.MethodGet and nits
kush-elastic added a commit to kush-elastic/beats that referenced this pull request May 2, 2022
* [filebeat][httpjson] Extend filebeat httpjson input for chained calls

* Updates based on comments

* Update on httpjson and nits

* Resolve comments from Tiago

* address comments and improved json_parser

* Add entry to CHANGELOG.next.asciidoc

* mage update and mage check for ci linting

* address comments

* Addressed comments

* remove else as we have continue, close resp.Body in id collection mechanism and update json_parser to convert any type values to string

* nits

* mage check

* Changes requested from elastic team

* requested changes on logging level

* requested changes for ids collection

* Requested changes on input-httpjson.asciidoc and added tests for chain in input_test.go

* Added regular tests with chain

* Changes based on CI

* Add New diagram for chain feature and updates based on CI changes

* CI linter problem

* removed unnecessary ci changes

* golangci eventsCh->events

* ignore few CI errors

* update GET to http.MethodGet

* Review changes- GET->http.MethodGet and nits
chrisberkhout pushed a commit that referenced this pull request Jun 1, 2023
* [filebeat][httpjson] Extend filebeat httpjson input for chained calls

* Updates based on comments

* Update on httpjson and nits

* Resolve comments from Tiago

* address comments and improved json_parser

* Add entry to CHANGELOG.next.asciidoc

* mage update and mage check for ci linting

* address comments

* Addressed comments

* remove else as we have continue, close resp.Body in id collection mechanism and update json_parser to convert any type values to string

* nits

* mage check

* Changes requested from elastic team

* requested changes on logging level

* requested changes for ids collection

* Requested changes on input-httpjson.asciidoc and added tests for chain in input_test.go

* Added regular tests with chain

* Changes based on CI

* Add New diagram for chain feature and updates based on CI changes

* CI linter problem

* removed unnecessary ci changes

* golangci eventsCh->events

* ignore few CI errors

* update GET to http.MethodGet

* Review changes- GET->http.MethodGet and nits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team Team:Integrations Label for the Integrations team v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.