Add script processor #10850

andrewkroh · 2019-02-20T20:44:06Z

I recommend reading the docs then reviewing the non-vendor changes which is ~1400 lines.

The script processor executes Javascript code to process an event. The processor uses a pure
Go implementation of ECMAScript 5.1. This can be useful in situations where one of the other
processors doesn’t provide the functionality you need to filter events.

The processor can be configured by embedding Javascript in your configuration file or by pointing
the processor at external file(s). See the included documentation for the full details.

processors:
- script:
    lang: javascript
    source: >
      function process(event) {
          event.Tag("js");
      }

For observability it can add metrics with a histogram of the execution time and a counter
for the number of exceptions that occur.

Benchmark Results

Here's a benchmark to get a rough idea of how long it takes to process an event in the processor's runtime.

goos: darwin
goarch: amd64
pkg: github.com/elastic/beats/libbeat/processors/script/javascript
BenchmarkBeatEventV0/Put-12                         2000000           712 ns/op
BenchmarkBeatEventV0/Object_Put_Key-12              2000000           615 ns/op
BenchmarkBeatEventV0/Get-12                         2000000           769 ns/op
BenchmarkBeatEventV0/Get_Undefined_Key-12           1000000          1010 ns/op
BenchmarkBeatEventV0/fields_get_key-12              2000000           836 ns/op
BenchmarkBeatEventV0/Get_@metadata-12               2000000           676 ns/op
BenchmarkBeatEventV0/Put_@metadata-12               2000000           736 ns/op
BenchmarkBeatEventV0/Delete_@metadata-12            2000000           651 ns/op
BenchmarkBeatEventV0/Cancel-12                      3000000           565 ns/op
BenchmarkBeatEventV0/Tag-12                         2000000           865 ns/op
BenchmarkBeatEventV0/AppendTo-12                    2000000           630 ns/op

webmat

Love this!

I think it may be worth aligning option names with the equivalent names in ingest node. E.g. "code" => "source" & so on.

CHANGELOG.next.asciidoc

libbeat/processors/script/javascript/beatevent_v0.go

libbeat/docs/processors-using.asciidoc

libbeat/processors/script/javascript/beatevent_v0_test.go

libbeat/docs/processors-using.asciidoc

ph · 2019-02-27T20:23:34Z

libbeat/processors/script/javascript/session.go

+// session is a javascript runtime environment used throughout the life of
+// the processor instance.
+type session struct {
+	vm             *goja.Runtime


Looking at the doc

Is it goroutine-safe?
No. An instance of goja.Runtime can only be used by a single goroutine at a time. You can create as many instances of Runtime as you like but it's not possible to pass object values between runtimes.

IIRC, I think this might be fine because when we call publish on the client we actually have a mutex around the processor calls. But if we ever change the contract that could cause issue.

If we change that assumption we'll have to take a look at each processor to make sure they are thread-safe.

ph · 2019-02-27T20:27:18Z

libbeat/processors/script/javascript/javascript.go

+	}
+	rtn, err := run(event)
+	return rtn, annotateError(p.ID, err)
+}


We are actually opening the door for all kinds of scripts here, what comes to my mind are really long execution times or ReDoS cases, looking at the readme the VM support interruption

So I think we should set a default timeout and allow people to overrides that.

roncohen · 2019-02-28T16:27:32Z

I love the idea of this!

However, it would be great if you could elaborate more on the motivation for adding this. I think taking on 120k lines of code and a Javascript interpreter is a pretty big deal. Especially considering:

there's no way to selectively include it in your binary. If you use libbeat, it's going to go into your binary. That means inflated binaries, compile time, security concerns etc.
it's a whole ECMAScript interpreter implemented more or less by a single contributor. He writes in the README:

It's very unlikely that I will be adding new functionality any time soon.

there are some comments on timeouts. What about security in more general terms? Are we worried about people writing insecure scripts?

How about making this processor a "real module" that beats can import if they need it?

andrewkroh · 2019-02-28T16:32:11Z

there's no way to selectively include it in your binary. If you use libbeat, it's going to go into your binary.

Big +1 on this from me. I'd rather give each Beat the choice to opt in to importing the processor.

andrewkroh

I've made updates to address the comments.

Added a timeout option.
Removed automatic inclusion of the processor by libbeat. I imported it in the log processing beats and made the documentation conditional.
Added a test for Geting and Object and mentioned this in the docs.

