Skip to content

Commit

Permalink
[APM]: Inferred types for aggregations (#37360)
Browse files Browse the repository at this point in the history
* [APM]: Inferred types for aggregations

Previously, aggregations returned by the ESClient were 'any' by default, and the return type had to be explicitly defined by the consumer to get any type safety. This leads to both type duplication and errors because of wrong assumptions.

This change infers the aggregation return type from the parameters passed to ESClient.search.

* Fix idx error

* Safeguard against querying against non-existing indices in functional tests

* Improve metric typings

* Automatically infer params from function arguments

* Remove unnecessary type hints
  • Loading branch information
dgieselaar authored Jun 17, 2019
1 parent 16fffe0 commit 4526e2a
Show file tree
Hide file tree
Showing 38 changed files with 597 additions and 822 deletions.
10 changes: 8 additions & 2 deletions x-pack/plugins/apm/public/hooks/useTransactionList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,20 @@ const getRelativeImpact = (
);

function getWithRelativeImpact(items: TransactionListAPIResponse) {
const impacts = items.map(({ impact }) => impact);
const impacts = items
.map(({ impact }) => impact)
.filter(impact => impact !== null) as number[];

const impactMin = Math.min(...impacts);
const impactMax = Math.max(...impacts);

return items.map(item => {
return {
...item,
impactRelative: getRelativeImpact(item.impact, impactMin, impactMax)
impactRelative:
item.impact !== null
? getRelativeImpact(item.impact, impactMin, impactMax)
: null
};
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { BucketAgg, ESFilter } from 'elasticsearch';
import { ESFilter } from 'elasticsearch';
import {
ERROR_GROUP_ID,
PROCESSOR_EVENT,
Expand Down Expand Up @@ -61,13 +61,8 @@ export async function getBuckets({
}
};

interface Aggs {
distribution: {
buckets: Array<BucketAgg<number>>;
};
}
const resp = await client.search(params);

const resp = await client.search<void, Aggs>(params);
const buckets = resp.aggregations.distribution.buckets.map(bucket => ({
key: bucket.key,
count: bucket.doc_count
Expand Down
89 changes: 38 additions & 51 deletions x-pack/plugins/apm/server/lib/errors/get_error_groups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { SearchParams } from 'elasticsearch';
import { idx } from '@kbn/elastic-idx';
import {
ERROR_CULPRIT,
Expand Down Expand Up @@ -37,7 +36,10 @@ export async function getErrorGroups({
}) {
const { start, end, uiFiltersES, client, config } = setup;

const params: SearchParams = {
// sort buckets by last occurrence of error
const sortByLatestOccurrence = sortField === 'latestOccurrenceAt';

const params = {
index: config.get<string>('apm_oss.errorIndices'),
body: {
size: 0,
Expand All @@ -56,7 +58,11 @@ export async function getErrorGroups({
terms: {
field: ERROR_GROUP_ID,
size: 500,
order: { _count: sortDirection }
order: sortByLatestOccurrence
? {
max_timestamp: sortDirection
}
: { _count: sortDirection }
},
aggs: {
sample: {
Expand All @@ -72,24 +78,22 @@ export async function getErrorGroups({
sort: [{ '@timestamp': 'desc' }],
size: 1
}
}
},
...(sortByLatestOccurrence
? {
max_timestamp: {
max: {
field: '@timestamp'
}
}
}
: {})
}
}
}
}
};

// sort buckets by last occurrence of error
if (sortField === 'latestOccurrenceAt') {
params.body.aggs.error_groups.terms.order = {
max_timestamp: sortDirection
};

params.body.aggs.error_groups.aggs.max_timestamp = {
max: { field: '@timestamp' }
};
}

interface SampleError {
'@timestamp': APMError['@timestamp'];
error: {
Expand All @@ -105,44 +109,27 @@ export async function getErrorGroups({
};
}

interface Bucket {
key: string;
doc_count: number;
sample: {
hits: {
total: number;
max_score: number | null;
hits: Array<{
_source: SampleError;
}>;
};
};
}

interface Aggs {
error_groups: {
buckets: Bucket[];
};
}
const resp = await client.search(params);

const resp = await client.search<void, Aggs>(params);
const buckets = idx(resp, _ => _.aggregations.error_groups.buckets) || [];
// aggregations can be undefined when no matching indices are found.
// this is an exception rather than the rule so the ES type does not account for this.
const hits = (idx(resp, _ => _.aggregations.error_groups.buckets) || []).map(
bucket => {
const source = bucket.sample.hits.hits[0]._source as SampleError;
const message =
idx(source, _ => _.error.log.message) ||
idx(source, _ => _.error.exception[0].message);

const hits = buckets.map(bucket => {
const source = bucket.sample.hits.hits[0]._source;
const message =
idx(source, _ => _.error.log.message) ||
idx(source, _ => _.error.exception[0].message);

return {
message,
occurrenceCount: bucket.doc_count,
culprit: idx(source, _ => _.error.culprit),
groupId: idx(source, _ => _.error.grouping_key),
latestOccurrenceAt: source['@timestamp'],
handled: idx(source, _ => _.error.exception[0].handled)
};
});
return {
message,
occurrenceCount: bucket.doc_count,
culprit: idx(source, _ => _.error.culprit),
groupId: idx(source, _ => _.error.grouping_key),
latestOccurrenceAt: source['@timestamp'],
handled: idx(source, _ => _.error.exception[0].handled)
};
}
);

return hits;
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { SearchParams } from 'elasticsearch';
import {
PROCESSOR_EVENT,
TRACE_ID,
Expand All @@ -17,25 +16,14 @@ export interface ErrorsPerTransaction {
[transactionId: string]: number;
}

interface TraceErrorsAggBucket {
key: string;
doc_count: number;
}

interface TraceErrorsAggResponse {
transactions: {
buckets: TraceErrorsAggBucket[];
};
}

export async function getTraceErrorsPerTransaction(
traceId: string,
setup: Setup
): Promise<ErrorsPerTransaction> {
const { start, end, client, config } = setup;

const params: SearchParams = {
index: [config.get('apm_oss.errorIndices')],
const params = {
index: config.get<string>('apm_oss.errorIndices'),
body: {
size: 0,
query: {
Expand All @@ -57,7 +45,7 @@ export async function getTraceErrorsPerTransaction(
}
};

const resp = await client.search<never, TraceErrorsAggResponse>(params);
const resp = await client.search(params);

return resp.aggregations.transactions.buckets.reduce(
(acc, bucket) => ({
Expand Down
8 changes: 4 additions & 4 deletions x-pack/plugins/apm/server/lib/helpers/es_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,10 @@ export function getESClient(req: Legacy.Request) {
const query = (req.query as unknown) as APMRequestQuery;

return {
search: async <Hits = unknown, Aggs = unknown>(
params: SearchParams,
search: async <Hits = unknown, U extends SearchParams = {}>(
params: U,
apmOptions?: APMOptions
): Promise<AggregationSearchResponse<Hits, Aggs>> => {
): Promise<AggregationSearchResponse<Hits, U>> => {
const nextParams = await getParamsForSearchRequest(
req,
params,
Expand All @@ -112,7 +112,7 @@ export function getESClient(req: Legacy.Request) {
}

return cluster.callWithRequest(req, 'search', nextParams) as Promise<
AggregationSearchResponse<Hits, Aggs>
AggregationSearchResponse<Hits, U>
>;
},
index: <Body>(params: IndexDocumentParams<Body>) => {
Expand Down

This file was deleted.

Loading

0 comments on commit 4526e2a

Please sign in to comment.