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

Add Significant Text agg to AggConfigs #122166

Merged
merged 6 commits into from
Jan 5, 2022

Conversation

Dosant
Copy link
Contributor

@Dosant Dosant commented Dec 30, 2021

Summary

close #67394

This adds significant text aggregation to agg configs and expressions.

This aggregation is similar to significant terms but works only on fields of type TEXT. It also doesn't support child aggregations.
Agg docs

For now, I excluded this agg in visualize editor (everywhere where we excluded multi_terms), but we can consider enabling it, as it gives some user-facing value. For example, we can retrive significant terms from TEXT fields for tag cloud:

Screen Shot 2021-12-30 at 15 38 25

But since this agg doesn't support child aggregations it only works for the COUNT metric. If a user changes count to something else, then there is an error. So I suppose that more works needs to be done in visualize editor before we can enable those.

* Significant text is available only for ES_FIELD_TYPES.TEXT,
* This information is not available from field.type, so we have to check this using underlying esTypes
*/
filterField: (field) =>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Significant text aggregation only allows fields of types TEXT,
for this had to add a new way to filter supported fields for an aggregation.

With this we will also be able to allow integer fields for significant terms as mentioned here: #40368 (comment)

@Dosant Dosant changed the title significant text agg wip Add Significant Text agg to AggConfigs Dec 30, 2021
@Dosant Dosant added Feature:Aggregations Aggregation infrastructure (AggConfig, esaggs, ...) release_note:skip Skip the PR/issue when compiling release notes Team:AppServicesSv v8.1.0 labels Dec 30, 2021
@Dosant
Copy link
Contributor Author

Dosant commented Jan 4, 2022

@elasticmachine merge upstream

@Dosant Dosant marked this pull request as ready for review January 4, 2022 11:50
@Dosant Dosant requested review from a team as code owners January 4, 2022 11:50
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServicesSv)

@Dosant Dosant requested review from ppisljar and flash1293 January 4, 2022 11:50
Copy link
Member

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

LGTM

@flash1293
Copy link
Contributor

@elasticmachine merge upstream

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

VisEditors changes LGTM, thanks for adding this. I will bring this topic up with the team, I agree that this is a helpful feature and should be exposed.

@Dosant
Copy link
Contributor Author

Dosant commented Jan 5, 2022

@elasticmachine merge upstream

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
data 506 508 +2

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
data 2930 2943 +13

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
data 441.0KB 444.8KB +3.7KB
visTypeHeatmap 10.2KB 10.3KB +60.0B
visTypeMetric 8.7KB 8.7KB +20.0B
visTypePie 7.8KB 7.8KB +40.0B
visTypeVislib 18.6KB 18.7KB +40.0B
visTypeXy 40.9KB 41.1KB +240.0B
total +4.1KB
Unknown metric groups

API count

id before after diff
data 3325 3340 +15

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:Aggregations Aggregation infrastructure (AggConfig, esaggs, ...) release_note:skip Skip the PR/issue when compiling release notes v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[data.search.aggs] Support significant text in AggConfigs
6 participants