[DOCS] Refactor script processor docs #72691

jrodewig · 2021-05-04T13:17:59Z

Changes:

Updates the intro link for scripts and the ingest context.
Rewrites the options section to better align with the create stored script API docs.
Updates the examples to provide more realistic examples and use the simulate pipeline API.

Closes #72645

Preview

https://elasticsearch_72691.docs-preview.app.elstc.co/guide/en/elasticsearch/reference/master/script-processor.html

elasticmachine · 2021-05-04T15:26:04Z

Pinging @elastic/es-core-features (Team:Core/Features)

elasticmachine · 2021-05-04T15:26:04Z

Pinging @elastic/es-docs (Team:Docs)

danhermann

LGTM. Thanks, @jrodewig!

ppf2

LGTM thx for addressing this confusion!

jdconrad

Thanks for updating these docs! This is a solid improvement. I have a few requests including a couple of minor corrections.

jdconrad · 2021-05-04T18:00:26Z

docs/reference/ingest/processors/script.asciidoc

+| Name        | Required  | Default    | Description
+| `lang`      | no        | "painless" | <<scripting-available-languages,Script language>>.
+| `id`        | no        | -          | ID of a <<create-stored-script-api,stored script>>.
+                                         If no `source` is specified, this parameter is required.


I think it's confusing to have this table say 'no' to all parameters in an either/or situation. One of id/source is required, and while this is specified as part of the description, is there a way we could change the no to a '*' or some other indicator that at least one of these is required because it's easy to miss on quick glance.

I agree this isn't ideal, but it's consistent with the other ingest processors. Changing this would break the later include::common-options.asciidoc[] statement, which is used in all our processor reference docs.

I've opened #72717 to address your concerns as a separate effort.

jdconrad · 2021-05-04T18:23:50Z

docs/reference/ingest/processors/script.asciidoc


-You can access the current ingest document from within the script context by using the `ctx` variable.
+To access an incoming document's source fields with a Painless script, use the
+`ctx.<field>` syntax. The `ctx._source.<field>` and `ctx['_source.<field>']`


Some notes here:

I would include a bit more detail about how the documents contents are a set of maps, lists, and primitives based on the parsed JSON from the original source document.

I wonder if the not included should be a note section to isolate that. I initially read that as I can use ctx._source because the not supported is at the end of the sentence. ctx['_source.<field>'] should be ctx['_source']['my_field'].

I would prefer to present the appropriate way to access fields as ctx['my_field'] instead of ctx.my_field since the first way can allow any field with special characters, but the second way is just a limited shortcut.

