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.

Sign up for GitHub

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

Jump to bottom

New processor: urldecode #17505

Merged
merged 9 commits into from
Apr 7, 2020
Merged

New processor: urldecode #17505

merged 9 commits into from
Apr 7, 2020

Conversation

sincejune
Copy link
Contributor

What does this PR do?

This PR introduces a new processor urldecode. To decode fields from URL encoded format use this processor with urldecode.

Why is it important?

This PR makes Filebeat can decode URL-encoded strings itself and then write to different outputs.

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.

@sincejune sincejune requested review from a team as code owners April 5, 2020 18:03
@elasticmachine
Copy link
Collaborator

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
@elasticmachine
Copy link
Collaborator

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?

@andresrc andresrc added [zube]: Inbox Team:Services (Deprecated) Label for the former Integrations-Services team labels Apr 6, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-services (Team:Services)

@andresrc andresrc assigned urso and unassigned urso Apr 6, 2020
@ycombinator ycombinator self-requested a review April 6, 2020 14:44
@ycombinator
Copy link
Contributor

Note to self (for review): Look at behavior of ES ingest node urldecode processor: https://www.elastic.co/guide/en/elasticsearch/reference/current/urldecode-processor.html.

ycombinator
ycombinator requested changes Apr 6, 2020
View reviewed changes
Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution, @sincejune! This PR is looking quite good; I just left a few nitpicks/minor comments.

@ycombinator
Copy link
Contributor

I just noticed that this processor is placed under the actions package. Would you mind moving it to it's own package like we have done for some of the newer processors (see add_id or fingerprint as examples)?

@sincejune
Copy link
Contributor Author

sincejune commented Apr 7, 2020 •
edited
Loading

Thanks @ycombinator , I moved urldecode processor into libbeat/processors/urldecode/ in 9e94570, but looks like the travis-ci didn't test the commit.

(And I re-requested the review from you, accidentally removed elastic/siem. Could you help me add them?)

@sincejune sincejune removed request for a team April 7, 2020 08:19
@ycombinator ycombinator requested a review from a team April 7, 2020 09:34
ycombinator
ycombinator requested changes Apr 7, 2020
View reviewed changes
Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

A couple more minor suggestions after the move to the new package. After that, I'll approve this PR.

ycombinator
ycombinator approved these changes Apr 7, 2020
View reviewed changes
Copy link
Contributor

@ycombinator ycombinator left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM. I'll merge it once CI looks good.

Thanks for the contribution, @sincejune!

@ycombinator
Copy link
Contributor

CI failures are unrelated. Merging.

@ycombinator ycombinator merged commit 96c3018 into elastic:master Apr 7, 2020
@ycombinator ycombinator mentioned this pull request Apr 7, 2020
Merged
@sincejune
Copy link
Contributor Author

Thanks for helping this, @ycombinator !

@sincejune sincejune deleted the feature/add-urldecode-processor branch April 8, 2020 07:26
@sincejune sincejune mentioned this pull request Apr 8, 2020
Merged
6 tasks
ycombinator added a commit that referenced this pull request Apr 13, 2020
@ycombinator @sincejune
[7.x] New processor: urldecode (#17505) (#17589)
d8c2f32 
* New processor: urldecode (#17505)

* add urldecode processor

* update reference yml files

* update doc with PR number

* remove unexpected format

* update from feedback

* move urldecode processor into its own package

* update from feedback

* CHANGELOG grooming

Co-authored-by: Chao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ycombinator ycombinator ycombinator approved these changes

Assignees
No one assigned
Labels
libbeat :Processors Team:Services (Deprecated) Label for the former Integrations-Services team v7.8.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sincejune @elasticmachine @ycombinator @urso @andresrc