Allow histogram fields in average and sum aggregations #66891
Conversation
timroes
added
release_note:enhancement
Feature:Aggregations
|
Pinging @elastic/kibana-app (Team:KibanaApp) |
| }); | ||
|
|
||
| it('with mean aggregation', async () => { | ||
| // Percentile values (which are used by media behind the scenes) are not deterministic, |
There was a problem hiding this comment.
typo media. Also shouldn't the test case be called 'with median aggregation' instead of mean?
|
|
||
| it('with percentile ranks aggregation', async () => { | ||
| const data = await renderTableForAggregation('Percentile Ranks'); | ||
| expect(data).to.eql([['0%']]); |
There was a problem hiding this comment.
Nit: This test would be more helpful if it would test a different percentile rank than the default 0 - 0% is a typical error value.
There was a problem hiding this comment.
Going to ignore this for now, since the the Percentile Ranks has the same non-deterministic problem than the others, so will rather fix that as soon as that's solved.
| await PageObjects.visualize.navigateToNewVisualization(); | ||
| await PageObjects.visualize.clickDataTable(); | ||
| await PageObjects.visualize.clickNewSearch('histogram-test'); | ||
| await PageObjects.visChart.waitForVisualization(); |
There was a problem hiding this comment.
Seems to be an extra wait, should work fine without it since the clickNewSearch is waiting for header.waitUntilLoadingHasFinished();
There was a problem hiding this comment.
Better save then sorry ;)
|
|
||
| const renderTableForAggregation = async ( | ||
| aggregation: string | ||
| ): Promise<string[][] | string[][][]> => { |
There was a problem hiding this comment.
Extra explicit returning type, typed fine from returning return await PageObjects.visChart.getTableVisContent();
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* Allow histogram fields in average and sum aggs * Fix PR review
* master: (33 commits) [Saved Objects] adds support for including hidden types in saved objects client (elastic#66879) [Discover] Deangularize timechart header (elastic#66532) [Discover] Improve and unskip a11y context view test (elastic#66959) [SIEM] Refactor Timeline.timelineType draft to Timeline.status draft (elastic#66864) docs: update RUM documentation link (elastic#67042) [QA] fixup coverage ingestion tests. (elastic#66905) [Metrics UI] Add support for multiple groupings to Metrics Explorer (and Alerts) (elastic#66503) [Metrics UI] Add sorting for name and value to Inventory View (elastic#66644) [Metrics UI] Change Metric Threshold Alert charts to use bar charts (elastic#66672) [Uptime] Use React.lazy for alert type registration (elastic#66829) [Reporting] Consolidate API Integration Test configs (elastic#66637) Allow histogram fields in average and sum aggregations (elastic#66891) Fix saved object share link (elastic#66771) move role reset into the top level after clause (elastic#66971) Automate the labels for any PRs affecting files for the Ingest Management team (elastic#67022) [SIEMDPOINT] Move endpoint to siem (elastic#66907) server.uuid so is not used (elastic#66963) Revert "[ci/stats] fix git metadata collection (elastic#66840)" [Uptime] Unmount uptime app properly (elastic#66950) [Visualize] Bar chart: Show missing values on chart setting (elastic#66375) ...
Summary
Allow fields of type
histogramto be used insumandaverageaggregations, since ES added support for it in (Sum: elastic/elasticsearch#55916, Avg: elastic/elasticsearch#56099).You can see the original PR (#59387) for a list of console commands to get an index with a
histogramfield in it for testing purposes.Checklist
Delete any items that are not applicable to this PR.
[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support[ ] Documentation was added for features that require explanation or tutorials[ ] This was checked for keyboard-only and screenreader accessibility[ ] This renders correctly on smaller devices using a responsive layout. (You can test this in your browser[ ] This was checked for cross-browser compatibility, including a check against IE11For maintainers
[ ] This was checked for breaking API changes and was labeled appropriately