[Lens] Formula overall functions#99461
Conversation
…a into lens/formula-error-handling
|
Pinging @elastic/kibana-app (Team:KibanaApp) |
|
@elasticmachine merge upstream |
| movingAverage, | ||
| mapColumn, | ||
| math, | ||
| ]; |
There was a problem hiding this comment.
This style was removed last week, instead you should move this list into src/plugins/expressions/common/service/expressions_services.ts.
| inputTypes: ['datatable'], | ||
|
|
||
| help: i18n.translate('expressions.functions.overallSum.help', { | ||
| defaultMessage: 'Calculates the overall sum of a column in a data table', |
There was a problem hiding this comment.
This doc still calls it "overall sum" instead of talking about the 4 possible functions. The same goes for most of the other i18n help strings here. Please update them all
| case 'average': | ||
| valueCounter[bucketIdentifier] = (valueCounter[bucketIdentifier] ?? 0) + 1; | ||
| case 'sum': | ||
| accumulators[bucketIdentifier] = accumulatorValue + Number(currentValue); |
There was a problem hiding this comment.
It looks like you're handling array-value fields in average but not in sum, is that right? Will this fail for array values, or null?
| selectionStyle: 'hidden', | ||
| requiredReferences: [ | ||
| { | ||
| input: ['field', 'managedReference'], |
There was a problem hiding this comment.
Seems like it could take a fullReference as input, not entirely sure whether it should. Thoughts?
| return true; | ||
| }, | ||
|
|
||
| filterable: false, |
There was a problem hiding this comment.
If this is not filterable, why is there logic to pass KQL from the previous to this column?
| operationType: 'overall_max'; | ||
| }; | ||
|
|
||
| export const overallMaxOperation: OperationDefinition< |
There was a problem hiding this comment.
Is there such a big difference between each overall function that they need to be in separate files? It looks to me like the main difference is the text, but we could generate these functions the same as we generate the simple metrics?
| const nonHistogramColumns = buckets.filter( | ||
| (colId) => | ||
| layer.columns[colId].operationType !== 'date_histogram' && | ||
| layer.columns[colId].operationType !== 'range' |
There was a problem hiding this comment.
It's kind of unusual to exclude range operations from this, it's not something we do for the other group-by logic. I think this should be consistent with the rest so I'm inclined to remove the range filter.
There was a problem hiding this comment.
I added this so it's possible to use overall calculations on number histograms. I think it makes sense because number histograms are often a distribution of some sort and it makes sense to treat it as a single data series in respect to overall calculation. This is an important use case for o11y.
|
@wylieconlon I think I addressed all of your comments, could you check again? It's also handling array values now. |
💚 Build SucceededMetrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
wylieconlon
left a comment
There was a problem hiding this comment.
Changes LGTM! Now we can update the docs for overall functions
💚 Backport successful
This backport PR will be merged automatically after passing CI. |
Fixes #94597
Adds new formula functions:
overall_sum,overall_min,overall_max,overall_averageThey calculate the respective overall metric for the current series. If there are no separate series, it's calculating the overall metric by all rows. A series is defined by a histogram or date histogram dimension.
Examples
overall_sumwill group by both fieldA and fieldB valuesoverall_sumwill create one large single groupoverall_sumwill create one large single groupExample use cases
Percentage of sum over a number histogram (this is the o11y use case)
Divergence from the mean overall:
Divergence from the mean over time (separate for each series):
Relative percent range over time (separate for each series):
What do you think @ghudgins @dej611 @mbondyra @wylieconlon ?