INGEST: Document Processor Conditional #33388

original-brownbear · 2018-09-04T16:44:45Z

elasticmachine · 2018-09-04T16:44:46Z

Pinging @elastic/es-core-infra

jakelandis · 2018-09-04T16:56:38Z

docs/reference/ingest/ingest-node.asciidoc

+[source,js]
+--------------------------------------------------
+{
+  "bytes": {


Can we use the set processor for this example ?

Sure that seems less confusing :)

jakelandis · 2018-09-04T17:05:00Z

docs/reference/ingest/ingest-node.asciidoc

+then the processor will be executed for the given document otherwise it will be skipped.
+The `if` field takes a map with the script settings used defined in <<script-processor, script-options>>
+and accesses a read only version of the document via the same `ctx` variable used by scripts in the
+<<script-processor>>.


Should we link to the painless doc ?

Do you think it would useful to also have additional examples (not necessarily the full the processor) of how to implement logstash's ~= ? It seemed to be the most common operator to use for this type of check.

Hmm yea it could make sense to add more examples (I agree, it's way too hard to figure out how to do that in Painless from what we currently have in the docs).

WDYT @rjernst ?

rjernst

Looks good, my only concern is making sure the example works in docs tests.

rjernst · 2018-09-04T17:19:37Z

docs/reference/ingest/ingest-node.asciidoc

@@ -721,12 +721,29 @@ All processors are defined in the following way within a pipeline definition:
 // NOTCONSOLE

 Each processor defines its own configuration parameters, but all processors have
-the ability to declare `tag` and `on_failure` fields. These fields are optional.
+the ability to declare `tag` ,`on_failure` and `if` fields. These fields are optional.


nit: space before comma should be after

rjernst · 2018-09-04T17:22:53Z

docs/reference/ingest/ingest-node.asciidoc


 A `tag` is simply a string identifier of the specific instantiation of a certain
 processor in a pipeline. The `tag` field does not affect the processor's behavior,
 but is very useful for bookkeeping and tracing errors to specific processors.

+The `if` field must contain a script that returns a boolean value. If the script evaluates to `true`
+then the processor will be executed for the given document otherwise it will be skipped.
+The `if` field takes a map with the script settings used defined in <<script-processor, script-options>>


if doesn't take a map, right? I think you mean "object" (since we are talking about json), and that is optional (the example you have below here has if as the script source directly. You would only need an object if you wanted to pass params (unlikely for ingest I think) or use a different scripting language (not really possible since expressions don't support this, but theoretically a native script could be written as a custom script engine).

Right .. "object" wins :)

Hmm on that note and kinda off topic but: maybe we should/could enable expressions here the same way we do for bucket selector aggregation (interpret 1.0 as true) to become consistent there?

I think it would be difficult and not very useful, since expressions only have access to numerics. Expressions operate on doc values, but ingest does not have doc values, so it would require a lot of hacking.

ok makes sense :)

rjernst · 2018-09-04T17:29:11Z

docs/reference/ingest/ingest-node.asciidoc

+  }
+}
+--------------------------------------------------
+// NOTCONSOLE


Why is this omitted from tests? Can we make an example that will work?

I figured just with the example config we don't have enough to go by for running anything, we'd need to actually index a document against a concrete pipeline here to make a test out of it right?
(that may be a little confusing given we only want an example of the if field here? Idk, you decide :))

IIRC we can have extra setup that is hidden from the generated documentation. I think having the examples always "work" is key to keeping the documentation up to date as apis change.

Ok :) Will look into that tomorrow morning :)

I can't add a test for a snipped like this it seems. I get this error:

Execution failed for task ':docs:buildRestTests'. > reference/ingest/ingest-node.asciidoc[737:745](js)// CONSOLE: Didn't match (?:(?<comment>#.+)|(?:(?:(?<method>GET|PUT|POST|HEAD|OPTIONS|DELETE)\s+(?<pathAndQuery>[^\n]+)(?<body>(?:\n(?!GET|PUT|POST|HEAD|OPTIONS|DELETE|startyaml|#)[^\n]+)+)?)|(?:startyaml(?s)(?<yaml>.+?)(?-s)endyaml)))\n+: { "set": { "if": "ctx.bar == 'expectedValue'", "field": "foo", "value": "bar" } }

if I try anything here. => Seems I at least need a snippet that includes an actual request. That would break with the style of the following docs that also just show a quick outline of the configuration (without tests) for each processor?

…-docs

original-brownbear · 2018-09-05T10:08:02Z

@rjernst @jakelandis

Fixed space around comma
Fixed wording "map" -> "object"
Made the example use "set" instead of "bytes" proc

... but:

Tests from the snippet in the form used here seem impossible with our test framework (unless I'm still misunderstanding something about it) ... we could use a different snippet though :)?

rjernst · 2018-09-07T23:39:06Z

@nik9000 Can you provide any suggestions on making the snippet test discussed here work?

jakelandis · 2018-10-15T21:55:43Z

@original-brownbear @rjernst - I think a standalone page (in addition to what is here), something like "Conditional Execution" would be beneficial. With specific mentions about the regex requirements, conditional dropping of events, and conditional pipelines. I can take the TODO to create that content (and implement the tests there). Hopefully that will unblock this PR.

thoughts ?

original-brownbear · 2018-10-16T07:40:54Z

@jakelandis I think that (standalone page) would make sense. Putting the example scripts and the how-to use the conditional processor could be pretty helpful for people trying to convert a LS config (or just their problem) into our code without having to jump between the Painless docs and pipeline docs :)

rjernst

