Geo-Match Enrich Processor #47243

talevy · 2019-09-27T18:23:00Z

Geo-Match Enrich Processor

this commit introduces a geo-match enrich processor that looks up a specific
geo_point field in the enrich-index for all entries that have a geo_shape match field
that meets some specific relation criteria with the input field.

For example, the enrich index may contain documents with zipcodes and their respective
geo_shape. Ingesting documents with a geo_point field can be enriched with which zipcode
they associate according to which shape they are contained within.

this commit also refactors some of the MatchProcessor by moving a lot of the shared code to
AbstractEnrichProcessor.

Closes #42639.

this commit introduces a geo-match enrich processor that looks up a specific `geo_point` field in the enrich-index for all entries that have a geo_shape match field that meets some specific relation criteria with the input field. For example, the enrich index may contain documents with zipcodes and their respective geo_shape. Ingesting documents with a geo_point field can be enriched with which zipcode they associate according to which shape they are contained within. this commit also refactors some of the MatchProcessor by moving a lot of the shared code to AbstractEnrichProcessor. Closes elastic#42639.

elasticmachine · 2019-09-27T18:23:02Z

Pinging @elastic/es-core-features

elasticmachine · 2019-09-27T18:23:04Z

Pinging @elastic/es-analytics-geo

talevy · 2019-09-27T18:24:49Z

Note to review:

this still need more tests
I was not sure how to extend EnrichPolicy to support passing in the shape_relation for the geo-enrich-processor without conflating the policy with things unrelated to Match

Please let me know if this looks reasonable. If so, then I will continue to clean it up further, otherwise I will await further instruction to refactor!

martijnvg

This looks great @talevy! The current abstraction looks good. I left a few comments.

I was not sure how to extend EnrichPolicy to support passing in the shape_relation for the geo-enrich-processor without conflating the policy with things unrelated to Match

I think shape_relation should be a pipeline configuration option and not a policy configuration, because this setting doesn't affect the enrich index being created and it is merely a query parameter.

martijnvg · 2019-09-30T07:17:13Z

x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichPolicyRunner.java

            // No need to also configure index_options, because keyword type defaults to 'docs'.
+        } else if (EnrichPolicy.GEO_MATCH_TYPE.equals(policy.getType())) {
+            matchFieldMapping = (builder) -> builder.field("type", "geo_shape");


I don't believe that geo_shape field type uses doc values by default, right?
Do you know whether the default is going to change in the future?
I just want to make sure we never store more than what is needed for enrich. (since doc values are not being used for querying)

the problem is that since doc-values aren't supported, neither is doc-values: false. I was debating whether this should be fixed upstream so that one is allowed to set that. Currently, it errors out if you attempt to set it

I will follow-up in another PR to expose doc-values parsing to geo_shape. I think it should understand that parameter, even though it only allows it to be false. For now, I think this is the best that can be done. Thankfully, I know when doc-values on shapes will be supported so I will be sure to update this!

👍 That sounds good to me.

x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichPolicyRunner.java

x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/GeoMatchProcessor.java

x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/AbstractEnrichProcessor.java

x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichProcessorFactory.java

talevy · 2019-10-02T22:45:12Z

run elasticsearch-ci/bwc
run elasticsearch-ci/default-distro

talevy · 2019-10-03T15:23:30Z

@elasticmachine update branch

martijnvg

LGTM, thanks for working on this!

martijnvg · 2019-10-03T20:27:00Z

x-pack/plugin/enrich/src/test/java/org/elasticsearch/xpack/enrich/BasicEnrichTests.java

+        assertThat(statsResponse.getCoordinatorStats().size(), equalTo(1));
+        String localNodeId = getInstanceFromNode(ClusterService.class).localNode().getId();
+        assertThat(statsResponse.getCoordinatorStats().get(0).getNodeId(), equalTo(localNodeId));
+        assertThat(statsResponse.getCoordinatorStats().get(0).getRemoteRequestsTotal(), greaterThanOrEqualTo(1L));


nit: this should be equalTo(1), since only 1 index request is ingested.
This test suite also restarts the node between each test, so this should be ok.

ah good catch. didn't update this.

talevy · 2019-10-03T20:31:35Z

I've only added support for geo_point for now. after merging, I will open an issue for supporting geo_shape. The problem is that parsing either geo_point or geo_shape gets really confusing since geo_point supports so many different field value types. I didn't want to hold up this PR because of that

talevy · 2019-10-04T17:40:30Z

run elasticsearch-ci/2
run elasticsearch-ci/bwc
run elasticsearch-ci/default-distro

this commit introduces a geo-match enrich processor that looks up a specific `geo_point` field in the enrich-index for all entries that have a geo_shape match field that meets some specific relation criteria with the input field. For example, the enrich index may contain documents with zipcodes and their respective geo_shape. Ingesting documents with a geo_point field can be enriched with which zipcode they associate according to which shape they are contained within. this commit also refactors some of the MatchProcessor by moving a lot of the shared code to AbstractEnrichProcessor. Closes elastic#42639.

this commit introduces a geo-match enrich processor that looks up a specific `geo_point` field in the enrich-index for all entries that have a geo_shape match field that meets some specific relation criteria with the input field. For example, the enrich index may contain documents with zipcodes and their respective geo_shape. Ingesting documents with a geo_point field can be enriched with which zipcode they associate according to which shape they are contained within. this commit also refactors some of the MatchProcessor by moving a lot of the shared code to AbstractEnrichProcessor. Closes #42639.

polyfractal · 2019-10-16T13:55:40Z

@talevy This merged into enrich, and that branch merged into master/7.5 right? Should we label this ticket with versions too so it's clear this landed as well?

martijnvg · 2019-10-16T14:22:23Z

@polyfractal I think the fact that #48039 is versioned, is sufficient? Also PRs on feature branches are typically not versioned and labelled non-issue in order to avoid confusion about features that have not been released yet.

polyfractal · 2019-10-16T14:33:15Z

@martijnvg 👍 makes sense, that works for me. Was just trying to hunt down if this made it as part of the enrich feature or not, since it was also being tracked semi-independent of the enrich feature.

I think I was also a bit confused on what it did too... thought it was a separate processor, rather than being a policy of enrich. So that makes more sense now too :)

