Rollup support for TSVB #28762

alexwizp · 2019-01-15T15:04:07Z

Summary

Roll-up functionality support for TSVB.

Index pattern and rollups

In TSVB under Panel Options in the Index Pattern field, users will be able to add rollup index, regular index pattern or a mix or roll up and index pattern.
- Same restrictions as Visual editor apply: TSVB will not support rollups names using wildcards; Only 1 rollup can be used
Based on the input in Panel Options in the Index Pattern field, TSVB will be able to query:
- regular indices
- rollup index
- index combining a rollup and index pattern
Using TSVB user will be able to create multiple y-axis with different index patterns or rollups indices in the same chart.
Users specifying the index in TSVB will need to be explicit regarding the rollup name, TSVB index field will not include rollup index when using a wildcard

Aggregations

Similar to kibana visual editor TSVB will limit aggregations available in the TSVB UI based on the capabilities of the roll-up index pattern using the rollup capabilities api
TSVB will limit fields in rollup based on aggregation type
Pipeline aggregations (derivatives, moving average, etc) should include rolled up data (TSVB is often used to create an advanced calculation, aggregation supported should include the pipeline aggregations)
TSVB unique aggregations should work on rolled up data. For example:
- Math
- Serial aggregation
All TSVB visualizations should work with rollups
TSVB group by will not support filters and will only support term or everything

Time intervals

TSVB intervals should be a multiple of what is configured in the roll-up job. Since intervals are typed into TSVB it will be good to warn user ahead of time what is the minimal interval and not allow smaller intervals
It will be possible to set >= interval using the minimal intervals, which will still work by multiplying the minimal intervals
TSVB chart should support zoom in to a narrow time range or to the most granular possible in case it is a combination chart [Rollup] Loosen validations when only raw data is queried elasticsearch#35744
In case user zoom in to an interval smaller than the minimum interval in a roll-up only chart, TSVB should use the smallest interval by default

Time zones

A rollup job has always a configured time zone for the date field it rolled up the data on. The date_histogram on that field mus NOT use the Kibana configured timezone, but the timezone that is configured in the rollup job instead, since the request will fail otherwise.

Checklist

markov00

Overall code LGTM.
Please change all default exports to named exports as written in our styleguide.

x-pack/plugins/rollup/server/lib/search_strategies/rollup_search_strategy.js

src/legacy/core_plugins/metrics/server/lib/vis_data/series/get_request_params.js

src/legacy/core_plugins/metrics/index.js

src/legacy/core_plugins/metrics/server/lib/search_strategies/search_strategies_register.js

src/legacy/core_plugins/metrics/server/lib/search_strategies/searh_requests/search_request.js

x-pack/plugins/rollup/index.js

x-pack/plugins/rollup/server/lib/search_strategies/rollup_search_capabilities.js

elasticmachine · 2019-01-22T14:48:06Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-01-23T08:19:28Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-01-23T11:07:18Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-01-23T11:36:45Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-01-23T16:21:31Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

alexwizp · 2019-01-23T16:51:23Z

x-pack/plugins/rollup/server/routes/api/index_patterns.js

