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

[New Package] httpjson custom input #2154

Merged
merged 7 commits into from
Dec 14, 2021
Merged

Conversation

P1llus
Copy link
Member

@P1llus P1llus commented Nov 11, 2021

What does this PR do?

This PR adds a package to utilize the httpjson input with custom integration, similar to winlog and log packages.

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).

@elasticmachine
Copy link

Pinging @elastic/integrations (Team:Integrations)

@elasticmachine
Copy link

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

@P1llus
Copy link
Member Author

P1llus commented Nov 11, 2021

@ruflin @narph @andrewkroh
Initial implementation of the httpjson input package, a few things I would love to get extra feedback on is:

  1. Ensure the docs are written okay, and that the description of the manifest.yml items for the UI is good and understandable.
  2. The title/description of the package itself.
  3. I have added plenty of system tests, if you need any more just let me know.
  4. There is currently no icon for this, the custom log input package uses a generic icon from our marketing or EUI styling, but there is nothing similar to match this usecase, any ideas?

This integration works very similarly to winlog and log packages, and is one of the many new input packages coming up. Started with this one as it is the most requested one at the moment.

@P1llus P1llus requested review from a team November 11, 2021 16:50
@elasticmachine
Copy link

elasticmachine commented Nov 11, 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 preview

Expand to view the summary

Build stats

  • Start Time: 2021-12-14T17:15:13.239+0000

  • Duration: 15 min 1 sec

  • Commit: ed16f06

Test stats 🧪

Test Results
Failed 0
Passed 6
Skipped 0
Total 6

🤖 GitHub comments

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

  • /test : Re-trigger the build.


## Configuration

The extensive documentation for the input is currently available [here](https://www.elastic.co/guide/en/beats/filebeat/current/filebeat-input-httpjson.html).
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we move all the documentations over into integrations so we don't have to link to Filebeat here?

Copy link
Member Author

@P1llus P1llus Nov 11, 2021

Choose a reason for hiding this comment

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

@kaiyan-sheng Sure that's always a possibility as well, I was just unsure how large we would want the integration pages in fleet to be, as httpjson's documentation is large.

There is also lots of links to the httpjson documentation in the field descriptions of each option as well, but always up to discuss what is best :)

I personally think we should move it to live inside the integration as well.

@narph
Copy link
Contributor

narph commented Nov 15, 2021

@ruflin @narph @andrewkroh Initial implementation of the httpjson input package, a few things I would love to get extra feedback on is:

  1. Ensure the docs are written okay, and that the description of the manifest.yml items for the UI is good and understandable.
  2. The title/description of the package itself.
  3. I have added plenty of system tests, if you need any more just let me know.
  4. There is currently no icon for this, the custom log input package uses a generic icon from our marketing or EUI styling, but there is nothing similar to match this usecase, any ideas?

Code LGTM.

  1. I would add the configuration options directly in the documentation, I guess if anything changes in Filebeat and is not available just yet in the integration might raise some confusion.

  2. Would the JSON icon do the trick here?

@P1llus
Copy link
Member Author

P1llus commented Nov 16, 2021

@narph @kaiyan-sheng The place of the documentation is something we are still discussing internally, would you be able to see if there is anything else that should also change?
For example the wording of the UI field descriptions etc?
We might go ahead and merge this while discussing the decision on the documentation location, but that is still up to debate.

@P1llus
Copy link
Member Author

P1llus commented Nov 16, 2021

For documentation it will most likely have to wait for: elastic/observability-docs#424

packages/httpjson/_dev/build/docs/README.md Outdated Show resolved Hide resolved
packages/httpjson/_dev/build/docs/README.md Outdated Show resolved Hide resolved
packages/httpjson/_dev/build/docs/README.md Outdated Show resolved Hide resolved
@masci masci linked an issue Nov 19, 2021 that may be closed by this pull request
8 tasks
@P1llus
Copy link
Member Author

P1llus commented Nov 22, 2021

Going ahead with the plans for these input packages, until the documentation has been moved to the new elastic-agent location, we will reference the filebeat documentation equivalent when needed.
Once documentation has been copied over to its new location, we will simply update the links to reference the new location.
Unless there is more reviews or questions around this, we will go ahead and merge this

@jamiehynds
Copy link

jamiehynds commented Dec 8, 2021

@andrewkroh could you please review/approve this PR, to ensure that we can publish the httpjson package (in line with the udp/tcp packages that shipped in 7.16)? There is ongoing discussion on docs, but I don't think linking to Filebeat input docs should be a blocker. We can update the links once agent input docs are available (elastic/observability-docs#424)

packages/httpjson/_dev/build/docs/README.md Outdated Show resolved Hide resolved
packages/httpjson/_dev/build/docs/README.md Outdated Show resolved Hide resolved
P1llus and others added 3 commits December 14, 2021 18:13
Co-authored-by: Andrew Kroh <andrew.kroh@elastic.co>
Co-authored-by: Andrew Kroh <andrew.kroh@elastic.co>
@P1llus P1llus merged commit e7e6286 into elastic:master Dec 14, 2021
eyalkraft pushed a commit to build-security/integrations that referenced this pull request Mar 30, 2022
* initial commit

* update changelog and modify min version

* add fixes for 7.16 versions

* adding safer tags, ingest pipeline support and documentation fixes based on PR comments

* Update packages/httpjson/_dev/build/docs/README.md

Co-authored-by: Andrew Kroh <andrew.kroh@elastic.co>

* Update packages/httpjson/_dev/build/docs/README.md

Co-authored-by: Andrew Kroh <andrew.kroh@elastic.co>

* Update docs

Co-authored-by: Andrew Kroh <andrew.kroh@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Integration:httpjson Custom API New Integration Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create httpjson input package
8 participants