From af0e4d4049056f4f62fa3f1d9dbf10fe5d3567ce Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 25 Jan 2021 14:50:37 +0100 Subject: [PATCH 01/12] breaking error table api --- .../service_overview_errors_table/index.tsx | 106 ++++++++++++------ .../shared/charts/spark_plot/index.tsx | 3 +- .../get_service_error_groups/index.ts | 66 +---------- .../get_service_error_groups_metrics/index.ts | 97 ++++++++++++++++ .../apm/server/routes/create_apm_api.ts | 2 + x-pack/plugins/apm/server/routes/services.ts | 48 ++++++-- 6 files changed, 210 insertions(+), 112 deletions(-) create mode 100644 x-pack/plugins/apm/server/lib/services/get_service_error_groups_metrics/index.ts diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx index d14ef648c22d..330203edaf3f 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx @@ -11,12 +11,15 @@ import { EuiTitle, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -import React, { useState } from 'react'; +import React, { useMemo, useState } from 'react'; import { asInteger } from '../../../../../common/utils/formatters'; import { useApmServiceContext } from '../../../../context/apm_service/use_apm_service_context'; import { useUrlParams } from '../../../../context/url_params_context/use_url_params'; import { FETCH_STATUS, useFetcher } from '../../../../hooks/use_fetcher'; -import { callApmApi } from '../../../../services/rest/createCallApmApi'; +import { + APIReturnType, + callApmApi, +} from '../../../../services/rest/createCallApmApi'; import { px, unit } from '../../../../style/variables'; import { SparkPlot } from '../../../shared/charts/spark_plot'; import { ErrorDetailLink } from '../../../shared/Links/apm/ErrorDetailLink'; @@ -30,16 +33,6 @@ interface Props { serviceName: string; } -interface ErrorGroupItem { - name: string; - last_seen: number; - group_id: string; - occurrences: { - value: number; - timeseries: Array<{ x: number; y: number }> | null; - }; -} - type SortDirection = 'asc' | 'desc'; type SortField = 'name' | 'last_seen' | 'occurrences'; @@ -49,24 +42,17 @@ const DEFAULT_SORT = { field: 'occurrences' as const, }; -export function ServiceOverviewErrorsTable({ serviceName }: Props) { - const { - urlParams: { start, end }, - uiFilters, - } = useUrlParams(); - const { transactionType } = useApmServiceContext(); - const [tableOptions, setTableOptions] = useState<{ - pageIndex: number; - sort: { - direction: SortDirection; - field: SortField; - }; - }>({ - pageIndex: 0, - sort: DEFAULT_SORT, - }); +type ErrorGroupItem = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups'>; +type GroupIdsErrorMetrics = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups/metrics'>; - const columns: Array> = [ +function getColumns({ + serviceName, + groupIdsErrorMetrics, +}: { + serviceName: string; + groupIdsErrorMetrics: GroupIdsErrorMetrics; +}): Array> { + return [ { field: 'name', name: i18n.translate('xpack.apm.serviceOverview.errorsTableColumnName', { @@ -110,11 +96,14 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { } ), width: px(unit * 12), - render: (_, { occurrences }) => { + render: (_, { occurrences, group_id: errorGroupId }) => { + const timeseries = groupIdsErrorMetrics + ? groupIdsErrorMetrics[errorGroupId]?.timeseries + : undefined; return ( ({ + pageIndex: 0, + sort: DEFAULT_SORT, + }); const { data = { @@ -153,9 +160,7 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { start, end, uiFilters: JSON.stringify(uiFilters), - size: PAGE_SIZE, numBuckets: 20, - pageIndex: tableOptions.pageIndex, sortField: tableOptions.sort.field, sortDirection: tableOptions.sort.direction, transactionType, @@ -191,6 +196,36 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { tableOptions: { pageIndex, sort }, } = data; + const groupIds = useMemo( + () => items.map(({ group_id: groupId }) => groupId), + [items] + ); + const { data: groupIdsErrorMetrics } = useFetcher( + () => { + if (groupIds.length && start && end && transactionType) { + return callApmApi({ + endpoint: 'GET /api/apm/services/{serviceName}/error_groups/metrics', + params: { + path: { serviceName }, + query: { + start, + end, + uiFilters: JSON.stringify(uiFilters), + numBuckets: 20, + transactionType, + groupIds: groupIds.join(), + }, + }, + }); + } + }, + // only fetches metrics when groupIds change + // eslint-disable-next-line + [groupIds] + ); + + const columns = getColumns({ serviceName, groupIdsErrorMetrics }); + return ( @@ -222,7 +257,10 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { > | null; + series?: Coordinate[] | null; valueLabel: React.ReactNode; compact?: boolean; }) { diff --git a/x-pack/plugins/apm/server/lib/services/get_service_error_groups/index.ts b/x-pack/plugins/apm/server/lib/services/get_service_error_groups/index.ts index 99382a124191..7f5bcd790208 100644 --- a/x-pack/plugins/apm/server/lib/services/get_service_error_groups/index.ts +++ b/x-pack/plugins/apm/server/lib/services/get_service_error_groups/index.ts @@ -18,7 +18,6 @@ import { TRANSACTION_TYPE, } from '../../../../common/elasticsearch_fieldnames'; import { Setup, SetupTimeRange } from '../../helpers/setup_request'; -import { getBucketSize } from '../../helpers/get_bucket_size'; import { getErrorName } from '../../helpers/get_error_name'; export type ServiceErrorGroupItem = ValuesType< @@ -28,17 +27,13 @@ export type ServiceErrorGroupItem = ValuesType< export async function getServiceErrorGroups({ serviceName, setup, - size, numBuckets, - pageIndex, sortDirection, sortField, transactionType, }: { serviceName: string; setup: Setup & SetupTimeRange; - size: number; - pageIndex: number; numBuckets: number; sortDirection: 'asc' | 'desc'; sortField: 'name' | 'last_seen' | 'occurrences'; @@ -46,8 +41,6 @@ export async function getServiceErrorGroups({ }) { const { apmEventClient, start, end, esFilter } = setup; - const { intervalString } = getBucketSize({ start, end, numBuckets }); - const response = await apmEventClient.search({ apm: { events: [ProcessorEvent.error], @@ -114,69 +107,12 @@ export async function getServiceErrorGroups({ return group[sortField]; }, [sortDirection] - ).slice(pageIndex * size, pageIndex * size + size); - - const sortedErrorGroupIds = sortedAndSlicedErrorGroups.map( - (group) => group.group_id ); - const timeseriesResponse = await apmEventClient.search({ - apm: { - events: [ProcessorEvent.error], - }, - body: { - size: 0, - query: { - bool: { - filter: [ - { terms: { [ERROR_GROUP_ID]: sortedErrorGroupIds } }, - { term: { [SERVICE_NAME]: serviceName } }, - { term: { [TRANSACTION_TYPE]: transactionType } }, - { range: rangeFilter(start, end) }, - ...esFilter, - ], - }, - }, - aggs: { - error_groups: { - terms: { - field: ERROR_GROUP_ID, - size, - }, - aggs: { - timeseries: { - date_histogram: { - field: '@timestamp', - fixed_interval: intervalString, - min_doc_count: 0, - extended_bounds: { - min: start, - max: end, - }, - }, - }, - }, - }, - }, - }, - }); - return { total_error_groups: errorGroups.length, is_aggregation_accurate: (response.aggregations?.error_groups.sum_other_doc_count ?? 0) === 0, - error_groups: sortedAndSlicedErrorGroups.map((errorGroup) => ({ - ...errorGroup, - occurrences: { - ...errorGroup.occurrences, - timeseries: - timeseriesResponse.aggregations?.error_groups.buckets - .find((bucket) => bucket.key === errorGroup.group_id) - ?.timeseries.buckets.map((dateBucket) => ({ - x: dateBucket.key, - y: dateBucket.doc_count, - })) ?? null, - }, - })), + error_groups: sortedAndSlicedErrorGroups, }; } diff --git a/x-pack/plugins/apm/server/lib/services/get_service_error_groups_metrics/index.ts b/x-pack/plugins/apm/server/lib/services/get_service_error_groups_metrics/index.ts new file mode 100644 index 000000000000..6ebacf93b9e6 --- /dev/null +++ b/x-pack/plugins/apm/server/lib/services/get_service_error_groups_metrics/index.ts @@ -0,0 +1,97 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import { Coordinate } from '../../../../typings/timeseries'; +import { + ERROR_GROUP_ID, + SERVICE_NAME, + TRANSACTION_TYPE, +} from '../../../../common/elasticsearch_fieldnames'; +import { ProcessorEvent } from '../../../../common/processor_event'; +import { rangeFilter } from '../../../../common/utils/range_filter'; +import { getBucketSize } from '../../helpers/get_bucket_size'; +import { Setup, SetupTimeRange } from '../../helpers/setup_request'; + +export async function getServiceErrorGroupsMetrics({ + serviceName, + setup, + numBuckets, + transactionType, + groupIds, +}: { + serviceName: string; + setup: Setup & SetupTimeRange; + numBuckets: number; + transactionType: string; + groupIds: string[]; +}): Promise | undefined> { + const { apmEventClient, start, end, esFilter } = setup; + + const { intervalString } = getBucketSize({ start, end, numBuckets }); + + const timeseriesResponse = await apmEventClient.search({ + apm: { + events: [ProcessorEvent.error], + }, + body: { + size: 0, + query: { + bool: { + filter: [ + { terms: { [ERROR_GROUP_ID]: groupIds } }, + { term: { [SERVICE_NAME]: serviceName } }, + { term: { [TRANSACTION_TYPE]: transactionType } }, + { range: rangeFilter(start, end) }, + ...esFilter, + ], + }, + }, + aggs: { + error_groups: { + terms: { + field: ERROR_GROUP_ID, + size: 500, + }, + aggs: { + timeseries: { + date_histogram: { + field: '@timestamp', + fixed_interval: intervalString, + min_doc_count: 0, + extended_bounds: { + min: start, + max: end, + }, + }, + }, + }, + }, + }, + }, + }); + + if (!timeseriesResponse.aggregations) { + return undefined; + } + + return timeseriesResponse.aggregations.error_groups.buckets.reduce( + (acc, bucket) => { + const groupId = bucket.key; + return { + ...acc, + [groupId]: { + timeseries: bucket.timeseries.buckets.map((timeseriesBucket) => { + return { + x: timeseriesBucket.key, + y: timeseriesBucket.doc_count, + }; + }), + }, + }; + }, + {} + ); +} diff --git a/x-pack/plugins/apm/server/routes/create_apm_api.ts b/x-pack/plugins/apm/server/routes/create_apm_api.ts index aeaa6f90a742..823df38547b5 100644 --- a/x-pack/plugins/apm/server/routes/create_apm_api.ts +++ b/x-pack/plugins/apm/server/routes/create_apm_api.ts @@ -22,6 +22,7 @@ import { serviceAnnotationsRoute, serviceAnnotationsCreateRoute, serviceErrorGroupsRoute, + serviceErrorGroupsMetricsRoute, serviceThroughputRoute, serviceDependenciesRoute, serviceMetadataDetailsRoute, @@ -135,6 +136,7 @@ const createApmApi = () => { .add(serviceMetadataDetailsRoute) .add(serviceMetadataIconsRoute) .add(serviceInstancesRoute) + .add(serviceErrorGroupsMetricsRoute) // Agent configuration .add(getSingleAgentConfigurationRoute) diff --git a/x-pack/plugins/apm/server/routes/services.ts b/x-pack/plugins/apm/server/routes/services.ts index bfc2ebf062ac..c71a3f89f970 100644 --- a/x-pack/plugins/apm/server/routes/services.ts +++ b/x-pack/plugins/apm/server/routes/services.ts @@ -24,6 +24,7 @@ import { getThroughput } from '../lib/services/get_throughput'; import { getServiceInstances } from '../lib/services/get_service_instances'; import { getServiceMetadataDetails } from '../lib/services/get_service_metadata_details'; import { getServiceMetadataIcons } from '../lib/services/get_service_metadata_icons'; +import { getServiceErrorGroupsMetrics } from '../lib/services/get_service_error_groups_metrics'; export const servicesRoute = createRoute({ endpoint: 'GET /api/apm/services', @@ -264,9 +265,7 @@ export const serviceErrorGroupsRoute = createRoute({ rangeRt, uiFiltersRt, t.type({ - size: toNumberRt, numBuckets: toNumberRt, - pageIndex: toNumberRt, sortDirection: t.union([t.literal('asc'), t.literal('desc')]), sortField: t.union([ t.literal('last_seen'), @@ -283,21 +282,12 @@ export const serviceErrorGroupsRoute = createRoute({ const { path: { serviceName }, - query: { - numBuckets, - pageIndex, - size, - sortDirection, - sortField, - transactionType, - }, + query: { numBuckets, sortDirection, sortField, transactionType }, } = context.params; return getServiceErrorGroups({ serviceName, setup, - size, numBuckets, - pageIndex, sortDirection, sortField, transactionType, @@ -305,6 +295,40 @@ export const serviceErrorGroupsRoute = createRoute({ }, }); +export const serviceErrorGroupsMetricsRoute = createRoute({ + endpoint: 'GET /api/apm/services/{serviceName}/error_groups/metrics', + params: t.type({ + path: t.type({ + serviceName: t.string, + }), + query: t.intersection([ + rangeRt, + uiFiltersRt, + t.type({ + numBuckets: toNumberRt, + transactionType: t.string, + groupIds: t.string, + }), + ]), + }), + options: { tags: ['access:apm'] }, + handler: async ({ context, request }) => { + const setup = await setupRequest(context, request); + + const { + path: { serviceName }, + query: { numBuckets, transactionType, groupIds }, + } = context.params; + return getServiceErrorGroupsMetrics({ + serviceName, + setup, + numBuckets, + transactionType, + groupIds: groupIds.split(','), + }); + }, +}); + export const serviceThroughputRoute = createRoute({ endpoint: 'GET /api/apm/services/{serviceName}/throughput', params: t.type({ From d258dca4291df8ef2b0b838c0d854eed3fc41406 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 25 Jan 2021 15:16:25 +0100 Subject: [PATCH 02/12] shows loading state while fetching metrics --- .../service_overview_errors_table/index.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx index 330203edaf3f..d34350b83e19 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx @@ -200,7 +200,10 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { () => items.map(({ group_id: groupId }) => groupId), [items] ); - const { data: groupIdsErrorMetrics } = useFetcher( + const { + data: groupIdsErrorMetrics, + status: groupIdsErrorMetricsStatus, + } = useFetcher( () => { if (groupIds.length && start && end && transactionType) { return callApmApi({ @@ -268,7 +271,10 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { pageSizeOptions: [PAGE_SIZE], hidePerPageOptions: true, }} - loading={status === FETCH_STATUS.LOADING} + loading={ + status === FETCH_STATUS.LOADING || + groupIdsErrorMetricsStatus === FETCH_STATUS.LOADING + } onChange={(newTableOptions: { page?: { index: number; From 020370debe71f7edb383df9bdd30c4a2b1fdc19f Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 25 Jan 2021 15:54:34 +0100 Subject: [PATCH 03/12] adding api tests --- .../apm_api_integration/basic/tests/index.ts | 2 + .../basic/tests/services/error_groups.ts | 121 ++++++++++++++++++ .../tests/services/error_groups_metrics.ts | 96 ++++++++++++++ 3 files changed, 219 insertions(+) create mode 100644 x-pack/test/apm_api_integration/basic/tests/services/error_groups.ts create mode 100644 x-pack/test/apm_api_integration/basic/tests/services/error_groups_metrics.ts diff --git a/x-pack/test/apm_api_integration/basic/tests/index.ts b/x-pack/test/apm_api_integration/basic/tests/index.ts index 4e66c6e6f76c..7f426c7aa9e5 100644 --- a/x-pack/test/apm_api_integration/basic/tests/index.ts +++ b/x-pack/test/apm_api_integration/basic/tests/index.ts @@ -27,6 +27,8 @@ export default function apmApiIntegrationTests({ loadTestFile }: FtrProviderCont loadTestFile(require.resolve('./services/transaction_types')); loadTestFile(require.resolve('./services/service_details')); loadTestFile(require.resolve('./services/service_icons')); + loadTestFile(require.resolve('./services/error_groups')); + loadTestFile(require.resolve('./services/error_groups_metrics')); }); describe('Service overview', function () { diff --git a/x-pack/test/apm_api_integration/basic/tests/services/error_groups.ts b/x-pack/test/apm_api_integration/basic/tests/services/error_groups.ts new file mode 100644 index 000000000000..8eef606dcdd3 --- /dev/null +++ b/x-pack/test/apm_api_integration/basic/tests/services/error_groups.ts @@ -0,0 +1,121 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import expect from '@kbn/expect'; +import url from 'url'; +import { FtrProviderContext } from '../../../../common/ftr_provider_context'; +import archives from '../../../common/fixtures/es_archiver/archives_metadata'; + +export default function ApiTest({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + const esArchiver = getService('esArchiver'); + + const archiveName = 'apm_8.0.0'; + const { start, end } = archives[archiveName]; + + describe('Error groups', () => { + describe('when data is not loaded ', () => { + it('handles the empty state', async () => { + const response = await supertest.get( + url.format({ + pathname: `/api/apm/services/opbeans-java/error_groups`, + query: { + start, + end, + uiFilters: '{}', + numBuckets: 20, + sortDirection: 'desc', + sortField: 'occurrences', + transactionType: 'request', + }, + }) + ); + + expect(response.status).to.be(200); + expectSnapshot(response.body).toMatchInline(` + Object { + "error_groups": Array [], + "is_aggregation_accurate": true, + "total_error_groups": 0, + } + `); + }); + }); + + describe('when data is loaded', () => { + before(() => esArchiver.load(archiveName)); + after(() => esArchiver.unload(archiveName)); + + it('returns the correct data', async () => { + const response = await supertest.get( + url.format({ + pathname: `/api/apm/services/opbeans-java/error_groups`, + query: { + start, + end, + uiFilters: '{}', + numBuckets: 20, + sortDirection: 'desc', + sortField: 'occurrences', + transactionType: 'request', + }, + }) + ); + + expect(response.status).to.be(200); + + expectSnapshot(response.body).toMatchInline(` + Object { + "error_groups": Array [ + Object { + "group_id": "051f95eabf120ebe2f8b0399fe3e54c5", + "last_seen": 1607437366098, + "name": "Could not write JSON: Null return value from advice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue(); nested exception is com.fasterxml.jackson.databind.JsonMappingException: Null return value from advice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue() (through reference chain: co.elastic.apm.opbeans.repositories.Stats[\\"numbers\\"]->com.sun.proxy.$Proxy132[\\"revenue\\"])", + "occurrences": Object { + "value": 5, + }, + }, + Object { + "group_id": "3bb34b98031a19c277bf59c3db82d3f3", + "last_seen": 1607436860546, + "name": "java.io.IOException: Connection reset by peer", + "occurrences": Object { + "value": 3, + }, + }, + Object { + "group_id": "b1c3ff13ec52de11187facf9c6a82538", + "last_seen": 1607437482385, + "name": "java.io.IOException: Connection reset by peer", + "occurrences": Object { + "value": 2, + }, + }, + Object { + "group_id": "9581687a53eac06aba50ba17cbd959c5", + "last_seen": 1607437468244, + "name": "Could not write JSON: Unable to find co.elastic.apm.opbeans.model.Customer with id 7173; nested exception is com.fasterxml.jackson.databind.JsonMappingException: Unable to find co.elastic.apm.opbeans.model.Customer with id 7173 (through reference chain: co.elastic.apm.opbeans.model.Customer_$$_jvst101_3[\\"email\\"])", + "occurrences": Object { + "value": 1, + }, + }, + Object { + "group_id": "97c2eef51fec10d177ade955670a2f15", + "last_seen": 1607437475563, + "name": "Request method 'POST' not supported", + "occurrences": Object { + "value": 1, + }, + }, + ], + "is_aggregation_accurate": true, + "total_error_groups": 5, + } + `); + }); + }); + }); +} diff --git a/x-pack/test/apm_api_integration/basic/tests/services/error_groups_metrics.ts b/x-pack/test/apm_api_integration/basic/tests/services/error_groups_metrics.ts new file mode 100644 index 000000000000..5539f5374fc3 --- /dev/null +++ b/x-pack/test/apm_api_integration/basic/tests/services/error_groups_metrics.ts @@ -0,0 +1,96 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import expect from '@kbn/expect'; +import url from 'url'; +import { FtrProviderContext } from '../../../../common/ftr_provider_context'; +import archives from '../../../common/fixtures/es_archiver/archives_metadata'; + +export default function ApiTest({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + const esArchiver = getService('esArchiver'); + + const archiveName = 'apm_8.0.0'; + const { start, end } = archives[archiveName]; + const groupIds = [ + '051f95eabf120ebe2f8b0399fe3e54c5', + '3bb34b98031a19c277bf59c3db82d3f3', + 'b1c3ff13ec52de11187facf9c6a82538', + '9581687a53eac06aba50ba17cbd959c5', + '97c2eef51fec10d177ade955670a2f15', + ].join(); + + describe('Error groups metrics', () => { + describe('when data is not loaded ', () => { + it('handles the empty state', async () => { + const response = await supertest.get( + url.format({ + pathname: `/api/apm/services/opbeans-java/error_groups/metrics`, + query: { + start, + end, + uiFilters: '{}', + numBuckets: 20, + transactionType: 'request', + groupIds, + }, + }) + ); + + expect(response.status).to.be(200); + expectSnapshot(response.body).toMatchInline(`Object {}`); + }); + }); + + describe('when data is loaded', () => { + before(() => esArchiver.load(archiveName)); + after(() => esArchiver.unload(archiveName)); + + it('returns the correct data', async () => { + const response = await supertest.get( + url.format({ + pathname: `/api/apm/services/opbeans-java/error_groups/metrics`, + query: { + start, + end, + uiFilters: '{}', + numBuckets: 20, + transactionType: 'request', + groupIds, + }, + }) + ); + + expect(response.status).to.be(200); + + const errorMetric = response.body['051f95eabf120ebe2f8b0399fe3e54c5']; + + expectSnapshot(errorMetric.timeseries.filter(({ y }: any) => y > 0).length).toMatchInline( + `4` + ); + }); + + it('returns empty data', async () => { + const response = await supertest.get( + url.format({ + pathname: `/api/apm/services/opbeans-java/error_groups/metrics`, + query: { + start, + end, + uiFilters: '{}', + numBuckets: 20, + transactionType: 'request', + groupIds: 'foo', + }, + }) + ); + + expect(response.status).to.be(200); + expectSnapshot(response.body).toMatchInline(`Object {}`); + }); + }); + }); +} From a97ae4c08e99e26104492ba6813e6591a7d09944 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 1 Feb 2021 17:50:18 +0100 Subject: [PATCH 04/12] removing pagination from server --- .../get_column.tsx | 93 ++++++++ .../service_overview_errors_table/index.tsx | 211 +++++------------- .../get_service_error_groups/index.ts | 24 +- .../apm/server/routes/create_apm_api.ts | 4 +- x-pack/plugins/apm/server/routes/services.ts | 18 +- 5 files changed, 166 insertions(+), 184 deletions(-) create mode 100644 x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx new file mode 100644 index 000000000000..db003e82dae0 --- /dev/null +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx @@ -0,0 +1,93 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import { EuiBasicTableColumn } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import React from 'react'; +import { asInteger } from '../../../../../common/utils/formatters'; +import { px, unit } from '../../../../style/variables'; +import { SparkPlot } from '../../../shared/charts/spark_plot'; +import { ErrorDetailLink } from '../../../shared/Links/apm/ErrorDetailLink'; +import { TimestampTooltip } from '../../../shared/TimestampTooltip'; +import { TruncateWithTooltip } from '../../../shared/truncate_with_tooltip'; +import { APIReturnType } from '../../../../services/rest/createCallApmApi'; + +type ErrorGroupItem = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups'>; +type GroupIdsErrorAggResults = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups/agg_results'>; + +export function getColumns({ + serviceName, + groupIdsErrorAggResults, +}: { + serviceName: string; + groupIdsErrorAggResults: GroupIdsErrorAggResults; +}): Array> { + return [ + { + field: 'name', + name: i18n.translate('xpack.apm.serviceOverview.errorsTableColumnName', { + defaultMessage: 'Name', + }), + render: (_, { name, group_id: errorGroupId }) => { + return ( + + {name} + + } + /> + ); + }, + }, + { + field: 'last_seen', + name: i18n.translate( + 'xpack.apm.serviceOverview.errorsTableColumnLastSeen', + { + defaultMessage: 'Last seen', + } + ), + render: (_, { last_seen: lastSeen }) => { + return ; + }, + width: px(unit * 9), + }, + { + field: 'occurrences', + name: i18n.translate( + 'xpack.apm.serviceOverview.errorsTableColumnOccurrences', + { + defaultMessage: 'Occurrences', + } + ), + width: px(unit * 12), + render: (_, { occurrences, group_id: errorGroupId }) => { + const timeseries = groupIdsErrorAggResults + ? groupIdsErrorAggResults[errorGroupId]?.timeseries + : undefined; + return ( + + ); + }, + }, + ]; +} diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx index d34350b83e19..01a83bc41857 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx @@ -5,29 +5,21 @@ */ import { EuiBasicTable, - EuiBasicTableColumn, EuiFlexGroup, EuiFlexItem, EuiTitle, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -import React, { useMemo, useState } from 'react'; -import { asInteger } from '../../../../../common/utils/formatters'; +import { isEmpty, orderBy } from 'lodash'; +import React, { useState } from 'react'; import { useApmServiceContext } from '../../../../context/apm_service/use_apm_service_context'; import { useUrlParams } from '../../../../context/url_params_context/use_url_params'; import { FETCH_STATUS, useFetcher } from '../../../../hooks/use_fetcher'; -import { - APIReturnType, - callApmApi, -} from '../../../../services/rest/createCallApmApi'; -import { px, unit } from '../../../../style/variables'; -import { SparkPlot } from '../../../shared/charts/spark_plot'; -import { ErrorDetailLink } from '../../../shared/Links/apm/ErrorDetailLink'; +import { callApmApi } from '../../../../services/rest/createCallApmApi'; import { ErrorOverviewLink } from '../../../shared/Links/apm/ErrorOverviewLink'; import { TableFetchWrapper } from '../../../shared/table_fetch_wrapper'; -import { TimestampTooltip } from '../../../shared/TimestampTooltip'; -import { TruncateWithTooltip } from '../../../shared/truncate_with_tooltip'; import { ServiceOverviewTableContainer } from '../service_overview_table_container'; +import { getColumns } from './get_column'; interface Props { serviceName: string; @@ -42,84 +34,6 @@ const DEFAULT_SORT = { field: 'occurrences' as const, }; -type ErrorGroupItem = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups'>; -type GroupIdsErrorMetrics = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups/metrics'>; - -function getColumns({ - serviceName, - groupIdsErrorMetrics, -}: { - serviceName: string; - groupIdsErrorMetrics: GroupIdsErrorMetrics; -}): Array> { - return [ - { - field: 'name', - name: i18n.translate('xpack.apm.serviceOverview.errorsTableColumnName', { - defaultMessage: 'Name', - }), - render: (_, { name, group_id: errorGroupId }) => { - return ( - - {name} - - } - /> - ); - }, - }, - { - field: 'last_seen', - name: i18n.translate( - 'xpack.apm.serviceOverview.errorsTableColumnLastSeen', - { - defaultMessage: 'Last seen', - } - ), - render: (_, { last_seen: lastSeen }) => { - return ; - }, - width: px(unit * 9), - }, - { - field: 'occurrences', - name: i18n.translate( - 'xpack.apm.serviceOverview.errorsTableColumnOccurrences', - { - defaultMessage: 'Occurrences', - } - ), - width: px(unit * 12), - render: (_, { occurrences, group_id: errorGroupId }) => { - const timeseries = groupIdsErrorMetrics - ? groupIdsErrorMetrics[errorGroupId]?.timeseries - : undefined; - return ( - - ); - }, - }, - ]; -} - export function ServiceOverviewErrorsTable({ serviceName }: Props) { const { urlParams: { start, end }, @@ -137,14 +51,13 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { sort: DEFAULT_SORT, }); + const { pageIndex, sort } = tableOptions; + const { data = { totalItemCount: 0, items: [], - tableOptions: { - pageIndex: 0, - sort: DEFAULT_SORT, - }, + requestId: '', }, status, } = useFetcher(() => { @@ -160,74 +73,78 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { start, end, uiFilters: JSON.stringify(uiFilters), - numBuckets: 20, - sortField: tableOptions.sort.field, - sortDirection: tableOptions.sort.direction, transactionType, }, }, }).then((response) => { return { + requestId: response.requestId, items: response.error_groups, totalItemCount: response.total_error_groups, - tableOptions: { - pageIndex: tableOptions.pageIndex, - sort: { - field: tableOptions.sort.field, - direction: tableOptions.sort.direction, - }, - }, }; }); - }, [ - start, - end, - serviceName, - uiFilters, - tableOptions.pageIndex, - tableOptions.sort.field, - tableOptions.sort.direction, - transactionType, - ]); + }, [start, end, serviceName, uiFilters, transactionType]); - const { + const { items, totalItemCount, requestId } = data; + const currentPageErrorGroups = orderBy( items, - totalItemCount, - tableOptions: { pageIndex, sort }, - } = data; + (group) => { + if (sort.field === 'occurrences') { + return group.occurrences.value; + } + return group[sort.field]; + }, + sort.direction + ).slice(pageIndex * PAGE_SIZE, (pageIndex + 1) * PAGE_SIZE); - const groupIds = useMemo( - () => items.map(({ group_id: groupId }) => groupId), - [items] + const groupIds = JSON.stringify( + currentPageErrorGroups.map(({ group_id: groupId }) => groupId) ); const { - data: groupIdsErrorMetrics, - status: groupIdsErrorMetricsStatus, + data: groupIdsErrorAggResults, + status: groupIdsErrorAggResultsStatus, } = useFetcher( () => { - if (groupIds.length && start && end && transactionType) { - return callApmApi({ - endpoint: 'GET /api/apm/services/{serviceName}/error_groups/metrics', - params: { - path: { serviceName }, - query: { - start, - end, - uiFilters: JSON.stringify(uiFilters), - numBuckets: 20, - transactionType, - groupIds: groupIds.join(), + async function fetchAggResults() { + if ( + !isEmpty(requestId) && + groupIds && + start && + end && + transactionType + ) { + const aggResults = await callApmApi({ + endpoint: + 'GET /api/apm/services/{serviceName}/error_groups/agg_results', + params: { + path: { serviceName }, + query: { + start, + end, + uiFilters: JSON.stringify(uiFilters), + numBuckets: 20, + transactionType, + groupIds, + }, }, - }, - }); + isCachable: true, + }); + return { [requestId]: aggResults }; + } } + return fetchAggResults(); }, - // only fetches metrics when groupIds change - // eslint-disable-next-line - [groupIds] + // only fetches agg results when requestId changes or group ids change + // eslint-disable-next-line react-hooks/exhaustive-deps + [requestId, groupIds] ); - const columns = getColumns({ serviceName, groupIdsErrorMetrics }); + const columns = getColumns({ + serviceName, + groupIdsErrorAggResults: groupIdsErrorAggResults + ? groupIdsErrorAggResults[requestId] + : undefined, + }); return ( @@ -260,10 +177,7 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { > diff --git a/x-pack/plugins/apm/server/lib/services/get_service_error_groups/index.ts b/x-pack/plugins/apm/server/lib/services/get_service_error_groups/index.ts index 7f5bcd790208..956d83655117 100644 --- a/x-pack/plugins/apm/server/lib/services/get_service_error_groups/index.ts +++ b/x-pack/plugins/apm/server/lib/services/get_service_error_groups/index.ts @@ -6,6 +6,7 @@ import { ValuesType } from 'utility-types'; import { orderBy } from 'lodash'; +import uuid from 'uuid'; import { NOT_AVAILABLE_LABEL } from '../../../../common/i18n'; import { PromiseReturnType } from '../../../../../observability/typings/common'; import { rangeFilter } from '../../../../common/utils/range_filter'; @@ -27,16 +28,10 @@ export type ServiceErrorGroupItem = ValuesType< export async function getServiceErrorGroups({ serviceName, setup, - numBuckets, - sortDirection, - sortField, transactionType, }: { serviceName: string; setup: Setup & SetupTimeRange; - numBuckets: number; - sortDirection: 'asc' | 'desc'; - sortField: 'name' | 'last_seen' | 'occurrences'; transactionType: string; }) { const { apmEventClient, start, end, esFilter } = setup; @@ -95,24 +90,17 @@ export async function getServiceErrorGroups({ }, })) ?? []; - // Sort error groups first, and only get timeseries for data in view. - // This is to limit the possibility of creating too many buckets. - - const sortedAndSlicedErrorGroups = orderBy( + const sortedErrorGroups = orderBy( errorGroups, - (group) => { - if (sortField === 'occurrences') { - return group.occurrences.value; - } - return group[sortField]; - }, - [sortDirection] + (group) => group.occurrences.value, + 'desc' ); return { + requestId: uuid(), total_error_groups: errorGroups.length, is_aggregation_accurate: (response.aggregations?.error_groups.sum_other_doc_count ?? 0) === 0, - error_groups: sortedAndSlicedErrorGroups, + error_groups: sortedErrorGroups, }; } diff --git a/x-pack/plugins/apm/server/routes/create_apm_api.ts b/x-pack/plugins/apm/server/routes/create_apm_api.ts index 823df38547b5..c6b7a2a2cf8a 100644 --- a/x-pack/plugins/apm/server/routes/create_apm_api.ts +++ b/x-pack/plugins/apm/server/routes/create_apm_api.ts @@ -22,7 +22,7 @@ import { serviceAnnotationsRoute, serviceAnnotationsCreateRoute, serviceErrorGroupsRoute, - serviceErrorGroupsMetricsRoute, + serviceErrorGroupsAggResultsRoute, serviceThroughputRoute, serviceDependenciesRoute, serviceMetadataDetailsRoute, @@ -136,7 +136,7 @@ const createApmApi = () => { .add(serviceMetadataDetailsRoute) .add(serviceMetadataIconsRoute) .add(serviceInstancesRoute) - .add(serviceErrorGroupsMetricsRoute) + .add(serviceErrorGroupsAggResultsRoute) // Agent configuration .add(getSingleAgentConfigurationRoute) diff --git a/x-pack/plugins/apm/server/routes/services.ts b/x-pack/plugins/apm/server/routes/services.ts index defb5eb78086..86cc2fc2194f 100644 --- a/x-pack/plugins/apm/server/routes/services.ts +++ b/x-pack/plugins/apm/server/routes/services.ts @@ -265,13 +265,6 @@ export const serviceErrorGroupsRoute = createRoute({ rangeRt, uiFiltersRt, t.type({ - numBuckets: toNumberRt, - sortDirection: t.union([t.literal('asc'), t.literal('desc')]), - sortField: t.union([ - t.literal('last_seen'), - t.literal('occurrences'), - t.literal('name'), - ]), transactionType: t.string, }), ]), @@ -282,21 +275,18 @@ export const serviceErrorGroupsRoute = createRoute({ const { path: { serviceName }, - query: { numBuckets, sortDirection, sortField, transactionType }, + query: { transactionType }, } = context.params; return getServiceErrorGroups({ serviceName, setup, - numBuckets, - sortDirection, - sortField, transactionType, }); }, }); -export const serviceErrorGroupsMetricsRoute = createRoute({ - endpoint: 'GET /api/apm/services/{serviceName}/error_groups/metrics', +export const serviceErrorGroupsAggResultsRoute = createRoute({ + endpoint: 'GET /api/apm/services/{serviceName}/error_groups/agg_results', params: t.type({ path: t.type({ serviceName: t.string, @@ -324,7 +314,7 @@ export const serviceErrorGroupsMetricsRoute = createRoute({ setup, numBuckets, transactionType, - groupIds: groupIds.split(','), + groupIds: JSON.parse(groupIds), }); }, }); From 92e9e532869832a0247bfef4259b0dcbe410f11f Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Tue, 2 Feb 2021 15:01:25 +0100 Subject: [PATCH 05/12] adding API test --- .../test/apm_api_integration/tests/index.ts | 2 + .../tests/services/error_groups.ts | 208 ++++++++---------- .../services/error_groups_agg_results.ts | 196 +++++++++++++++++ .../tests/services/error_groups_metrics.ts | 102 --------- 4 files changed, 292 insertions(+), 216 deletions(-) create mode 100644 x-pack/test/apm_api_integration/tests/services/error_groups_agg_results.ts delete mode 100644 x-pack/test/apm_api_integration/tests/services/error_groups_metrics.ts diff --git a/x-pack/test/apm_api_integration/tests/index.ts b/x-pack/test/apm_api_integration/tests/index.ts index eef82c714b2d..95068b33cbd6 100644 --- a/x-pack/test/apm_api_integration/tests/index.ts +++ b/x-pack/test/apm_api_integration/tests/index.ts @@ -43,6 +43,8 @@ export default function apmApiIntegrationTests(providerContext: FtrProviderConte loadTestFile(require.resolve('./services/throughput')); loadTestFile(require.resolve('./services/top_services')); loadTestFile(require.resolve('./services/transaction_types')); + loadTestFile(require.resolve('./services/error_groups')); + loadTestFile(require.resolve('./services/error_groups_agg_results')); loadTestFile(require.resolve('./settings/anomaly_detection/basic')); loadTestFile(require.resolve('./settings/anomaly_detection/no_access_user')); diff --git a/x-pack/test/apm_api_integration/tests/services/error_groups.ts b/x-pack/test/apm_api_integration/tests/services/error_groups.ts index 6e9d774e3af4..5b50869766a9 100644 --- a/x-pack/test/apm_api_integration/tests/services/error_groups.ts +++ b/x-pack/test/apm_api_integration/tests/services/error_groups.ts @@ -4,124 +4,104 @@ * you may not use this file except in compliance with the Elastic License. */ -// /* -// * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one -// * or more contributor license agreements. Licensed under the Elastic License; -// * you may not use this file except in compliance with the Elastic License. -// */ +import url from 'url'; +import expect from '@kbn/expect'; +import archives_metadata from '../../common/fixtures/es_archiver/archives_metadata'; +import { FtrProviderContext } from '../../common/ftr_provider_context'; +import { registry } from '../../common/registry'; -// import expect from '@kbn/expect'; -// import url from 'url'; -// import { FtrProviderContext } from '../../../../common/ftr_provider_context'; -// import archives from '../../../common/fixtures/es_archiver/archives_metadata'; +export default function ApiTest({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); -// export default function ApiTest({ getService }: FtrProviderContext) { -// const supertest = getService('supertest'); -// const esArchiver = getService('esArchiver'); + const archiveName = 'apm_8.0.0'; + const metadata = archives_metadata[archiveName]; + const { start, end } = metadata; -// const archiveName = 'apm_8.0.0'; -// const { start, end } = archives[archiveName]; + registry.when('Error groups when data is not loaded', { config: 'basic', archives: [] }, () => { + it('handles empty state', async () => { + const response = await supertest.get( + url.format({ + pathname: `/api/apm/services/opbeans-java/error_groups`, + query: { + start, + end, + uiFilters: '{}', + transactionType: 'request', + }, + }) + ); -// describe('Error groups', () => { -// describe('when data is not loaded ', () => { -// it('handles the empty state', async () => { -// const response = await supertest.get( -// url.format({ -// pathname: `/api/apm/services/opbeans-java/error_groups`, -// query: { -// start, -// end, -// uiFilters: '{}', -// numBuckets: 20, -// sortDirection: 'desc', -// sortField: 'occurrences', -// transactionType: 'request', -// }, -// }) -// ); + expect(response.status).to.be(200); -// expect(response.status).to.be(200); -// expectSnapshot(response.body).toMatchInline(` -// Object { -// "error_groups": Array [], -// "is_aggregation_accurate": true, -// "total_error_groups": 0, -// } -// `); -// }); -// }); + expect(response.status).to.be(200); + expect(response.body.error_groups).to.empty(); + expect(response.body.requestId).not.to.empty(); + expect(response.body.is_aggregation_accurate).to.eql(true); + expect(response.body.total_error_groups).to.eql(0); + }); + }); -// describe('when data is loaded', () => { -// before(() => esArchiver.load(archiveName)); -// after(() => esArchiver.unload(archiveName)); + registry.when( + 'Error groups when data is loaded', + { config: 'basic', archives: [archiveName] }, + () => { + it('returns the correct data', async () => { + const response = await supertest.get( + url.format({ + pathname: `/api/apm/services/opbeans-java/error_groups`, + query: { start, end, uiFilters: '{}', transactionType: 'request' }, + }) + ); -// it('returns the correct data', async () => { -// const response = await supertest.get( -// url.format({ -// pathname: `/api/apm/services/opbeans-java/error_groups`, -// query: { -// start, -// end, -// uiFilters: '{}', -// numBuckets: 20, -// sortDirection: 'desc', -// sortField: 'occurrences', -// transactionType: 'request', -// }, -// }) -// ); - -// expect(response.status).to.be(200); - -// expectSnapshot(response.body).toMatchInline(` -// Object { -// "error_groups": Array [ -// Object { -// "group_id": "051f95eabf120ebe2f8b0399fe3e54c5", -// "last_seen": 1607437366098, -// "name": "Could not write JSON: Null return value from advice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue(); nested exception is com.fasterxml.jackson.databind.JsonMappingException: Null return value from advice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue() (through reference chain: co.elastic.apm.opbeans.repositories.Stats[\\"numbers\\"]->com.sun.proxy.$Proxy132[\\"revenue\\"])", -// "occurrences": Object { -// "value": 5, -// }, -// }, -// Object { -// "group_id": "3bb34b98031a19c277bf59c3db82d3f3", -// "last_seen": 1607436860546, -// "name": "java.io.IOException: Connection reset by peer", -// "occurrences": Object { -// "value": 3, -// }, -// }, -// Object { -// "group_id": "b1c3ff13ec52de11187facf9c6a82538", -// "last_seen": 1607437482385, -// "name": "java.io.IOException: Connection reset by peer", -// "occurrences": Object { -// "value": 2, -// }, -// }, -// Object { -// "group_id": "9581687a53eac06aba50ba17cbd959c5", -// "last_seen": 1607437468244, -// "name": "Could not write JSON: Unable to find co.elastic.apm.opbeans.model.Customer with id 7173; nested exception is com.fasterxml.jackson.databind.JsonMappingException: Unable to find co.elastic.apm.opbeans.model.Customer with id 7173 (through reference chain: co.elastic.apm.opbeans.model.Customer_$$_jvst101_3[\\"email\\"])", -// "occurrences": Object { -// "value": 1, -// }, -// }, -// Object { -// "group_id": "97c2eef51fec10d177ade955670a2f15", -// "last_seen": 1607437475563, -// "name": "Request method 'POST' not supported", -// "occurrences": Object { -// "value": 1, -// }, -// }, -// ], -// "is_aggregation_accurate": true, -// "total_error_groups": 5, -// } -// `); -// }); -// }); -// }); -// } + expect(response.status).to.be(200); + expect(response.body.requestId).not.to.empty(); + expect(response.body.is_aggregation_accurate).to.eql(true); + expect(response.body.total_error_groups).to.eql(5); + expectSnapshot(response.body.error_groups).toMatchInline(` + Array [ + Object { + "group_id": "051f95eabf120ebe2f8b0399fe3e54c5", + "last_seen": 1607437366098, + "name": "Could not write JSON: Null return value fromadvice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue(); nested exception is com.fasterxml.jackson.databind.JsonMappingException: Null return value fromadvice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue() (through reference chain: co.elastic.apm.opbeans.repositories.Stats["numbers"]->com.sun.proxy.$Proxy132["revenue"])", + "occurrences": Object { + "value": 5, + }, + }, + Object { + "group_id": "3bb34b98031a19c277bf59c3db82d3f3", + "last_seen": 1607436860546, + "name": "java.io.IOException: Connection reset by peer", + "occurrences": Object { + "value": 3, + }, + }, + Object { + "group_id": "b1c3ff13ec52de11187facf9c6a82538", + "last_seen": 1607437482385, + "name": "java.io.IOException: Connection reset by peer", + "occurrences": Object { + "value": 2, + }, + }, + Object { + "group_id": "9581687a53eac06aba50ba17cbd959c5", + "last_seen": 1607437468244, + "name": "Could not write JSON: Unable to find co.elastic.apm.opbeans.model.Customer with id 7173; nested exception is com.fasterxml.jackson.databind.JsonMappingException: Unable to find co.elastic.apm.opbeans.model.Customer with id 7173 (through reference chain: co.elastic.apm.opbeans.model.Customer_$$_jvst101_3["email"])", + "occurrences": Object { + "value": 1, + }, + }, + Object { + "group_id": "97c2eef51fec10d177ade955670a2f15", + "last_seen": 1607437475563, + "name": "Request method 'POST' not supported", + "occurrences": Object { + "value": 1, + }, + }, + ] + `); + }); + } + ); +} diff --git a/x-pack/test/apm_api_integration/tests/services/error_groups_agg_results.ts b/x-pack/test/apm_api_integration/tests/services/error_groups_agg_results.ts new file mode 100644 index 000000000000..04f1a862b91e --- /dev/null +++ b/x-pack/test/apm_api_integration/tests/services/error_groups_agg_results.ts @@ -0,0 +1,196 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +// /* +// * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// * or more contributor license agreements. Licensed under the Elastic License; +// * you may not use this file except in compliance with the Elastic License. +// */ + +// import expect from '@kbn/expect'; +// import url from 'url'; +// import { FtrProviderContext } from '../../../../common/ftr_provider_context'; +// import archives from '../../../common/fixtures/es_archiver/archives_metadata'; + +// export default function ApiTest({ getService }: FtrProviderContext) { +// const supertest = getService('supertest'); +// const esArchiver = getService('esArchiver'); + +// const archiveName = 'apm_8.0.0'; +// const { start, end } = archives[archiveName]; +// const groupIds = [ +// '051f95eabf120ebe2f8b0399fe3e54c5', +// '3bb34b98031a19c277bf59c3db82d3f3', +// 'b1c3ff13ec52de11187facf9c6a82538', +// '9581687a53eac06aba50ba17cbd959c5', +// '97c2eef51fec10d177ade955670a2f15', +// ].join(); + +// describe('Error groups metrics', () => { +// describe('when data is not loaded ', () => { +// it('handles the empty state', async () => { +// const response = await supertest.get( +// url.format({ +// pathname: `/api/apm/services/opbeans-java/error_groups/metrics`, +// query: { +// start, +// end, +// uiFilters: '{}', +// numBuckets: 20, +// transactionType: 'request', +// groupIds, +// }, +// }) +// ); + +// expect(response.status).to.be(200); +// expectSnapshot(response.body).toMatchInline(`Object {}`); +// }); +// }); + +// describe('when data is loaded', () => { +// before(() => esArchiver.load(archiveName)); +// after(() => esArchiver.unload(archiveName)); + +// it('returns the correct data', async () => { +// const response = await supertest.get( +// url.format({ +// pathname: `/api/apm/services/opbeans-java/error_groups/metrics`, +// query: { +// start, +// end, +// uiFilters: '{}', +// numBuckets: 20, +// transactionType: 'request', +// groupIds, +// }, +// }) +// ); + +// expect(response.status).to.be(200); + +// const errorMetric = response.body['051f95eabf120ebe2f8b0399fe3e54c5']; + +// expectSnapshot(errorMetric.timeseries.filter(({ y }: any) => y > 0).length).toMatchInline( +// `4` +// ); +// }); + +// it('returns empty data', async () => { +// const response = await supertest.get( +// url.format({ +// pathname: `/api/apm/services/opbeans-java/error_groups/metrics`, +// query: { +// start, +// end, +// uiFilters: '{}', +// numBuckets: 20, +// transactionType: 'request', +// groupIds: 'foo', +// }, +// }) +// ); + +// expect(response.status).to.be(200); +// expectSnapshot(response.body).toMatchInline(`Object {}`); +// }); +// }); +// }); +// } + +import url from 'url'; +import expect from '@kbn/expect'; +import archives_metadata from '../../common/fixtures/es_archiver/archives_metadata'; +import { FtrProviderContext } from '../../common/ftr_provider_context'; +import { registry } from '../../common/registry'; + +export default function ApiTest({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + + const archiveName = 'apm_8.0.0'; + const metadata = archives_metadata[archiveName]; + const { start, end } = metadata; + const groupIds = JSON.stringify([ + '051f95eabf120ebe2f8b0399fe3e54c5', + '3bb34b98031a19c277bf59c3db82d3f3', + 'b1c3ff13ec52de11187facf9c6a82538', + '9581687a53eac06aba50ba17cbd959c5', + '97c2eef51fec10d177ade955670a2f15', + ]); + + registry.when( + 'Error groups agg results when data is not loaded', + { config: 'basic', archives: [] }, + () => { + it('handles empty state', async () => { + const response = await supertest.get( + url.format({ + pathname: `/api/apm/services/opbeans-java/error_groups/agg_results`, + query: { + start, + end, + uiFilters: '{}', + numBuckets: 20, + transactionType: 'request', + groupIds, + }, + }) + ); + expect(response.status).to.be(200); + expectSnapshot(response.body).toMatchInline(`Object {}`); + }); + } + ); + + registry.when( + 'Error groups agg results when data is loaded', + { config: 'basic', archives: [archiveName] }, + () => { + it('returns the correct data', async () => { + const response = await supertest.get( + url.format({ + pathname: `/api/apm/services/opbeans-java/error_groups/agg_results`, + query: { + start, + end, + uiFilters: '{}', + numBuckets: 20, + transactionType: 'request', + groupIds, + }, + }) + ); + + expect(response.status).to.be(200); + + const errorMetric = response.body['051f95eabf120ebe2f8b0399fe3e54c5']; + + expectSnapshot(errorMetric.timeseries.filter(({ y }: any) => y > 0).length).toMatchInline( + `4` + ); + }); + + it('returns empty data', async () => { + const response = await supertest.get( + url.format({ + pathname: `/api/apm/services/opbeans-java/error_groups/agg_results`, + query: { + start, + end, + uiFilters: '{}', + numBuckets: 20, + transactionType: 'request', + groupIds: JSON.stringify(['foo']), + }, + }) + ); + + expect(response.status).to.be(200); + expectSnapshot(response.body).toMatchInline(`Object {}`); + }); + } + ); +} diff --git a/x-pack/test/apm_api_integration/tests/services/error_groups_metrics.ts b/x-pack/test/apm_api_integration/tests/services/error_groups_metrics.ts deleted file mode 100644 index e7bf7291ede2..000000000000 --- a/x-pack/test/apm_api_integration/tests/services/error_groups_metrics.ts +++ /dev/null @@ -1,102 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -// /* -// * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one -// * or more contributor license agreements. Licensed under the Elastic License; -// * you may not use this file except in compliance with the Elastic License. -// */ - -// import expect from '@kbn/expect'; -// import url from 'url'; -// import { FtrProviderContext } from '../../../../common/ftr_provider_context'; -// import archives from '../../../common/fixtures/es_archiver/archives_metadata'; - -// export default function ApiTest({ getService }: FtrProviderContext) { -// const supertest = getService('supertest'); -// const esArchiver = getService('esArchiver'); - -// const archiveName = 'apm_8.0.0'; -// const { start, end } = archives[archiveName]; -// const groupIds = [ -// '051f95eabf120ebe2f8b0399fe3e54c5', -// '3bb34b98031a19c277bf59c3db82d3f3', -// 'b1c3ff13ec52de11187facf9c6a82538', -// '9581687a53eac06aba50ba17cbd959c5', -// '97c2eef51fec10d177ade955670a2f15', -// ].join(); - -// describe('Error groups metrics', () => { -// describe('when data is not loaded ', () => { -// it('handles the empty state', async () => { -// const response = await supertest.get( -// url.format({ -// pathname: `/api/apm/services/opbeans-java/error_groups/metrics`, -// query: { -// start, -// end, -// uiFilters: '{}', -// numBuckets: 20, -// transactionType: 'request', -// groupIds, -// }, -// }) -// ); - -// expect(response.status).to.be(200); -// expectSnapshot(response.body).toMatchInline(`Object {}`); -// }); -// }); - -// describe('when data is loaded', () => { -// before(() => esArchiver.load(archiveName)); -// after(() => esArchiver.unload(archiveName)); - -// it('returns the correct data', async () => { -// const response = await supertest.get( -// url.format({ -// pathname: `/api/apm/services/opbeans-java/error_groups/metrics`, -// query: { -// start, -// end, -// uiFilters: '{}', -// numBuckets: 20, -// transactionType: 'request', -// groupIds, -// }, -// }) -// ); - -// expect(response.status).to.be(200); - -// const errorMetric = response.body['051f95eabf120ebe2f8b0399fe3e54c5']; - -// expectSnapshot(errorMetric.timeseries.filter(({ y }: any) => y > 0).length).toMatchInline( -// `4` -// ); -// }); - -// it('returns empty data', async () => { -// const response = await supertest.get( -// url.format({ -// pathname: `/api/apm/services/opbeans-java/error_groups/metrics`, -// query: { -// start, -// end, -// uiFilters: '{}', -// numBuckets: 20, -// transactionType: 'request', -// groupIds: 'foo', -// }, -// }) -// ); - -// expect(response.status).to.be(200); -// expectSnapshot(response.body).toMatchInline(`Object {}`); -// }); -// }); -// }); -// } From 926de0b234ff07f61f652d62b1d6ce6bcf0a83f4 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Wed, 3 Feb 2021 17:12:13 +0100 Subject: [PATCH 06/12] refactoring --- .../service_overview.test.tsx | 8 +- .../get_column.tsx | 10 +- .../service_overview_errors_table/index.tsx | 65 +++-- .../get_service_error_groups/index.ts | 3 - .../apm/server/routes/create_apm_api.ts | 4 +- x-pack/plugins/apm/server/routes/services.ts | 35 +-- .../test/apm_api_integration/tests/index.ts | 3 +- .../tests/service_overview/error_groups.ts | 228 ------------------ .../tests/services/error_groups.ts | 49 +--- .../services/error_groups_agg_results.ts | 196 --------------- .../tests/services/error_groups_statistics.ts | 95 ++++++++ 11 files changed, 155 insertions(+), 541 deletions(-) delete mode 100644 x-pack/test/apm_api_integration/tests/service_overview/error_groups.ts delete mode 100644 x-pack/test/apm_api_integration/tests/services/error_groups_agg_results.ts create mode 100644 x-pack/test/apm_api_integration/tests/services/error_groups_statistics.ts diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview.test.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview.test.tsx index 46c2a4c322c9..28ca56f9f758 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview.test.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview.test.tsx @@ -79,11 +79,10 @@ describe('ServiceOverview', () => { status: FETCH_STATUS.SUCCESS, }); + /* eslint-disable @typescript-eslint/naming-convention */ const calls = { - // eslint-disable-next-line @typescript-eslint/naming-convention 'GET /api/apm/services/{serviceName}/error_groups': { error_groups: [], - total_error_groups: 0, }, 'GET /api/apm/services/{serviceName}/transactions/groups/overview': { transactionGroups: [], @@ -91,9 +90,12 @@ describe('ServiceOverview', () => { isAggregationAccurate: true, }, 'GET /api/apm/services/{serviceName}/dependencies': [], - // eslint-disable-next-line @typescript-eslint/naming-convention 'GET /api/apm/services/{serviceName}/service_overview_instances': [], + 'GET /api/apm/services/{serviceName}/error_groups/statistics': { + timeseries: [], + }, }; + /* eslint-enable @typescript-eslint/naming-convention */ jest .spyOn(callApmApiModule, 'createCallApmApi') diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx index db003e82dae0..5fdb9a5d18a3 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx @@ -15,14 +15,14 @@ import { TruncateWithTooltip } from '../../../shared/truncate_with_tooltip'; import { APIReturnType } from '../../../../services/rest/createCallApmApi'; type ErrorGroupItem = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups'>; -type GroupIdsErrorAggResults = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups/agg_results'>; +type GroupIdsErrorStatistics = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups/statistics'>; export function getColumns({ serviceName, - groupIdsErrorAggResults, + groupIdsErrorStatistics, }: { serviceName: string; - groupIdsErrorAggResults: GroupIdsErrorAggResults; + groupIdsErrorStatistics: GroupIdsErrorStatistics; }): Array> { return [ { @@ -69,8 +69,8 @@ export function getColumns({ ), width: px(unit * 12), render: (_, { occurrences, group_id: errorGroupId }) => { - const timeseries = groupIdsErrorAggResults - ? groupIdsErrorAggResults[errorGroupId]?.timeseries + const timeseries = groupIdsErrorStatistics + ? groupIdsErrorStatistics[errorGroupId]?.timeseries : undefined; return ( { return { - requestId: response.requestId, + requestId: uuid(), items: response.error_groups, - totalItemCount: response.total_error_groups, }; }); }, [start, end, serviceName, uiFilters, transactionType] ); - const { items, totalItemCount, requestId } = data; + const { items, requestId } = data; const currentPageErrorGroups = orderBy( items, (group) => { @@ -100,41 +99,33 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { ).slice(pageIndex * PAGE_SIZE, (pageIndex + 1) * PAGE_SIZE); const groupIds = JSON.stringify( - currentPageErrorGroups.map(({ group_id: groupId }) => groupId) + currentPageErrorGroups.map(({ group_id: groupId }) => groupId).sort() ); const { - data: groupIdsErrorAggResults, - status: groupIdsErrorAggResultsStatus, + data: groupIdsErrorStatistics, + status: groupIdsErrorStatisticsStatus, } = useFetcher( (callApmApi) => { - async function fetchAggResults() { - if ( - !isEmpty(requestId) && - groupIds && - start && - end && - transactionType - ) { - const aggResults = await callApmApi({ - endpoint: - 'GET /api/apm/services/{serviceName}/error_groups/agg_results', - params: { - path: { serviceName }, - query: { - start, - end, - uiFilters: JSON.stringify(uiFilters), - numBuckets: 20, - transactionType, - groupIds, - }, + if (!isEmpty(requestId) && groupIds && start && end && transactionType) { + return callApmApi({ + endpoint: + 'GET /api/apm/services/{serviceName}/error_groups/statistics', + params: { + path: { serviceName }, + query: { + start, + end, + uiFilters: JSON.stringify(uiFilters), + numBuckets: 20, + transactionType, + groupIds, }, - isCachable: true, - }); - return { [requestId]: aggResults }; - } + }, + isCachable: true, + }).then((response) => { + return { [requestId]: response }; + }); } - return fetchAggResults(); }, // only fetches agg results when requestId changes or group ids change // eslint-disable-next-line react-hooks/exhaustive-deps @@ -143,8 +134,8 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { const columns = getColumns({ serviceName, - groupIdsErrorAggResults: groupIdsErrorAggResults - ? groupIdsErrorAggResults[requestId] + groupIdsErrorStatistics: groupIdsErrorStatistics + ? groupIdsErrorStatistics[requestId] : undefined, }); @@ -183,13 +174,13 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { pagination={{ pageIndex, pageSize: PAGE_SIZE, - totalItemCount, + totalItemCount: items.length, pageSizeOptions: [PAGE_SIZE], hidePerPageOptions: true, }} loading={ status === FETCH_STATUS.LOADING || - groupIdsErrorAggResultsStatus === FETCH_STATUS.LOADING + groupIdsErrorStatisticsStatus === FETCH_STATUS.LOADING } onChange={(newTableOptions: { page?: { diff --git a/x-pack/plugins/apm/server/lib/services/get_service_error_groups/index.ts b/x-pack/plugins/apm/server/lib/services/get_service_error_groups/index.ts index 956d83655117..0a9ea176f312 100644 --- a/x-pack/plugins/apm/server/lib/services/get_service_error_groups/index.ts +++ b/x-pack/plugins/apm/server/lib/services/get_service_error_groups/index.ts @@ -6,7 +6,6 @@ import { ValuesType } from 'utility-types'; import { orderBy } from 'lodash'; -import uuid from 'uuid'; import { NOT_AVAILABLE_LABEL } from '../../../../common/i18n'; import { PromiseReturnType } from '../../../../../observability/typings/common'; import { rangeFilter } from '../../../../common/utils/range_filter'; @@ -97,8 +96,6 @@ export async function getServiceErrorGroups({ ); return { - requestId: uuid(), - total_error_groups: errorGroups.length, is_aggregation_accurate: (response.aggregations?.error_groups.sum_other_doc_count ?? 0) === 0, error_groups: sortedErrorGroups, diff --git a/x-pack/plugins/apm/server/routes/create_apm_api.ts b/x-pack/plugins/apm/server/routes/create_apm_api.ts index c6b7a2a2cf8a..15235797e0c4 100644 --- a/x-pack/plugins/apm/server/routes/create_apm_api.ts +++ b/x-pack/plugins/apm/server/routes/create_apm_api.ts @@ -22,7 +22,7 @@ import { serviceAnnotationsRoute, serviceAnnotationsCreateRoute, serviceErrorGroupsRoute, - serviceErrorGroupsAggResultsRoute, + serviceErrorGroupsStatisticsRoute, serviceThroughputRoute, serviceDependenciesRoute, serviceMetadataDetailsRoute, @@ -136,7 +136,7 @@ const createApmApi = () => { .add(serviceMetadataDetailsRoute) .add(serviceMetadataIconsRoute) .add(serviceInstancesRoute) - .add(serviceErrorGroupsAggResultsRoute) + .add(serviceErrorGroupsStatisticsRoute) // Agent configuration .add(getSingleAgentConfigurationRoute) diff --git a/x-pack/plugins/apm/server/routes/services.ts b/x-pack/plugins/apm/server/routes/services.ts index 86cc2fc2194f..e03e4a56f18c 100644 --- a/x-pack/plugins/apm/server/routes/services.ts +++ b/x-pack/plugins/apm/server/routes/services.ts @@ -4,27 +4,28 @@ * you may not use this file except in compliance with the Elastic License. */ -import * as t from 'io-ts'; import Boom from '@hapi/boom'; +import * as t from 'io-ts'; import { uniq } from 'lodash'; -import { setupRequest } from '../lib/helpers/setup_request'; -import { getServiceAgentName } from '../lib/services/get_service_agent_name'; -import { getServices } from '../lib/services/get_services'; -import { getServiceTransactionTypes } from '../lib/services/get_service_transaction_types'; -import { getServiceNodeMetadata } from '../lib/services/get_service_node_metadata'; -import { createRoute } from './create_route'; -import { uiFiltersRt, rangeRt } from './default_api_types'; -import { getServiceAnnotations } from '../lib/services/annotations'; import { dateAsStringRt } from '../../common/runtime_types/date_as_string_rt'; +import { jsonRt } from '../../common/runtime_types/json_rt'; +import { toNumberRt } from '../../common/runtime_types/to_number_rt'; import { getSearchAggregatedTransactions } from '../lib/helpers/aggregated_transactions'; -import { getServiceErrorGroups } from '../lib/services/get_service_error_groups'; +import { setupRequest } from '../lib/helpers/setup_request'; +import { getServiceAnnotations } from '../lib/services/annotations'; +import { getServices } from '../lib/services/get_services'; +import { getServiceAgentName } from '../lib/services/get_service_agent_name'; import { getServiceDependencies } from '../lib/services/get_service_dependencies'; -import { toNumberRt } from '../../common/runtime_types/to_number_rt'; -import { getThroughput } from '../lib/services/get_throughput'; +import { getServiceErrorGroups } from '../lib/services/get_service_error_groups'; +import { getServiceErrorGroupsMetrics } from '../lib/services/get_service_error_groups_metrics'; import { getServiceInstances } from '../lib/services/get_service_instances'; import { getServiceMetadataDetails } from '../lib/services/get_service_metadata_details'; import { getServiceMetadataIcons } from '../lib/services/get_service_metadata_icons'; -import { getServiceErrorGroupsMetrics } from '../lib/services/get_service_error_groups_metrics'; +import { getServiceNodeMetadata } from '../lib/services/get_service_node_metadata'; +import { getServiceTransactionTypes } from '../lib/services/get_service_transaction_types'; +import { getThroughput } from '../lib/services/get_throughput'; +import { createRoute } from './create_route'; +import { rangeRt, uiFiltersRt } from './default_api_types'; export const servicesRoute = createRoute({ endpoint: 'GET /api/apm/services', @@ -285,8 +286,8 @@ export const serviceErrorGroupsRoute = createRoute({ }, }); -export const serviceErrorGroupsAggResultsRoute = createRoute({ - endpoint: 'GET /api/apm/services/{serviceName}/error_groups/agg_results', +export const serviceErrorGroupsStatisticsRoute = createRoute({ + endpoint: 'GET /api/apm/services/{serviceName}/error_groups/statistics', params: t.type({ path: t.type({ serviceName: t.string, @@ -297,7 +298,7 @@ export const serviceErrorGroupsAggResultsRoute = createRoute({ t.type({ numBuckets: toNumberRt, transactionType: t.string, - groupIds: t.string, + groupIds: t.string.pipe(jsonRt), }), ]), }), @@ -314,7 +315,7 @@ export const serviceErrorGroupsAggResultsRoute = createRoute({ setup, numBuckets, transactionType, - groupIds: JSON.parse(groupIds), + groupIds, }); }, }); diff --git a/x-pack/test/apm_api_integration/tests/index.ts b/x-pack/test/apm_api_integration/tests/index.ts index 95068b33cbd6..98dfaba2eb94 100644 --- a/x-pack/test/apm_api_integration/tests/index.ts +++ b/x-pack/test/apm_api_integration/tests/index.ts @@ -33,7 +33,6 @@ export default function apmApiIntegrationTests(providerContext: FtrProviderConte loadTestFile(require.resolve('./service_maps/service_maps')); loadTestFile(require.resolve('./service_overview/dependencies')); - loadTestFile(require.resolve('./service_overview/error_groups')); loadTestFile(require.resolve('./service_overview/instances')); loadTestFile(require.resolve('./services/agent_name')); @@ -44,7 +43,7 @@ export default function apmApiIntegrationTests(providerContext: FtrProviderConte loadTestFile(require.resolve('./services/top_services')); loadTestFile(require.resolve('./services/transaction_types')); loadTestFile(require.resolve('./services/error_groups')); - loadTestFile(require.resolve('./services/error_groups_agg_results')); + loadTestFile(require.resolve('./services/error_groups_statistics')); loadTestFile(require.resolve('./settings/anomaly_detection/basic')); loadTestFile(require.resolve('./settings/anomaly_detection/no_access_user')); diff --git a/x-pack/test/apm_api_integration/tests/service_overview/error_groups.ts b/x-pack/test/apm_api_integration/tests/service_overview/error_groups.ts deleted file mode 100644 index fc649c60103c..000000000000 --- a/x-pack/test/apm_api_integration/tests/service_overview/error_groups.ts +++ /dev/null @@ -1,228 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import expect from '@kbn/expect'; -import qs from 'querystring'; -import { pick, uniqBy } from 'lodash'; -import { FtrProviderContext } from '../../common/ftr_provider_context'; -import archives from '../../common/fixtures/es_archiver/archives_metadata'; -import { registry } from '../../common/registry'; - -export default function ApiTest({ getService }: FtrProviderContext) { - const supertest = getService('supertest'); - - const archiveName = 'apm_8.0.0'; - const { start, end } = archives[archiveName]; - - registry.when( - 'Service overview error groups when data is not loaded', - { config: 'basic', archives: [] }, - () => { - it('handles the empty state', async () => { - const response = await supertest.get( - `/api/apm/services/opbeans-java/error_groups?${qs.stringify({ - start, - end, - uiFilters: '{}', - size: 5, - numBuckets: 20, - pageIndex: 0, - sortDirection: 'desc', - sortField: 'occurrences', - transactionType: 'request', - })}` - ); - - expect(response.status).to.be(200); - expect(response.body).to.eql({ - total_error_groups: 0, - error_groups: [], - is_aggregation_accurate: true, - }); - }); - } - ); - - registry.when( - 'Service overview error groups when data is loaded', - { config: 'basic', archives: [archiveName] }, - () => { - it('returns the correct data', async () => { - const response = await supertest.get( - `/api/apm/services/opbeans-java/error_groups?${qs.stringify({ - start, - end, - uiFilters: '{}', - size: 5, - numBuckets: 20, - pageIndex: 0, - sortDirection: 'desc', - sortField: 'occurrences', - transactionType: 'request', - })}` - ); - - expect(response.status).to.be(200); - - expectSnapshot(response.body.total_error_groups).toMatchInline(`5`); - - expectSnapshot(response.body.error_groups.map((group: any) => group.name)).toMatchInline(` - Array [ - "Could not write JSON: Null return value from advice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue(); nested exception is com.fasterxml.jackson.databind.JsonMappingException: Null return value from advice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue() (through reference chain: co.elastic.apm.opbeans.repositories.Stats[\\"numbers\\"]->com.sun.proxy.$Proxy132[\\"revenue\\"])", - "java.io.IOException: Connection reset by peer", - "java.io.IOException: Connection reset by peer", - "Could not write JSON: Unable to find co.elastic.apm.opbeans.model.Customer with id 7173; nested exception is com.fasterxml.jackson.databind.JsonMappingException: Unable to find co.elastic.apm.opbeans.model.Customer with id 7173 (through reference chain: co.elastic.apm.opbeans.model.Customer_$$_jvst101_3[\\"email\\"])", - "Request method 'POST' not supported", - ] - `); - - expectSnapshot(response.body.error_groups.map((group: any) => group.occurrences.value)) - .toMatchInline(` - Array [ - 5, - 3, - 2, - 1, - 1, - ] - `); - - const firstItem = response.body.error_groups[0]; - - expectSnapshot(pick(firstItem, 'group_id', 'last_seen', 'name', 'occurrences.value')) - .toMatchInline(` - Object { - "group_id": "051f95eabf120ebe2f8b0399fe3e54c5", - "last_seen": 1607437366098, - "name": "Could not write JSON: Null return value from advice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue(); nested exception is com.fasterxml.jackson.databind.JsonMappingException: Null return value from advice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue() (through reference chain: co.elastic.apm.opbeans.repositories.Stats[\\"numbers\\"]->com.sun.proxy.$Proxy132[\\"revenue\\"])", - "occurrences": Object { - "value": 5, - }, - } - `); - - const visibleDataPoints = firstItem.occurrences.timeseries.filter(({ y }: any) => y > 0); - expectSnapshot(visibleDataPoints.length).toMatchInline(`4`); - }); - - it('sorts items in the correct order', async () => { - const descendingResponse = await supertest.get( - `/api/apm/services/opbeans-java/error_groups?${qs.stringify({ - start, - end, - uiFilters: '{}', - size: 5, - numBuckets: 20, - pageIndex: 0, - sortDirection: 'desc', - sortField: 'occurrences', - transactionType: 'request', - })}` - ); - - expect(descendingResponse.status).to.be(200); - - const descendingOccurrences = descendingResponse.body.error_groups.map( - (item: any) => item.occurrences.value - ); - - expect(descendingOccurrences).to.eql(descendingOccurrences.concat().sort().reverse()); - - const ascendingResponse = await supertest.get( - `/api/apm/services/opbeans-java/error_groups?${qs.stringify({ - start, - end, - uiFilters: '{}', - size: 5, - numBuckets: 20, - pageIndex: 0, - sortDirection: 'desc', - sortField: 'occurrences', - transactionType: 'request', - })}` - ); - - const ascendingOccurrences = ascendingResponse.body.error_groups.map( - (item: any) => item.occurrences.value - ); - - expect(ascendingOccurrences).to.eql(ascendingOccurrences.concat().sort().reverse()); - }); - - it('sorts items by the correct field', async () => { - const response = await supertest.get( - `/api/apm/services/opbeans-java/error_groups?${qs.stringify({ - start, - end, - uiFilters: '{}', - size: 5, - numBuckets: 20, - pageIndex: 0, - sortDirection: 'desc', - sortField: 'last_seen', - transactionType: 'request', - })}` - ); - - expect(response.status).to.be(200); - - const dates = response.body.error_groups.map((group: any) => group.last_seen); - - expect(dates).to.eql(dates.concat().sort().reverse()); - }); - - it('paginates through the items', async () => { - const size = 1; - - const firstPage = await supertest.get( - `/api/apm/services/opbeans-java/error_groups?${qs.stringify({ - start, - end, - uiFilters: '{}', - size, - numBuckets: 20, - pageIndex: 0, - sortDirection: 'desc', - sortField: 'occurrences', - transactionType: 'request', - })}` - ); - - expect(firstPage.status).to.eql(200); - - const totalItems = firstPage.body.total_error_groups; - - const pages = Math.floor(totalItems / size); - - const items = await new Array(pages) - .fill(undefined) - .reduce(async (prevItemsPromise, _, pageIndex) => { - const prevItems = await prevItemsPromise; - - const thisPage = await supertest.get( - `/api/apm/services/opbeans-java/error_groups?${qs.stringify({ - start, - end, - uiFilters: '{}', - size, - numBuckets: 20, - pageIndex, - sortDirection: 'desc', - sortField: 'occurrences', - transactionType: 'request', - })}` - ); - - return prevItems.concat(thisPage.body.error_groups); - }, Promise.resolve([])); - - expect(items.length).to.eql(totalItems); - - expect(uniqBy(items, 'group_id').length).to.eql(totalItems); - }); - } - ); -} diff --git a/x-pack/test/apm_api_integration/tests/services/error_groups.ts b/x-pack/test/apm_api_integration/tests/services/error_groups.ts index 5b50869766a9..a106e94e4009 100644 --- a/x-pack/test/apm_api_integration/tests/services/error_groups.ts +++ b/x-pack/test/apm_api_integration/tests/services/error_groups.ts @@ -35,9 +35,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { expect(response.status).to.be(200); expect(response.body.error_groups).to.empty(); - expect(response.body.requestId).not.to.empty(); expect(response.body.is_aggregation_accurate).to.eql(true); - expect(response.body.total_error_groups).to.eql(0); }); }); @@ -54,53 +52,8 @@ export default function ApiTest({ getService }: FtrProviderContext) { ); expect(response.status).to.be(200); - expect(response.body.requestId).not.to.empty(); expect(response.body.is_aggregation_accurate).to.eql(true); - expect(response.body.total_error_groups).to.eql(5); - expectSnapshot(response.body.error_groups).toMatchInline(` - Array [ - Object { - "group_id": "051f95eabf120ebe2f8b0399fe3e54c5", - "last_seen": 1607437366098, - "name": "Could not write JSON: Null return value fromadvice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue(); nested exception is com.fasterxml.jackson.databind.JsonMappingException: Null return value fromadvice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue() (through reference chain: co.elastic.apm.opbeans.repositories.Stats["numbers"]->com.sun.proxy.$Proxy132["revenue"])", - "occurrences": Object { - "value": 5, - }, - }, - Object { - "group_id": "3bb34b98031a19c277bf59c3db82d3f3", - "last_seen": 1607436860546, - "name": "java.io.IOException: Connection reset by peer", - "occurrences": Object { - "value": 3, - }, - }, - Object { - "group_id": "b1c3ff13ec52de11187facf9c6a82538", - "last_seen": 1607437482385, - "name": "java.io.IOException: Connection reset by peer", - "occurrences": Object { - "value": 2, - }, - }, - Object { - "group_id": "9581687a53eac06aba50ba17cbd959c5", - "last_seen": 1607437468244, - "name": "Could not write JSON: Unable to find co.elastic.apm.opbeans.model.Customer with id 7173; nested exception is com.fasterxml.jackson.databind.JsonMappingException: Unable to find co.elastic.apm.opbeans.model.Customer with id 7173 (through reference chain: co.elastic.apm.opbeans.model.Customer_$$_jvst101_3["email"])", - "occurrences": Object { - "value": 1, - }, - }, - Object { - "group_id": "97c2eef51fec10d177ade955670a2f15", - "last_seen": 1607437475563, - "name": "Request method 'POST' not supported", - "occurrences": Object { - "value": 1, - }, - }, - ] - `); + expect(response.body.error_groups.length).to.be(5); }); } ); diff --git a/x-pack/test/apm_api_integration/tests/services/error_groups_agg_results.ts b/x-pack/test/apm_api_integration/tests/services/error_groups_agg_results.ts deleted file mode 100644 index 04f1a862b91e..000000000000 --- a/x-pack/test/apm_api_integration/tests/services/error_groups_agg_results.ts +++ /dev/null @@ -1,196 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -// /* -// * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one -// * or more contributor license agreements. Licensed under the Elastic License; -// * you may not use this file except in compliance with the Elastic License. -// */ - -// import expect from '@kbn/expect'; -// import url from 'url'; -// import { FtrProviderContext } from '../../../../common/ftr_provider_context'; -// import archives from '../../../common/fixtures/es_archiver/archives_metadata'; - -// export default function ApiTest({ getService }: FtrProviderContext) { -// const supertest = getService('supertest'); -// const esArchiver = getService('esArchiver'); - -// const archiveName = 'apm_8.0.0'; -// const { start, end } = archives[archiveName]; -// const groupIds = [ -// '051f95eabf120ebe2f8b0399fe3e54c5', -// '3bb34b98031a19c277bf59c3db82d3f3', -// 'b1c3ff13ec52de11187facf9c6a82538', -// '9581687a53eac06aba50ba17cbd959c5', -// '97c2eef51fec10d177ade955670a2f15', -// ].join(); - -// describe('Error groups metrics', () => { -// describe('when data is not loaded ', () => { -// it('handles the empty state', async () => { -// const response = await supertest.get( -// url.format({ -// pathname: `/api/apm/services/opbeans-java/error_groups/metrics`, -// query: { -// start, -// end, -// uiFilters: '{}', -// numBuckets: 20, -// transactionType: 'request', -// groupIds, -// }, -// }) -// ); - -// expect(response.status).to.be(200); -// expectSnapshot(response.body).toMatchInline(`Object {}`); -// }); -// }); - -// describe('when data is loaded', () => { -// before(() => esArchiver.load(archiveName)); -// after(() => esArchiver.unload(archiveName)); - -// it('returns the correct data', async () => { -// const response = await supertest.get( -// url.format({ -// pathname: `/api/apm/services/opbeans-java/error_groups/metrics`, -// query: { -// start, -// end, -// uiFilters: '{}', -// numBuckets: 20, -// transactionType: 'request', -// groupIds, -// }, -// }) -// ); - -// expect(response.status).to.be(200); - -// const errorMetric = response.body['051f95eabf120ebe2f8b0399fe3e54c5']; - -// expectSnapshot(errorMetric.timeseries.filter(({ y }: any) => y > 0).length).toMatchInline( -// `4` -// ); -// }); - -// it('returns empty data', async () => { -// const response = await supertest.get( -// url.format({ -// pathname: `/api/apm/services/opbeans-java/error_groups/metrics`, -// query: { -// start, -// end, -// uiFilters: '{}', -// numBuckets: 20, -// transactionType: 'request', -// groupIds: 'foo', -// }, -// }) -// ); - -// expect(response.status).to.be(200); -// expectSnapshot(response.body).toMatchInline(`Object {}`); -// }); -// }); -// }); -// } - -import url from 'url'; -import expect from '@kbn/expect'; -import archives_metadata from '../../common/fixtures/es_archiver/archives_metadata'; -import { FtrProviderContext } from '../../common/ftr_provider_context'; -import { registry } from '../../common/registry'; - -export default function ApiTest({ getService }: FtrProviderContext) { - const supertest = getService('supertest'); - - const archiveName = 'apm_8.0.0'; - const metadata = archives_metadata[archiveName]; - const { start, end } = metadata; - const groupIds = JSON.stringify([ - '051f95eabf120ebe2f8b0399fe3e54c5', - '3bb34b98031a19c277bf59c3db82d3f3', - 'b1c3ff13ec52de11187facf9c6a82538', - '9581687a53eac06aba50ba17cbd959c5', - '97c2eef51fec10d177ade955670a2f15', - ]); - - registry.when( - 'Error groups agg results when data is not loaded', - { config: 'basic', archives: [] }, - () => { - it('handles empty state', async () => { - const response = await supertest.get( - url.format({ - pathname: `/api/apm/services/opbeans-java/error_groups/agg_results`, - query: { - start, - end, - uiFilters: '{}', - numBuckets: 20, - transactionType: 'request', - groupIds, - }, - }) - ); - expect(response.status).to.be(200); - expectSnapshot(response.body).toMatchInline(`Object {}`); - }); - } - ); - - registry.when( - 'Error groups agg results when data is loaded', - { config: 'basic', archives: [archiveName] }, - () => { - it('returns the correct data', async () => { - const response = await supertest.get( - url.format({ - pathname: `/api/apm/services/opbeans-java/error_groups/agg_results`, - query: { - start, - end, - uiFilters: '{}', - numBuckets: 20, - transactionType: 'request', - groupIds, - }, - }) - ); - - expect(response.status).to.be(200); - - const errorMetric = response.body['051f95eabf120ebe2f8b0399fe3e54c5']; - - expectSnapshot(errorMetric.timeseries.filter(({ y }: any) => y > 0).length).toMatchInline( - `4` - ); - }); - - it('returns empty data', async () => { - const response = await supertest.get( - url.format({ - pathname: `/api/apm/services/opbeans-java/error_groups/agg_results`, - query: { - start, - end, - uiFilters: '{}', - numBuckets: 20, - transactionType: 'request', - groupIds: JSON.stringify(['foo']), - }, - }) - ); - - expect(response.status).to.be(200); - expectSnapshot(response.body).toMatchInline(`Object {}`); - }); - } - ); -} diff --git a/x-pack/test/apm_api_integration/tests/services/error_groups_statistics.ts b/x-pack/test/apm_api_integration/tests/services/error_groups_statistics.ts new file mode 100644 index 000000000000..56b814f00d78 --- /dev/null +++ b/x-pack/test/apm_api_integration/tests/services/error_groups_statistics.ts @@ -0,0 +1,95 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +import url from 'url'; +import expect from '@kbn/expect'; +import archives_metadata from '../../common/fixtures/es_archiver/archives_metadata'; +import { FtrProviderContext } from '../../common/ftr_provider_context'; +import { registry } from '../../common/registry'; + +export default function ApiTest({ getService }: FtrProviderContext) { + const supertest = getService('supertest'); + + const archiveName = 'apm_8.0.0'; + const metadata = archives_metadata[archiveName]; + const { start, end } = metadata; + const groupIds = [ + '051f95eabf120ebe2f8b0399fe3e54c5', + '3bb34b98031a19c277bf59c3db82d3f3', + 'b1c3ff13ec52de11187facf9c6a82538', + '9581687a53eac06aba50ba17cbd959c5', + '97c2eef51fec10d177ade955670a2f15', + ]; + + registry.when( + 'Error groups agg results when data is not loaded', + { config: 'basic', archives: [] }, + () => { + it('handles empty state', async () => { + const response = await supertest.get( + url.format({ + pathname: `/api/apm/services/opbeans-java/error_groups/statistics`, + query: { + start, + end, + uiFilters: '{}', + numBuckets: 20, + transactionType: 'request', + groupIds: JSON.stringify(groupIds), + }, + }) + ); + expect(response.status).to.be(200); + expect(response.body).to.empty(); + }); + } + ); + + registry.when( + 'Error groups agg results when data is loaded', + { config: 'basic', archives: [archiveName] }, + () => { + it('returns the correct data', async () => { + const response = await supertest.get( + url.format({ + pathname: `/api/apm/services/opbeans-java/error_groups/statistics`, + query: { + start, + end, + uiFilters: '{}', + numBuckets: 20, + transactionType: 'request', + groupIds: JSON.stringify(groupIds), + }, + }) + ); + + expect(response.status).to.be(200); + expect(Object.keys(response.body).length).to.be(5); + const errorMetric = response.body[groupIds[0]]; + expect(errorMetric.timeseries.length).to.be(31); + }); + + it('returns empty data', async () => { + const response = await supertest.get( + url.format({ + pathname: `/api/apm/services/opbeans-java/error_groups/statistics`, + query: { + start, + end, + uiFilters: '{}', + numBuckets: 20, + transactionType: 'request', + groupIds: JSON.stringify(['foo']), + }, + }) + ); + + expect(response.status).to.be(200); + expect(response.body).to.empty(); + }); + } + ); +} From 47792e0b51d6843a15d58bb63511796cb71d4be1 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Fri, 5 Feb 2021 14:46:05 +0100 Subject: [PATCH 07/12] fixing license --- .../service_overview_errors_table/get_column.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx index 5fdb9a5d18a3..ff89eac81a3c 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx @@ -1,8 +1,10 @@ /* * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. + * 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 { EuiBasicTableColumn } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import React from 'react'; From 4b6f1c4170f58fec1216d952ed649daa12e08922 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 8 Feb 2021 14:12:16 +0100 Subject: [PATCH 08/12] renaming apis --- .../service_overview.test.tsx | 4 +- .../get_column.tsx | 16 +++--- .../service_overview_errors_table/index.tsx | 16 +++--- .../apm/server/routes/create_apm_api.ts | 8 +-- x-pack/plugins/apm/server/routes/services.ts | 10 ++-- .../test/apm_api_integration/tests/index.ts | 4 +- ... => error_groups_comparison_statistics.ts} | 10 ++-- ....ts => error_groups_primary_statistics.ts} | 50 ++++++++++--------- 8 files changed, 63 insertions(+), 55 deletions(-) rename x-pack/test/apm_api_integration/tests/services/{error_groups_statistics.ts => error_groups_comparison_statistics.ts} (93%) rename x-pack/test/apm_api_integration/tests/services/{error_groups.ts => error_groups_primary_statistics.ts} (61%) diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview.test.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview.test.tsx index ffabdf2d3700..6d8652f23e72 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview.test.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview.test.tsx @@ -82,7 +82,7 @@ describe('ServiceOverview', () => { /* eslint-disable @typescript-eslint/naming-convention */ const calls = { - 'GET /api/apm/services/{serviceName}/error_groups': { + 'GET /api/apm/services/{serviceName}/error_groups/primary_statistics': { error_groups: [], }, 'GET /api/apm/services/{serviceName}/transactions/groups/overview': { @@ -92,7 +92,7 @@ describe('ServiceOverview', () => { }, 'GET /api/apm/services/{serviceName}/dependencies': [], 'GET /api/apm/services/{serviceName}/service_overview_instances': [], - 'GET /api/apm/services/{serviceName}/error_groups/statistics': { + 'GET /api/apm/services/{serviceName}/error_groups/comparison_statistics': { timeseries: [], }, }; diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx index ff89eac81a3c..8a167d7fc529 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx @@ -16,16 +16,18 @@ import { TimestampTooltip } from '../../../shared/TimestampTooltip'; import { TruncateWithTooltip } from '../../../shared/truncate_with_tooltip'; import { APIReturnType } from '../../../../services/rest/createCallApmApi'; -type ErrorGroupItem = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups'>; -type GroupIdsErrorStatistics = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups/statistics'>; +type ErrorGroupsPrimaryStatistics = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups/primary_statistics'>; +type ErrorGroupsComparisonStatistics = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups/comparison_statistics'>; export function getColumns({ serviceName, - groupIdsErrorStatistics, + errorGroupsComparisonStatistics, }: { serviceName: string; - groupIdsErrorStatistics: GroupIdsErrorStatistics; -}): Array> { + errorGroupsComparisonStatistics: ErrorGroupsComparisonStatistics; +}): Array< + EuiBasicTableColumn +> { return [ { field: 'name', @@ -71,8 +73,8 @@ export function getColumns({ ), width: px(unit * 12), render: (_, { occurrences, group_id: errorGroupId }) => { - const timeseries = groupIdsErrorStatistics - ? groupIdsErrorStatistics[errorGroupId]?.timeseries + const timeseries = errorGroupsComparisonStatistics + ? errorGroupsComparisonStatistics[errorGroupId]?.timeseries : undefined; return ( groupId).sort() ); const { - data: groupIdsErrorStatistics, - status: groupIdsErrorStatisticsStatus, + data: errorGroupsComparisonStatistics, + status: errorGroupsComparisonStatisticsStatus, } = useFetcher( (callApmApi) => { if (!isEmpty(requestId) && groupIds && start && end && transactionType) { return callApmApi({ endpoint: - 'GET /api/apm/services/{serviceName}/error_groups/statistics', + 'GET /api/apm/services/{serviceName}/error_groups/comparison_statistics', params: { path: { serviceName }, query: { @@ -136,9 +137,8 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { const columns = getColumns({ serviceName, - groupIdsErrorStatistics: groupIdsErrorStatistics - ? groupIdsErrorStatistics[requestId] - : undefined, + errorGroupsComparisonStatistics: + errorGroupsComparisonStatistics?.[requestId], }); return ( @@ -182,7 +182,7 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { }} loading={ status === FETCH_STATUS.LOADING || - groupIdsErrorStatisticsStatus === FETCH_STATUS.LOADING + errorGroupsComparisonStatisticsStatus === FETCH_STATUS.LOADING } onChange={(newTableOptions: { page?: { diff --git a/x-pack/plugins/apm/server/routes/create_apm_api.ts b/x-pack/plugins/apm/server/routes/create_apm_api.ts index d0c37af8b071..21c60cc2f7d0 100644 --- a/x-pack/plugins/apm/server/routes/create_apm_api.ts +++ b/x-pack/plugins/apm/server/routes/create_apm_api.ts @@ -23,8 +23,8 @@ import { serviceNodeMetadataRoute, serviceAnnotationsRoute, serviceAnnotationsCreateRoute, - serviceErrorGroupsRoute, - serviceErrorGroupsStatisticsRoute, + serviceErrorGroupsPrimaryStatisticsRoute, + serviceErrorGroupsComparisonStatisticsRoute, serviceThroughputRoute, serviceDependenciesRoute, serviceMetadataDetailsRoute, @@ -125,13 +125,13 @@ const createApmApi = () => { .add(serviceNodeMetadataRoute) .add(serviceAnnotationsRoute) .add(serviceAnnotationsCreateRoute) - .add(serviceErrorGroupsRoute) + .add(serviceErrorGroupsPrimaryStatisticsRoute) .add(serviceThroughputRoute) .add(serviceDependenciesRoute) .add(serviceMetadataDetailsRoute) .add(serviceMetadataIconsRoute) .add(serviceInstancesRoute) - .add(serviceErrorGroupsStatisticsRoute) + .add(serviceErrorGroupsComparisonStatisticsRoute) // Agent configuration .add(getSingleAgentConfigurationRoute) diff --git a/x-pack/plugins/apm/server/routes/services.ts b/x-pack/plugins/apm/server/routes/services.ts index c15c84805bae..a595dce50b9b 100644 --- a/x-pack/plugins/apm/server/routes/services.ts +++ b/x-pack/plugins/apm/server/routes/services.ts @@ -257,8 +257,9 @@ export const serviceAnnotationsCreateRoute = createRoute({ }, }); -export const serviceErrorGroupsRoute = createRoute({ - endpoint: 'GET /api/apm/services/{serviceName}/error_groups', +export const serviceErrorGroupsPrimaryStatisticsRoute = createRoute({ + endpoint: + 'GET /api/apm/services/{serviceName}/error_groups/primary_statistics', params: t.type({ path: t.type({ serviceName: t.string, @@ -287,8 +288,9 @@ export const serviceErrorGroupsRoute = createRoute({ }, }); -export const serviceErrorGroupsStatisticsRoute = createRoute({ - endpoint: 'GET /api/apm/services/{serviceName}/error_groups/statistics', +export const serviceErrorGroupsComparisonStatisticsRoute = createRoute({ + endpoint: + 'GET /api/apm/services/{serviceName}/error_groups/comparison_statistics', params: t.type({ path: t.type({ serviceName: t.string, diff --git a/x-pack/test/apm_api_integration/tests/index.ts b/x-pack/test/apm_api_integration/tests/index.ts index dfbdec1cf19c..822b6c281ca4 100644 --- a/x-pack/test/apm_api_integration/tests/index.ts +++ b/x-pack/test/apm_api_integration/tests/index.ts @@ -43,8 +43,8 @@ export default function apmApiIntegrationTests(providerContext: FtrProviderConte loadTestFile(require.resolve('./services/throughput')); loadTestFile(require.resolve('./services/top_services')); loadTestFile(require.resolve('./services/transaction_types')); - loadTestFile(require.resolve('./services/error_groups')); - loadTestFile(require.resolve('./services/error_groups_statistics')); + loadTestFile(require.resolve('./services/error_groups_primary_statistics')); + loadTestFile(require.resolve('./services/error_groups_comparison_statistics')); loadTestFile(require.resolve('./settings/anomaly_detection/basic')); loadTestFile(require.resolve('./settings/anomaly_detection/no_access_user')); diff --git a/x-pack/test/apm_api_integration/tests/services/error_groups_statistics.ts b/x-pack/test/apm_api_integration/tests/services/error_groups_comparison_statistics.ts similarity index 93% rename from x-pack/test/apm_api_integration/tests/services/error_groups_statistics.ts rename to x-pack/test/apm_api_integration/tests/services/error_groups_comparison_statistics.ts index 7e07eb81a1b3..db7a86758747 100644 --- a/x-pack/test/apm_api_integration/tests/services/error_groups_statistics.ts +++ b/x-pack/test/apm_api_integration/tests/services/error_groups_comparison_statistics.ts @@ -26,13 +26,13 @@ export default function ApiTest({ getService }: FtrProviderContext) { ]; registry.when( - 'Error groups agg results when data is not loaded', + 'Error groups comparison statistics when data is not loaded', { config: 'basic', archives: [] }, () => { it('handles empty state', async () => { const response = await supertest.get( url.format({ - pathname: `/api/apm/services/opbeans-java/error_groups/statistics`, + pathname: `/api/apm/services/opbeans-java/error_groups/comparison_statistics`, query: { start, end, @@ -50,13 +50,13 @@ export default function ApiTest({ getService }: FtrProviderContext) { ); registry.when( - 'Error groups agg results when data is loaded', + 'Error groups comparison statistics when data is loaded', { config: 'basic', archives: [archiveName] }, () => { it('returns the correct data', async () => { const response = await supertest.get( url.format({ - pathname: `/api/apm/services/opbeans-java/error_groups/statistics`, + pathname: `/api/apm/services/opbeans-java/error_groups/comparison_statistics`, query: { start, end, @@ -77,7 +77,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { it('returns empty data', async () => { const response = await supertest.get( url.format({ - pathname: `/api/apm/services/opbeans-java/error_groups/statistics`, + pathname: `/api/apm/services/opbeans-java/error_groups/comparison_statistics`, query: { start, end, diff --git a/x-pack/test/apm_api_integration/tests/services/error_groups.ts b/x-pack/test/apm_api_integration/tests/services/error_groups_primary_statistics.ts similarity index 61% rename from x-pack/test/apm_api_integration/tests/services/error_groups.ts rename to x-pack/test/apm_api_integration/tests/services/error_groups_primary_statistics.ts index 00ac45797fc3..f3de86315b1f 100644 --- a/x-pack/test/apm_api_integration/tests/services/error_groups.ts +++ b/x-pack/test/apm_api_integration/tests/services/error_groups_primary_statistics.ts @@ -18,36 +18,40 @@ export default function ApiTest({ getService }: FtrProviderContext) { const metadata = archives_metadata[archiveName]; const { start, end } = metadata; - registry.when('Error groups when data is not loaded', { config: 'basic', archives: [] }, () => { - it('handles empty state', async () => { - const response = await supertest.get( - url.format({ - pathname: `/api/apm/services/opbeans-java/error_groups`, - query: { - start, - end, - uiFilters: '{}', - transactionType: 'request', - }, - }) - ); - - expect(response.status).to.be(200); - - expect(response.status).to.be(200); - expect(response.body.error_groups).to.empty(); - expect(response.body.is_aggregation_accurate).to.eql(true); - }); - }); + registry.when( + 'Error groups primary statistics when data is not loaded', + { config: 'basic', archives: [] }, + () => { + it('handles empty state', async () => { + const response = await supertest.get( + url.format({ + pathname: `/api/apm/services/opbeans-java/error_groups/primary_statistics`, + query: { + start, + end, + uiFilters: '{}', + transactionType: 'request', + }, + }) + ); + + expect(response.status).to.be(200); + + expect(response.status).to.be(200); + expect(response.body.error_groups).to.empty(); + expect(response.body.is_aggregation_accurate).to.eql(true); + }); + } + ); registry.when( - 'Error groups when data is loaded', + 'Error groups primary statistics when data is loaded', { config: 'basic', archives: [archiveName] }, () => { it('returns the correct data', async () => { const response = await supertest.get( url.format({ - pathname: `/api/apm/services/opbeans-java/error_groups`, + pathname: `/api/apm/services/opbeans-java/error_groups/primary_statistics`, query: { start, end, uiFilters: '{}', transactionType: 'request' }, }) ); From 63fcf6aa3cff8c9a735d8d05173930b9d9a561ba Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 8 Feb 2021 17:48:47 +0100 Subject: [PATCH 09/12] fixing some stuff --- .../service_overview.test.tsx | 3 --- .../get_column.tsx | 17 +++++++--------- .../service_overview_errors_table/index.tsx | 13 ++++++------ ...vice_error_group_comparison_statistics.ts} | 14 ++++++------- ...service_error_group_primary_statistics.ts} | 20 +++++++------------ x-pack/plugins/apm/server/routes/services.ts | 8 ++++---- 6 files changed, 31 insertions(+), 44 deletions(-) rename x-pack/plugins/apm/server/lib/services/{get_service_error_groups_statistics/index.ts => get_service_error_group_comparison_statistics.ts} (83%) rename x-pack/plugins/apm/server/lib/services/{get_service_error_groups/index.ts => get_service_error_group_primary_statistics.ts} (76%) diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview.test.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview.test.tsx index 6d8652f23e72..f119a049d167 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview.test.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview.test.tsx @@ -92,9 +92,6 @@ describe('ServiceOverview', () => { }, 'GET /api/apm/services/{serviceName}/dependencies': [], 'GET /api/apm/services/{serviceName}/service_overview_instances': [], - 'GET /api/apm/services/{serviceName}/error_groups/comparison_statistics': { - timeseries: [], - }, }; /* eslint-enable @typescript-eslint/naming-convention */ diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx index 8a167d7fc529..2d89c560ef19 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx @@ -16,18 +16,16 @@ import { TimestampTooltip } from '../../../shared/TimestampTooltip'; import { TruncateWithTooltip } from '../../../shared/truncate_with_tooltip'; import { APIReturnType } from '../../../../services/rest/createCallApmApi'; -type ErrorGroupsPrimaryStatistics = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups/primary_statistics'>; -type ErrorGroupsComparisonStatistics = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups/comparison_statistics'>; +type ErrorGroupPrimaryStatistics = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups/primary_statistics'>; +type ErrorGroupComparisonStatistics = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups/comparison_statistics'>; export function getColumns({ serviceName, - errorGroupsComparisonStatistics, + errorGroupComparisonStatistics, }: { serviceName: string; - errorGroupsComparisonStatistics: ErrorGroupsComparisonStatistics; -}): Array< - EuiBasicTableColumn -> { + errorGroupComparisonStatistics: ErrorGroupComparisonStatistics; +}): Array> { return [ { field: 'name', @@ -73,9 +71,8 @@ export function getColumns({ ), width: px(unit * 12), render: (_, { occurrences, group_id: errorGroupId }) => { - const timeseries = errorGroupsComparisonStatistics - ? errorGroupsComparisonStatistics[errorGroupId]?.timeseries - : undefined; + const timeseries = + errorGroupComparisonStatistics?.[errorGroupId]?.timeseries; return ( groupId).sort() ); const { - data: errorGroupsComparisonStatistics, - status: errorGroupsComparisonStatisticsStatus, + data: errorGroupComparisonStatistics, + status: errorGroupComparisonStatisticsStatus, } = useFetcher( (callApmApi) => { - if (!isEmpty(requestId) && groupIds && start && end && transactionType) { + if (currentPageErrorGroups.length && start && end && transactionType) { return callApmApi({ endpoint: 'GET /api/apm/services/{serviceName}/error_groups/comparison_statistics', @@ -137,8 +137,7 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { const columns = getColumns({ serviceName, - errorGroupsComparisonStatistics: - errorGroupsComparisonStatistics?.[requestId], + errorGroupComparisonStatistics: errorGroupComparisonStatistics?.[requestId], }); return ( @@ -182,7 +181,7 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { }} loading={ status === FETCH_STATUS.LOADING || - errorGroupsComparisonStatisticsStatus === FETCH_STATUS.LOADING + errorGroupComparisonStatisticsStatus === FETCH_STATUS.LOADING } onChange={(newTableOptions: { page?: { diff --git a/x-pack/plugins/apm/server/lib/services/get_service_error_groups_statistics/index.ts b/x-pack/plugins/apm/server/lib/services/get_service_error_group_comparison_statistics.ts similarity index 83% rename from x-pack/plugins/apm/server/lib/services/get_service_error_groups_statistics/index.ts rename to x-pack/plugins/apm/server/lib/services/get_service_error_group_comparison_statistics.ts index 0b10fabd744c..cfa8e2086e5a 100644 --- a/x-pack/plugins/apm/server/lib/services/get_service_error_groups_statistics/index.ts +++ b/x-pack/plugins/apm/server/lib/services/get_service_error_group_comparison_statistics.ts @@ -5,18 +5,18 @@ * 2.0. */ -import { Coordinate } from '../../../../typings/timeseries'; import { ERROR_GROUP_ID, SERVICE_NAME, TRANSACTION_TYPE, -} from '../../../../common/elasticsearch_fieldnames'; -import { ProcessorEvent } from '../../../../common/processor_event'; -import { rangeFilter } from '../../../../common/utils/range_filter'; -import { getBucketSize } from '../../helpers/get_bucket_size'; -import { Setup, SetupTimeRange } from '../../helpers/setup_request'; +} from '../../../common/elasticsearch_fieldnames'; +import { ProcessorEvent } from '../../../common/processor_event'; +import { rangeFilter } from '../../../common/utils/range_filter'; +import { Coordinate } from '../../../typings/timeseries'; +import { getBucketSize } from '../helpers/get_bucket_size'; +import { Setup, SetupTimeRange } from '../helpers/setup_request'; -export async function getServiceErrorGroupsStatistics({ +export async function getServiceErrorGroupComparisonStatistics({ serviceName, setup, numBuckets, diff --git a/x-pack/plugins/apm/server/lib/services/get_service_error_groups/index.ts b/x-pack/plugins/apm/server/lib/services/get_service_error_group_primary_statistics.ts similarity index 76% rename from x-pack/plugins/apm/server/lib/services/get_service_error_groups/index.ts rename to x-pack/plugins/apm/server/lib/services/get_service_error_group_primary_statistics.ts index 79f0b6c19c09..cff57cb52e59 100644 --- a/x-pack/plugins/apm/server/lib/services/get_service_error_groups/index.ts +++ b/x-pack/plugins/apm/server/lib/services/get_service_error_group_primary_statistics.ts @@ -5,27 +5,21 @@ * 2.0. */ -import { ValuesType } from 'utility-types'; import { orderBy } from 'lodash'; -import { NOT_AVAILABLE_LABEL } from '../../../../common/i18n'; -import { PromiseReturnType } from '../../../../../observability/typings/common'; -import { rangeFilter } from '../../../../common/utils/range_filter'; -import { ProcessorEvent } from '../../../../common/processor_event'; import { ERROR_EXC_MESSAGE, ERROR_GROUP_ID, ERROR_LOG_MESSAGE, SERVICE_NAME, TRANSACTION_TYPE, -} from '../../../../common/elasticsearch_fieldnames'; -import { Setup, SetupTimeRange } from '../../helpers/setup_request'; -import { getErrorName } from '../../helpers/get_error_name'; +} from '../../../common/elasticsearch_fieldnames'; +import { NOT_AVAILABLE_LABEL } from '../../../common/i18n'; +import { ProcessorEvent } from '../../../common/processor_event'; +import { rangeFilter } from '../../../common/utils/range_filter'; +import { getErrorName } from '../helpers/get_error_name'; +import { Setup, SetupTimeRange } from '../helpers/setup_request'; -export type ServiceErrorGroupItem = ValuesType< - PromiseReturnType ->; - -export async function getServiceErrorGroups({ +export async function getServiceErrorGroupPrimaryStatistics({ serviceName, setup, transactionType, diff --git a/x-pack/plugins/apm/server/routes/services.ts b/x-pack/plugins/apm/server/routes/services.ts index a595dce50b9b..d2f7575df878 100644 --- a/x-pack/plugins/apm/server/routes/services.ts +++ b/x-pack/plugins/apm/server/routes/services.ts @@ -17,8 +17,8 @@ import { getServiceAnnotations } from '../lib/services/annotations'; import { getServices } from '../lib/services/get_services'; import { getServiceAgentName } from '../lib/services/get_service_agent_name'; import { getServiceDependencies } from '../lib/services/get_service_dependencies'; -import { getServiceErrorGroups } from '../lib/services/get_service_error_groups'; -import { getServiceErrorGroupsStatistics } from '../lib/services/get_service_error_groups_statistics'; +import { getServiceErrorGroupPrimaryStatistics } from '../lib/services/get_service_error_group_primary_statistics'; +import { getServiceErrorGroupComparisonStatistics } from '../lib/services/get_service_error_group_comparison_statistics'; import { getServiceInstances } from '../lib/services/get_service_instances'; import { getServiceMetadataDetails } from '../lib/services/get_service_metadata_details'; import { getServiceMetadataIcons } from '../lib/services/get_service_metadata_icons'; @@ -280,7 +280,7 @@ export const serviceErrorGroupsPrimaryStatisticsRoute = createRoute({ path: { serviceName }, query: { transactionType }, } = context.params; - return getServiceErrorGroups({ + return getServiceErrorGroupPrimaryStatistics({ serviceName, setup, transactionType, @@ -313,7 +313,7 @@ export const serviceErrorGroupsComparisonStatisticsRoute = createRoute({ path: { serviceName }, query: { numBuckets, transactionType, groupIds }, } = context.params; - return getServiceErrorGroupsStatistics({ + return getServiceErrorGroupComparisonStatistics({ serviceName, setup, numBuckets, From ac5f9a55c4e751d32c45313141547dce71ba0187 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 15 Feb 2021 09:45:40 -0500 Subject: [PATCH 10/12] addressing PR comments --- .../get_column.tsx | 2 +- .../service_overview_errors_table/index.tsx | 36 +++-- ..._service_error_group_primary_statistics.ts | 13 +- .../error_groups_comparison_statistics.snap | 133 ++++++++++++++++++ .../error_groups_primary_statistics.snap | 39 +++++ .../error_groups_comparison_statistics.ts | 21 ++- .../error_groups_primary_statistics.ts | 3 +- 7 files changed, 210 insertions(+), 37 deletions(-) create mode 100644 x-pack/test/apm_api_integration/tests/services/__snapshots__/error_groups_comparison_statistics.snap create mode 100644 x-pack/test/apm_api_integration/tests/services/__snapshots__/error_groups_primary_statistics.snap diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx index 2d89c560ef19..94913c1678d2 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/get_column.tsx @@ -82,7 +82,7 @@ export function getColumns({ { defaultMessage: `{occurrencesCount} occ.`, values: { - occurrencesCount: asInteger(occurrences.value), + occurrencesCount: asInteger(occurrences), }, } )} diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx index 379e93f56870..8fe02db4e24b 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx @@ -14,7 +14,6 @@ import { import { i18n } from '@kbn/i18n'; import { orderBy } from 'lodash'; import React, { useState } from 'react'; -import uuid from 'uuid'; import { useApmServiceContext } from '../../../../context/apm_service/use_apm_service_context'; import { useUrlParams } from '../../../../context/url_params_context/use_url_params'; import { FETCH_STATUS, useFetcher } from '../../../../hooks/use_fetcher'; @@ -36,6 +35,10 @@ const DEFAULT_SORT = { field: 'occurrences' as const, }; +const INITIAL_STATE = { + items: [], +}; + export function ServiceOverviewErrorsTable({ serviceName }: Props) { const { urlParams: { start, end }, @@ -55,18 +58,11 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { const { pageIndex, sort } = tableOptions; - const { - data = { - items: [], - requestId: '', - }, - status, - } = useFetcher( + const { data = INITIAL_STATE, status } = useFetcher( (callApmApi) => { if (!start || !end || !transactionType) { return; } - return callApmApi({ endpoint: 'GET /api/apm/services/{serviceName}/error_groups/primary_statistics', @@ -81,7 +77,6 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { }, }).then((response) => { return { - requestId: uuid(), items: response.error_groups, }; }); @@ -89,15 +84,10 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { [start, end, serviceName, uiFilters, transactionType] ); - const { items, requestId } = data; + const { items } = data; const currentPageErrorGroups = orderBy( items, - (group) => { - if (sort.field === 'occurrences') { - return group.occurrences.value; - } - return group[sort.field]; - }, + sort.field, sort.direction ).slice(pageIndex * PAGE_SIZE, (pageIndex + 1) * PAGE_SIZE); @@ -109,7 +99,13 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { status: errorGroupComparisonStatisticsStatus, } = useFetcher( (callApmApi) => { - if (currentPageErrorGroups.length && start && end && transactionType) { + if ( + status === FETCH_STATUS.SUCCESS && + currentPageErrorGroups.length && + start && + end && + transactionType + ) { return callApmApi({ endpoint: 'GET /api/apm/services/{serviceName}/error_groups/comparison_statistics', @@ -127,9 +123,9 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { }); } }, - // only fetches agg results when requestId changes or group ids change + // only fetches agg results when status changes or group ids change // eslint-disable-next-line react-hooks/exhaustive-deps - [requestId, groupIds], + [status, groupIds], { preservePreviousData: false } ); diff --git a/x-pack/plugins/apm/server/lib/services/get_service_error_groups/get_service_error_group_primary_statistics.ts b/x-pack/plugins/apm/server/lib/services/get_service_error_groups/get_service_error_group_primary_statistics.ts index 8287b24fb8b0..973b3cf4d4e8 100644 --- a/x-pack/plugins/apm/server/lib/services/get_service_error_groups/get_service_error_group_primary_statistics.ts +++ b/x-pack/plugins/apm/server/lib/services/get_service_error_groups/get_service_error_group_primary_statistics.ts @@ -5,7 +5,6 @@ * 2.0. */ -import { orderBy } from 'lodash'; import { ERROR_EXC_MESSAGE, ERROR_GROUP_ID, @@ -82,21 +81,13 @@ export function getServiceErrorGroupPrimaryStatistics({ last_seen: new Date( bucket.sample.hits.hits[0]?._source['@timestamp'] ).getTime(), - occurrences: { - value: bucket.doc_count, - }, + occurrences: bucket.doc_count, })) ?? []; - const sortedErrorGroups = orderBy( - errorGroups, - (group) => group.occurrences.value, - 'desc' - ); - return { is_aggregation_accurate: (response.aggregations?.error_groups.sum_other_doc_count ?? 0) === 0, - error_groups: sortedErrorGroups, + error_groups: errorGroups, }; }); } diff --git a/x-pack/test/apm_api_integration/tests/services/__snapshots__/error_groups_comparison_statistics.snap b/x-pack/test/apm_api_integration/tests/services/__snapshots__/error_groups_comparison_statistics.snap new file mode 100644 index 000000000000..a536a6de67ff --- /dev/null +++ b/x-pack/test/apm_api_integration/tests/services/__snapshots__/error_groups_comparison_statistics.snap @@ -0,0 +1,133 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`APM API tests basic apm_8.0.0 Error groups comparison statistics when data is loaded returns the correct data 1`] = ` +Object { + "groupId": "051f95eabf120ebe2f8b0399fe3e54c5", + "timeseries": Array [ + Object { + "x": 1607435820000, + "y": 0, + }, + Object { + "x": 1607435880000, + "y": 0, + }, + Object { + "x": 1607435940000, + "y": 0, + }, + Object { + "x": 1607436000000, + "y": 0, + }, + Object { + "x": 1607436060000, + "y": 0, + }, + Object { + "x": 1607436120000, + "y": 0, + }, + Object { + "x": 1607436180000, + "y": 0, + }, + Object { + "x": 1607436240000, + "y": 0, + }, + Object { + "x": 1607436300000, + "y": 1, + }, + Object { + "x": 1607436360000, + "y": 0, + }, + Object { + "x": 1607436420000, + "y": 0, + }, + Object { + "x": 1607436480000, + "y": 0, + }, + Object { + "x": 1607436540000, + "y": 0, + }, + Object { + "x": 1607436600000, + "y": 1, + }, + Object { + "x": 1607436660000, + "y": 0, + }, + Object { + "x": 1607436720000, + "y": 0, + }, + Object { + "x": 1607436780000, + "y": 0, + }, + Object { + "x": 1607436840000, + "y": 0, + }, + Object { + "x": 1607436900000, + "y": 0, + }, + Object { + "x": 1607436960000, + "y": 0, + }, + Object { + "x": 1607437020000, + "y": 0, + }, + Object { + "x": 1607437080000, + "y": 0, + }, + Object { + "x": 1607437140000, + "y": 0, + }, + Object { + "x": 1607437200000, + "y": 2, + }, + Object { + "x": 1607437260000, + "y": 0, + }, + Object { + "x": 1607437320000, + "y": 1, + }, + Object { + "x": 1607437380000, + "y": 0, + }, + Object { + "x": 1607437440000, + "y": 0, + }, + Object { + "x": 1607437500000, + "y": 0, + }, + Object { + "x": 1607437560000, + "y": 0, + }, + Object { + "x": 1607437620000, + "y": 0, + }, + ], +} +`; diff --git a/x-pack/test/apm_api_integration/tests/services/__snapshots__/error_groups_primary_statistics.snap b/x-pack/test/apm_api_integration/tests/services/__snapshots__/error_groups_primary_statistics.snap new file mode 100644 index 000000000000..cd744a3e5049 --- /dev/null +++ b/x-pack/test/apm_api_integration/tests/services/__snapshots__/error_groups_primary_statistics.snap @@ -0,0 +1,39 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`APM API tests basic apm_8.0.0 Error groups primary statistics when data is loaded returns the correct data 1`] = ` +Object { + "error_groups": Array [ + Object { + "group_id": "051f95eabf120ebe2f8b0399fe3e54c5", + "last_seen": 1607437366098, + "name": "Could not write JSON: Null return value from advice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue(); nested exception is com.fasterxml.jackson.databind.JsonMappingException: Null return value from advice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue() (through reference chain: co.elastic.apm.opbeans.repositories.Stats[\\"numbers\\"]->com.sun.proxy.$Proxy132[\\"revenue\\"])", + "occurrences": 5, + }, + Object { + "group_id": "3bb34b98031a19c277bf59c3db82d3f3", + "last_seen": 1607436860546, + "name": "java.io.IOException: Connection reset by peer", + "occurrences": 3, + }, + Object { + "group_id": "b1c3ff13ec52de11187facf9c6a82538", + "last_seen": 1607437482385, + "name": "java.io.IOException: Connection reset by peer", + "occurrences": 2, + }, + Object { + "group_id": "9581687a53eac06aba50ba17cbd959c5", + "last_seen": 1607437468244, + "name": "Could not write JSON: Unable to find co.elastic.apm.opbeans.model.Customer with id 7173; nested exception is com.fasterxml.jackson.databind.JsonMappingException: Unable to find co.elastic.apm.opbeans.model.Customer with id 7173 (through reference chain: co.elastic.apm.opbeans.model.Customer_$$_jvst101_3[\\"email\\"])", + "occurrences": 1, + }, + Object { + "group_id": "97c2eef51fec10d177ade955670a2f15", + "last_seen": 1607437475563, + "name": "Request method 'POST' not supported", + "occurrences": 1, + }, + ], + "is_aggregation_accurate": true, +} +`; diff --git a/x-pack/test/apm_api_integration/tests/services/error_groups_comparison_statistics.ts b/x-pack/test/apm_api_integration/tests/services/error_groups_comparison_statistics.ts index db7a86758747..50690897eba7 100644 --- a/x-pack/test/apm_api_integration/tests/services/error_groups_comparison_statistics.ts +++ b/x-pack/test/apm_api_integration/tests/services/error_groups_comparison_statistics.ts @@ -10,6 +10,9 @@ import expect from '@kbn/expect'; import archives_metadata from '../../common/fixtures/es_archiver/archives_metadata'; import { FtrProviderContext } from '../../common/ftr_provider_context'; import { registry } from '../../common/registry'; +import { APIReturnType } from '../../../../plugins/apm/public/services/rest/createCallApmApi'; + +type ErrorGroupsComparisonStatistics = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups/comparison_statistics'>; export default function ApiTest({ getService }: FtrProviderContext) { const supertest = getService('supertest'); @@ -69,12 +72,22 @@ export default function ApiTest({ getService }: FtrProviderContext) { ); expect(response.status).to.be(200); - expect(Object.keys(response.body).length).to.be(5); - const errorMetric = response.body[groupIds[0]]; - expect(errorMetric.timeseries.length).to.be(31); + + const errorGroupsComparisonStatistics = response.body as ErrorGroupsComparisonStatistics; + expect(Object.keys(errorGroupsComparisonStatistics).length).to.be.eql(groupIds.length); + + groupIds.map((groupId) => { + expect(errorGroupsComparisonStatistics[groupId]).not.to.be.empty(); + }); + + const errorgroupsComparisonStatistics = errorGroupsComparisonStatistics[groupIds[0]]; + expect( + errorgroupsComparisonStatistics.timeseries.map(({ y }) => isFinite(y)).length + ).to.be.greaterThan(0); + expectSnapshot(errorgroupsComparisonStatistics).toMatch(); }); - it('returns empty data', async () => { + it('returns an empty list when requested groupIds are not available in the given time range', async () => { const response = await supertest.get( url.format({ pathname: `/api/apm/services/opbeans-java/error_groups/comparison_statistics`, diff --git a/x-pack/test/apm_api_integration/tests/services/error_groups_primary_statistics.ts b/x-pack/test/apm_api_integration/tests/services/error_groups_primary_statistics.ts index f3de86315b1f..51394935c1c2 100644 --- a/x-pack/test/apm_api_integration/tests/services/error_groups_primary_statistics.ts +++ b/x-pack/test/apm_api_integration/tests/services/error_groups_primary_statistics.ts @@ -58,7 +58,8 @@ export default function ApiTest({ getService }: FtrProviderContext) { expect(response.status).to.be(200); expect(response.body.is_aggregation_accurate).to.eql(true); - expect(response.body.error_groups.length).to.be(5); + expect(response.body.error_groups.length).to.be.greaterThan(0); + expectSnapshot(response.body).toMatch(); }); } ); From 3b023027c3506fcd8a87b67277fb9beae1066464 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Thu, 18 Feb 2021 14:54:36 -0500 Subject: [PATCH 11/12] adding request id --- .../service_overview_errors_table/index.tsx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx index bb7655e3bd4e..109bf0483f2b 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_errors_table/index.tsx @@ -14,6 +14,7 @@ import { import { i18n } from '@kbn/i18n'; import { orderBy } from 'lodash'; import React, { useState } from 'react'; +import uuid from 'uuid'; import { useApmServiceContext } from '../../../../context/apm_service/use_apm_service_context'; import { useUrlParams } from '../../../../context/url_params_context/use_url_params'; import { FETCH_STATUS, useFetcher } from '../../../../hooks/use_fetcher'; @@ -37,6 +38,7 @@ const DEFAULT_SORT = { const INITIAL_STATE = { items: [], + requestId: undefined, }; export function ServiceOverviewErrorsTable({ serviceName }: Props) { @@ -78,6 +80,7 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { }, }).then((response) => { return { + requestId: uuid(), items: response.error_groups, }; }); @@ -85,7 +88,7 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { [environment, start, end, serviceName, uiFilters, transactionType] ); - const { items } = data; + const { requestId, items } = data; const currentPageErrorGroups = orderBy( items, sort.field, @@ -101,7 +104,7 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { } = useFetcher( (callApmApi) => { if ( - status === FETCH_STATUS.SUCCESS && + requestId && currentPageErrorGroups.length && start && end && @@ -124,9 +127,9 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) { }); } }, - // only fetches agg results when status changes or group ids change + // only fetches agg results when requestId or group ids change // eslint-disable-next-line react-hooks/exhaustive-deps - [status, groupIds], + [requestId, groupIds], { preservePreviousData: false } ); From 9af5714a744339bcf046441ac01095fffff4cdc8 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Fri, 19 Feb 2021 10:23:34 -0500 Subject: [PATCH 12/12] addressing PR comments --- x-pack/plugins/apm/server/routes/services.ts | 2 +- .../plugins/apm/server/routes/transactions.ts | 2 +- .../error_groups_primary_statistics.snap | 39 ------------- .../error_groups_comparison_statistics.ts | 4 +- .../error_groups_primary_statistics.ts | 57 +++++++++++++++++-- 5 files changed, 57 insertions(+), 47 deletions(-) delete mode 100644 x-pack/test/apm_api_integration/tests/services/__snapshots__/error_groups_primary_statistics.snap diff --git a/x-pack/plugins/apm/server/routes/services.ts b/x-pack/plugins/apm/server/routes/services.ts index af5d997a606a..2ce41f3d1e1a 100644 --- a/x-pack/plugins/apm/server/routes/services.ts +++ b/x-pack/plugins/apm/server/routes/services.ts @@ -325,7 +325,7 @@ export const serviceErrorGroupsComparisonStatisticsRoute = createRoute({ t.type({ numBuckets: toNumberRt, transactionType: t.string, - groupIds: t.string.pipe(jsonRt), + groupIds: jsonRt.pipe(t.array(t.string)), }), ]), }), diff --git a/x-pack/plugins/apm/server/routes/transactions.ts b/x-pack/plugins/apm/server/routes/transactions.ts index 5a4be216a817..960cc7f52642 100644 --- a/x-pack/plugins/apm/server/routes/transactions.ts +++ b/x-pack/plugins/apm/server/routes/transactions.ts @@ -117,7 +117,7 @@ export const transactionGroupsComparisonStatisticsRoute = createRoute({ rangeRt, uiFiltersRt, t.type({ - transactionNames: jsonRt, + transactionNames: jsonRt.pipe(t.array(t.string)), numBuckets: toNumberRt, transactionType: t.string, latencyAggregationType: latencyAggregationTypeRt, diff --git a/x-pack/test/apm_api_integration/tests/services/__snapshots__/error_groups_primary_statistics.snap b/x-pack/test/apm_api_integration/tests/services/__snapshots__/error_groups_primary_statistics.snap deleted file mode 100644 index cd744a3e5049..000000000000 --- a/x-pack/test/apm_api_integration/tests/services/__snapshots__/error_groups_primary_statistics.snap +++ /dev/null @@ -1,39 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`APM API tests basic apm_8.0.0 Error groups primary statistics when data is loaded returns the correct data 1`] = ` -Object { - "error_groups": Array [ - Object { - "group_id": "051f95eabf120ebe2f8b0399fe3e54c5", - "last_seen": 1607437366098, - "name": "Could not write JSON: Null return value from advice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue(); nested exception is com.fasterxml.jackson.databind.JsonMappingException: Null return value from advice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue() (through reference chain: co.elastic.apm.opbeans.repositories.Stats[\\"numbers\\"]->com.sun.proxy.$Proxy132[\\"revenue\\"])", - "occurrences": 5, - }, - Object { - "group_id": "3bb34b98031a19c277bf59c3db82d3f3", - "last_seen": 1607436860546, - "name": "java.io.IOException: Connection reset by peer", - "occurrences": 3, - }, - Object { - "group_id": "b1c3ff13ec52de11187facf9c6a82538", - "last_seen": 1607437482385, - "name": "java.io.IOException: Connection reset by peer", - "occurrences": 2, - }, - Object { - "group_id": "9581687a53eac06aba50ba17cbd959c5", - "last_seen": 1607437468244, - "name": "Could not write JSON: Unable to find co.elastic.apm.opbeans.model.Customer with id 7173; nested exception is com.fasterxml.jackson.databind.JsonMappingException: Unable to find co.elastic.apm.opbeans.model.Customer with id 7173 (through reference chain: co.elastic.apm.opbeans.model.Customer_$$_jvst101_3[\\"email\\"])", - "occurrences": 1, - }, - Object { - "group_id": "97c2eef51fec10d177ade955670a2f15", - "last_seen": 1607437475563, - "name": "Request method 'POST' not supported", - "occurrences": 1, - }, - ], - "is_aggregation_accurate": true, -} -`; diff --git a/x-pack/test/apm_api_integration/tests/services/error_groups_comparison_statistics.ts b/x-pack/test/apm_api_integration/tests/services/error_groups_comparison_statistics.ts index 50690897eba7..a13a76e2ddb4 100644 --- a/x-pack/test/apm_api_integration/tests/services/error_groups_comparison_statistics.ts +++ b/x-pack/test/apm_api_integration/tests/services/error_groups_comparison_statistics.ts @@ -74,9 +74,9 @@ export default function ApiTest({ getService }: FtrProviderContext) { expect(response.status).to.be(200); const errorGroupsComparisonStatistics = response.body as ErrorGroupsComparisonStatistics; - expect(Object.keys(errorGroupsComparisonStatistics).length).to.be.eql(groupIds.length); + expect(Object.keys(errorGroupsComparisonStatistics).sort()).to.eql(groupIds.sort()); - groupIds.map((groupId) => { + groupIds.forEach((groupId) => { expect(errorGroupsComparisonStatistics[groupId]).not.to.be.empty(); }); diff --git a/x-pack/test/apm_api_integration/tests/services/error_groups_primary_statistics.ts b/x-pack/test/apm_api_integration/tests/services/error_groups_primary_statistics.ts index 51394935c1c2..8a334ca567f0 100644 --- a/x-pack/test/apm_api_integration/tests/services/error_groups_primary_statistics.ts +++ b/x-pack/test/apm_api_integration/tests/services/error_groups_primary_statistics.ts @@ -10,6 +10,9 @@ import expect from '@kbn/expect'; import archives_metadata from '../../common/fixtures/es_archiver/archives_metadata'; import { FtrProviderContext } from '../../common/ftr_provider_context'; import { registry } from '../../common/registry'; +import { APIReturnType } from '../../../../plugins/apm/public/services/rest/createCallApmApi'; + +type ErrorGroupsPrimaryStatistics = APIReturnType<'GET /api/apm/services/{serviceName}/error_groups/primary_statistics'>; export default function ApiTest({ getService }: FtrProviderContext) { const supertest = getService('supertest'); @@ -52,14 +55,60 @@ export default function ApiTest({ getService }: FtrProviderContext) { const response = await supertest.get( url.format({ pathname: `/api/apm/services/opbeans-java/error_groups/primary_statistics`, - query: { start, end, uiFilters: '{}', transactionType: 'request' }, + query: { + start, + end, + uiFilters: '{}', + transactionType: 'request', + environment: 'production', + }, }) ); expect(response.status).to.be(200); - expect(response.body.is_aggregation_accurate).to.eql(true); - expect(response.body.error_groups.length).to.be.greaterThan(0); - expectSnapshot(response.body).toMatch(); + + const errorGroupPrimaryStatistics = response.body as ErrorGroupsPrimaryStatistics; + + expect(errorGroupPrimaryStatistics.is_aggregation_accurate).to.eql(true); + expect(errorGroupPrimaryStatistics.error_groups.length).to.be.greaterThan(0); + + expectSnapshot(errorGroupPrimaryStatistics.error_groups.map(({ name }) => name)) + .toMatchInline(` + Array [ + "Could not write JSON: Null return value from advice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue(); nested exception is com.fasterxml.jackson.databind.JsonMappingException: Null return value from advice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue() (through reference chain: co.elastic.apm.opbeans.repositories.Stats[\\"numbers\\"]->com.sun.proxy.$Proxy132[\\"revenue\\"])", + "java.io.IOException: Connection reset by peer", + "java.io.IOException: Connection reset by peer", + "Could not write JSON: Unable to find co.elastic.apm.opbeans.model.Customer with id 7173; nested exception is com.fasterxml.jackson.databind.JsonMappingException: Unable to find co.elastic.apm.opbeans.model.Customer with id 7173 (through reference chain: co.elastic.apm.opbeans.model.Customer_$$_jvst101_3[\\"email\\"])", + "Request method 'POST' not supported", + ] + `); + + const occurences = errorGroupPrimaryStatistics.error_groups.map( + ({ occurrences }) => occurrences + ); + + occurences.forEach((occurence) => expect(occurence).to.be.greaterThan(0)); + + expectSnapshot(occurences).toMatchInline(` + Array [ + 5, + 3, + 2, + 1, + 1, + ] + `); + + const firstItem = errorGroupPrimaryStatistics.error_groups[0]; + + expectSnapshot(firstItem).toMatchInline(` + Object { + "group_id": "051f95eabf120ebe2f8b0399fe3e54c5", + "last_seen": 1607437366098, + "name": "Could not write JSON: Null return value from advice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue(); nested exception is com.fasterxml.jackson.databind.JsonMappingException: Null return value from advice does not match primitive return type for: public abstract double co.elastic.apm.opbeans.repositories.Numbers.getRevenue() (through reference chain: co.elastic.apm.opbeans.repositories.Stats[\\"numbers\\"]->com.sun.proxy.$Proxy132[\\"revenue\\"])", + "occurrences": 5, + } + `); }); } );