talevy · 2019-10-16T16:56:53Z

what Martijn said :)

talevy added 2 commits September 27, 2019 07:07

init

e67b11c

talevy added :Analytics/Geo Indexing, search aggregations of geo points and shapes :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP labels Sep 27, 2019

talevy requested review from martijnvg, hub-cap and jbaiera September 27, 2019 18:23

martijnvg reviewed Sep 30, 2019

View reviewed changes

talevy added 4 commits September 30, 2019 10:28

Merge remote-tracking branch 'elastic/enrich' into enrich-geo

d0bc160

respond to review

cdce178

undo field mapper changes to geo_shape. unintentional

3cc42e2

oh my, forgot another miscode

0947a8a

talevy requested a review from martijnvg October 1, 2019 13:57

talevy added 2 commits October 2, 2019 14:02

support multiple geo-points as input

1c473cc

Merge remote-tracking branch 'elastic/enrich' into enrich-geo

e733070

martijnvg approved these changes Oct 3, 2019

View reviewed changes

talevy mentioned this pull request Oct 3, 2019

Asymmetry between supported point format in geo_point and geo_shape field types #47535

Closed

Merge remote-tracking branch 'elastic/enrich' into enrich-geo

5f32e31

talevy force-pushed the enrich-geo branch from 9fbec10 to 5f32e31 Compare October 4, 2019 20:51

talevy merged commit c20fa4d into elastic:enrich Oct 7, 2019

talevy deleted the enrich-geo branch October 7, 2019 18:24

talevy added the backport pending label Oct 7, 2019

talevy removed the backport pending label Oct 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Geo-Match Enrich Processor #47243

Geo-Match Enrich Processor #47243

talevy commented Sep 27, 2019

elasticmachine commented Sep 27, 2019

elasticmachine commented Sep 27, 2019

talevy commented Sep 27, 2019 •

edited

Loading

martijnvg left a comment

martijnvg Sep 30, 2019

talevy Sep 30, 2019 •

edited

Loading

talevy Sep 30, 2019

martijnvg Oct 1, 2019

talevy commented Oct 2, 2019

talevy commented Oct 3, 2019

martijnvg left a comment

martijnvg Oct 3, 2019

talevy Oct 3, 2019

talevy commented Oct 3, 2019 •

edited

Loading

talevy commented Oct 4, 2019

polyfractal commented Oct 16, 2019

martijnvg commented Oct 16, 2019

polyfractal commented Oct 16, 2019

talevy commented Oct 16, 2019

Geo-Match Enrich Processor #47243

Geo-Match Enrich Processor #47243

Conversation

talevy commented Sep 27, 2019

elasticmachine commented Sep 27, 2019

elasticmachine commented Sep 27, 2019

talevy commented Sep 27, 2019 • edited Loading

martijnvg left a comment

Choose a reason for hiding this comment

martijnvg Sep 30, 2019

Choose a reason for hiding this comment

talevy Sep 30, 2019 • edited Loading

Choose a reason for hiding this comment

talevy Sep 30, 2019

Choose a reason for hiding this comment

martijnvg Oct 1, 2019

Choose a reason for hiding this comment

talevy commented Oct 2, 2019

talevy commented Oct 3, 2019

martijnvg left a comment

Choose a reason for hiding this comment

martijnvg Oct 3, 2019

Choose a reason for hiding this comment

talevy Oct 3, 2019

Choose a reason for hiding this comment

talevy commented Oct 3, 2019 • edited Loading

talevy commented Oct 4, 2019

polyfractal commented Oct 16, 2019

martijnvg commented Oct 16, 2019

polyfractal commented Oct 16, 2019

talevy commented Oct 16, 2019

talevy commented Sep 27, 2019 •

edited

Loading

talevy Sep 30, 2019 •

edited

Loading

talevy commented Oct 3, 2019 •

edited

Loading