This looks fine to get in since we need conditional processor docs, but I would still like suggestions from @nik9000 on ways to test this so we don't need NOTCONSOLE.

…-docs

original-brownbear · 2018-10-23T08:09:21Z

@rjernst alright thanks! Merged master into this PR since its been a while => will merge once green then.

* master: (24 commits) ingest: better support for conditionals with simulate?verbose (elastic#34155) [Rollup] Job deletion should be invoked on the allocated task (elastic#34574) [DOCS] .Security index is never auto created (elastic#34589) CCR: Requires soft-deletes on the follower (elastic#34725) re-enable bwc tests (elastic#34743) Empty GetAliases authorization fix (elastic#34444) INGEST: Document Processor Conditional (elastic#33388) [CCR] Add total fetch time leader stat (elastic#34577) SQL: Support pattern against compatible indices (elastic#34718) [CCR] Auto follow pattern APIs adjustments (elastic#34518) [Test] Remove dead code from ExceptionSerializationTests (elastic#34713) A small typo in migration-assistance doc (elastic#34704) ingest: processor stats (elastic#34724) SQL: Implement IN(value1, value2, ...) expression. (elastic#34581) Tests: Add checks to GeoDistanceQueryBuilderTests (elastic#34273) INGEST: Rename Pipeline Processor Param. (elastic#34733) Core: Move IndexNameExpressionResolver to java time (elastic#34507) [DOCS] Force Merge: clarify execution and storage requirements (elastic#33882) TESTING.asciidoc fix examples using forbidden annotation (elastic#34515) SQL: Implement `CONVERT`, an alternative to `CAST` (elastic#34660) ...

* INGEST: Document Processor Conditional Relates elastic#33188

* INGEST: Document Processor Conditional Relates #33188

jakelandis · 2018-10-24T18:06:46Z

6.5 backport: fcad4e7

* INGEST: Document Processor Conditional Relates #33188

INGEST: Document Processor Conditional

04d1013

Relates elastic#33188

original-brownbear added >docs General docs changes :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP v7.0.0 v6.5.0 labels Sep 4, 2018

original-brownbear requested review from talevy, jasontedor, rjernst and jakelandis and removed request for jasontedor September 4, 2018 16:44

jakelandis reviewed Sep 4, 2018

View reviewed changes

rjernst reviewed Sep 4, 2018

View reviewed changes

original-brownbear added 4 commits September 5, 2018 11:39

Merge remote-tracking branch 'elastic/master' into ingest-conditional…

b0d3eb8

…-docs

CR: Fix whitespace around comma

1472452

Move to set proc

a45e0f6

CR: wording 'map' -> 'object'

8add065

original-brownbear added the review label Sep 8, 2018

rjernst removed the review label Oct 10, 2018

jakelandis mentioned this pull request Oct 15, 2018

ingest: Documentation improvements for ingest node #33188

Closed

11 tasks

rjernst approved these changes Oct 23, 2018

View reviewed changes

Merge remote-tracking branch 'elastic/master' into ingest-conditional…

f33f36f

…-docs

original-brownbear merged commit f0f7329 into elastic:master Oct 23, 2018

original-brownbear deleted the ingest-conditional-docs branch October 23, 2018 15:37

jakelandis added the backport pending label Oct 23, 2018

jakelandis pushed a commit to jakelandis/elasticsearch that referenced this pull request Oct 23, 2018

INGEST: Document Processor Conditional (elastic#33388)

f5c26ba

* INGEST: Document Processor Conditional Relates elastic#33188

jakelandis pushed a commit that referenced this pull request Oct 24, 2018

INGEST: Document Processor Conditional (#33388)

fcad4e7

* INGEST: Document Processor Conditional Relates #33188

jakelandis removed the backport pending label Oct 24, 2018

kcm pushed a commit that referenced this pull request Oct 30, 2018

INGEST: Document Processor Conditional (#33388)

1fcfb84

* INGEST: Document Processor Conditional Relates #33188

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

INGEST: Document Processor Conditional #33388

INGEST: Document Processor Conditional #33388

original-brownbear commented Sep 4, 2018

elasticmachine commented Sep 4, 2018

jakelandis Sep 4, 2018

original-brownbear Sep 4, 2018

jakelandis Sep 4, 2018

original-brownbear Sep 4, 2018

rjernst left a comment

rjernst Sep 4, 2018

rjernst Sep 4, 2018

original-brownbear Sep 4, 2018

rjernst Sep 4, 2018

original-brownbear Sep 4, 2018

rjernst Sep 4, 2018

original-brownbear Sep 4, 2018

rjernst Sep 4, 2018

original-brownbear Sep 4, 2018

original-brownbear Sep 5, 2018

original-brownbear commented Sep 5, 2018

rjernst commented Sep 7, 2018

jakelandis commented Oct 15, 2018

original-brownbear commented Oct 16, 2018

rjernst left a comment

original-brownbear commented Oct 23, 2018 •

edited

Loading

jakelandis commented Oct 24, 2018

INGEST: Document Processor Conditional #33388

INGEST: Document Processor Conditional #33388

Conversation

original-brownbear commented Sep 4, 2018

elasticmachine commented Sep 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rjernst left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

original-brownbear commented Sep 5, 2018

rjernst commented Sep 7, 2018

jakelandis commented Oct 15, 2018

original-brownbear commented Oct 16, 2018

rjernst left a comment

Choose a reason for hiding this comment

original-brownbear commented Oct 23, 2018 • edited Loading

jakelandis commented Oct 24, 2018

original-brownbear commented Oct 23, 2018 •

edited

Loading