Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[Filebeat][httpjson] httpjson chain calls #29816
[Filebeat][httpjson] httpjson chain calls #29816
Changes from 19 commits
07e006a
d062e26
664d95b
e288c0d
27e71a2
24b1e87
28daafc
915bc6d
ea115bb
1dfb2b0
d42d505
3aa2a11
5b9e7fe
19694db
6e9170d
7386e91
d8e5327
7975a49
df162e9
f43d410
f59e8a0
844bd79
c64dc1d
60e5c14
1534361
d7df3c4
879b945
6563fc6
c11c048
616415b
d55af74
a0a5d94
6739df7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs contain a sequence diagram that explains the high level flow. I think the chain feature should be added to the diagram.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure the example is too clear, is it referencing that the value can or should be different for every chain step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, As value can be different. user can mentioned different calls in specified way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this always required? Can there be situations were a step does not expect a replaceable part?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, It's always required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe would be good to have this examples in the docs if they are different from the current ones
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, They are same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Question]
chainConfig
only contains a single fieldstepConfig
, is it really needed? Are they both used so the final yaml looks like that:Maybe we could use the
stepConfig
directly:I believe it would result in a yaml like this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is related to #29816 (comment). Changing type would be better than just documenting the situation, though does this preclude extending the computation model in the future, and if it does, does that matter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, we thought of that but then decided to go with using a step to make it easy in the future to extend it to add some more processing in-between steps whenever required, for example: