Implementing Histogram Aggregation

[nebrass]

The following changes are proposed:

• Implementing Histogram Aggregation

Screenshot:

[eladiw]

some comments

[AsafMah]

Your PR still doesn't support "Extended Bounds", that can be enabled by the UI (you have the property, you do nothing with it).
What it does is send empty buckets for the entire range of the extended bounds.

If we consider that a non-important feature we can skip it, just have it documented

[AsafMah]

You have an off by one on hard bounds -
try "dayOfWeek" with hard bounds of 3 to 4
in kibana it gives you 3 and 4
in kusto it gives you 2,3,4,5

[AsafMah]

Some comments

[nebrass]

Hello @AsafMah & @eladiw thank you for your comments and suggestions !
I made some changes to fix the hard_bounds issue
for the extended_bounds I will mention in the wiki that it was not implemented for lack of knowledge about the feature itself..

[tamirkamara]

Query and visitor look good now!
I'll let the others review the rest of the code.

[AsafMah]

@tamirkamara are you sure?

There is something wrong with the logic of the bins, no matter what options I choose it puts 99.9% of the data in the "0" bin, the rest in the next bin, and all the other bins are emtpy.

Even when entering a manual interval of 500 I got the following query:

let _data = database("kibana").StormEvents ;
let _extdata = _data
| extend ['2%False'] = bin(todouble(['StormSummary'].['TotalDamages']), 7000000);
let _summarizablemetrics = _extdata
| summarize count() by ['2%False']
| order by ['2%False'] asc;
(_summarizablemetrics
| as aggs);
(_data | count | as hitsTotal);
(_data | limit 0 | as hits)

[tamirkamara]

I didn't test it myself @AsafMah, only looked at the visitor logic. With the newly added base classes it's not easy to see the exact query that gets generated...
Are you saying it's always seeing 7000000 as the bin size?

[AsafMah]

I didn't test it myself @AsafMah, only looked at the visitor logic. With the newly added base classes it's not easy to see the exact query that gets generated... Are you saying it's always seeing 7000000 as the bin size?

No, it always sets the bin size so it's basically all in the first bin.
If I set the max bars to 5 I get this:

And if I set max bars to 3 I get this:

@nebrass have you tested it?

[nebrass]

@AsafMah I just made a test and the result is not as expected as you mentioned below:

The generated query:

let _data = database("dev").StormEvents | where (['StartTime'] >= todatetime("2007-01-18T15:52:40.4150000Z") and ['StartTime'] <= todatetime("2022-01-18T15:52:40.4150000Z"));
let _extdata = _data
| extend ['2%False'] = bin(['InjuriesDirect'], 500);
let _summarizablemetrics = _extdata
| summarize count() by ['2%False']
| order by ['2%False'] asc;
(_summarizablemetrics | as aggs);
(_data | count | as hitsTotal);
(_data | limit 0 | as hits)

and the resulted data is the same:

[eladiw]

Nice job.
lgtm

All comments were resolved and discussed

[AsafMah]

@AsafMah I just made a test and the result is not as expected as you mentioned below:

The generated query:

let _data = database("dev").StormEvents | where (['StartTime'] >= todatetime("2007-01-18T15:52:40.4150000Z") and ['StartTime'] <= todatetime("2022-01-18T15:52:40.4150000Z"));
let _extdata = _data
| extend ['2%False'] = bin(['InjuriesDirect'], 500);
let _summarizablemetrics = _extdata
| summarize count() by ['2%False']
| order by ['2%False'] asc;
(_summarizablemetrics | as aggs);
(_data | count | as hitsTotal);
(_data | limit 0 | as hits)

and the resulted data is the same:

Try it with a field with bigger values (DamageCrops)

[eladiw]

changing the approval until all comments are resolved to avoid accidental merge

[AsafMah]

Overall looks good!
Some minor comments here

Also fix the open comments & merge conflict files and we can merge

[AsafMah]

Looks good!

[eladiw]

🚀