Skip to content

Commit

Permalink
[APM] Make aggs optional in ES response type def (#43451) (#43562)
Browse files Browse the repository at this point in the history
* [APM] Make aggregations optional in ES response type definition

Closes #43436.

Our type definition for ElasticSearch aggregations suggests that the aggregations key will always be defined if we request aggregations. However, that is not the case, as there is an edge case where if no matching indices were found, the aggregations key will be undefined.

* Review feedback

* Support undefined values in number formatters

* Expect 200s for all API requests even without matching indices

* Bail early for asDynamicBytes as well
  • Loading branch information
dgieselaar authored Aug 20, 2019
1 parent 479590f commit ee5c0ff
Show file tree
Hide file tree
Showing 17 changed files with 85 additions and 95 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const INITIAL_DATA = {
p99: []
},
tpmBuckets: [],
overallAvgDuration: undefined
overallAvgDuration: null
},
anomalyTimeseries: undefined
};
Expand Down
53 changes: 22 additions & 31 deletions x-pack/legacy/plugins/apm/public/utils/formatters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -187,53 +187,40 @@ export function asPercent(
return numeral(decimal).format('0.0%');
}

type ByteFormatter = (value: number | null) => string;

function asKilobytes(value: number | null) {
if (value === null || isNaN(value)) {
return '';
}
function asKilobytes(value: number) {
return `${asDecimal(value / 1000)} KB`;
}

function asMegabytes(value: number | null) {
if (value === null || isNaN(value)) {
return '';
}
function asMegabytes(value: number) {
return `${asDecimal(value / 1e6)} MB`;
}

function asGigabytes(value: number | null) {
if (value === null || isNaN(value)) {
return '';
}
function asGigabytes(value: number) {
return `${asDecimal(value / 1e9)} GB`;
}

function asTerabytes(value: number | null) {
if (value === null || isNaN(value)) {
return '';
}
function asTerabytes(value: number) {
return `${asDecimal(value / 1e12)} TB`;
}

export function asBytes(value: number | null | undefined) {
if (value === null || value === undefined || isNaN(value)) {
return '';
}
function asBytes(value: number) {
return `${asDecimal(value)} B`;
}

export function asDynamicBytes(value: number | null | undefined) {
if (value === null || value === undefined || isNaN(value)) {
return '';
}
return unmemoizedFixedByteFormatter(value)(value);
}
const bailIfNumberInvalid = (cb: (val: number) => string) => {
return (val: number | null | undefined) => {
if (val === null || val === undefined || isNaN(val)) {
return '';
}
return cb(val);
};
};

type GetByteFormatter = (max: number) => ByteFormatter;
export const asDynamicBytes = bailIfNumberInvalid((value: number) => {
return unmemoizedFixedByteFormatter(value)(value);
});

const unmemoizedFixedByteFormatter: GetByteFormatter = max => {
const unmemoizedFixedByteFormatter = (max: number) => {
if (max > 1e12) {
return asTerabytes;
}
Expand All @@ -253,4 +240,8 @@ const unmemoizedFixedByteFormatter: GetByteFormatter = max => {
return asBytes;
};

export const getFixedByteFormatter = memoize(unmemoizedFixedByteFormatter);
export const getFixedByteFormatter = memoize((max: number) => {
const formatter = unmemoizedFixedByteFormatter(max);

return bailIfNumberInvalid(formatter);
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import { ESFilter } from 'elasticsearch';
import { idx } from '@kbn/elastic-idx';
import {
ERROR_GROUP_ID,
PROCESSOR_EVENT,
Expand Down Expand Up @@ -63,7 +64,9 @@ export async function getBuckets({

const resp = await client.search(params);

const buckets = resp.aggregations.distribution.buckets.map(bucket => ({
const buckets = (
idx(resp.aggregations, _ => _.distribution.buckets) || []
).map(bucket => ({
key: bucket.key,
count: bucket.doc_count
}));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { idx } from '@kbn/elastic-idx';
import {
PROCESSOR_EVENT,
TRACE_ID,
Expand Down Expand Up @@ -47,7 +48,7 @@ export async function getTraceErrorsPerTransaction(

const resp = await client.search(params);

return resp.aggregations.transactions.buckets.reduce(
return (idx(resp.aggregations, _ => _.transactions.buckets) || []).reduce(
(acc, bucket) => ({
...acc,
[bucket.key]: bucket.doc_count
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
import theme from '@elastic/eui/dist/eui_theme_light.json';
import { AggregationSearchResponse, AggregatedValue } from 'elasticsearch';
import { idx } from '@kbn/elastic-idx';
import { ChartBase } from './types';

const colors = [
Expand Down Expand Up @@ -49,23 +50,23 @@ export function transformDataToMetricsChart<Params extends AggregatedParams>(
chartBase: ChartBase
) {
const { aggregations, hits } = result;
const { timeseriesData } = aggregations;
const timeseriesData = idx(aggregations, _ => _.timeseriesData);

return {
title: chartBase.title,
key: chartBase.key,
yUnit: chartBase.yUnit,
totalHits: hits.total,
series: Object.keys(chartBase.series).map((seriesKey, i) => {
const agg = aggregations[seriesKey];
const overallValue = idx(aggregations, _ => _[seriesKey].value);

return {
title: chartBase.series[seriesKey].title,
key: seriesKey,
type: chartBase.type,
color: chartBase.series[seriesKey].color || colors[i],
overallValue: agg.value,
data: timeseriesData.buckets.map(bucket => {
overallValue,
data: (idx(timeseriesData, _ => _.buckets) || []).map(bucket => {
const { value } = bucket[seriesKey] as AggregatedValue;
const y = value === null || isNaN(value) ? null : value;
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { idx } from '@kbn/elastic-idx';
import { Setup } from '../../../helpers/setup_request';
import {
PROCESSOR_EVENT,
Expand Down Expand Up @@ -52,6 +53,6 @@ export async function getAllEnvironments({
};

const resp = await client.search(params);
const buckets = resp.aggregations.environments.buckets;
const buckets = idx(resp.aggregations, _ => _.environments.buckets) || [];
return buckets.map(bucket => bucket.key);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { idx } from '@kbn/elastic-idx';
import { Setup } from '../../../helpers/setup_request';
import {
SERVICE_NAME,
Expand Down Expand Up @@ -42,6 +43,6 @@ export async function getUnavailableEnvironments({
};

const resp = await client.search(params);
const buckets = resp.aggregations.environments.buckets;
const buckets = idx(resp.aggregations, _ => _.environments.buckets) || [];
return buckets.map(bucket => bucket.key);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { idx } from '@kbn/elastic-idx';
import { Setup } from '../../helpers/setup_request';
import { PromiseReturnType } from '../../../../typings/common';
import {
Expand Down Expand Up @@ -44,6 +45,6 @@ export async function getServiceNames({ setup }: { setup: Setup }) {
};

const resp = await client.search(params);
const buckets = resp.aggregations.services.buckets;
const buckets = idx(resp.aggregations, _ => _.services.buckets) || [];
return buckets.map(bucket => bucket.key);
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ function calculateRelativeImpacts(transactionGroups: ITransactionGroup[]) {

export type ITransactionGroup = ReturnType<typeof getTransactionGroup>;
function getTransactionGroup(
bucket: ESResponse['aggregations']['transactions']['buckets'][0],
bucket: Required<ESResponse>['aggregations']['transactions']['buckets'][0],
minutes: number
) {
const averageResponseTime = bucket.avg.value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import { flatten, sortByOrder } from 'lodash';
import { idx } from '@kbn/elastic-idx';
import {
SERVICE_NAME,
SPAN_SUBTYPE,
Expand Down Expand Up @@ -112,8 +113,8 @@ export async function getTransactionBreakdown({

const formatBucket = (
aggs:
| typeof resp['aggregations']
| typeof resp['aggregations']['by_date']['buckets'][0]
| Required<typeof resp>['aggregations']
| Required<typeof resp>['aggregations']['by_date']['buckets'][0]
) => {
const sumAllSelfTimes = aggs.sum_all_self_times.value || 0;

Expand All @@ -135,11 +136,12 @@ export async function getTransactionBreakdown({
return breakdowns;
};

const visibleKpis = sortByOrder(
formatBucket(resp.aggregations),
'percentage',
'desc'
).slice(0, MAX_KPIS);
const visibleKpis = resp.aggregations
? sortByOrder(formatBucket(resp.aggregations), 'percentage', 'desc').slice(
0,
MAX_KPIS
)
: [];

const kpis = sortByOrder(visibleKpis, 'name').map((kpi, index) => {
return {
Expand All @@ -150,7 +152,9 @@ export async function getTransactionBreakdown({

const kpiNames = kpis.map(kpi => kpi.name);

const timeseriesPerSubtype = resp.aggregations.by_date.buckets.reduce(
const bucketsByDate = idx(resp.aggregations, _ => _.by_date.buckets) || [];

const timeseriesPerSubtype = bucketsByDate.reduce(
(prev, bucket) => {
const formattedValues = formatBucket(bucket);
const time = bucket.key;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import { ESResponse } from './fetcher';

type IBucket = ReturnType<typeof getBucket>;
function getBucket(
bucket: ESResponse['aggregations']['ml_avg_response_times']['buckets'][0]
bucket: Required<
ESResponse
>['aggregations']['ml_avg_response_times']['buckets'][0]
) {
return {
x: bucket.key,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ export function timeseriesTransformer({
bucketSize: number;
}) {
const aggs = timeseriesResponse.aggregations;
const overallAvgDuration = idx(aggs, _ => _.overall_avg_duration.value);
const overallAvgDuration =
idx(aggs, _ => _.overall_avg_duration.value) || null;
const responseTimeBuckets = idx(aggs, _ => _.response_times.buckets);
const { avg, p95, p99 } = getResponseTime(responseTimeBuckets);
const transactionResultBuckets = idx(
Expand All @@ -41,7 +42,9 @@ export function timeseriesTransformer({
}

export function getTpmBuckets(
transactionResultBuckets: ESResponse['aggregations']['transaction_results']['buckets'] = [],
transactionResultBuckets: Required<
ESResponse
>['aggregations']['transaction_results']['buckets'] = [],
bucketSize: number
) {
const buckets = transactionResultBuckets.map(
Expand All @@ -67,7 +70,9 @@ export function getTpmBuckets(
}

function getResponseTime(
responseTimeBuckets: ESResponse['aggregations']['response_times']['buckets'] = []
responseTimeBuckets: Required<
ESResponse
>['aggregations']['response_times']['buckets'] = []
) {
return responseTimeBuckets.reduce(
(acc, bucket) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ type DistributionBucketResponse = PromiseReturnType<typeof bucketFetcher>;

export type IBucket = ReturnType<typeof getBucket>;
function getBucket(
bucket: DistributionBucketResponse['aggregations']['distribution']['buckets'][0]
bucket: Required<
DistributionBucketResponse
>['aggregations']['distribution']['buckets'][0]
) {
const sampleSource = idx(bucket, _ => _.sample.hits.hits[0]._source) as
| Transaction
Expand All @@ -32,7 +34,9 @@ function getBucket(
}

export function bucketTransformer(response: DistributionBucketResponse) {
const buckets = response.aggregations.distribution.buckets.map(getBucket);
const buckets = (
idx(response.aggregations, _ => _.distribution.buckets) || []
).map(getBucket);

return {
totalHits: response.hits.total,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,5 @@ export async function getDistributionMax(
};

const resp = await client.search(params);
return resp.aggregations.stats.max;
return resp.aggregations ? resp.aggregations.stats.max : null;
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,20 @@ export async function getLocalUIFilters({

const response = await client.search(params);

const { aggregations } = response;

if (!aggregations) {
return [];
}

return localFilterNames.map(key => {
const aggregations = response.aggregations[key];
const aggregationsForFilter = aggregations[key];
const filter = localUIFilters[key];

return {
...filter,
options: sortByOrder(
aggregations.by_terms.buckets.map(bucket => {
aggregationsForFilter.by_terms.buckets.map(bucket => {
return {
name: bucket.key,
count:
Expand Down
2 changes: 1 addition & 1 deletion x-pack/legacy/plugins/apm/typings/elasticsearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ declare module 'elasticsearch' {
> &
(SearchParams extends { body: Required<AggregationOptionMap> }
? {
aggregations: AggregationResultMap<SearchParams['body']['aggs']>;
aggregations?: AggregationResultMap<SearchParams['body']['aggs']>;
}
: {});

Expand Down
Loading

0 comments on commit ee5c0ff

Please sign in to comment.