From 381d09de699ef316d9d3fb1f09b7b9caad1a790d Mon Sep 17 00:00:00 2001 From: achyutjhunjhunwala Date: Tue, 20 Jun 2023 12:14:08 +0200 Subject: [PATCH 1/7] Add query to check for overflow bucket in service groups --- .../lib/service_group_query_with_overflow.ts | 29 +++++++++++++++++++ .../get_services/get_service_alerts.ts | 4 +-- .../get_service_transaction_stats.ts | 4 +-- .../get_services_without_transactions.ts | 4 +-- 4 files changed, 35 insertions(+), 6 deletions(-) create mode 100644 x-pack/plugins/apm/server/lib/service_group_query_with_overflow.ts diff --git a/x-pack/plugins/apm/server/lib/service_group_query_with_overflow.ts b/x-pack/plugins/apm/server/lib/service_group_query_with_overflow.ts new file mode 100644 index 0000000000000..61c1128408bf5 --- /dev/null +++ b/x-pack/plugins/apm/server/lib/service_group_query_with_overflow.ts @@ -0,0 +1,29 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { QueryDslQueryContainer } from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; +import { kqlQuery } from '@kbn/observability-plugin/server'; +import { SERVICE_NAME } from '@kbn/ux-plugin/common/elasticsearch_fieldnames'; +import { ServiceGroup } from '../../common/service_groups'; + +export function serviceGroupWithOverflowQuery( + serviceGroup?: ServiceGroup | null +): QueryDslQueryContainer[] { + if (serviceGroup) { + const serviceGroupQuery = kqlQuery(serviceGroup?.kuery); + const otherBucketQuery = kqlQuery(`${SERVICE_NAME} : "_other"`); + + return [ + { + bool: { + should: [...serviceGroupQuery, ...otherBucketQuery], + }, + }, + ]; + } + return []; +} diff --git a/x-pack/plugins/apm/server/routes/services/get_services/get_service_alerts.ts b/x-pack/plugins/apm/server/routes/services/get_services/get_service_alerts.ts index f212172364f2e..a8968f9bbb390 100644 --- a/x-pack/plugins/apm/server/routes/services/get_services/get_service_alerts.ts +++ b/x-pack/plugins/apm/server/routes/services/get_services/get_service_alerts.ts @@ -24,8 +24,8 @@ import { SERVICE_NAME } from '../../../../common/es_fields/apm'; import { ServiceGroup } from '../../../../common/service_groups'; import { ApmAlertsClient } from '../../../lib/helpers/get_apm_alerts_client'; import { environmentQuery } from '../../../../common/utils/environment_query'; -import { serviceGroupQuery } from '../../../lib/service_group_query'; import { MAX_NUMBER_OF_SERVICES } from './get_services_items'; +import { serviceGroupWithOverflowQuery } from '../../../lib/service_group_query_with_overflow'; interface ServiceAggResponse { buckets: Array< @@ -69,7 +69,7 @@ export async function getServicesAlerts({ ...termQuery(ALERT_STATUS, ALERT_STATUS_ACTIVE), ...rangeQuery(start, end), ...kqlQuery(kuery), - ...serviceGroupQuery(serviceGroup), + ...serviceGroupWithOverflowQuery(serviceGroup), ...termQuery(SERVICE_NAME, serviceName), ...environmentQuery(environment), ], diff --git a/x-pack/plugins/apm/server/routes/services/get_services/get_service_transaction_stats.ts b/x-pack/plugins/apm/server/routes/services/get_services/get_service_transaction_stats.ts index 3cd6cfd2698db..9557d130522a6 100644 --- a/x-pack/plugins/apm/server/routes/services/get_services/get_service_transaction_stats.ts +++ b/x-pack/plugins/apm/server/routes/services/get_services/get_service_transaction_stats.ts @@ -27,8 +27,8 @@ import { calculateFailedTransactionRate, getOutcomeAggregation, } from '../../../lib/helpers/transaction_error_rate'; -import { serviceGroupQuery } from '../../../lib/service_group_query'; import { maybe } from '../../../../common/utils/maybe'; +import { serviceGroupWithOverflowQuery } from '../../../lib/service_group_query_with_overflow'; interface AggregationParams { environment: string; @@ -102,7 +102,7 @@ export async function getServiceTransactionStats({ ...rangeQuery(start, end), ...environmentQuery(environment), ...kqlQuery(kuery), - ...serviceGroupQuery(serviceGroup), + ...serviceGroupWithOverflowQuery(serviceGroup), ], }, }, diff --git a/x-pack/plugins/apm/server/routes/services/get_services/get_services_without_transactions.ts b/x-pack/plugins/apm/server/routes/services/get_services/get_services_without_transactions.ts index 05af47e61e8ec..0eedb8494f21b 100644 --- a/x-pack/plugins/apm/server/routes/services/get_services/get_services_without_transactions.ts +++ b/x-pack/plugins/apm/server/routes/services/get_services/get_services_without_transactions.ts @@ -14,12 +14,12 @@ import { SERVICE_NAME, } from '../../../../common/es_fields/apm'; import { environmentQuery } from '../../../../common/utils/environment_query'; -import { serviceGroupQuery } from '../../../lib/service_group_query'; import { ServiceGroup } from '../../../../common/service_groups'; import { RandomSampler } from '../../../lib/helpers/get_random_sampler'; import { APMEventClient } from '../../../lib/helpers/create_es_client/create_apm_event_client'; import { ApmDocumentType } from '../../../../common/document_type'; import { RollupInterval } from '../../../../common/rollup'; +import { serviceGroupWithOverflowQuery } from '../../../lib/service_group_query_with_overflow'; export interface ServicesWithoutTransactionsResponse { services: Array<{ @@ -82,7 +82,7 @@ export async function getServicesWithoutTransactions({ ...rangeQuery(start, end), ...environmentQuery(environment), ...kqlQuery(kuery), - ...serviceGroupQuery(serviceGroup), + ...serviceGroupWithOverflowQuery(serviceGroup), ], }, }, From 8018344a68f464fca881b93f6b040e505bbbe973 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 20 Jun 2023 10:26:51 +0000 Subject: [PATCH 2/7] [CI] Auto-commit changed files from 'node scripts/lint_ts_projects --fix' --- x-pack/plugins/apm/tsconfig.json | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugins/apm/tsconfig.json b/x-pack/plugins/apm/tsconfig.json index faa3fa932096f..5afac767d4c52 100644 --- a/x-pack/plugins/apm/tsconfig.json +++ b/x-pack/plugins/apm/tsconfig.json @@ -93,6 +93,7 @@ "@kbn/dashboard-plugin", "@kbn/controls-plugin", "@kbn/core-http-server", + "@kbn/ux-plugin", ], "exclude": [ "target/**/*", From 6b7eb605e2107a27167ab72ab040316f3ba36eab Mon Sep 17 00:00:00 2001 From: achyutjhunjhunwala Date: Tue, 20 Jun 2023 12:29:44 +0200 Subject: [PATCH 3/7] Fix wrong import --- .../plugins/apm/server/lib/service_group_query_with_overflow.ts | 2 +- x-pack/plugins/apm/tsconfig.json | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/plugins/apm/server/lib/service_group_query_with_overflow.ts b/x-pack/plugins/apm/server/lib/service_group_query_with_overflow.ts index 61c1128408bf5..5dda47c30c624 100644 --- a/x-pack/plugins/apm/server/lib/service_group_query_with_overflow.ts +++ b/x-pack/plugins/apm/server/lib/service_group_query_with_overflow.ts @@ -7,7 +7,7 @@ import { QueryDslQueryContainer } from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; import { kqlQuery } from '@kbn/observability-plugin/server'; -import { SERVICE_NAME } from '@kbn/ux-plugin/common/elasticsearch_fieldnames'; +import { SERVICE_NAME } from '../../common/es_fields/apm'; import { ServiceGroup } from '../../common/service_groups'; export function serviceGroupWithOverflowQuery( diff --git a/x-pack/plugins/apm/tsconfig.json b/x-pack/plugins/apm/tsconfig.json index 5afac767d4c52..faa3fa932096f 100644 --- a/x-pack/plugins/apm/tsconfig.json +++ b/x-pack/plugins/apm/tsconfig.json @@ -93,7 +93,6 @@ "@kbn/dashboard-plugin", "@kbn/controls-plugin", "@kbn/core-http-server", - "@kbn/ux-plugin", ], "exclude": [ "target/**/*", From ec8942f7fc89ad1c63cf94f9917d1099e402a997 Mon Sep 17 00:00:00 2001 From: achyutjhunjhunwala Date: Thu, 22 Jun 2023 11:13:57 +0200 Subject: [PATCH 4/7] Fix review comments --- .../lib/service_group_query_with_overflow.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/apm/server/lib/service_group_query_with_overflow.ts b/x-pack/plugins/apm/server/lib/service_group_query_with_overflow.ts index 5dda47c30c624..f0f1009b7d036 100644 --- a/x-pack/plugins/apm/server/lib/service_group_query_with_overflow.ts +++ b/x-pack/plugins/apm/server/lib/service_group_query_with_overflow.ts @@ -6,7 +6,7 @@ */ import { QueryDslQueryContainer } from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; -import { kqlQuery } from '@kbn/observability-plugin/server'; +import { kqlQuery, termQuery } from '@kbn/observability-plugin/server'; import { SERVICE_NAME } from '../../common/es_fields/apm'; import { ServiceGroup } from '../../common/service_groups'; @@ -15,12 +15,23 @@ export function serviceGroupWithOverflowQuery( ): QueryDslQueryContainer[] { if (serviceGroup) { const serviceGroupQuery = kqlQuery(serviceGroup?.kuery); - const otherBucketQuery = kqlQuery(`${SERVICE_NAME} : "_other"`); + const otherBucketQuery = termQuery(SERVICE_NAME, '_other'); return [ { bool: { - should: [...serviceGroupQuery, ...otherBucketQuery], + should: [ + { + bool: { + filter: serviceGroupQuery, + }, + }, + { + bool: { + filter: otherBucketQuery, + }, + }, + ], }, }, ]; From 12b5db3679afb735da0f5da20ae7bde06b3c09b6 Mon Sep 17 00:00:00 2001 From: achyutjhunjhunwala Date: Thu, 22 Jun 2023 17:49:50 +0200 Subject: [PATCH 5/7] Add API Tests --- .../service_group_with_overflow/es_utils.ts | 51 +++++++++ .../generate_data.ts | 57 ++++++++++ .../service_group_with_overflow.spec.ts | 103 ++++++++++++++++++ 3 files changed, 211 insertions(+) create mode 100644 x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/es_utils.ts create mode 100644 x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/generate_data.ts create mode 100644 x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/service_group_with_overflow.spec.ts diff --git a/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/es_utils.ts b/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/es_utils.ts new file mode 100644 index 0000000000000..03a0def567957 --- /dev/null +++ b/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/es_utils.ts @@ -0,0 +1,51 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export function createServiceTransactionMetricsDocs({ + time, + service, + agentName, + overflowCount, +}: { + time: number; + service: { + name: string; + environment?: string; + language?: string; + }; + agentName?: string; + overflowCount?: number; +}) { + return { + processor: { + event: 'metric' as const, + }, + '@timestamp': new Date(time).toISOString(), + ...(agentName && { + agent: { + name: agentName, + }, + }), + event: { + ingested: new Date(time).toISOString(), + }, + metricset: { + name: 'service_transaction', + }, + service, + ...(overflowCount && { + service_transaction: { + aggregation: { + overflow_count: overflowCount, + }, + }, + }), + observer: { + version: '8.9.0', + }, + }; +} diff --git a/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/generate_data.ts b/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/generate_data.ts new file mode 100644 index 0000000000000..21c14f36866f0 --- /dev/null +++ b/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/generate_data.ts @@ -0,0 +1,57 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +import { apm, timerange } from '@kbn/apm-synthtrace-client'; +import type { ApmSynthtraceEsClient } from '@kbn/apm-synthtrace'; + +export async function generateData({ + synthtraceEsClient, + start, + end, +}: { + synthtraceEsClient: ApmSynthtraceEsClient; + start: number; + end: number; +}) { + const synthServices = [ + apm + .service({ name: 'synth-go', environment: 'testing', agentName: 'go' }) + .instance('instance-1'), + apm + .service({ name: 'synth-java', environment: 'testing', agentName: 'java' }) + .instance('instance-2'), + ]; + + await synthtraceEsClient.index( + synthServices.map((service) => + timerange(start, end) + .interval('5m') + .rate(1) + .generator((timestamp) => + service + .transaction({ + transactionName: 'GET /api/product/list', + transactionType: 'request', + }) + .duration(2000) + .timestamp(timestamp) + .children( + service + .span({ + spanName: '/_search', + spanType: 'db', + spanSubtype: 'elasticsearch', + }) + .destination('elasticsearch') + .duration(100) + .success() + .timestamp(timestamp) + ) + .errors(service.error({ message: 'error 1', type: 'foo' }).timestamp(timestamp)) + ) + ) + ); +} diff --git a/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/service_group_with_overflow.spec.ts b/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/service_group_with_overflow.spec.ts new file mode 100644 index 0000000000000..b110a4570d883 --- /dev/null +++ b/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/service_group_with_overflow.spec.ts @@ -0,0 +1,103 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +import expect from '@kbn/expect'; +import { ValuesType } from 'utility-types'; +import { ENVIRONMENT_ALL } from '@kbn/apm-plugin/common/environment_filter_values'; +import { ApmDocumentType } from '@kbn/apm-plugin/common/document_type'; +import { RollupInterval } from '@kbn/apm-plugin/common/rollup'; +import { FtrProviderContext } from '../../../common/ftr_provider_context'; +import { createServiceGroupApi, deleteAllServiceGroups } from '../service_groups_api_methods'; +import { createServiceTransactionMetricsDocs } from './es_utils'; +import { generateData } from './generate_data'; + +export default function ApiTest({ getService }: FtrProviderContext) { + const registry = getService('registry'); + const apmApiClient = getService('apmApiClient'); + const es = getService('es'); + const synthtraceEsClient = getService('synthtraceEsClient'); + + registry.when( + 'Display overflow bucket in Service Groups', + { config: 'basic', archives: [] }, + () => { + const indexName = 'metrics-apm.service_transaction.1m-default'; + const start = '2023-06-21T06:50:15.910Z'; + const end = '2023-06-21T06:59:15.910Z'; + const startTime = new Date(start).getTime() + 1000; + let serviceGroupId; + + after(async () => { + await deleteAllServiceGroups(apmApiClient); + synthtraceEsClient.clean(); + }); + + before(async () => { + await generateData({ start, end, synthtraceEsClient }); + + const docs = [ + createServiceTransactionMetricsDocs({ + time: startTime, + service: { + name: '_other', + }, + overflowCount: 13, + }), + ]; + + const bulkActions = docs.reduce( + (prev, doc) => { + return [...prev, { create: { _index: indexName } }, doc]; + }, + [] as Array< + | { + index: { + _index: string; + }; + } + | ValuesType + > + ); + + await es.bulk({ + body: bulkActions, + refresh: 'wait_for', + }); + + const serviceGroup = { + groupName: 'overflowGroup', + kuery: 'service.name: synth-go or service.name: synth-java', + }; + const createResponse = await createServiceGroupApi({ apmApiClient, ...serviceGroup }); + expect(createResponse.status).to.be(200); + serviceGroupId = createResponse.body.id; + }); + + it('get the overflow bucket even though its not added explicitly in the Service Group', async () => { + const response = await apmApiClient.readUser({ + endpoint: `GET /internal/apm/services`, + params: { + query: { + start, + end, + environment: ENVIRONMENT_ALL.value, + kuery: '', + serviceGroup: serviceGroupId, + probability: 1, + documentType: ApmDocumentType.ServiceTransactionMetric, + rollupInterval: RollupInterval.OneMinute, + }, + }, + }); + + const overflowBucket = response.body.items.find( + (service) => service.serviceName === '_other' + ); + expect(overflowBucket.serviceName).to.equal('_other'); + }); + } + ); +} From bf3038bcf501f39e67318481d3f6a69975315e58 Mon Sep 17 00:00:00 2001 From: achyutjhunjhunwala Date: Thu, 22 Jun 2023 17:51:41 +0200 Subject: [PATCH 6/7] Refactor other name --- .../service_group_with_overflow.spec.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/service_group_with_overflow.spec.ts b/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/service_group_with_overflow.spec.ts index b110a4570d883..42243992763fe 100644 --- a/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/service_group_with_overflow.spec.ts +++ b/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/service_group_with_overflow.spec.ts @@ -28,6 +28,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { const start = '2023-06-21T06:50:15.910Z'; const end = '2023-06-21T06:59:15.910Z'; const startTime = new Date(start).getTime() + 1000; + const OVERFLOW_SERVICE_NAME = '_other'; let serviceGroupId; after(async () => { @@ -42,7 +43,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { createServiceTransactionMetricsDocs({ time: startTime, service: { - name: '_other', + name: OVERFLOW_SERVICE_NAME, }, overflowCount: 13, }), @@ -94,9 +95,9 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); const overflowBucket = response.body.items.find( - (service) => service.serviceName === '_other' + (service) => service.serviceName === OVERFLOW_SERVICE_NAME ); - expect(overflowBucket.serviceName).to.equal('_other'); + expect(overflowBucket.serviceName).to.equal(OVERFLOW_SERVICE_NAME); }); } ); From 3ed952d318ac92cdb2943dfa36c27e8e874e6431 Mon Sep 17 00:00:00 2001 From: achyutjhunjhunwala Date: Thu, 22 Jun 2023 19:17:31 +0200 Subject: [PATCH 7/7] Fix checktypes issue --- .../service_group_with_overflow/generate_data.ts | 4 ++-- .../service_group_with_overflow.spec.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/generate_data.ts b/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/generate_data.ts index 21c14f36866f0..e688e6ac6836a 100644 --- a/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/generate_data.ts +++ b/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/generate_data.ts @@ -13,8 +13,8 @@ export async function generateData({ end, }: { synthtraceEsClient: ApmSynthtraceEsClient; - start: number; - end: number; + start: string; + end: string; }) { const synthServices = [ apm diff --git a/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/service_group_with_overflow.spec.ts b/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/service_group_with_overflow.spec.ts index 42243992763fe..dba8a21521a89 100644 --- a/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/service_group_with_overflow.spec.ts +++ b/x-pack/test/apm_api_integration/tests/service_groups/service_group_with_overflow/service_group_with_overflow.spec.ts @@ -29,7 +29,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { const end = '2023-06-21T06:59:15.910Z'; const startTime = new Date(start).getTime() + 1000; const OVERFLOW_SERVICE_NAME = '_other'; - let serviceGroupId; + let serviceGroupId: string; after(async () => { await deleteAllServiceGroups(apmApiClient); @@ -55,7 +55,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { }, [] as Array< | { - index: { + create: { _index: string; }; } @@ -97,7 +97,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { const overflowBucket = response.body.items.find( (service) => service.serviceName === OVERFLOW_SERVICE_NAME ); - expect(overflowBucket.serviceName).to.equal(OVERFLOW_SERVICE_NAME); + expect(overflowBucket?.serviceName).to.equal(OVERFLOW_SERVICE_NAME); }); } );