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] Make httpjson use cursor input when using date cursor #20751

Merged
merged 13 commits into from
Sep 29, 2020

Conversation

marc-gr
Copy link
Contributor

@marc-gr marc-gr commented Aug 24, 2020

What does this PR do?

Creates a custom input manager that will initialize a cursor input manager whenever a date cursor is configured.

Why is it important?

It is a recurrent requirement to keep state between beat restarts for the httpjson input. With this change, whenever a cursor is set up, the state will be kept.

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.

Related issues

Relates to #19071 #19486

@marc-gr marc-gr added enhancement in progress Pull request is currently in progress. Filebeat Filebeat Team:SIEM v7.10.0 labels Aug 24, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/siem (Team:SIEM)

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Aug 24, 2020
@elasticmachine
Copy link
Collaborator

elasticmachine commented Aug 24, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #20751 updated]

  • Start Time: 2020-09-29T10:46:01.907+0000

  • Duration: 105 min 37 sec

Test stats 🧪

Test Results
Failed 0
Passed 20305
Skipped 1858
Total 22163

@marc-gr marc-gr force-pushed the httpjson_cursor branch 2 times, most recently from aea7847 to c0560a3 Compare August 31, 2020 10:06
@marc-gr marc-gr requested a review from a team September 4, 2020 12:55
@marc-gr marc-gr added review and removed in progress Pull request is currently in progress. labels Sep 8, 2020
@@ -67,7 +67,8 @@ func (o *OAuth2) IsEnabled() bool {

// Client wraps the given http.Client and returns a new one that will use the oauth authentication.
func (o *OAuth2) Client(ctx context.Context, client *http.Client) (*http.Client, error) {
Copy link

Choose a reason for hiding this comment

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

Where do we need this type? If the implementation is internal to the httpjson package, why not keep it in the httpjson package and unexport it (implementation detail). If this type (and corresponding types/values) are more generic, consider to move it to the libbeat/common/transport package (or a sub-package).

@@ -23,10 +25,10 @@ type dateCursor struct {
dateFormat string

value string
valueTpl *Template
valueTpl *template.Template
Copy link

@urso urso Sep 14, 2020

Choose a reason for hiding this comment

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

this type mixes the computation/extraction of the cursor and the actual data. In case we want to store (or pass around) the data, consider to split the actual data (in the case 'value'), such that the 'computation' becomes immutable.

Edit: looks like cursorState is already the state type we want to have separated.

@marc-gr marc-gr merged commit 8f9d54b into elastic:master Sep 29, 2020
@marc-gr marc-gr deleted the httpjson_cursor branch September 29, 2020 14:20
v1v added a commit to v1v/beats that referenced this pull request Sep 30, 2020
…ci-build-label-support

* upstream/master:
  [JJBB] Set shallow cloning to 10 (elastic#21409)
  docs: add link to release notes for 7.9.2 (elastic#21405) (elastic#21419)
  docs: Prepare Changelog for 7.9.2 (elastic#21229) (elastic#21403)
  fix: mark flaky tests (elastic#21300)
  fix: use a fixed version of setuptools (elastic#21393)
  Move Kubernetes events metricset to its own block in reference config (elastic#21407)
  [libbeat] Enable WriteAheadLimit in the disk queue (elastic#21391)
  docs: fix apt/yum formatting (elastic#21362)
  Fix shutdown tracking in s3 input (elastic#21380)
  [libbeat] Fix position writing in the disk queue
  Add UBI 8 image to the dependencies report (elastic#21374)
  Fix debug message to show actual SQS message ID (elastic#20614)
  [Elastic Agent] Rename *ConfigChange to PolicyChange (elastic#20779)
  [Elastic Agent] Add install/uninstall sub-command (elastic#21206)
  [Filebeat][httpjson] Make httpjson use cursor input when using date cursor (elastic#20751)
  feat: prepare release pipelines (elastic#21238)
  Add IP validation to Security module (elastic#21325)
marc-gr added a commit to marc-gr/beats that referenced this pull request Oct 1, 2020
…ursor (elastic#20751)

* Fix duplicate import

* Move config to its own package

* Minor improvements

* Fix tests

* Create input manager

* Change requester to accept and store a cursor

* Modify input to be embedded

* Create stateless and cursor inputs

* Initialize new input manager on publish

* Add changelog entry and format files

* Move test data folder

* Change tests

* Apply requested changes

(cherry picked from commit 8f9d54b)
marc-gr added a commit that referenced this pull request Oct 2, 2020
…ursor (#20751) (#21384)

* Fix duplicate import

* Move config to its own package

* Minor improvements

* Fix tests

* Create input manager

* Change requester to accept and store a cursor

* Modify input to be embedded

* Create stateless and cursor inputs

* Initialize new input manager on publish

* Add changelog entry and format files

* Move test data folder

* Change tests

* Apply requested changes

(cherry picked from commit 8f9d54b)
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.

4 participants