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

[TSDB] Metric fields in the field caps API #88695

Merged
merged 19 commits into from
Aug 4, 2022

Conversation

csoulios
Copy link
Contributor

@csoulios csoulios commented Jul 21, 2022

To assist the user in configuring the visualizations correctly while leveraging TSDB functionality, information about TSDB configuration should be exposed via the field caps API per field.

Especially for metrics fields, it must be clear which fields are metrics and if they belong to only time-series indexes or mixed time-series and non-time-series indexes.

To further distinguish metric fields when they belong to any of the following indices:

  • Standard (non-time-series) indexes
  • Time series indexes
  • Downsampled time series indexes

This PR modifies the field caps API so that the mapping parameters time_series_dimension and time_series_dimension are presented only when they are set on fields of time-series indexes. Those
parameters are completely ignored when they are set on standard (non-time-series) indexes.

This PR revisits some of the conventions adopted by #78790

@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine changed the base branch from master to main July 22, 2022 23:04
@csoulios csoulios marked this pull request as ready for review August 4, 2022 15:07
@csoulios csoulios requested review from nik9000 and romseygeek August 4, 2022 15:07
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Aug 4, 2022
@elasticsearchmachine
Copy link
Collaborator

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

@@ -80,44 +83,47 @@ public void assertSmallSimple(Object d1, Object d2, CheckedConsumer<XContentBuil
dimensionMapping.accept(mapping);
mapping.field("time_series_dimension", true);
mapping.endObject();
});
// Add a keyword dimension as a routing parameter
mapping.startObject("k").field("type", "keyword").field("time_series_dimension", true).endObject();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding a keyword dimension field named k, simply because there always must be a routing dimension. It's value is always k

@@ -137,11 +137,11 @@ field types are all described as the `keyword` type family.

`time_series_dimension`::
preview:[]
Whether this field is used as a time series dimension.
Whether this field is used as a time series dimension on all indices.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kilfoyle Maybe you have a better take on the doc changes here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good now! 👍

Copy link
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

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

LGTM. Very comprehensive tests for such a small functional change!

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

Looks good on my side.

Copy link
Contributor

@kilfoyle kilfoyle left a comment

Choose a reason for hiding this comment

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

All good for the text changes!

@csoulios
Copy link
Contributor Author

csoulios commented Aug 4, 2022

Thanks everyone for reviewing this so fast!

@csoulios csoulios merged commit b81f418 into elastic:main Aug 4, 2022
@csoulios csoulios deleted the field-caps-metrics branch August 4, 2022 17:42
@droberts195
Copy link
Contributor

#89171 (comment) relates to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :StorageEngine/TSDB You know, for Metrics Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants