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

[data views] Time series counter fields are not aggregatable #150954

Merged
merged 2 commits into from
Feb 15, 2023

Conversation

mattkime
Copy link
Contributor

@mattkime mattkime commented Feb 10, 2023

Summary

Part of elastic/elasticsearch#93539

Currently, field caps reports time series counter fields as aggregatable as they support several aggs. Unfortunately, they don't support ALL aggs so its possible create an unsupported agg in Kibana which results in an ES error. In order to prevent this, the data view field list will report all time series counter fields as non-aggregatable until support for specific aggs can be provided throughout Kibana.

To test - Create a TSDS with a time series counter field and verify that its not aggregatable in data view management. Also check other apps to make sure the field isn't available for aggregations. https://www.elastic.co/guide/en/elasticsearch/reference/master/set-up-tsds.html

Checklist

Delete any items that are not applicable to this PR.

@thomasneirynck thomasneirynck self-requested a review February 13, 2023 14:27
@mattkime mattkime changed the title time series counter fields are not aggregatable [data views] Time series counter fields are not aggregatable Feb 13, 2023
@mattkime mattkime added Feature:Discover Discover Application release_note:skip Skip the PR/issue when compiling release notes labels Feb 13, 2023
@mattkime mattkime marked this pull request as ready for review February 13, 2023 20:45
@mattkime mattkime requested a review from a team as a code owner February 13, 2023 20:45
@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

Copy link
Contributor

@davismcphee davismcphee left a comment

Choose a reason for hiding this comment

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

Pulled and tested locally, following the provided docs and adding a test field with "time_series_metric": "counter". Confirmed it appears as non-aggregatable in the UI. LGTM!

Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

code review.

Confirmed in Kibana errors are still being thrown with regular data-streams. Will first need fix on ES-side outlined here: elastic/elasticsearch#93749 (comment) . Ie. allow all aggs on counter-fields when index-mode is not set to time_series

@mattkime mattkime added the auto-backport Deprecated - use backport:version if exact versions are needed label Feb 15, 2023
@mattkime mattkime merged commit 410f019 into elastic:main Feb 15, 2023
kibanamachine pushed a commit that referenced this pull request Feb 15, 2023
## Summary

Part of elastic/elasticsearch#93539

Currently, field caps reports time series counter fields as aggregatable
as they support several aggs. Unfortunately, they don't support ALL aggs
so its possible create an unsupported agg in Kibana which results in an
ES error. In order to prevent this, the data view field list will report
all time series counter fields as non-aggregatable until support for
specific aggs can be provided throughout Kibana.

To test - Create a TSDS with a time series counter field and verify that
its not aggregatable in data view management. Also check other apps to
make sure the field isn't available for aggregations.
https://www.elastic.co/guide/en/elasticsearch/reference/master/set-up-tsds.html

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 410f019)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.7

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

kibanamachine added a commit that referenced this pull request Feb 15, 2023
…150954) (#151240)

# Backport

This will backport the following commits from `main` to `8.7`:
- [[data views] Time series counter fields are not aggregatable
(#150954)](#150954)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Matthew
Kime","email":"matt@mattki.me"},"sourceCommit":{"committedDate":"2023-02-15T00:38:23Z","message":"[data
views] Time series counter fields are not aggregatable (#150954)\n\n##
Summary\r\n\r\nPart of
https://github.com/elastic/elasticsearch/issues/93539\r\n\r\nCurrently,
field caps reports time series counter fields as aggregatable\r\nas they
support several aggs. Unfortunately, they don't support ALL aggs\r\nso
its possible create an unsupported agg in Kibana which results in
an\r\nES error. In order to prevent this, the data view field list will
report\r\nall time series counter fields as non-aggregatable until
support for\r\nspecific aggs can be provided throughout
Kibana.\r\n\r\nTo test - Create a TSDS with a time series counter field
and verify that\r\nits not aggregatable in data view management. Also
check other apps to\r\nmake sure the field isn't available for
aggregations.\r\nhttps://www.elastic.co/guide/en/elasticsearch/reference/master/set-up-tsds.html\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"410f0199408b2c55389ed7d6dfd3170cdede2c84","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Discover","release_note:skip","auto-backport","v8.7.0","v8.8.0","Feature:
TSDS"],"number":150954,"url":"https://github.com/elastic/kibana/pull/150954","mergeCommit":{"message":"[data
views] Time series counter fields are not aggregatable (#150954)\n\n##
Summary\r\n\r\nPart of
https://github.com/elastic/elasticsearch/issues/93539\r\n\r\nCurrently,
field caps reports time series counter fields as aggregatable\r\nas they
support several aggs. Unfortunately, they don't support ALL aggs\r\nso
its possible create an unsupported agg in Kibana which results in
an\r\nES error. In order to prevent this, the data view field list will
report\r\nall time series counter fields as non-aggregatable until
support for\r\nspecific aggs can be provided throughout
Kibana.\r\n\r\nTo test - Create a TSDS with a time series counter field
and verify that\r\nits not aggregatable in data view management. Also
check other apps to\r\nmake sure the field isn't available for
aggregations.\r\nhttps://www.elastic.co/guide/en/elasticsearch/reference/master/set-up-tsds.html\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"410f0199408b2c55389ed7d6dfd3170cdede2c84"}},"sourceBranch":"main","suggestedTargetBranches":["8.7"],"targetPullRequestStates":[{"branch":"8.7","label":"v8.7.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/150954","number":150954,"mergeCommit":{"message":"[data
views] Time series counter fields are not aggregatable (#150954)\n\n##
Summary\r\n\r\nPart of
https://github.com/elastic/elasticsearch/issues/93539\r\n\r\nCurrently,
field caps reports time series counter fields as aggregatable\r\nas they
support several aggs. Unfortunately, they don't support ALL aggs\r\nso
its possible create an unsupported agg in Kibana which results in
an\r\nES error. In order to prevent this, the data view field list will
report\r\nall time series counter fields as non-aggregatable until
support for\r\nspecific aggs can be provided throughout
Kibana.\r\n\r\nTo test - Create a TSDS with a time series counter field
and verify that\r\nits not aggregatable in data view management. Also
check other apps to\r\nmake sure the field isn't available for
aggregations.\r\nhttps://www.elastic.co/guide/en/elasticsearch/reference/master/set-up-tsds.html\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"410f0199408b2c55389ed7d6dfd3170cdede2c84"}}]}]
BACKPORT-->

Co-authored-by: Matthew Kime <matt@mattki.me>
justinkambic pushed a commit to justinkambic/kibana that referenced this pull request Feb 23, 2023
…#150954)

## Summary

Part of elastic/elasticsearch#93539

Currently, field caps reports time series counter fields as aggregatable
as they support several aggs. Unfortunately, they don't support ALL aggs
so its possible create an unsupported agg in Kibana which results in an
ES error. In order to prevent this, the data view field list will report
all time series counter fields as non-aggregatable until support for
specific aggs can be provided throughout Kibana.

To test - Create a TSDS with a time series counter field and verify that
its not aggregatable in data view management. Also check other apps to
make sure the field isn't available for aggregations.
https://www.elastic.co/guide/en/elasticsearch/reference/master/set-up-tsds.html

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
@mattkime mattkime mentioned this pull request Mar 2, 2023
@ppisljar ppisljar mentioned this pull request Mar 6, 2023
4 tasks
ppisljar added a commit that referenced this pull request Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed Feature:Discover Discover Application Feature: TSDS release_note:skip Skip the PR/issue when compiling release notes v8.7.0 v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants