METRON-877 Extract core implementation and UDF support, create metron-stellar module

[mattf-apache]

Contributor Comments

It appears that github permanently closes any PR that gets rebased, hence for the history and discussion to date, I can only point you at the previous incarnation of this PR: #610

The design document has been available for some time at https://docs.google.com/document/d/1EP7Jt4ePHe2A-_oboLl2QbN1muh7uKeET_kbpIgjcJM/edit#
I have now added a new section, starting on p. 7, titled "Retrospective: Changes made for Parts 1 & 2" that details the specific changes actually done in this PR to extract Stellar.

Pull Request Checklist

In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following:

For all changes:

• Is there a JIRA ticket associated with this PR? If not one needs to be created at Metron Jira.
• 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.
• Has your PR been rebased against the latest commit within the target branch (typically master)?

For code changes:

The intent of these changes is to achieve an architecture change WITHOUT any functional change. The continued working of all unit tests, integration tests and real-world applications should result. In addition, it generates (in metron-stellar/stellar-common/target) a stellar-common uber-jar (including all dependencies), which among other things can run the Stellar shell as a stand-alone Java application (although missing those Stellar functions that remain as part of Metron):

java -cp target/stellar-common-0.4.0-uber.jar \
    org.apache.metron.stellar.common.shell.StellarShell

For testing within Metron, the stellar classes are still incorporated into the metron-common jar and RPM, so test installation should work as before.

For a general test plan for all Stellar functionality within Metron, please see #468 's test plan.

• 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?

• Have you ensured that the full suite of tests and checks have been executed in the root incubating-metron folder via:

  mvn -q clean integration-test install && build_utils/verify_licenses.sh 
  

• Have you written or updated unit tests and or integration tests to verify your changes?

• If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

• [IN PROCESS] 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?

[ottobackwards]

So, the test plan for this is...? The test work etc?

[ottobackwards]

So we have a StatefulExecutor, is there an idea that there will be more types of executors? Will there be a non-stateful executor, maybe for performance reasons or other use cases? Maybe in the future some write up on this would help with the intent

[ottobackwards]

As a more stand alone module, I think the readme should be broken down a little bit, instead of just being 'here is the language'.

A top level statement document, with what stellar is at a high level, why it exists and when it has value to be used.
Below that more technical guides - language, examples, faq, discovery and loading.
I would also like to see a developer's guide.

I wouldn't hold this PR up for this, but maybe it should be added to the follow on list.

[ottobackwards]

Each example could be the seed of future development of encapsulating that functionality. For example - how to use stellar in spark could be evolved into a stellar-spark integration package.

[ottobackwards]

• 1 on inspection, built and ran the tests - This is a lot of work, great job. As with other large PR's ( cough METRON-777 ) I think that this could be a feature branch. Although this did have more involvement from other contributors ( @cestella ), the feature branch would help the complete review pool

[mattf-apache]

@ottobackwards , thanks very much for tackling this admittedly large review.

So, the test plan for this is...?

"For a general test plan for all Stellar functionality within Metron, please see #468 's test plan."
It's long, hence not reproduced here.

So we have a StatefulExecutor...

The base executor is org.apache.metron.stellar.common.BaseStellarProcessor, formerly org.apache.metron.common.stellar.BaseStellarProcessor. It parses and evaluates Stellar expressions, with caching of the parse, and with given context (variables), variableResolver, and functionResolver. It can return any Stellar data type. It also provides a validate() method.

The "stateful" executor, org.apache.metron.stellar.common.StellarStatefulExecutor, assists with Aggregators, where state needs to persist across multiple invocations of the same entity, and in fact was formerly located at org.apache.metron.profiler.stellar.StellarExecutor. It was renamed when I relocated it, to avoid conflict with the Shell executor, org.apache.metron.stellar.common.shell.StellarExecutor, which is significantly different. I deemed the stateful executor to be Stellar functionality rather than Profiler, and moved it. The Aggregator functions will also be moved into Stellar, but that will be in Step 3, which will be a different PR.

I agree in future it would be nice to have design documents, for Stellar and other parts of Metron.

I think the readme should be [expanded into a Language Reference Manual :-) ]
...maybe it should be added to the follow on list.

Yah, that would be good. I've opened METRON-998, as a follow-on task.

I think that this could be a feature branch.

Since this started as a regular PR, I'd like to finish this one that way. But I agree, before starting Part 3 I'll petition to create a feature branch for the next set(s) of work.

[ottobackwards]

Thanks Matt, I think your write up above should be captured somewhere... We don't want to lose it in the PR

[mattf-apache]

Hi @ottobackwards , I've captured those two paragraphs in a new section of the GoogleDoc, titled "Design Observations".

[mattf-apache]

@ottobackwards , is the "1" above a +1?
@cestella , since this is in your bailiwick, would you like to review it?
Thanks!

[ottobackwards]

Yes, sorry.
+1

[cestella]

Ah, yes, I do want that. Sorry!

[cestella]

+1 by inspection, this is great work, @mattf-horton

[ottobackwards]

so can this be merged?

[mattf-apache]

Thanks, @ottobackwards and @cestella !
Merge to current master was trivial, as seen here: 87a44f6
Proceeding with commit.

[ottobackwards]

super! great work @mattf-horton

[mattf-apache]

This has been committed to git-wip-us.apache.org/repos/asf/metron. Propagation seems a little slow today.