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

[TSVB] Remove the broken "aggregate function" in TSVB table #91149

Closed
wylieconlon opened this issue Feb 11, 2021 · 11 comments · Fixed by #119967
Closed

[TSVB] Remove the broken "aggregate function" in TSVB table #91149

wylieconlon opened this issue Feb 11, 2021 · 11 comments · Fixed by #119967
Assignees
Labels
enhancement New value added to drive a business result Feature:TSVB TSVB (Time Series Visual Builder) Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@wylieconlon
Copy link
Contributor

This is an issue originally reported on Discuss. If you go to TSVB > Table > Options > Aggregate function, it gives you form controls that configure a Series Agg, but the controls don't work.

This feature should be removed because it's confusing and redundant.

@wylieconlon wylieconlon added Feature:TSVB TSVB (Time Series Visual Builder) enhancement New value added to drive a business result Team:Visualizations Visualization editors, elastic-charts and infrastructure :KibanaApp/fix-it-week labels Feb 11, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@wylieconlon
Copy link
Contributor Author

@simianhacker @timroes Are you aware of this function ever working? Or is this change risk-free because it never worked?

@wylieconlon
Copy link
Contributor Author

I just created a deployment using 7.0.1 and observed that the feature has existed since then, and the feature appears to be doing something but not necessarily something useful.

@simianhacker
Copy link
Member

Yes, this worked when it was originally developed.

@wylieconlon
Copy link
Contributor Author

wylieconlon commented Feb 18, 2021

@simianhacker can you clarify a bit more? When I tested it in 7.0 I wasn't sure what it was doing, since it seems like it's doing something different from what the series agg does.

To be more specific, the "Series agg" reduces the total number of visible series. It seems like the "aggregate function" is not capable of reducing the number of visible series.

@alexwizp
Copy link
Contributor

@wylieconlon honestly, I think the initial idea of that was to do something like our new 'Entire Timerange mode' feature. But I'm not sure.

@simianhacker
Copy link
Member

Here is the use case that worked at one point. Let's say your listing all your hosts and you want to have a column for network traffic. Keep in mind that network traffic for system.network.out.bytes is recorded for every interface which is identified by system.network.name. What you really want is OVERALL network traffic which is all the traffic aggregated together across all the interfaces. To do that you would setup the metrics tab (for a series) like this:

image

and the options tab like this

image

The query would throw a terms agg in there and then do something similar to the series_agg to aggregate all the values together for that column.

Here is the original PR: #12813

@stratoula
Copy link
Contributor

Although, it doesn't seem to work as @simianhacker described I think we should first gather metrics on how many visualizations are using it and then decide if we want to remove it or fix it.

@flash1293
Copy link
Contributor

Discussed offline, we decided to go with @stratoula s suggestion of implementing telemetry before removing it completely.

@simianhacker
Copy link
Member

Please fix this feature and don't remove it, it's the only way to do any quasi-multiple-groupings of data.

@flash1293
Copy link
Contributor

I traced this back - it broke in 7.2 during rollup integration in #28762 more specifically because of this change: https://github.com/elastic/kibana/pull/28762/files#diff-8356ba367c7c14f240fbaa3bcd6a3d4042c5d3e2a02193ffe2355ee5cda300dcR28 If i'm removing this part it works as expected.

In case of aggregate function in TSVB table, there's no timeseries agg directly in the pivot split, so it behaves as if it would be a rollup response. @alexwizp could you check whether there is an easy way to fix this?

Legacy visualizations automation moved this from Long-term goals to Done Jan 4, 2022
alexwizp added a commit that referenced this issue Jan 4, 2022
* [TSVB] Fix the broken "aggregate function" in TSVB table

Closes: #91149

* [TSVB] Table series filter and aggregation function applied at the same time cause an error

# Conflicts:
#	src/plugins/vis_types/timeseries/server/lib/vis_data/request_processors/table/split_by_everything.ts
#	src/plugins/vis_types/timeseries/server/lib/vis_data/request_processors/table/split_by_terms.ts

* some work

* filter terms columns

* fix error message on no pivot_id

* fix CI

* enable aggregation function for entire timerange

* fix PR comments

* update check_aggs

* fix series aggs for table

* unify error messages

* fix pr comment: restrictions: UIRestrictions = DEFAULT_UI_RESTRICTION

* fix i18n translation error

* fixes translations

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
gbamparop pushed a commit to gbamparop/kibana that referenced this issue Jan 12, 2022
)

* [TSVB] Fix the broken "aggregate function" in TSVB table

Closes: elastic#91149

* [TSVB] Table series filter and aggregation function applied at the same time cause an error

# Conflicts:
#	src/plugins/vis_types/timeseries/server/lib/vis_data/request_processors/table/split_by_everything.ts
#	src/plugins/vis_types/timeseries/server/lib/vis_data/request_processors/table/split_by_terms.ts

* some work

* filter terms columns

* fix error message on no pivot_id

* fix CI

* enable aggregation function for entire timerange

* fix PR comments

* update check_aggs

* fix series aggs for table

* unify error messages

* fix pr comment: restrictions: UIRestrictions = DEFAULT_UI_RESTRICTION

* fix i18n translation error

* fixes translations

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:TSVB TSVB (Time Series Visual Builder) Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
No open projects
7 participants