METRON-295: Script parsing bolt #338

rkarthik29 · 2016-11-02T19:13:20Z

addd support for js,python and groovy parser bolt

…metron into metron-295

ottobackwards · 2016-11-02T19:26:33Z

This is interesting. I would think we would want some tooling like https://github.com/YelpArchive/pyleus around building a jar that contains all the python dependencies for a script for people to do non-trivial things in python.

I am not sure how you would do the same for javascript or groovy.

ottobackwards · 2016-11-02T19:28:47Z

metron-platform/metron-integration-test/.pydevproject

@@ -0,0 +1,5 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>


I think this should go in the .gitignore

ottobackwards · 2016-11-02T19:29:59Z

What is the workflow or steps around deploying the scripts?

cestella

Interesting contribution; thanks so much for that. We have definitely talked about this for some time. We really need to think about where scripts should live and how parsers can uptake new scripts on-the-fly.

cestella · 2016-11-02T19:32:52Z

metron-platform/metron-parsers/src/main/java/org/apache/metron/parsers/ScriptParser.java

+
+	@Override
+	public void init() {
+		// TODO Auto-generated method stub


probably don't need that

cestella · 2016-11-02T19:34:20Z

metron-platform/metron-parsers/src/main/java/org/apache/metron/parsers/ScriptParser.java

+	}
+	//Should this be sent to the interface as a default method?
+	public InputStream openInputStream(String streamName) throws IOException {
+	    FileSystem fs = FileSystem.get(new Configuration());


So this is following the pattern of what's currently in Grok. That is being moved to take the grok statements from zookeeper because otherwise you cannot update the parser without bouncing the topology. Should we follow suit here?

We should be consistent with this for now. If we change them to live in zookeeper we have to change all the parsers. We can't have some live in HDFS and others live in zookeeper

Well, we currently have active work to move the only parser that refers to reference files to zookeeper. We put ourselves in an inconsistent state as soon as that work comes in if we have this pull from HDFS.

I think until that work has been merged in the only consistent state is pulling from HDFS

I agree with the desire for consistency; however, I would potentially be concerned about the user experience of putting scripts (especially ones with external library dependencies) into zookeeper. We would need to make this as clean as possible.

cestella · 2016-11-02T19:35:19Z

metron-platform/metron-parsers/src/main/java/org/apache/metron/parsers/ScriptParser.java

+
+	@Override
+	public boolean validate(JSONObject message) {
+		// TODO Auto-generated method stub


don't need that TODO

cestella · 2016-11-02T19:35:49Z

metron-platform/metron-parsers/src/test/java/org/apache/metron/parsers/GroovyParserTest.java

+
+	@Override
+	public String getTimestampField() {
+		// TODO Auto-generated method stub


cestella · 2016-11-02T19:35:53Z

metron-platform/metron-parsers/src/test/java/org/apache/metron/parsers/GroovyParserTest.java

+
+	@Override
+	public String getDateFormat() {
+		// TODO Auto-generated method stub


cestella · 2016-11-02T19:37:09Z

metron-platform/metron-parsers/src/test/java/org/apache/metron/parsers/PythonParserTest.java

+
+	@Override
+	public String getParseFunction() {
+		// TODO Auto-generated method stub


cestella · 2016-11-02T19:37:14Z

metron-platform/metron-parsers/src/test/java/org/apache/metron/parsers/PythonParserTest.java

+
+	@Override
+	public String getLanguage() {
+		// TODO Auto-generated method stub


cestella · 2016-11-02T19:37:19Z

metron-platform/metron-parsers/src/test/java/org/apache/metron/parsers/PythonParserTest.java

+
+	@Override
+	public List<String> getTimeFields() {
+		// TODO Auto-generated method stub


cestella · 2016-11-02T19:37:24Z

metron-platform/metron-parsers/src/test/java/org/apache/metron/parsers/PythonParserTest.java

+
+	@Override
+	public String getDateFormat() {
+		// TODO Auto-generated method stub


cestella · 2016-11-02T19:37:29Z

metron-platform/metron-parsers/src/test/java/org/apache/metron/parsers/PythonParserTest.java

+
+	@Override
+	public String getTimestampField() {
+		// TODO Auto-generated method stub


ottobackwards · 2016-11-02T20:10:54Z

I think the tooling around creating the script parser is very important ( packaging, dependencies ), although that could be tracked in another jira as a follow on. But it may be impactful to this.

cestella · 2016-11-02T20:18:15Z

There has been a lot of thought and modification around updating external content in #308. It seems to me that some of this should be reused or presumed here. We have a similar issue, the need to update an external reference/execution file from zookeeper.

cestella · 2016-11-02T20:21:29Z

It may be, of course, that this shouldn't be pulled from zookeeper and should be kept a file. I'm not sure how I feel either way. If we keep it a file, I am unclear how we manage to reparse the script (and collateral) upon change without bouncing the topology.

ottobackwards · 2016-11-02T20:26:00Z

I don't think the script change is the same as configuration/rule change necessarily.
Maybe what is missing are the requirements?

does the script have to be editable and trigger reload the same as a configuration
does the script have to include dependencies such that it doesn't introduce problems maintaining environments across the cluster
does script X have to support all 'features' ( jython limitations for cpython modules )
Etc etc

cestella · 2016-11-02T20:27:45Z

@ottobackwards Yeah, that's sensible. As far as the features, I think it's ok for this to support the JVM-supported scripting languages with all their limitations (IMO).

ottobackwards · 2016-11-02T20:35:59Z

yeah, as a Minimum Viable, but those limitations need to be spelled out, per language. Actually all the points above may have per language implications

cestella · 2016-11-02T20:38:48Z

@rkarthik29 Could you add a section here describing the parser's config and limitations/abilities?

rkarthik29 · 2016-11-02T20:48:59Z

i am coming in late to this party, but i think the core issue , how would the bolt know that the script has changed so it can reload the parser. I was thinking if the previous bolt could add aversion tag to the raw message.. The parser bolt compares this version or checksum to the last one it had, if different reruns init and stores this value into the bolt. Else just drops the version an processes the raw message. A simple checksum may be. Since the scripts can be really evolved, we could give user more leverage if we allowed them to keep in HDFS or a filesystem. even a rest api. Please let me know if i am way off.

cestella · 2016-11-02T20:55:58Z

I'm increasingly becoming of the opinion that the code as it stands is probably the right path for the the moment. When we are through the changes from making the grok parser pull from Zookeeper, we can revise if necessary.

The main issue, as I see it, is as @rkarthik29 stated: how do you know that the scripts changed. And there are now two parsers that need non-trivially sized external files to function properly, so it may make some sense for there to be more concerted thought to how to abstract that properly.

merrimanr · 2016-11-02T21:08:22Z

@rkarthik29 One of the advantages of keeping the configuration (scripts in this case) in Zookeeper is that you get notified when something changes. You don't need to track versions. Unfortunately, until #308 gets merged in, the ParserBolt does not notify the parser class that the config has changed and only provides the configuration on startup (hence the need to restart the topology).

I think it makes sense to keep it in HDFS for this PR and accept the current restriction of having to restart the topology for it to take effect. Until we merge #308 or agree on the another approach, I wouldn't worry about script changes being updated on the fly. There has already been a lot of discussion and work put into this so it doesn't make sense to start over. We could even update #308 to include this parser once we get closer to merging that.

james-sirota · 2016-11-02T23:10:31Z

I will try to verify this on a production deployment with production data on Friday. I'll let you know how it goes

james-sirota · 2016-11-02T23:12:20Z

By the way sounds like the Travis build fails the RAT check. [ERROR] Failed to execute goal org.apache.rat:apache-rat-plugin:0.12:check (default) on project metron-integration-test: Too many files with unapproved license: 3 See RAT report in: /home/travis/build/apache/incubator-metron/metron-platform/metron-integration-test/target/rat.txt -> [Help 1]

rkarthik29 · 2016-11-07T17:39:15Z

The pull request is due to some errors with executing test cases. I am not sure if they are related to the parser, if someone guides me i can look at it.

# Conflicts: # metron-platform/metron-parsers/pom.xml

rkarthik29 · 2016-11-09T15:16:23Z

what happens to the PR now?

ottobackwards · 2016-11-09T15:27:53Z

How do scripts with dependencies work? - I think there are still questions in the comments here that should be answered

rkarthik29 · 2016-11-09T15:40:54Z

I think that should be a separate lira. This feature does a very simple thing, it allows a user to provide a function in the format JsonObject parse(rawMessage) in a language of his choice. The concept is that he can now use standard library functions to parse the rawMessage into JSon, which metron can then use.

dependencies for external libraries i think will need to be tested. This could be the strategy...

For JS script, you could just include the min.js link or just put your script in a http server and refer it in the script.
For groovy script, could be as simple as dropping a jar in the proper location, i guess the bolt class path.
for python, i think we should be able to put the modules in a modules folder and i think python script can refer to it from there, we will have to look at that packaging.

I think it is a little more complicated and i think a separate would be needed to track it.

rkarthik29 · 2016-11-09T15:46:47Z

this is exactly what we would need to do for python...
http://docs.aws.amazon.com/lambda/latest/dg/lambda-python-how-to-create-deployment-package.html

ottobackwards · 2016-11-09T15:57:19Z

So that is similar to the pyleus stuff. Although pyleus actually produces jar topologies as a unit. The question is, with only standard library support for any of these languages how useful is this? Does that need to be accounted for now or can it be another jira as you mention. I'm not advocating either way, but if we do say it will be another jira, and there will be further work here, we should explicitly state that and create the jiras to put that plan in place.

ottobackwards · 2016-11-09T15:57:54Z

And the documentation for the feature need to account for the limitations etc

rkarthik29 · 2016-11-09T16:04:34Z

yes, i agree, right now i don't think dependencies in a project would require the code for this feature to change. I think it is just documenting on how to include such dependency.

These languages are pretty good in their support for string parsing and extracting data, right out of the box. They support xml, json , again right out of the box. With that being said, just the standard scripts should be pretty useful.

rkarthik29 · 2016-11-09T16:05:57Z

on a separate note, One feature that could really be a jira that causes code change, could be the one where we can have callbacks from these scripts, especially JS. Which means my script can call an external rest Api over AJAX and once it gets a response, will call back a function in the bolt with the JSONObject, asynchronous. This can give user tremendous amount of extendibility. May be a way to use a centralized rule engine , for their alerts. I know JS supports it, so should groovy. Not sure about python. Will the bolt support such a thing?

kylerichardson

Thanks for pulling this together. I can see some real value coming out of this parsing option. Some follow-on work will be needed to support more advanced use cases (e.g. inclusion of additional libraries).

Just a few minor comments. Overall a great start. Thanks for the contribution!

kylerichardson · 2016-11-09T16:04:21Z

metron-platform/metron-parsers/pom.xml.orig

+  the specific language governing permissions and limitations under the License. 
+  -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"


This appears to be a local backup of the POM file. Can it be removed in favor of the git history?

kylerichardson · 2016-11-09T16:05:40Z

metron-platform/metron-parsers/src/main/resources/scripts/groovy/common

+
+def MetronMessage(){
+ def message=[:];
+ message["source"]="userlog";


For the other languages you have passed in a name variable and used it for the source, can we do that here to be consistent?

kylerichardson · 2016-11-09T16:06:21Z

metron-platform/metron-parsers/src/main/resources/scripts/python/common

+
+def MetronMessage(name):
+	message={}
+	message["source"]="userlog"


Can we update this to set the source to the passed in name variable?

kylerichardson · 2016-11-09T16:25:34Z

metron-platform/metron-parsers/src/main/java/org/apache/metron/parsers/ScriptParser.java

+	}
+	//Should this be sent to the interface as a default method?
+	public InputStream openInputStream(String streamName) throws IOException {
+	    FileSystem fs = FileSystem.get(new Configuration());


I agree with the desire for consistency; however, I would potentially be concerned about the user experience of putting scripts (especially ones with external library dependencies) into zookeeper. We would need to make this as clean as possible.

kylerichardson · 2016-11-09T16:37:05Z

metron-platform/metron-parsers/src/main/java/org/apache/metron/parsers/ScriptParser.java

+		                + originalMessage + " and the parsed message was: " + message + " . Check the function at: "
+		                + this.scriptPath);
+
+		      message.put("original_string", originalMessage);


To move toward consistency, can this "original_string" field be updated to use the new Constants.Fields.ORIGINAL.getName()?

randerzander · 2016-11-09T17:04:08Z

@ottobackwards , Python's standard libraries are useful by themselves.

Since first tasks with Metron seem often related to ingesting a new source, Python lowers the barrier to entry over Java or Grok parsers.

james-sirota · 2016-11-09T17:24:24Z

i agree with @randerzander. We can file a follow-on jira to get library support for Python and other languages in another follow-on Jira to this

rkarthik29 · 2016-11-11T03:13:54Z

@james-sirota I will fix the conflicts once we are ready to merge.

ottobackwards · 2017-07-02T14:21:13Z

I am not sure what the status of this pr is. I think that we may want to re-think this a little in light of METRON-777

ottobackwards · 2017-11-08T02:46:17Z

What is the status of this effort?

ottobackwards · 2017-12-23T05:40:37Z

Re-reading this, I think it is dead, unless there is an update triggered by this.

I think I have the resolution to the update issue.

Post 777 and parser extensions, extensions can / will be deployed through the ui.
A script parser will deploy scripts, as we do grok rules to a directory, we can have the deployment also make an entry in zookeeper with the checksum of the script or some such. That can be the item that is in zookeeper and monitored for change. If we deploy a new version of the script by installing it in the UI, it will save it to hdfs, and the change the entry in zookeeper, triggering reload.

If this PR is dead, I may take this up

nickwallen · 2018-04-30T13:55:37Z

@rkarthik29 Please close this PR unless you believe it is still needed. This PR will be closed per the Metron Development Guidelines on Inactive PRs, unless you provide some reasoning as to why it is stil needed. Thanks!

nickwallen · 2018-05-14T12:58:14Z

-1

There has been no response from contributor. This PR will be closed according to the Metron Development Guidelines on Inactive PRs.

Kathik Narayanan added 4 commits November 1, 2016 16:36

working on js parser

754a1ba

renamed JSParser to ScriptPArser

f4d0cec

script parsing support for js,python and groovy.

91bf93d

Merge branch 'metron-295' of https://github.com/rkarthik29/incubator-…

2f63772

…metron into metron-295

ottobackwards reviewed Nov 2, 2016

View reviewed changes

cestella reviewed Nov 2, 2016

View reviewed changes

removed TODOs

d1af778

fix for RAT check error

0f15c73

Kathik Narayanan added 3 commits November 7, 2016 13:12

added license for groovy and python jars.

5f2545a

Merge branch 'master' into metron-295

29d47e5

# Conflicts: # metron-platform/metron-parsers/pom.xml

fixed a typo

932934e

kylerichardson reviewed Nov 9, 2016

View reviewed changes

Humbedooh closed this May 14, 2018

		@@ -0,0 +1,5 @@
		<?xml version="1.0" encoding="UTF-8" standalone="no"?>

METRON-295: Script parsing bolt #338

METRON-295: Script parsing bolt #338

Uh oh!

Conversation

rkarthik29 commented Nov 2, 2016

Uh oh!

ottobackwards commented Nov 2, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ottobackwards commented Nov 2, 2016

Uh oh!

cestella left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ottobackwards commented Nov 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cestella commented Nov 2, 2016

Uh oh!

cestella commented Nov 2, 2016

Uh oh!

ottobackwards commented Nov 2, 2016

Uh oh!

cestella commented Nov 2, 2016

Uh oh!

ottobackwards commented Nov 2, 2016

Uh oh!

cestella commented Nov 2, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rkarthik29 commented Nov 2, 2016

Uh oh!

cestella commented Nov 2, 2016

Uh oh!

merrimanr commented Nov 2, 2016

Uh oh!

james-sirota commented Nov 2, 2016

Uh oh!

james-sirota commented Nov 2, 2016

Uh oh!

rkarthik29 commented Nov 7, 2016

Uh oh!

rkarthik29 commented Nov 9, 2016

Uh oh!

ottobackwards commented Nov 9, 2016

Uh oh!

rkarthik29 commented Nov 9, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rkarthik29 commented Nov 9, 2016

Uh oh!

ottobackwards commented Nov 2, 2016 •

edited

Loading

cestella commented Nov 2, 2016 •

edited

Loading

rkarthik29 commented Nov 9, 2016 •

edited

Loading