mmiklavc opened a new pull request #1409: METRON-2112 Normalize parser original_string handling URL: https://github.com/apache/metron/pull/1409 ## Contributor Comments https://issues.apache.org/jira/browse/METRON-2112 This PR introduces work to address the issue with `original_string` brought up in the DISCUSS thread [[DISCUSS] JsonMapParser original string functionality](https://lists.apache.org/thread.html/2dbf068cd2144ea3bda8d652b4e866c74b3ef9e96510c63ff27335b4@%3Cdev.metron.apache.org%3E) The discussion concluded with a solution that would: 1. Address the regression 2. Make the handling of original_string more generalized and universally applied across all parsers 3. Allow individual parsers to override the original_string, if desired. 4. Not screw up parser chaining This is discussed in more detail in the README, but here's a rundown of the settings: - Global config option added `parser.original.string.global`, defaults to `true`. By default, this will now enable the parser runner to append an `original_string` using the true raw source message. Note, for backwards compatibility, the implementation uses a putIfAbsent approach. This approach is to keep from completely breaking parser chaining due to the way enveloped message parsing works (hint: it's our only real special system-level case). Setting the property to false will mean the runner will not attempt to add `original_string` at all. - Modification to JsonMapParser to accept a new configuration option `overrideOriginalString`. The default of `false` addresses the regression introduced by jsonpquery and will not attempt to add an `original_string`. Setting this value to `true` will effectively override the global setting and apply an `original_string` per message generated that reflects the existing functionality. I also addressed a few random doc and test issues I noticed while modifying the code for this PR. e.g. global config sub-section links, missing/omitted test assertions, etc. More exhaustive test instructions to follow. In the meantime, this PR is ready for code and doc review. Most importantly, the functionality I've outlined here and in the README's should be reviewed. I have not modified any of the parsers besides JsonMapParser. If we think they should have their default functionality changed, I propose we open a separate DISCUSS thread for this and provide a migration path for existing users that may or may not be impacted by any change. I suspect that only JSON parsers should be affected. Considering the origin and impact of this change, I presume this will warrant a mention in Upgrading.md. I have not done this yet, but would like to hear any specific concerns or feedback on wording. ## Pull Request Checklist ### For all changes: - [x] Is there a JIRA ticket associated with this PR? If not one needs to be created at [Metron Jira](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel). - [x] Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. - [x] Has your PR been rebased against the latest commit within the target branch (typically master)? ### For code changes: - [ ] Have you included steps to reproduce the behavior or problem that is being changed or addressed? - [ ] Have you included steps or a guide to how the change may be verified and tested manually? - [x] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via: ``` mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh ``` - [x] Have you written or updated unit tests and or integration tests to verify your changes? - n/a If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? - [ ] Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent? ### For documentation related changes: - [ ] Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via `site-book/target/site/index.html`: ``` cd site-book mvn site ``` - n/a Have you ensured that any documentation diagrams have been updated, along with their source files, using [draw.io](https://www.draw.io/)? See [Metron Development Guidelines](https://cwiki.apache.org/confluence/display/METRON/Development+Guidelines) for instructions.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
