-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Support Fixed length extraction #17191
Support Fixed length extraction #17191
Conversation
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
1 similar comment
Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually? |
💚 CLA has been signed |
Hi @newly12, thank you for your contribution. In order to proceed with reviewing this PR and accepting your contribution, we first need to you to sign the Elastic Contributor License Agreement (CLA): https://www.elastic.co/contributor-agreement. Thanks! |
Pinging @elastic/integrations-services (Team:Services) |
@ycombinator, I will work with our internal legal team to have @newly12 added to our corporate CLA by EOD thursday. |
Sounds good, thanks @vjsamuel! |
@@ -30,7 +30,7 @@ an error; you need to either drop or rename the key before using dissect. | |||
For tokenization to be successful, all keys must be found and extracted, if one of them cannot be | |||
found an error will be logged and no modification is done on the original event. | |||
|
|||
NOTE: A key can contain any characters except reserved suffix or prefix modifiers: `/`,`&`, `+` |
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.
Could you also add the #
modifier to the table of modifiers further down in this same file? Also please add a section for fixed length extraction similar to sections for the other modifiers.
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.
@ycombinator I failed to find sections for modifier in this file, could you please point me to that? thx!
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.
I'm sorry, I was looking at the Elasticsearch dissect processor's documentation (https://www.elastic.co/guide/en/elasticsearch/reference/master/dissect-processor.html) when I made that comment. You're right, there is no such section in the Beat's dissect processor's documentation (https://www.elastic.co/guide/en/beats/filebeat/7.6/dissect.html). 🤦
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.
Thanks for the info. It is pretty good doc for dissect processor! It will be good to have similar doc in beats or a link to the modfier part..
Hi @newly12, thanks for this contribution! The code and tests look great; I just added a couple of comments about adding docs, etc. Also, please add an entry to the Thanks again! |
1cbbe63
to
1814167
Compare
Hi @ycombinator thanks a lot for reviewing this! I've updated |
Signed-off-by: Peter Deng <yundeng@ebay.com>
1814167
to
1b563cc
Compare
💔 Build FailedExpand to view the summary
Build stats
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
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.
LGTM. Thanks for the contribution, @newly12!
CI failures are unrelated. Merging. |
What does this PR do?
This PR allows dissect processor to do fixed-length field extraction.
Why is it important?
Before this PR dissect does not provide such ability.
Checklist
I have made corresponding changes to the documentationI have made corresponding change to the default configuration filesRelated issues
Fixes elastic/dissect-specification#6