Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DOCS] Document range enrich policy #79607

Merged
merged 20 commits into from
Oct 26, 2021

Conversation

mjmbischoff
Copy link
Contributor

Adding the docs similar to the match policy for the new range policy

@elasticsearchmachine elasticsearchmachine added v8.0.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels Oct 21, 2021
@martijnvg
Copy link
Member

The yaml test snippets fail with an error and assertion error:

java.lang.AssertionError: java.lang.AssertionError: callback must handle its own exceptions
»  	at org.elasticsearch.client.node.NodeClient.lambda$executeLocally$0(NodeClient.java:105) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.tasks.TaskManager$1.onResponse(TaskManager.java:169) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.tasks.TaskManager$1.onResponse(TaskManager.java:163) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.action.ActionListener$DelegatingActionListener.onResponse(ActionListener.java:184) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.action.admin.indices.get.TransportGetIndexAction.doMasterOperation(TransportGetIndexAction.java:113) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.action.admin.indices.get.TransportGetIndexAction.doMasterOperation(TransportGetIndexAction.java:39) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.action.support.master.info.TransportClusterInfoAction.masterOperation(TransportClusterInfoAction.java:45) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.action.support.master.info.TransportClusterInfoAction.masterOperation(TransportClusterInfoAction.java:24) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.action.support.master.TransportMasterNodeAction.executeMasterOperation(TransportMasterNodeAction.java:97) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.action.support.master.TransportMasterNodeAction$AsyncSingleAction.lambda$doStart$3(TransportMasterNodeAction.java:185) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.action.ActionRunnable$2.doRun(ActionRunnable.java:62) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.common.util.concurrent.AbstractRunnable.run(AbstractRunnable.java:26) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.common.util.concurrent.EsExecutors$DirectExecutorService.execute(EsExecutors.java:178) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.action.support.master.TransportMasterNodeAction$AsyncSingleAction.doStart(TransportMasterNodeAction.java:185) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.action.support.master.TransportMasterNodeAction.doExecute(TransportMasterNodeAction.java:128) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.action.support.master.TransportMasterNodeAction.doExecute(TransportMasterNodeAction.java:52) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.action.support.TransportAction$RequestFilterChain.proceed(TransportAction.java:77) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.action.support.ActionFilter$Simple.apply(ActionFilter.java:42) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.action.support.TransportAction$RequestFilterChain.proceed(TransportAction.java:75) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.action.support.TransportAction.execute(TransportAction.java:53) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.tasks.TaskManager.registerAndExecute(TaskManager.java:163) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.client.node.NodeClient.executeLocally(NodeClient.java:100) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.client.node.NodeClient.doExecute(NodeClient.java:80) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.client.support.AbstractClient.execute(AbstractClient.java:375) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.client.FilterClient.doExecute(FilterClient.java:54) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.xpack.enrich.EnrichPolicyRunner$3.doExecute(EnrichPolicyRunner.java:613) ~[?:?]
»  	at org.elasticsearch.client.support.AbstractClient.execute(AbstractClient.java:375) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.client.support.AbstractClient$IndicesAdmin.execute(AbstractClient.java:1254) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.client.support.AbstractClient$IndicesAdmin.getIndex(AbstractClient.java:1304) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.xpack.enrich.EnrichPolicyRunner.run(EnrichPolicyRunner.java:127) ~[?:?]
»  	at org.elasticsearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:678) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) ~[?:?]
»  	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) ~[?:?]
»  	at java.lang.Thread.run(Thread.java:834) [?:?]
»  Caused by: java.lang.AssertionError: callback must handle its own exceptions
»  	... 34 more
»  Caused by: org.elasticsearch.ElasticsearchException: Field 'range' has type [text] which doesn't appear to be a range type
»  	at org.elasticsearch.xpack.enrich.EnrichPolicyRunner.createRangeEnrichMappingBuilder(EnrichPolicyRunner.java:304) ~[?:?]
»  	at org.elasticsearch.xpack.enrich.EnrichPolicyRunner.resolveEnrichMapping(EnrichPolicyRunner.java:251) ~[?:?]
»  	at org.elasticsearch.xpack.enrich.EnrichPolicyRunner.prepareAndCreateEnrichIndex(EnrichPolicyRunner.java:375) ~[?:?]
»  	at org.elasticsearch.xpack.enrich.EnrichPolicyRunner.lambda$run$0(EnrichPolicyRunner.java:134) ~[?:?]
»  	at org.elasticsearch.action.ActionListener$DelegatingFailureActionListener.onResponse(ActionListener.java:217) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	at org.elasticsearch.client.node.NodeClient.lambda$executeLocally$0(NodeClient.java:103) ~[elasticsearch-8.0.0-SNAPSHOT.jar:8.0.0-SNAPSHOT]
»  	... 33 more

