Skip to content

Commit

Permalink
[APM] Sorting Instances table result on server-side (elastic#174164)
Browse files Browse the repository at this point in the history
closes elastic#167633

- Move sorting to the backend, so we don't miss any document.
- Query 1000 instances sort it and slice to return Top 100 instances.
- Adding API tests.



https://github.com/elastic/kibana/assets/55978943/3c1b746a-799a-4195-8a29-230b2547c53b
  • Loading branch information
cauemarcondes authored and nreese committed Jan 31, 2024
1 parent 2f2fa14 commit 7edd628
Show file tree
Hide file tree
Showing 19 changed files with 727 additions and 464 deletions.
18 changes: 18 additions & 0 deletions x-pack/plugins/apm/common/instances.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import * as t from 'io-ts';

export const instancesSortFieldRt = t.keyof({
serviceNodeName: null,
latency: null,
throughput: null,
errorRate: null,
cpuUsage: null,
memoryUsage: null,
});

export type InstancesSortField = t.TypeOf<typeof instancesSortFieldRt>;
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
*/

import { EuiFlexItem, EuiPanel } from '@elastic/eui';
import { orderBy } from 'lodash';
import React, { useState } from 'react';
import { v4 as uuidv4 } from 'uuid';
import { isTimeComparison } from '../../shared/time_comparison/get_comparison_options';
Expand All @@ -25,6 +24,7 @@ import {
TableOptions,
} from './service_overview_instances_table';
import { LatencyAggregationType } from '../../../../common/latency_aggregation_types';
import { InstancesSortField } from '../../../../common/instances';

interface ServiceOverviewInstancesChartAndTableProps {
chartHeight: number;
Expand All @@ -48,14 +48,6 @@ const INITIAL_STATE_DETAILED_STATISTICS: ApiResponseDetailedStats = {
previousPeriod: {},
};

export type SortField =
| 'serviceNodeName'
| 'latency'
| 'throughput'
| 'errorRate'
| 'cpuUsage'
| 'memoryUsage';

export type SortDirection = 'asc' | 'desc';
export const PAGE_SIZE = 5;
const DEFAULT_SORT = {
Expand Down Expand Up @@ -122,6 +114,8 @@ export function ServiceOverviewInstancesChartAndTable({
comparisonEnabled && isTimeComparison(offset)
? offset
: undefined,
sortField: tableOptions.sort.field,
sortDirection: tableOptions.sort.direction,
},
},
}
Expand Down Expand Up @@ -152,6 +146,7 @@ export function ServiceOverviewInstancesChartAndTable({
offset,
// not used, but needed to trigger an update when comparison feature is disabled/enabled by user
comparisonEnabled,
tableOptions.sort,
]
);

Expand All @@ -162,19 +157,10 @@ export function ServiceOverviewInstancesChartAndTable({
currentPeriodItemsCount,
} = mainStatsData;

const currentPeriodOrderedItems = orderBy(
// need top-level sortable fields for the managed table
currentPeriodItems.map((item) => ({
...item,
latency: item.latency ?? 0,
throughput: item.throughput ?? 0,
errorRate: item.errorRate ?? 0,
cpuUsage: item.cpuUsage ?? 0,
memoryUsage: item.memoryUsage ?? 0,
})),
field,
direction
).slice(pageIndex * PAGE_SIZE, (pageIndex + 1) * PAGE_SIZE);
const currentPageItems = currentPeriodItems.slice(
pageIndex * PAGE_SIZE,
(pageIndex + 1) * PAGE_SIZE
);

const {
data: detailedStatsData = INITIAL_STATE_DETAILED_STATISTICS,
Expand Down Expand Up @@ -208,7 +194,7 @@ export function ServiceOverviewInstancesChartAndTable({
numBuckets: 20,
transactionType,
serviceNodeIds: JSON.stringify(
currentPeriodOrderedItems.map((item) => item.serviceNodeName)
currentPageItems.map((item) => item.serviceNodeName)
),
offset:
comparisonEnabled && isTimeComparison(offset)
Expand Down Expand Up @@ -238,7 +224,7 @@ export function ServiceOverviewInstancesChartAndTable({
<EuiFlexItem grow={7}>
<EuiPanel hasBorder={true}>
<ServiceOverviewInstancesTable
mainStatsItems={currentPeriodOrderedItems}
mainStatsItems={currentPageItems}
mainStatsStatus={mainStatsStatus}
mainStatsItemCount={currentPeriodItemsCount}
detailedStatsLoading={isPending(detailedStatsStatus)}
Expand All @@ -252,7 +238,7 @@ export function ServiceOverviewInstancesChartAndTable({
pageIndex: newTableOptions.page?.index ?? 0,
sort: newTableOptions.sort
? {
field: newTableOptions.sort.field as SortField,
field: newTableOptions.sort.field as InstancesSortField,
direction: newTableOptions.sort.direction,
}
: DEFAULT_SORT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ type ServiceInstanceDetailedStatistics =
export function getColumns({
serviceName,
kuery,
agentName,
latencyAggregationType,
detailedStatsLoading,
detailedStatsData,
Expand All @@ -59,7 +58,6 @@ export function getColumns({
}: {
serviceName: string;
kuery: string;
agentName?: string;
latencyAggregationType?: LatencyAggregationType;
detailedStatsLoading: boolean;
detailedStatsData?: ServiceInstanceDetailedStatistics;
Expand Down Expand Up @@ -214,6 +212,7 @@ export function getColumns({
{ defaultMessage: 'CPU usage (avg.)' }
),
align: RIGHT_ALIGNMENT,
sortable: true,
render: (_, { serviceNodeName, cpuUsage }) => {
const currentPeriodTimestamp =
detailedStatsData?.currentPeriod?.[serviceNodeName]?.cpuUsage;
Expand Down Expand Up @@ -241,7 +240,6 @@ export function getColumns({
/>
);
},
sortable: true,
},
{
field: 'memoryUsage',
Expand All @@ -250,6 +248,7 @@ export function getColumns({
{ defaultMessage: 'Memory usage (avg.)' }
),
align: RIGHT_ALIGNMENT,
sortable: true,
render: (_, { serviceNodeName, memoryUsage }) => {
const currentPeriodTimestamp =
detailedStatsData?.currentPeriod?.[serviceNodeName]?.memoryUsage;
Expand Down Expand Up @@ -277,7 +276,6 @@ export function getColumns({
/>
);
},
sortable: true,
},
{
width: '40px',
Expand All @@ -292,7 +290,10 @@ export function getColumns({
anchorPosition="leftCenter"
button={
<EuiButtonIcon
aria-label="Edit"
aria-label={i18n.translate(
'xpack.apm.getColumns.euiButtonIcon.editLabel',
{ defaultMessage: 'Edit' }
)}
data-test-subj={`instanceActionsButton_${instanceItem.serviceNodeName}`}
iconType="boxesHorizontal"
onClick={() =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,19 @@ import {
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React, { ReactNode, useEffect, useState } from 'react';
import { useApmServiceContext } from '../../../../context/apm_service/use_apm_service_context';
import { FETCH_STATUS } from '../../../../hooks/use_fetcher';
import { APIReturnType } from '../../../../services/rest/create_call_apm_api';
import {
PAGE_SIZE,
SortDirection,
SortField,
} from '../service_overview_instances_chart_and_table';
import { OverviewTableContainer } from '../../../shared/overview_table_container';
import { getColumns } from './get_columns';
import { InstanceDetails } from './intance_details';
import { useApmParams } from '../../../../hooks/use_apm_params';
import { useBreakpoints } from '../../../../hooks/use_breakpoints';
import { LatencyAggregationType } from '../../../../../common/latency_aggregation_types';
import { InstancesSortField } from '../../../../../common/instances';

type ServiceInstanceMainStatistics =
APIReturnType<'GET /internal/apm/services/{serviceName}/service_overview_instances/main_statistics'>;
Expand All @@ -39,7 +38,7 @@ export interface TableOptions {
pageIndex: number;
sort: {
direction: SortDirection;
field: SortField;
field: InstancesSortField;
};
}

Expand Down Expand Up @@ -70,8 +69,6 @@ export function ServiceOverviewInstancesTable({
isLoading,
isNotInitiated,
}: Props) {
const { agentName } = useApmServiceContext();

const {
query: { kuery, latencyAggregationType, comparisonEnabled, offset },
} = useApmParams('/services/{serviceName}');
Expand Down Expand Up @@ -122,7 +119,6 @@ export function ServiceOverviewInstancesTable({
const shouldShowSparkPlots = !isXl;

const columns = getColumns({
agentName,
serviceName,
kuery,
latencyAggregationType: latencyAggregationType as LatencyAggregationType,
Expand Down Expand Up @@ -154,7 +150,9 @@ export function ServiceOverviewInstancesTable({
<EuiTitle size="xs">
<h2>
{i18n.translate('xpack.apm.serviceOverview.instancesTableTitle', {
defaultMessage: 'Instances',
defaultMessage:
'Top {count} {count, plural, one {instance} other {instances}}',
values: { count: mainStatsItemCount },
})}
</h2>
</EuiTitle>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@ async function getServiceInstancesDetailedStatistics(
const [transactionStats, systemMetricStats = []] = await Promise.all([
getServiceInstancesTransactionStatistics({
...params,
isComparisonSearch: true,
includeTimeseries: true,
}),
getServiceInstancesSystemMetricStatistics({
...params,
isComparisonSearch: true,
includeTimeseries: true,
}),
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export async function getServiceInstancesSystemMetricStatistics<
end,
serviceNodeIds,
numBuckets,
isComparisonSearch,
includeTimeseries,
offset,
}: {
apmEventClient: APMEventClient;
Expand All @@ -64,7 +64,7 @@ export async function getServiceInstancesSystemMetricStatistics<
environment: string;
kuery: string;
size?: number;
isComparisonSearch: T;
includeTimeseries: T;
offset?: string;
}): Promise<Array<ServiceInstanceSystemMetricStatistics<T>>> {
const { startWithOffset, endWithOffset } = getOffsetInMs({
Expand All @@ -85,7 +85,7 @@ export async function getServiceInstancesSystemMetricStatistics<
agg: TParams
) {
return {
...(isComparisonSearch
...(includeTimeseries
? {
avg: { avg: agg },
timeseries: {
Expand Down Expand Up @@ -136,7 +136,7 @@ export async function getServiceInstancesSystemMetricStatistics<
...rangeQuery(startWithOffset, endWithOffset),
...environmentQuery(environment),
...kqlQuery(kuery),
...(isComparisonSearch && serviceNodeIds
...(serviceNodeIds?.length
? [{ terms: { [SERVICE_NODE_NAME]: serviceNodeIds } }]
: []),
{
Expand All @@ -158,7 +158,7 @@ export async function getServiceInstancesSystemMetricStatistics<
field: SERVICE_NODE_NAME,
missing: SERVICE_NODE_NAME_MISSING,
...(size ? { size } : {}),
...(isComparisonSearch ? { include: serviceNodeIds } : {}),
...(serviceNodeIds?.length ? { include: serviceNodeIds } : {}),
},
aggs: subAggs,
},
Expand All @@ -179,7 +179,7 @@ export async function getServiceInstancesSystemMetricStatistics<
: 'memory_usage_system';

const cpuUsage =
// Timeseries is available when isComparisonSearch is true
// Timeseries is available when includeTimeseries is true
'timeseries' in serviceNodeBucket.cpu_usage
? serviceNodeBucket.cpu_usage.timeseries.buckets.map(
(dateBucket) => ({
Expand All @@ -191,7 +191,7 @@ export async function getServiceInstancesSystemMetricStatistics<

const memoryUsageValue = serviceNodeBucket[memoryMetricsKey];
const memoryUsage =
// Timeseries is available when isComparisonSearch is true
// Timeseries is available when includeTimeseries is true
'timeseries' in memoryUsageValue
? memoryUsageValue.timeseries.buckets.map((dateBucket) => ({
x: dateBucket.key,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export async function getServiceInstancesTransactionStatistics<
end,
serviceNodeIds,
numBuckets,
isComparisonSearch,
includeTimeseries,
offset,
}: {
latencyAggregationType: LatencyAggregationType;
Expand All @@ -73,7 +73,7 @@ export async function getServiceInstancesTransactionStatistics<
searchAggregatedTransactions: boolean;
start: number;
end: number;
isComparisonSearch: T;
includeTimeseries: T;
serviceNodeIds?: string[];
environment: string;
kuery: string;
Expand Down Expand Up @@ -123,7 +123,7 @@ export async function getServiceInstancesTransactionStatistics<
...getBackwardCompatibleDocumentTypeFilter(
searchAggregatedTransactions
),
...(isComparisonSearch && serviceNodeIds
...(serviceNodeIds?.length
? [{ terms: { [SERVICE_NODE_NAME]: serviceNodeIds } }]
: []),
],
Expand All @@ -136,9 +136,9 @@ export async function getServiceInstancesTransactionStatistics<
field: SERVICE_NODE_NAME,
missing: SERVICE_NODE_NAME_MISSING,
...(size ? { size } : {}),
...(isComparisonSearch ? { include: serviceNodeIds } : {}),
...(serviceNodeIds?.length ? { include: serviceNodeIds } : {}),
},
aggs: isComparisonSearch
aggs: includeTimeseries
? {
timeseries: {
date_histogram: {
Expand Down Expand Up @@ -174,7 +174,7 @@ export async function getServiceInstancesTransactionStatistics<
const { doc_count: count, key } = serviceNodeBucket;
const serviceNodeName = String(key);

// Timeseries is returned when isComparisonSearch is true
// Timeseries is returned when includeTimeseries is true
if ('timeseries' in serviceNodeBucket) {
const { timeseries } = serviceNodeBucket;
return {
Expand Down
Loading

0 comments on commit 7edd628

Please sign in to comment.