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

Always report dimension and time series metric attributes in field caps api #93749

Conversation

martijnvg
Copy link
Member

The time_series_dimension and time_series_metric attributes can be defined on non tsdb indices. The field caps only reports both attributes on tsdb indices. This change alters the field caps api to also return these attributes when specified on non tsdb indices.

Relates to #93539

…e defined on non tsdb indices. The field caps only reports both attributes on tsdb indices. This change alters the field caps api to also return these attributes when specified on non tsdb indices.

Relates to elastic#93539
@martijnvg martijnvg marked this pull request as ready for review February 13, 2023 20:22
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Feb 13, 2023
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

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

@thomasneirynck
Copy link
Contributor

Corresponding Kibana PR here elastic/kibana#150954

@csoulios
Copy link
Contributor

I think this reverts the change implemented at PR #88695

This PR would allow Kibana to distinguish standard from time-series indices. As Kibana does not read the index settings and the only way to get information for fields is from the field caps API, we need to provide a way to distinguish metric/dimension fields that belong to time-series indices vs standard indices.

Back then we had agreed with Kibana team that field caps will expose metrics and dimensions only for time-series indices. For standard indices, it does not make sense to expose those parameters.

@martijnvg
Copy link
Member Author

@mattkime @thomasneirynck Regarding the issue that you describe here, the time_series_metric not being reported in case the index isn't a time series index even if time_series_metric attribute has been specified in the mapping.
@csoulios pointed out (^) that this wouldn't allow Kibana to distinguish between a regular index and time series index. In practise what an index makes a time series index is based on whether the Index.mode setting has been specified to time_series.

So looks like this fix would break other things. I like to fix this problem differently. If time_series_metric has been set to counter then we threat it like a normal number field if the index is a regular index. This way the avg, sum and other aggregations that are strict about counter fields, just think it is a regular number field and validation doesn't fail. This way the field caps api doesn't have to include time_series_dimension and time_series_metric attributes in the response for non tsdb indices/data streams. Like we do now.

Ideally we shouldn't allow defining time_series_dimension or time_series_metric attributes on fields in non tsdb data streams/indices. But there may already be indices in the wild that use these attributes. And prohibiting this now, would mean the indices would load after clusters have been upgraded.

@thomasneirynck
Copy link
Contributor

thanks @martijnvg

then we threat it like a normal number field if the index is a regular index.

So to confirm, practically speaking, for 8.7:

For end-users, the combination of both PRs will be:

  • when tsdb index, counter-fields are hidden from certain UX (no errors)
  • when datastream index, counter-fields will support all aggs (possibly semantically incorrect, but no errors)

@martijnvg
Copy link
Member Author

ES will put up a PR with this change. ie. all aggs will work on counter fields when it's regular data-stream. This is possibly semantically incorrect, but tolerable, because it's PEBKAC and those counter fields should not be used outside tsdb-index.

A non tsdb data stream is unsorted and when defining counter fields on such data streams we can't detect counter resets. I guess logically it doesn't make sense either, but it is tolerable if we threat counter fields in non tsdb data streams as regular number fields.

Kibana does nothing beyond elastic/kibana#150954

Yes.

when tsdb index, counter-fields are hidden from certain UX (no errors)

Yes, and we're going to look into improving the field caps api so that fields don't need to be hidden.

when datastream index, counter-fields will support all aggs (possibly semantically incorrect, but no errors)

Yes.

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

Successfully merging this pull request may close these issues.

4 participants