It may be worth having a link to the access operator here. (https://www.elastic.co/guide/en/elasticsearch/painless/current/painless-operators-reference.html#map-access-operator)

Thanks for this feedback. I pushed 9a1f779 to update this section and use the preferred syntax.

jdconrad · 2021-05-04T18:28:45Z

docs/reference/ingest/processors/script.asciidoc

--------------------------------------------------
+[source,console]
+----
+POST _ingest/pipeline/_simulate


Do we explain what the simulate query is anywhere?

That's covered in the ingest pipeline docs. I don't feel we need to cover that here.

jdconrad · 2021-05-04T18:30:01Z

docs/reference/ingest/processors/script.asciidoc

+          "description": "Extract 'tags' from 'env' field",
+          "lang": "painless",
+          "source": """
+            String[] envSplit = ctx?.env.splitOnToken(params.delimiter);


Is there a case where ctx can actually be null? Also we still access envSplit anyway so if ctx were null this is still going to throw an NPE. I would not include the elvis operator here.
ctx?.env.splitOnToken(params.delimiter); --> ctx.env.splitOnToken(params.delimiter);

Good point. I've changed this to remove the shorthand syntax anyway.

jrodewig

Thanks for the review @jdconrad. I've responded to your comments and pushed 9a1f779 to address your feedback.

jrodewig · 2021-05-04T18:49:56Z

docs/reference/ingest/processors/script.asciidoc

--------------------------------------------------
+[source,console]
+----
+POST _ingest/pipeline/_simulate


That's covered in the ingest pipeline docs. I don't feel we need to cover that here.

jrodewig · 2021-05-04T19:00:48Z

docs/reference/ingest/processors/script.asciidoc

+| Name        | Required  | Default    | Description
+| `lang`      | no        | "painless" | <<scripting-available-languages,Script language>>.
+| `id`        | no        | -          | ID of a <<create-stored-script-api,stored script>>.
+                                         If no `source` is specified, this parameter is required.


I agree this isn't ideal, but it's consistent with the other ingest processors. Changing this would break the later include::common-options.asciidoc[] statement, which is used in all our processor reference docs.

I've opened #72717 to address your concerns as a separate effort.

jrodewig · 2021-05-04T19:03:15Z

docs/reference/ingest/processors/script.asciidoc

+          "description": "Extract 'tags' from 'env' field",
+          "lang": "painless",
+          "source": """
+            String[] envSplit = ctx?.env.splitOnToken(params.delimiter);


Good point. I've changed this to remove the shorthand syntax anyway.

jrodewig · 2021-05-04T19:08:10Z

docs/reference/ingest/processors/script.asciidoc


-You can access the current ingest document from within the script context by using the `ctx` variable.
+To access an incoming document's source fields with a Painless script, use the
+`ctx.<field>` syntax. The `ctx._source.<field>` and `ctx['_source.<field>']`


Thanks for this feedback. I pushed 9a1f779 to update this section and use the preferred syntax.

jdconrad

LGTM! Thanks for updating this.

jrodewig · 2021-05-04T19:55:33Z

Thanks all!

[DOCS] Refactor script processor docs

96b8340

jrodewig added :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >docs General docs changes v7.12.2 v7.13.1 v7.14.0 v8.0.0 labels May 4, 2021

jrodewig marked this pull request as ready for review May 4, 2021 15:26

elasticmachine added Team:Data Management Meta label for data/management team Team:Docs Meta label for docs team labels May 4, 2021

jrodewig requested review from danhermann and jdconrad May 4, 2021 15:26

Note that ctx._source is not supported

4786e9e

jrodewig requested a review from ppf2 May 4, 2021 15:29

danhermann approved these changes May 4, 2021

View reviewed changes

ppf2 approved these changes May 4, 2021

View reviewed changes

jdconrad requested changes May 4, 2021

View reviewed changes

Address review feedback

9a1f779

jrodewig commented May 4, 2021

View reviewed changes

jrodewig requested a review from jdconrad May 4, 2021 19:09

jrodewig added 4 commits May 4, 2021 15:10

Fix typos

324fcdd

Fix link

d44570e

Fix test

0d43f48

Reword

7827639

jdconrad approved these changes May 4, 2021

View reviewed changes

jrodewig merged commit bd84e8a into elastic:master May 4, 2021

jrodewig deleted the docs__fix-script-processor-docs branch May 4, 2021 19:55

This was referenced May 4, 2021

[7.x] [DOCS] Refactor script processor docs (#72691) #72719

Merged

[7.13] [DOCS] Refactor script processor docs (#72691) #72720

Merged

[7.12] [DOCS] Refactor script processor docs (#72691) #72721

Merged

jrodewig added a commit that referenced this pull request May 4, 2021

[DOCS] Refactor script processor docs (#72691) (#72719)

b187c75

jrodewig added a commit that referenced this pull request May 4, 2021

[DOCS] Refactor script processor docs (#72691) (#72720)

3356d7b

jrodewig added a commit that referenced this pull request May 4, 2021

[DOCS] Refactor script processor docs (#72691) (#72721)

407551d

mark-vieira added v7.13.0 and removed v7.13.1 labels May 14, 2021

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

[DOCS] Refactor script processor docs #72691

[DOCS] Refactor script processor docs #72691

Uh oh!

Conversation

jrodewig commented May 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Preview

Uh oh!

elasticmachine commented May 4, 2021

Uh oh!

elasticmachine commented May 4, 2021

Uh oh!

danhermann left a comment

Choose a reason for hiding this comment

Uh oh!

ppf2 left a comment

Choose a reason for hiding this comment

Uh oh!

jdconrad 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!

jrodewig 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!

jdconrad left a comment

Choose a reason for hiding this comment

Uh oh!

jrodewig commented May 4, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jrodewig commented May 4, 2021 •

edited

Loading