libbeat/docs/processors-using.asciidoc

libbeat/processors/script/javascript/beatevent_v0.go

libbeat/processors/script/javascript/beatevent_v0_test.go

andrewkroh · 2019-03-06T19:06:13Z

libbeat/processors/script/javascript/session.go

+// session is a javascript runtime environment used throughout the life of
+// the processor instance.
+type session struct {
+	vm             *goja.Runtime


If we change that assumption we'll have to take a look at each processor to make sure they are thread-safe.

ph

I am happy with the decision of keeping this experimental for now, I understand the usefulness of having the power of a scripting language in beats but I am also worried about the activity around the javascript engine that we vendor.

NIT: I would probably change the options to be close with the ingest node instead of the ruby filter, since we are usually interacting more often with pipelines.

ph · 2019-03-14T00:56:52Z

libbeat/processors/script/javascript/session.go

+		t := time.AfterFunc(s.timeout, func() {
+			s.vm.Interrupt(timeoutError)
+		})
+		defer t.Stop()


@andrewkroh By curiosity, what was the impact of this change in your benchmark? I am asking this because I know there is channel involved when under the hood.

You can see the resulting benchmark in this commit:
427d5d9

BenchmarkBeatEventV0/timeout_Put-12 1000000 1510 ns/op BenchmarkBeatEventV0/Object_Put_Key-12 2000000 631 ns/op

OK let's keep it as is, for now, we might be able to optimize it later.

andrewkroh · 2019-03-14T02:21:37Z

I am happy with the decision of keeping this experimental for now, I understand the usefulness of having the power of a scripting language in beats but I am also worried about the activity around the javascript engine that we vendor.

Yep. We can try it out. If we find a major show stopper bug with the engine that would take significant effort to fix we “cancel” the experiment.

And I’ll make the change to align the config more with the painless script processor in ingest node.

The script processor executes Javascript code to process an event. The processor uses a pure Go implementation of ECMAScript 5.1. This can be useful in situations where one of the other processors doesn’t provide the functionality you need to filter events. The processor can be configured by embedding Javascript in your configuration file or by pointing the processor at external file(s). See the included documentation for the full details. processors: - script: type: javascript code: > function process(event) { event.Tag("js"); } For observability it can add metrics with a histogram of the execution time and a counter for the number of exceptions that occur. Benchmark Results Here's a benchmark to get a rough idea of how long it takes to process an event in the processor's runtime goos: darwin goarch: amd64 pkg: github.com/elastic/beats/libbeat/processors/script/javascript BenchmarkBeatEventV0/Put-12 2000000 712 ns/op BenchmarkBeatEventV0/Object_Put_Key-12 2000000 615 ns/op BenchmarkBeatEventV0/Get-12 2000000 769 ns/op BenchmarkBeatEventV0/Get_Undefined_Key-12 1000000 1010 ns/op BenchmarkBeatEventV0/fields_get_key-12 2000000 836 ns/op BenchmarkBeatEventV0/Get_@metadata-12 2000000 676 ns/op BenchmarkBeatEventV0/Put_@metadata-12 2000000 736 ns/op BenchmarkBeatEventV0/Delete_@metadata-12 2000000 651 ns/op BenchmarkBeatEventV0/Cancel-12 3000000 565 ns/op BenchmarkBeatEventV0/Tag-12 2000000 865 ns/op BenchmarkBeatEventV0/AppendTo-12 2000000 630 ns/op

This adds an optional timeout configuration option to the processor that will timeout the execution by interrupting the JS code. I updated the benchmark to test each case with and without the timeout enabled. BenchmarkBeatEventV0/Put-12 2000000 707 ns/op BenchmarkBeatEventV0/timeout_Put-12 1000000 1510 ns/op BenchmarkBeatEventV0/Object_Put_Key-12 2000000 631 ns/op BenchmarkBeatEventV0/timeout_Object_Put_Key-12 1000000 1252 ns/op BenchmarkBeatEventV0/Get-12 2000000 750 ns/op BenchmarkBeatEventV0/timeout_Get-12 1000000 1495 ns/op BenchmarkBeatEventV0/Get_Undefined_Key-12 1000000 1039 ns/op BenchmarkBeatEventV0/timeout_Get_Undefined_Key-12 1000000 2108 ns/op BenchmarkBeatEventV0/fields_get_key-12 2000000 1044 ns/op BenchmarkBeatEventV0/timeout_fields_get_key-12 1000000 2051 ns/op BenchmarkBeatEventV0/Get_@metadata-12 2000000 750 ns/op BenchmarkBeatEventV0/timeout_Get_@metadata-12 1000000 1745 ns/op BenchmarkBeatEventV0/Put_@metadata-12 1000000 1048 ns/op BenchmarkBeatEventV0/timeout_Put_@metadata-12 500000 2623 ns/op BenchmarkBeatEventV0/Delete_@metadata-12 2000000 842 ns/op BenchmarkBeatEventV0/timeout_Delete_@metadata-12 1000000 1629 ns/op BenchmarkBeatEventV0/Cancel-12 2000000 759 ns/op BenchmarkBeatEventV0/timeout_Cancel-12 1000000 1329 ns/op BenchmarkBeatEventV0/Tag-12 1000000 1189 ns/op BenchmarkBeatEventV0/timeout_Tag-12 1000000 1973 ns/op BenchmarkBeatEventV0/AppendTo-12 2000000 644 ns/op BenchmarkBeatEventV0/timeout_AppendTo-12 1000000 1347 ns/op

Rather than have libbeat force the inclusion of the script processor in all Beats, make each Beat opt into the processor.

Module is an overloaded term.

andrewkroh · 2019-03-14T23:42:03Z

jenkins, test this

ph

LGTM, let's make a followup PR to reject it from CM.

JCMais · 2019-05-09T15:16:38Z

This sounds really interesting! I have a question related to that, will it make their way to v6 or it will be v7 only?

andrewkroh · 2019-05-10T12:08:18Z

@JCMais It will be a feature in v7+ only.

Follows the strategy used in #10850 to check for goroutine leaks on tests in some places related to the leaks investigated as part of #9302. Several things here: * New helper to reuse the goroutine checker, it also prints now the goroutines dump on failure. * Add goroutine checks for autodiscover tests * Add goroutine checks for CloseOnSignal and SubOutlet tests (they detect the issues solved by #11263) * Add goroutine checks for log input tests (they detect issues solved by #12125 and #12164)

The script processor executes Javascript code to process an event. The processor uses a pure Go implementation of ECMAScript 5.1. This can be useful in situations where one of the other processors doesn’t provide the functionality you need to filter events. The processor can be configured by embedding Javascript in your configuration file or by pointing the processor at external file(s). See the included documentation for the full details. processors: - script: type: javascript code: > function process(event) { event.Tag("js"); } For observability it can add metrics with a histogram of the execution time and a counter for the number of exceptions that occur. There is an optional timeout configuration option to the processor that will timeout the execution by interrupting the JS code. Rather than have libbeat force the inclusion of the script processor in all Beats, make each Beat opt into the processor. It is imported by the log processing Beats (filebeat, journalbeat, and winlogbeat). Benchmark Results Here's a benchmark to get a rough idea of how long it takes to process an event in the processor's runtime I updated the benchmark to test each case with and without the timeout enabled. BenchmarkBeatEventV0/Put-12 2000000 707 ns/op BenchmarkBeatEventV0/timeout_Put-12 1000000 1510 ns/op BenchmarkBeatEventV0/Object_Put_Key-12 2000000 631 ns/op BenchmarkBeatEventV0/timeout_Object_Put_Key-12 1000000 1252 ns/op BenchmarkBeatEventV0/Get-12 2000000 750 ns/op BenchmarkBeatEventV0/timeout_Get-12 1000000 1495 ns/op BenchmarkBeatEventV0/Get_Undefined_Key-12 1000000 1039 ns/op BenchmarkBeatEventV0/timeout_Get_Undefined_Key-12 1000000 2108 ns/op BenchmarkBeatEventV0/fields_get_key-12 2000000 1044 ns/op BenchmarkBeatEventV0/timeout_fields_get_key-12 1000000 2051 ns/op BenchmarkBeatEventV0/Get_@metadata-12 2000000 750 ns/op BenchmarkBeatEventV0/timeout_Get_@metadata-12 1000000 1745 ns/op BenchmarkBeatEventV0/Put_@metadata-12 1000000 1048 ns/op BenchmarkBeatEventV0/timeout_Put_@metadata-12 500000 2623 ns/op BenchmarkBeatEventV0/Delete_@metadata-12 2000000 842 ns/op BenchmarkBeatEventV0/timeout_Delete_@metadata-12 1000000 1629 ns/op BenchmarkBeatEventV0/Cancel-12 2000000 759 ns/op BenchmarkBeatEventV0/timeout_Cancel-12 1000000 1329 ns/op BenchmarkBeatEventV0/Tag-12 1000000 1189 ns/op BenchmarkBeatEventV0/timeout_Tag-12 1000000 1973 ns/op BenchmarkBeatEventV0/AppendTo-12 2000000 644 ns/op BenchmarkBeatEventV0/timeout_AppendTo-12 1000000 1347 ns/op

andrewkroh added review libbeat :Processors labels Feb 20, 2019

andrewkroh requested a review from a team as a code owner February 20, 2019 20:44

webmat reviewed Feb 27, 2019

View reviewed changes

ph suggested changes Feb 27, 2019

View reviewed changes

andrewkroh force-pushed the feature/libbeat/js-processor branch from 2b6fb06 to 67c0820 Compare March 6, 2019 18:13

andrewkroh commented Mar 6, 2019

View reviewed changes

andrewkroh requested a review from ph March 6, 2019 19:10

ph approved these changes Mar 14, 2019

View reviewed changes

andrewkroh added 9 commits March 14, 2019 19:39

Add github.com/dop251/goja to vendor

3f29594

Add PR number to changelog

471606e

Include script process in log processing Beats

0703267

Rather than have libbeat force the inclusion of the script processor in all Beats, make each Beat opt into the processor.

Add test for Get with a non-leaf field name

e276c4e

Explicitly mention Get can return an object

003ea91

Follow Ingest Node script processor parameter naming

86c8ccb

Rename AddModule to AddSessionHook

ea47f77

Module is an overloaded term.

andrewkroh force-pushed the feature/libbeat/js-processor branch from e8477db to ea47f77 Compare March 14, 2019 23:39

Fix changelog

8a032df

ph approved these changes Mar 14, 2019

View reviewed changes

andrewkroh merged commit 0693a22 into elastic:master Mar 15, 2019

jsoriano mentioned this pull request May 8, 2019

Check goroutines in places related to recent found leaks #12106

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add script processor #10850

Add script processor #10850

andrewkroh commented Feb 20, 2019 •

edited

Loading

webmat left a comment

ph Feb 27, 2019

andrewkroh Mar 6, 2019

ph Feb 27, 2019

andrewkroh Feb 27, 2019

roncohen commented Feb 28, 2019 •

edited

Loading

andrewkroh commented Feb 28, 2019

andrewkroh left a comment

andrewkroh Mar 6, 2019

ph left a comment

ph Mar 14, 2019 •

edited

Loading

andrewkroh Mar 14, 2019

ph Mar 14, 2019

andrewkroh commented Mar 14, 2019

andrewkroh commented Mar 14, 2019

ph left a comment

JCMais commented May 9, 2019 •

edited

Loading

andrewkroh commented May 10, 2019

Add script processor #10850

Add script processor #10850

Conversation

andrewkroh commented Feb 20, 2019 • edited Loading

webmat left a comment

Choose a reason for hiding this comment

ph Feb 27, 2019

Choose a reason for hiding this comment

andrewkroh Mar 6, 2019

Choose a reason for hiding this comment

ph Feb 27, 2019

Choose a reason for hiding this comment

andrewkroh Feb 27, 2019

Choose a reason for hiding this comment

roncohen commented Feb 28, 2019 • edited Loading

andrewkroh commented Feb 28, 2019

andrewkroh left a comment

Choose a reason for hiding this comment

andrewkroh Mar 6, 2019

Choose a reason for hiding this comment

ph left a comment

Choose a reason for hiding this comment

ph Mar 14, 2019 • edited Loading

Choose a reason for hiding this comment

andrewkroh Mar 14, 2019

Choose a reason for hiding this comment

ph Mar 14, 2019

Choose a reason for hiding this comment

andrewkroh commented Mar 14, 2019

andrewkroh commented Mar 14, 2019

ph left a comment

Choose a reason for hiding this comment

JCMais commented May 9, 2019 • edited Loading

andrewkroh commented May 10, 2019

andrewkroh commented Feb 20, 2019 •

edited

Loading

roncohen commented Feb 28, 2019 •

edited

Loading

ph Mar 14, 2019 •

edited

Loading

JCMais commented May 9, 2019 •

edited

Loading