@@ -66,62 +66,10 @@ export function registerFieldsForWildcardRoute(server) {
        // Keep meta fields
        metaFields.forEach(field => fieldsFromFieldCapsApi[field] && rollupFields.push(fieldsFromFieldCapsApi[field]));

-        // Merge rollup capabilities information with field information


@jen-huang Could you please review that? We want to reuse your code

elasticmachine · 2019-01-24T11:10:26Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-01-24T15:00:10Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-01-24T16:31:09Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-01-25T08:56:28Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-01-25T15:42:06Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-01-28T10:01:20Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-02-20T16:58:40Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

alexwizp · 2019-02-21T14:38:29Z

@cchaos could you please have a look? We need review from elastic/kibana-design

cchaos

Glanced at the SASS changes, just an addition of another selector. LGTM

AlonaNadler · 2019-02-25T15:32:08Z

I checked the PR again, looks good
Few comments

After selecting aggregation when selecting the field it shows me each field with every metric (e.g. bytes.max_value ), I understand it is the way the field appears in the document

In the classic visual editor, it doesn't appear in the same way, it only shows the field bytes, assuming users already selected aggregation can we do it similarly in TSVB
Can we have an error when selecting an aggregation that can't work? currently, it shows zero which might be misleading without any additional context or error
I configured the index pattern in panel option to be on both rollup and regular index roll1*,kibana_sample_data_logs and specific series only on rollup1* minimum intervals are 30s.

Does it make sense that when my minimal intervals are configured on 30s and I'm zooming in to specific time range where TSVB selects 5s as intervals and I still see results on the series that uses only rollup?

timroes · 2019-02-25T18:25:06Z

After selecting aggregation when selecting the field it shows me each field with every metric (e.g. bytes.max_value ), I understand it is the way the field appears in the document

This should not happen. It should show only the "original field name", because that's the field name we need to query against. So it should behave like the classical vis editor as you've pointed out.

alexwizp · 2019-02-25T20:59:46Z

After selecting aggregation when selecting the field it shows me each field with every metric (e.g. bytes.max_value ), I understand it is the way the field appears in the document

This should not happen. It should show only the "original field name", because that's the field name we need to query against. So it should behave like the classical vis editor as you've pointed out.'

@timroes @AlonaNadler Let me explain. In example above Alona use the index pattern with wildcard (roll1*,kibana_sample_data_logs). From our requirements we cannot use Rollup search strategy for indexes with wildcard. See selected text below:

For Rollup Search Strategy we reuse logic from classical vis editor. See x-pack/plugins/rollup/server/lib/merge_capabilities_with_fields.js file in this PR. It means that if we detect that we should use Rollup Search Strategy we show bytes and filter all extra fields like e.g. bytes.max_value.

For Default Search Strategy all works like in the master branch.
@AlonaNadler @timroes Could you please checkout the master branch (without my changes) and try to create the same visualization with (roll1*,kibana_sample_data_logs)? bytes.max_value will be there

AlonaNadler · 2019-02-26T00:23:36Z

Initially, the team who implemented rollups in Kibana had a requirement to not support wildcard in rollup index pattern in Kibana. Apparently, there was a change and it is possible to use a wildcard with index pattern,
I followed up with the team and they said :

"we revised it so that it is possible to create a rollup index pattern with a wildcard now. the original reason was to prevent users from matching against more than one rollup index (since rollup search only supports querying against one rollup index). however, allowing users to use wildcard lets them search one rollup index + any number of regular indices. for example rollup_logstash,logstash-* as a single pattern"

I didn't' know about this was changed and in the rollup requirements, I added it as a requirement to not support wildcard in rollup, which was implemented in this PR, apologies my fault.

Is there any chance to revert this back?

alexwizp · 2019-02-26T08:19:39Z

It's not a problem to remove 'if' condition but let me show you one case:

Open you fresh Kibana instance and install 2 of our sample data: e.g Flights and E-Commerce (two indexes should be created here: kibana_sample_data_flights, kibana_sample_data_ecommerce)
Go to TSVB and create new visualization with unsupported for Rollup aggregation (in my example I just set 'Filter' for the Group By field). Index pattern should be "kibana_*".
Save you visualization.

Actual behavior: user see data for all indexes with prefix "kibana_". In real world it can be e.g company name. TSVB works as expected.

Let's go next:

Open Management -> Rollup Job and create a Rollup index. Prefix should be like before "kibana_". For my example I created a rollup job with name "kibana_rollup"
Open visualization which we saved in step 3

Expected behaviour: I expect to see the aggregated data by kibana_* wildcard like in step 3
Actual behaviour: I see the Error Message that Filter aggregation is not supported by Rollup search.

@AlonaNadler @timroes Are you ok with that? If not probably we don't need to revert it

elasticmachine · 2019-02-26T08:42:28Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

rayafratkina · 2019-02-26T14:08:56Z

We should have a separate conversation and PR about wildcard support. It's not worth holding up this PR while we figure out what do.

* Added a feature of rollup search on the UI side Signed-off-by: Alexey Antonov <alexwizp@gmail.com> * Rollup Feature - initial commit * Revert "Added a feature of rollup search on the UI side" This reverts commit 9568b0970b16f5102f50b748bb4d691a8612c2c2. # Conflicts: # src/legacy/core_plugins/metrics/public/components/index_pattern.js * Remove the 'label' property from the search strategies * Changed search by strategy from the last * add single search request * rollup_search_strategy add base implementation of isViable method * rollup_search_strategy add base implementation of isViable method -fix * Changed requests due to search request type * refactoring of import Base classes / remove '../../../../../../ * remove extra await * rollup_search_strategy. Refactoring of isRollupJobExists method * remove question * Add support of annotations and table data * skeleton for adding Search Strategy restrictions * Add rollup search capabilities * apply search strategy for annotations request * set fields capabilities for rollup strategy * add timezone, interval into SearchCapabilities * Add fields from capabilities * add timezone, interval into SearchCapabilities * fix default timezone * Merging of two Rollup Jobs was removed * move getFieldsForWildcard to searchStrategy * Fix TSVB search requests should have a timeout # Conflicts: # src/legacy/core_plugins/metrics/server/lib/vis_data/get_annotations.js # src/legacy/core_plugins/metrics/server/lib/vis_data/series/get_request_params.js * Add unit test * apply getEsShardTimeout for annorations/get_request_params, series/get_request_params * rename metrics -> tsvb * search_strategies_register refactoring: move 'add' method from class * Add merge rollup capabilities with fields * Add merge rollup capabilities with fields - small fixes * Add support of 'Everything' aggregation for Rollup Search * Return back metrics plugin * remove 'metrics' from the X-pack\rollup require * Fix test cases * fix broken test: fail: "apis InfraOps GraphQL Endpoints metrics should basically work" * rollup search - split by terms is not working * Add count metric * /get_bucket_size.js. Add support of 'auto' interval, Add support of gte intervals e.g.: >=1m * fix build_request_body test * [Rollup] [Phase 1] Error handling - rollup search errors should be more user friendly * [Rollup] [Phase 1] Table View - research the query to ES - sorting is not wokring * Merge elastic#26006 into rollup # Conflicts: # src/legacy/core_plugins/metrics/server/lib/vis_data/annorations/build_request_body.js # src/legacy/core_plugins/metrics/server/lib/vis_data/get_annotations.js # src/legacy/core_plugins/metrics/server/lib/vis_data/get_series_data.js # src/legacy/core_plugins/metrics/server/lib/vis_data/get_table_data.js # src/legacy/core_plugins/metrics/server/lib/vis_data/request_processors/annotations/query.js # src/legacy/core_plugins/metrics/server/lib/vis_data/series/__tests__/build_request_body.js # src/legacy/core_plugins/metrics/server/lib/vis_data/series/build_request_body.js # src/legacy/core_plugins/metrics/server/lib/vis_data/series/get_request_params.js * Add table view support * fix broken build * fix broken build * [Rollup] [Phase 1] - write new tests (rollup_search_request, rollup_search_strategy) * [Rollup] [Phase 1] - write tests for rollup_search_capabilities * Add test on default_search_capabilities, abstract_search_strategy, search_strategies_register * Add test cases for search_requests folder * [Rollup] [Phase 1] - write tests for rollup_search_strategy * FIx broken build * remove todo * fix calculation of interval value for rollup search * add unit tests * Remove default exports * fix PR comments * fix calendar intervals

* Added a feature of rollup search on the UI side Signed-off-by: Alexey Antonov <alexwizp@gmail.com> * Rollup Feature - initial commit * Revert "Added a feature of rollup search on the UI side" This reverts commit 9568b0970b16f5102f50b748bb4d691a8612c2c2. # Conflicts: # src/legacy/core_plugins/metrics/public/components/index_pattern.js * Remove the 'label' property from the search strategies * Changed search by strategy from the last * add single search request * rollup_search_strategy add base implementation of isViable method * rollup_search_strategy add base implementation of isViable method -fix * Changed requests due to search request type * refactoring of import Base classes / remove '../../../../../../ * remove extra await * rollup_search_strategy. Refactoring of isRollupJobExists method * remove question * Add support of annotations and table data * skeleton for adding Search Strategy restrictions * Add rollup search capabilities * apply search strategy for annotations request * set fields capabilities for rollup strategy * add timezone, interval into SearchCapabilities * Add fields from capabilities * add timezone, interval into SearchCapabilities * fix default timezone * Merging of two Rollup Jobs was removed * move getFieldsForWildcard to searchStrategy * Fix TSVB search requests should have a timeout # Conflicts: # src/legacy/core_plugins/metrics/server/lib/vis_data/get_annotations.js # src/legacy/core_plugins/metrics/server/lib/vis_data/series/get_request_params.js * Add unit test * apply getEsShardTimeout for annorations/get_request_params, series/get_request_params * rename metrics -> tsvb * search_strategies_register refactoring: move 'add' method from class * Add merge rollup capabilities with fields * Add merge rollup capabilities with fields - small fixes * Add support of 'Everything' aggregation for Rollup Search * Return back metrics plugin * remove 'metrics' from the X-pack\rollup require * Fix test cases * fix broken test: fail: "apis InfraOps GraphQL Endpoints metrics should basically work" * rollup search - split by terms is not working * Add count metric * /get_bucket_size.js. Add support of 'auto' interval, Add support of gte intervals e.g.: >=1m * fix build_request_body test * [Rollup] [Phase 1] Error handling - rollup search errors should be more user friendly * [Rollup] [Phase 1] Table View - research the query to ES - sorting is not wokring * Merge #26006 into rollup # Conflicts: # src/legacy/core_plugins/metrics/server/lib/vis_data/annorations/build_request_body.js # src/legacy/core_plugins/metrics/server/lib/vis_data/get_annotations.js # src/legacy/core_plugins/metrics/server/lib/vis_data/get_series_data.js # src/legacy/core_plugins/metrics/server/lib/vis_data/get_table_data.js # src/legacy/core_plugins/metrics/server/lib/vis_data/request_processors/annotations/query.js # src/legacy/core_plugins/metrics/server/lib/vis_data/series/__tests__/build_request_body.js # src/legacy/core_plugins/metrics/server/lib/vis_data/series/build_request_body.js # src/legacy/core_plugins/metrics/server/lib/vis_data/series/get_request_params.js * Add table view support * fix broken build * fix broken build * [Rollup] [Phase 1] - write new tests (rollup_search_request, rollup_search_strategy) * [Rollup] [Phase 1] - write tests for rollup_search_capabilities * Add test on default_search_capabilities, abstract_search_strategy, search_strategies_register * Add test cases for search_requests folder * [Rollup] [Phase 1] - write tests for rollup_search_strategy * FIx broken build * remove todo * fix calculation of interval value for rollup search * add unit tests * Remove default exports * fix PR comments * fix calendar intervals

elasticmachine · 2019-03-14T09:44:42Z

Pinging @elastic/kibana-app

Rollup fields were were merged twice with other fields, this caused duplicates. Fix of a bug introduced by #28762

Rollup fields were were merged twice with other fields, this caused duplicates. Fix of a bug introduced by elastic#28762

Rollup fields were were merged twice with other fields, this caused duplicates. Fix of a bug introduced by #28762

Rollup fields were were merged twice with other fields, this caused duplicates. Fix of a bug introduced by elastic#28762

alexwizp requested review from timroes and markov00 January 15, 2019 15:04

alexwizp added the WIP Work in progress label Jan 15, 2019

markov00 changed the title ~~Rollup~~ [WIP] Rollup support for TSVB Jan 15, 2019

markov00 reviewed Jan 15, 2019

View reviewed changes

x-pack/plugins/rollup/server/lib/search_strategies/rollup_search_strategy.js Outdated Show resolved Hide resolved

x-pack/plugins/rollup/server/lib/search_strategies/rollup_search_strategy.js Outdated Show resolved Hide resolved

alexwizp commented Jan 15, 2019

View reviewed changes

src/legacy/core_plugins/metrics/server/lib/vis_data/series/get_request_params.js Outdated Show resolved Hide resolved

alexwizp requested review from a team as code owners January 18, 2019 08:58

sulemanof force-pushed the rollup branch from b468203 to 194851a Compare January 18, 2019 09:00

alexwizp force-pushed the rollup branch from 194851a to f661066 Compare January 18, 2019 09:07