The assertion error can be fixed by moving prepareAndCreateEnrichIndex(...) method call inside the try block in the run method of EnrichPolicyRunner (to line 130). Fixing this should leave us just with the actual error (Field 'range' has type [text] which doesn't appear to be a range type) and not with many failing docs tests (node gets killed because of the assertion error).

I'm not sure why the actual error occurs. I will need to take a better look.

@martijnvg martijnvg added the :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP label Oct 25, 2021
@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Oct 25, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@martijnvg martijnvg self-requested a review October 25, 2021 11:46
@martijnvg martijnvg removed their assignment Oct 25, 2021
@martijnvg martijnvg added the >docs General docs changes label Oct 25, 2021
@elasticmachine elasticmachine added the Team:Docs Meta label for docs team label Oct 25, 2021
@elasticmachine
Copy link
Collaborator

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

@mjmbischoff
Copy link
Contributor Author

I left the call in the spot where it originally was, interesting as it does look like te other paths could raise exceptions as well. Moving it makes sense, feels a little bit weird to do it as part of this PR?

The remaining error is likely due to // TEST[continued] is missing on the second call, breaking the chain making it 3 separate tests. Since it relies on the mapping to indicate it was an ip range (10.100.0.0/16 gets conservatively mapped as a string by default) Because the chain is broken the document gets indexed with a text field, leading to the error.

… to try catch. Fixes "java.lang.AssertionError: java.lang.AssertionError: callback must handle its own exceptions"
…y moving to try catch. Fixes "java.lang.AssertionError: java.lang.AssertionError: callback must handle its own exceptions""

-> moving to separate PR.
This reverts commit 4d78c8e.
@martijnvg
Copy link
Member

Moving it makes sense, feels a little bit weird to do it as part of this PR?

Yes, let's do this in a separate PR.

@martijnvg
Copy link
Member

martijnvg commented Oct 25, 2021

The remaining error is likely due to // TEST[continued] is missing on the second call, breaking the chain making it 3 separate tests. Since it relies on the mapping to indicate it was an ip range (10.100.0.0/16 gets conservatively mapped as a string by default) Because the chain is broken the document gets indexed with a text field, leading to the error.

Yes, it could be that actual error is caused by another test snippet.

@martijnvg martijnvg requested a review from jrodewig October 25, 2021 15:26
@jrodewig jrodewig changed the title Adding docs for the range enrich policy [DOCS] Document range enrich policy Oct 25, 2021
Copy link
Contributor

@jrodewig jrodewig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mjmbischoff.

I pushed some commits to update the API docs, remove some unneeded snippet comments, publish your example, and clean up some of my own mistakes.

I left some suggestions and comments. My only major outstanding question is what happens if the processor's field contains an array? The docs aren't clear right now.

Copy link
Contributor

@jrodewig jrodewig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying @mjmbischoff. I pushed some additional commits to clean up some of my additions based on your comment and to move the include statements to the right place.

If those changes look okay to you, this LGTM. Since I contributed, I'd like to get final approval from @martijnvg before merging.

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @mjmbischoff and @jrodewig!
This LGTM

@mjmbischoff mjmbischoff merged commit c30ab86 into elastic:master Oct 26, 2021
@mjmbischoff mjmbischoff deleted the range_enrich_docs branch October 26, 2021 13:15
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Oct 26, 2021
* upstream/master: (209 commits)
  Enforce license expiration (elastic#79671)
  TSDB: Automatically add timestamp mapper (elastic#79136)
  [DOCS]  `_id` is required for bulk API's `update` action (elastic#79774)
  EQL: Add optional fields and limit joining keys on non-null values only (elastic#79677)
  [DOCS] Document range enrich policy (elastic#79607)
  [DOCS] Fix typos in 8.0 security migration (elastic#79802)
  Allow listing older repositories (elastic#78244)
  [ML] track inference model feature usage per node (elastic#79752)
  Remove IncrementalClusterStateWriter & related code (elastic#79738)
  Reuse previous indices lookup when possible (elastic#79004)
  Reduce merging in PersistedClusterStateService (elastic#79793)
  SQL: Adjust JDBC docs to use milliseconds for timeouts (elastic#79628)
  Remove endpoint for freezing indices (elastic#78918)
  [ML] add timeout parameter for DELETE trained_models API (elastic#79739)
  [ML] wait for .ml-state-write alias to be readable (elastic#79731)
  [Docs] Update edgengram-tokenizer.asciidoc (elastic#79577)
  [Test][Transform] fix UpdateTransformActionRequestTests failure (elastic#79787)
  Limit CS Update Task Description Size (elastic#79443)
  Apply the reader wrapper on can_match source (elastic#78988)
  [DOCS] Adds new transform limitation item and a note to the tutorial (elastic#79479)
  ...

# Conflicts:
#	server/src/main/java/org/elasticsearch/index/IndexMode.java
#	server/src/test/java/org/elasticsearch/index/TimeSeriesModeTests.java
elasticsearchmachine pushed a commit that referenced this pull request Oct 26, 2021
Adding docs for the range enrich policy

Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>

Co-authored-by: Michael Bischoff <michael.bischoff@elastic.co>
lockewritesdocs pushed a commit to lockewritesdocs/elasticsearch that referenced this pull request Oct 28, 2021
Adding docs for the range enrich policy

Co-authored-by: James Rodewig <40268737+jrodewig@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >docs General docs changes external-contributor Pull request authored by a developer outside the Elasticsearch team Team:Data Management Meta label for data/management team Team:Docs Meta label for docs team v7.16.0 v8.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants