Skip to content

Commit

Permalink
[Infrastructure UI] Filter out null bucket items from average calcula…
Browse files Browse the repository at this point in the history
…tion (elastic#152333)

## Summary

Closes [elastic#152328](elastic#152328)

This PR fixes the average calculation in the Snapshot API, filtering out
buckets with null values from it, which are more likely to appear with
queries that use small data ranges.

The results after this change are equal to what Elasticsearch would
calculate in the avg aggregation

### How to test

- Make sure you have metrics data (either through enabling the system
module in metricbeat or connecting your local kibana to an oblt-cli
cluster)
- Navigate to `Infrastructure` > `Hosts`
- Filter the results to see a single host
- Change the data range filter and compare the KPIs against the table.
- Validate other pages that use the Snapshot API (Inventory UI and
Metrics UI to see if the results there are still correct

---------

Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit f6a0b88)
  • Loading branch information
crespocarlos committed Mar 1, 2023
1 parent 932f1b8 commit 50865f4
Show file tree
Hide file tree
Showing 6 changed files with 9 additions and 4 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/infra/common/http_api/snapshot_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ export const SnapshotRequestRT = rt.intersection([
region: rt.string,
filterQuery: rt.union([rt.string, rt.null]),
overrideCompositeSize: rt.number,
dropPartialBuckets: rt.boolean,
}),
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export const Tile = ({ type, ...props }: Props) => {
metrics: [{ type }],
groupBy: null,
includeTimeseries: true,
dropPartialBuckets: false,
});

return <KPIChart id={`$metric-${type}`} type={type} nodes={nodes} loading={loading} {...props} />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export function useSnapshot({
groupBy = null,
sendRequestImmediately = true,
includeTimeseries = true,
dropPartialBuckets = true,
requestTs,
...args
}: UseSnapshotRequest) {
Expand All @@ -56,6 +57,7 @@ export function useSnapshot({
lookbackSize: 5,
},
includeTimeseries,
dropPartialBuckets,
};

const { error, loading, response, makeRequest } = useHTTPRequest(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,16 @@ import { applyMetadataToLastPath } from './apply_metadata_to_last_path';
const getMetricValue = (row: MetricsAPIRow) => {
if (!isNumber(row.metric_0)) return null;
const value = row.metric_0;
return isFinite(value) ? value : null;
return Number.isFinite(value) ? value : null;
};

const calculateMax = (rows: MetricsAPIRow[]) => {
return max(rows.map(getMetricValue)) || 0;
};

const calculateAvg = (rows: MetricsAPIRow[]): number => {
return sum(rows.map(getMetricValue)) / rows.length || 0;
const values = rows.map(getMetricValue).filter(Number.isFinite);
return sum(values) / Math.max(values.length, 1);
};

const getLastValue = (rows: MetricsAPIRow[]) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export const transformRequestToMetricsAPIRequest = async ({
? snapshotRequest.overrideCompositeSize
: compositeSize,
alignDataToEnd: true,
dropPartialBuckets: true,
dropPartialBuckets: snapshotRequest.dropPartialBuckets ?? true,
includeTimeseries: snapshotRequest.includeTimeseries,
};

Expand Down
2 changes: 1 addition & 1 deletion x-pack/test/api_integration/apis/metrics_ui/snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ export default function ({ getService }: FtrProviderContext) {
name: 'cpu',
value: null,
max: 0.47105555555555556,
avg: 0.0672936507936508,
avg: 0.47105555555555556,
};

expect(snapshot).to.have.property('nodes');
Expand Down

0 comments on commit 50865f4

Please sign in to comment.