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

Implement a first basic pipeline string parser for the currently existing pipeline operators #2877

Merged
merged 43 commits into from Jan 27, 2023

Conversation

Dakostu
Copy link
Member

@Dakostu Dakostu commented Jan 19, 2023

This PR adds the ability to parse & execute the currently implemented pipeline operators in the command line, via vast export <pipeline>.

Fixes tenzir/issues#6

TODO:

  • unit tests
  • integration tests
  • syntax cleanup; we can maybe "globalize" e.g. extractor rules
  • if slated for 3.0 release: changelog item saying this is experimental & one-liner in docu
    - [ ] if slated for 3.1 release: changelog item & docu

@Dakostu Dakostu added the feature New functionality label Jan 19, 2023
@Dakostu Dakostu requested a review from a team January 19, 2023 13:22
@dominiklohmann dominiklohmann requested review from patszt and removed request for a team January 23, 2023 08:33
@Dakostu Dakostu marked this pull request as ready for review January 24, 2023 09:57
libvast/src/plugin.cpp Outdated Show resolved Hide resolved
@Dakostu Dakostu requested a review from patszt January 24, 2023 14:24
@patszt
Copy link
Contributor

patszt commented Jan 25, 2023

Thanks for the good test coverage. I think the only code related issue is Dominik's suggestion about the return value from the option_set parse method.

Copy link
Member

@dominiklohmann dominiklohmann left a comment

Choose a reason for hiding this comment

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

Mostly minor things for the docs and some inline notes in the code. I didn't review the parsers in-depth. This is a great piece of work, and I'm really looking forward to use this every day.

libvast/src/pipeline.cpp Outdated Show resolved Hide resolved
libvast/src/system/spawn_exporter.cpp Outdated Show resolved Hide resolved
libvast/include/vast/concept/parseable/vast/option_set.hpp Outdated Show resolved Hide resolved
web/docs/understand/query-language/operators/replace.md Outdated Show resolved Hide resolved
web/docs/understand/query-language/operators/select.md Outdated Show resolved Hide resolved
web/docs/understand/query-language/operators/summarize.md Outdated Show resolved Hide resolved
web/docs/understand/query-language/operators/summarize.md Outdated Show resolved Hide resolved
web/docs/understand/query-language/pipelines.md Outdated Show resolved Hide resolved
Copy link
Contributor

@patszt patszt left a comment

Choose a reason for hiding this comment

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

Just a minor comment with regards to initialization. Great work!

libvast/include/vast/concept/parseable/vast/option_set.hpp Outdated Show resolved Hide resolved
@tobim
Copy link
Member

tobim commented Jan 26, 2023

@Dakostu would you mind squashing your commits into a smaller number of logical units? The PR consists of 78 commits now and many of them are small fixups. I understand it is already approved, but doing such a final pass will make it easier to navigate history should it become necessary.

patszt and others added 3 commits January 26, 2023 14:49
Implement string parsing for pseudonymize pipeline operator

Implement string parsing for extend pipeline operator

Implement string parsing for replace pipeline operator

Implement string parsing for rename pipeline operator

Allow aggregate pipelines from VAST nodes (quick hacky way to allow summarize operator)

Implement parsing for summarize pipeline operator

Implement aggregation function name assignment in summarize operator
@Dakostu Dakostu force-pushed the topic/expose-pipelines-in-export branch from 043243d to 52b42a7 Compare January 26, 2023 14:36
Add pipeline parsing unit tests

Add first batch of pipeline string parsing unit tests

Add unit test 'pipeline string parsing - options - operator with wrong short form option'

Implement unit test 'pipeline string parsing - options - operator with wrong long form option'

Implement simple pipeline parsers in new file parseable/vast/pipeline.hpp

Add unit test 'option set - missing option value'
Add integration test 'Pipeline operator parsing only summarize'

Add 'identity' to pipeline integration test

Add integration test 'Pipeline operator parsing after expression'

Add integration test 'Pipeline operator summarize after expression'

Add -b option to integration tests that need blocking import
@Dakostu Dakostu force-pushed the topic/expose-pipelines-in-export branch from 52b42a7 to 7d5b455 Compare January 26, 2023 14:39
@Dakostu
Copy link
Member Author

Dakostu commented Jan 26, 2023

@Dakostu would you mind squashing your commits into a smaller number of logical units? The PR consists of 78 commits now and many of them are small fixups. I understand it is already approved, but doing such a final pass will make it easier to navigate history should it become necessary.

I squashed the number down to 38.

Copy link
Member

@dominiklohmann dominiklohmann left a comment

Choose a reason for hiding this comment

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

I only looked at the last commit fixing the handling of the query language plugin. All comments I've left are trivial to address, so feel free to just resolve them after.

libvast/include/vast/system/spawn_arguments.hpp Outdated Show resolved Hide resolved
libvast/src/system/spawn_counter.cpp Outdated Show resolved Hide resolved
libvast/src/system/spawn_pivoter.cpp Outdated Show resolved Hide resolved
libvast/src/system/make_source.cpp Outdated Show resolved Hide resolved
@Dakostu Dakostu force-pushed the topic/expose-pipelines-in-export branch from 965d1e4 to 8fa5cf5 Compare January 27, 2023 09:19
@Dakostu Dakostu force-pushed the topic/expose-pipelines-in-export branch from 8fa5cf5 to 1ca24e6 Compare January 27, 2023 09:42
@Dakostu Dakostu merged commit 92275ca into master Jan 27, 2023
@Dakostu Dakostu deleted the topic/expose-pipelines-in-export branch January 27, 2023 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality
Projects
None yet
4 participants