TSVB Filter ratio denominator not handling negative values

[m-wcislo]

Kibana version:
v 8.4.3
Elasticsearch version:
v 8.4.3
Server OS version:
Rhel7
Browser version:
Firefox 109.0.1
Browser OS version:
Ubuntu 22.04

Describe the bug:
When using TSVB Metric, when adding Filter Ratio aggregation (Sum based) it seems that result is made 0 when the result of underlying sum for denominator is negative.
Steps to reproduce:
1.Data with field test which is runtime field with values both negative, positive and 0.
2.Create TSVB Metric with Filter ratio aggregation based on sum of test
3.Following was observed:

Numerator filter	Denominator filter	Result
test > 0	test > 0	1
test < 0	test > 0	correct negative value
test < 0	test < 0	0 (should be 1)
test > 0	test < 0	0 (should be correct negative value)

Expected behavior:
Denominator seems to have problems with negative values. So expected is to have 1 when both numerator and denominator filters same documents and counts same value (even when the sum is negative).

[elasticmachine]

Pinging @elastic/kibana-visualizations @elastic/kibana-visualizations-external (Team:Visualizations)

[stratoula]

This is the script that runs during filter ration function

"params.numerator != null && params.denominator != null && params.denominator > 0 ? params.numerator / params.denominator : 0"

It seems that when the denominator is negative it defaults to 0. This hasn't changed the last many minors so it doesn't see as a bug to me but as a business decision. Unfortunately at this point I dont know how the decision was made. I will change it to enhancement for now, we need to research why this decision was made and either close the issue or change the behavior.

[markov00]

@stratoula you are right, this was the formula that was used since the beginning of TSVB #9725
Probably @simianhacker can chime in to remember why that decision was made.

[stratoula]

Filter ratio is supported in Lens formulas and it doesn't have this limitation so I am closing this as it is supported in Lens with formulas.

[m-wcislo]

Do we have any explanation why it doesn't work in TSVB? Is it documented anywhere? The fact that it works in lens is only confirming that it might be bug in tsvb.

[stratoula]

It is how it worked in TSVB since the beginning, @simianhacker can give more information I assume why this has been decided.

[m-wcislo]

Well the fact that it work the same since it was implemented is nature of every bug. Can't understand why this issue is closed, nothing changed since the first comment. Also, seems it is not documented and works correctly in other parts of the system, which contradicts any business decision that could be made. On top of that it is simply wrong from mathematical perspective giving no warning to the users, until they find out their dashboards have wrong calculations.

[simianhacker]

From a product perspective, we decided to use ZERO since were were talking about a ratios. Maybe it would have been more appropriate to return null.

That being said, I could see an argument for changing the painless script to:

"params.numerator != null && params.denominator != null && params.denominator != 0 ? params.numerator / params.denominator : 0"

Which would support 5 / -3 which would give you a negative ratio of -1.6667.

[simianhacker]

BTW... I've created a PR for this change: #152053

[m-wcislo]

Perfect, thanks for fixing this.

[thomasneirynck]

my2c - this seems to be a bug. The fact it doesn't come up often is, I expect, likely because most users are actually not creating ratios with different signs in nominator/denominator (otherwise, wouldn't they also think it's odd their values come up 0 (?)).

If it's a low effort fix, I'm ok with merging, despite the minor change in behavior (which imho is buggy).

[simianhacker]

@m-wcislo I just merged the PR, the update will be in 8.8

@thomasneirynck & @m-wcislo As far as returning 0 when the denominator is 0, should we change that to null? I don't have a strong opinion one way or another. It's a very small change BUT it will change the results which might be unexpected. The negative ratio was a bug (fixed).

[thomasneirynck]

As far as returning 0 when the denominator is 0, should we change that to null

No big opinion either way fwiw. Maybe just keep as-is and not change the behavior.