Skip to content

Commit

Permalink
[APM]removing rest_total_hits_as_int and changing hits.total type (#4…
Browse files Browse the repository at this point in the history
…7814) (#48218)

* removing rest_total_hits_as_int and changing hits.total type

* changing hits.total type and replace it

* Changing elasticsearch response type to match the new structure(hits.total.value)

* Changing elasticsearch response type to match the new structure(hits.total.value)

* Changing elasticsearch response type to match the new structure(hits.total.value)

* Replacing totalHits to noHits

* Fixing unit test and updating snapshot

* Changing elasticsearch type names

* Changing elasticsearch type names
  • Loading branch information
cauemarcondes authored Oct 15, 2019
1 parent 30f69df commit 8af2940
Show file tree
Hide file tree
Showing 20 changed files with 80 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ interface IBucket {

// TODO: cleanup duplication of this in distribution/get_distribution.ts (ErrorDistributionAPIResponse) and transactions/distribution/index.ts (TransactionDistributionAPIResponse)
interface IDistribution {
totalHits: number;
noHits: boolean;
buckets: IBucket[];
bucketSize: number;
}
Expand Down Expand Up @@ -57,9 +57,7 @@ export function ErrorDistribution({ distribution, title }: Props) {
distribution.bucketSize
);

const isEmpty = distribution.totalHits === 0;

if (isEmpty) {
if (distribution.noHits) {
return (
<EmptyMessage
heading={i18n.translate('xpack.apm.errorGroupDetails.noErrorsLabel', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export const TransactionDistribution: FunctionComponent<Props> = (
]);

// no data in response
if (!distribution || !distribution.totalHits) {
if (!distribution || distribution.noHits) {
// only show loading state if there is no data - else show stale data until new data has loaded
if (isLoading) {
return <LoadingStatePrompt />;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,5 +247,5 @@
}
],
"overallAvgDuration": 467582.45401459857,
"totalHits": 999
"noHits": false
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { TransactionDistributionAPIResponse } from '../../server/lib/transaction

const INITIAL_DATA = {
buckets: [] as TransactionDistributionAPIResponse['buckets'],
totalHits: 0,
noHits: true,
bucketSize: 0
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export async function getBuckets({
}));

return {
totalHits: resp.hits.total,
noHits: resp.hits.total.value === 0,
buckets
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ export async function getErrorDistribution({
setup: Setup;
}) {
const bucketSize = getBucketSize(setup);
const { buckets, totalHits } = await getBuckets({
const { buckets, noHits } = await getBuckets({
serviceName,
groupId,
bucketSize,
setup
});

return {
totalHits,
noHits,
buckets,
bucketSize
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,6 @@ export async function getErrorGroup({
return {
transaction,
error,
occurrencesCount: resp.hits.total
occurrencesCount: resp.hits.total.value
};
}
15 changes: 9 additions & 6 deletions x-pack/legacy/plugins/apm/server/lib/helpers/es_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
IndexDocumentParams,
IndicesDeleteParams,
IndicesCreateParams,
AggregationSearchResponse
AggregationSearchResponseWithTotalHitsAsObject
} from 'elasticsearch';
import { Legacy } from 'kibana';
import { cloneDeep, has, isString, set } from 'lodash';
Expand Down Expand Up @@ -76,8 +76,7 @@ async function getParamsForSearchRequest(
const includeFrozen = await uiSettings.get('search:includeFrozen');
return {
...addFilterForLegacyData(apmIndices, params, apmOptions), // filter out pre-7.0 data
ignore_throttled: !includeFrozen, // whether to query frozen indices or not
rest_total_hits_as_int: true // ensure that ES returns accurate hits.total with pre-6.6 format
ignore_throttled: !includeFrozen // whether to query frozen indices or not
};
}

Expand All @@ -93,7 +92,7 @@ export function getESClient(req: Legacy.Request) {
search: async <Hits = unknown, U extends SearchParams = {}>(
params: U,
apmOptions?: APMOptions
): Promise<AggregationSearchResponse<Hits, U>> => {
): Promise<AggregationSearchResponseWithTotalHitsAsObject<Hits, U>> => {
const nextParams = await getParamsForSearchRequest(
req,
params,
Expand All @@ -111,8 +110,12 @@ export function getESClient(req: Legacy.Request) {
console.log(JSON.stringify(nextParams.body, null, 4));
}

return cluster.callWithRequest(req, 'search', nextParams) as Promise<
AggregationSearchResponse<Hits, U>
return (cluster.callWithRequest(
req,
'search',
nextParams
) as unknown) as Promise<
AggregationSearchResponseWithTotalHitsAsObject<Hits, U>
>;
},
index: <Body>(params: IndexDocumentParams<Body>) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ describe('setupRequest', () => {
}
}
},
ignore_throttled: true,
rest_total_hits_as_int: true
ignore_throttled: true
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export async function fetchAndTransformGcMetrics({
if (!aggregations) {
return {
...chartBase,
totalHits: 0,
noHits: true,
series: []
};
}
Expand Down Expand Up @@ -153,7 +153,7 @@ export async function fetchAndTransformGcMetrics({

return {
...chartBase,
totalHits: response.hits.total,
noHits: response.hits.total.value === 0,
series
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { ChartType, YUnit } from '../../../typings/timeseries';

test('transformDataToMetricsChart should transform an ES result into a chart object', () => {
const response = {
hits: { total: 5000 },
hits: { total: { value: 5000 } },
aggregations: {
a: { value: 1000 },
b: { value: 1000 },
Expand Down Expand Up @@ -58,6 +58,7 @@ test('transformDataToMetricsChart should transform an ES result into a chart obj
expect(chart).toMatchInlineSnapshot(`
Object {
"key": "test_chart_key",
"noHits": false,
"series": Array [
Object {
"color": "red",
Expand Down Expand Up @@ -124,7 +125,6 @@ Object {
},
],
"title": "Test Chart Title",
"totalHits": 5000,
"yUnit": "number",
}
`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
* you may not use this file except in compliance with the Elastic License.
*/
import theme from '@elastic/eui/dist/eui_theme_light.json';
import { AggregationSearchResponse, AggregatedValue } from 'elasticsearch';
import {
AggregationSearchResponseWithTotalHitsAsObject,
AggregatedValue
} from 'elasticsearch';
import { idx } from '@kbn/elastic-idx';
import { ChartBase } from './types';

Expand Down Expand Up @@ -46,7 +49,7 @@ interface AggregatedParams {
}

export function transformDataToMetricsChart<Params extends AggregatedParams>(
result: AggregationSearchResponse<unknown, Params>,
result: AggregationSearchResponseWithTotalHitsAsObject<unknown, Params>,
chartBase: ChartBase
) {
const { aggregations, hits } = result;
Expand All @@ -56,7 +59,7 @@ export function transformDataToMetricsChart<Params extends AggregatedParams>(
title: chartBase.title,
key: chartBase.key,
yUnit: chartBase.yUnit,
totalHits: hits.total,
noHits: hits.total.value === 0,
series: Object.keys(chartBase.series).map((seriesKey, i) => {
const overallValue = idx(aggregations, _ => _[seriesKey].value);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,6 @@ export async function getAgentStatus(setup: Setup) {
};

const resp = await client.search(params);
const hasHistorialAgentData = resp.hits.total > 0;
const hasHistorialAgentData = resp.hits.total.value > 0;
return hasHistorialAgentData;
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ export async function getLegacyDataStatus(setup: Setup) {
};

const resp = await client.search(params, { includeLegacyData: true });
const hasLegacyData = resp.hits.total > 0;
const hasLegacyData = resp.hits.total.value > 0;
return hasLegacyData;
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,15 @@ export async function getTraceItems(traceId: string, setup: Setup) {
{ _score: { order: 'asc' } },
{ [TRANSACTION_DURATION]: { order: 'desc' } },
{ [SPAN_DURATION]: { order: 'desc' } }
]
],
track_total_hits: true
}
};

const resp = await client.search<Transaction | Span>(params);

return {
items: resp.hits.hits.map(hit => hit._source),
exceedsMax: resp.hits.total > maxTraceItems
exceedsMax: resp.hits.total.value > maxTraceItems
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export function bucketTransformer(response: DistributionBucketResponse) {
).map(getBucket);

return {
totalHits: response.hits.total,
noHits: response.hits.total.value === 0,
buckets
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ export async function getTransactionDistribution({
);

if (distributionMax == null) {
return { totalHits: 0, buckets: [], bucketSize: 0 };
return { noHits: true, buckets: [], bucketSize: 0 };
}

const bucketSize = getBucketSize(distributionMax, setup);
const { buckets, totalHits } = await getBuckets(
const { buckets, noHits } = await getBuckets(
serviceName,
transactionName,
transactionType,
Expand All @@ -63,7 +63,7 @@ export async function getTransactionDistribution({
);

return {
totalHits,
noHits,
buckets,
bucketSize
};
Expand Down
30 changes: 28 additions & 2 deletions x-pack/legacy/plugins/apm/typings/elasticsearch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@ declare module 'elasticsearch' {
value: number | null;
}

interface HitsTotal {
value: number;
relation: 'eq' | 'gte';
}

type AggregationResultMap<AggregationOption> = IndexAsString<
{
[AggregationName in keyof AggregationOption]: {
Expand All @@ -101,7 +106,7 @@ declare module 'elasticsearch' {
>;
top_hits: {
hits: {
total: number;
total: HitsTotal;
max_score: number | null;
hits: Array<{
_source: AggregationOption[AggregationName] extends {
Expand Down Expand Up @@ -148,7 +153,10 @@ declare module 'elasticsearch' {
}
>;

export type AggregationSearchResponse<HitType, SearchParams> = Pick<
export type AggregationSearchResponseWithTotalHitsAsInt<
HitType,
SearchParams
> = Pick<
SearchResponse<HitType>,
Exclude<keyof SearchResponse<HitType>, 'aggregations'>
> &
Expand All @@ -158,6 +166,24 @@ declare module 'elasticsearch' {
}
: {});

type Hits<HitType> = Pick<
SearchResponse<HitType>['hits'],
Exclude<keyof SearchResponse<HitType>['hits'], 'total'>
> & {
total: HitsTotal;
};

export type AggregationSearchResponseWithTotalHitsAsObject<
HitType,
SearchParams
> = Pick<
AggregationSearchResponseWithTotalHitsAsInt<HitType, SearchParams>,
Exclude<
keyof AggregationSearchResponseWithTotalHitsAsInt<HitType, SearchParams>,
'hits'
>
> & { hits: Hits<HitType> };

export interface ESFilter {
[key: string]: {
[key: string]: string | string[] | number | StringMap | ESFilter[];
Expand Down
21 changes: 13 additions & 8 deletions x-pack/legacy/plugins/lens/server/routes/field_stats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import Boom from 'boom';
import DateMath from '@elastic/datemath';
import { schema } from '@kbn/config-schema';
import { AggregationSearchResponse } from 'elasticsearch';
import { AggregationSearchResponseWithTotalHitsAsInt } from 'elasticsearch';
import { CoreSetup } from 'src/core/server';
import { FieldStatsResponse, BASE_API_URL } from '../../common';

Expand Down Expand Up @@ -135,10 +135,9 @@ export async function getNumberHistogram(
},
};

const minMaxResult = (await aggSearchWithBody(searchBody)) as AggregationSearchResponse<
unknown,
{ body: { aggs: typeof searchBody } }
>;
const minMaxResult = (await aggSearchWithBody(
searchBody
)) as AggregationSearchResponseWithTotalHitsAsInt<unknown, { body: { aggs: typeof searchBody } }>;

const minValue = minMaxResult.aggregations!.sample.min_value.value;
const maxValue = minMaxResult.aggregations!.sample.max_value.value;
Expand Down Expand Up @@ -179,7 +178,9 @@ export async function getNumberHistogram(
},
},
};
const histogramResult = (await aggSearchWithBody(histogramBody)) as AggregationSearchResponse<
const histogramResult = (await aggSearchWithBody(
histogramBody
)) as AggregationSearchResponseWithTotalHitsAsInt<
unknown,
{ body: { aggs: typeof histogramBody } }
>;
Expand Down Expand Up @@ -213,7 +214,9 @@ export async function getStringSamples(
},
},
};
const topValuesResult = (await aggSearchWithBody(topValuesBody)) as AggregationSearchResponse<
const topValuesResult = (await aggSearchWithBody(
topValuesBody
)) as AggregationSearchResponseWithTotalHitsAsInt<
unknown,
{ body: { aggs: typeof topValuesBody } }
>;
Expand Down Expand Up @@ -260,7 +263,9 @@ export async function getDateHistogram(
const histogramBody = {
histo: { date_histogram: { field: field.name, fixed_interval: fixedInterval } },
};
const results = (await aggSearchWithBody(histogramBody)) as AggregationSearchResponse<
const results = (await aggSearchWithBody(
histogramBody
)) as AggregationSearchResponseWithTotalHitsAsInt<
unknown,
{ body: { aggs: typeof histogramBody } }
>;
Expand Down

0 comments on commit 8af2940

Please sign in to comment.