Skip to content

Support geo label position through REST vector tiles API#86458

Merged
craigtaverner merged 6 commits intoelastic:masterfrom
craigtaverner:label_position_rest_mvt
May 17, 2022
Merged

Support geo label position through REST vector tiles API#86458
craigtaverner merged 6 commits intoelastic:masterfrom
craigtaverner:label_position_rest_mvt

Conversation

@craigtaverner
Copy link
Copy Markdown
Contributor

@craigtaverner craigtaverner commented May 5, 2022

There is a need to provide sensibly calculated label positions for polygons and lines in Kibana maps (as requested in #86044). A very convenient way to satisfy this need is through a runtime field that the rest API can make use of when labels are requested. This has the advantage of providing painless access to the label position as well.

Building onto #86154, this PR adds support for the REST API to provide label positions to MVT queries, both for the HITS layer and the AGGS layer. To enable this feature, set with_labels to true as a query parameter to the vector tile search query.

Closes #86044

@craigtaverner craigtaverner added >enhancement :Analytics/Geo Indexing, search aggregations of geo points and shapes labels May 5, 2022
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Hi @craigtaverner, I've created a changelog YAML for you.

@craigtaverner craigtaverner force-pushed the label_position_rest_mvt branch from cd3df0f to f93565b Compare May 9, 2022 09:05
@craigtaverner craigtaverner changed the title Label position rest mvt Support geo label position through REST vector tiles API May 9, 2022
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Hi @craigtaverner, I've created a changelog YAML for you.

@craigtaverner craigtaverner force-pushed the label_position_rest_mvt branch 3 times, most recently from 2f65f05 to 0c6c2b1 Compare May 9, 2022 12:53
@craigtaverner craigtaverner marked this pull request as ready for review May 9, 2022 16:11
@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label May 9, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

@sethmlarson sethmlarson added the Team:Clients Meta label for clients team label May 9, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/clients-team (Team:Clients)

@craigtaverner craigtaverner force-pushed the label_position_rest_mvt branch from 153ac85 to 6354bd7 Compare May 10, 2022 10:01
Copy link
Copy Markdown
Contributor

@iverase iverase left a comment

Choose a reason for hiding this comment

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

@iverase
Copy link
Copy Markdown
Contributor

iverase commented May 10, 2022

In addition, can we update the description of the PR because it is used to generate the release notes?

featureBuilder.mergeFrom(labelPosFeature);
VectorTileUtils.addPropertyToFeature(featureBuilder, layerProps, ID_TAG, searchHit.getId());
VectorTileUtils.addPropertyToFeature(featureBuilder, layerProps, LABEL_POSITION_TAG, true);
hitsLayerBuilder.addFeatures(featureBuilder);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we agreed that we should add all the tags that we added to the feature that is not a tag. That brings the question to what happens if the user has defined an attribute with the same name as the label?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think both concerns are considered in 9fa8c00

VectorTileUtils.addPropertyToFeature(featureBuilder, layerProps, KEY_TAG, bucketKey);
VectorTileUtils.addPropertyToFeature(featureBuilder, layerProps, LABEL_POSITION_TAG, true);
aggLayerBuilder.addFeatures(featureBuilder);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we agreed that we should add all the tags that we added to the feature that is not a tag. That brings the question to what happens if the user has defined an attribute with the same name as the label?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think both concerns are considered in 9fa8c00

* Use _mvt_ prefix for all internal tags
* Remove runtime field from original feature
* Copy all original tags to label position geometries
* Same with aggs (copy to label position)
Copy link
Copy Markdown
Contributor

@iverase iverase left a comment

Choose a reason for hiding this comment

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

LGTM!

* `LineString` features will likewise provide a roughly central point selected
from the <<geoshape-indexing-approach,triangle-tree>>.
* The aggregation results will provide one central point for each aggregation bucket.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Use + instead of empty line so statement below is indented under with_labels option

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I will add that the label features inherit all the attributes from its corresponding original feature.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I had tried that before, but it indented too much (made it part of the last bullet point). However, now I tried again, this time with an extra newline before the + and voila! It indented the right amount!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I fixed both your suggestions in 1c69a4a

Copy link
Copy Markdown
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

doc changes LGTM

@craigtaverner craigtaverner merged commit db08d61 into elastic:master May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/Geo Indexing, search aggregations of geo points and shapes >enhancement Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) Team:Clients Meta label for clients team v8.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add label features to vector tile search API response

6 participants