From be46156293b0fddcfedf662cd71318f51ba774cf Mon Sep 17 00:00:00 2001 From: Oliver Gupte Date: Mon, 24 Oct 2022 12:51:43 -0400 Subject: [PATCH 01/14] [APM] Support specific fields when creating service groups (#142201) --- x-pack/plugins/apm/common/service_groups.ts | 56 ++++++++++++ .../utils}/get_kuery_fields.test.ts | 0 .../utils}/get_kuery_fields.ts | 0 .../service_group_save/select_services.tsx | 37 +++++--- .../collect_data_telemetry/tasks.ts | 2 +- .../server/routes/alerts/get_source_fields.ts | 55 +++++++++++ .../register_error_count_rule_type.ts | 13 ++- ...register_transaction_duration_rule_type.ts | 91 +++++++++++-------- ...gister_transaction_error_rate_rule_type.ts | 21 +++-- .../apm/server/routes/service_groups/route.ts | 10 +- 10 files changed, 227 insertions(+), 58 deletions(-) rename x-pack/plugins/apm/{server/lib/helpers => common/utils}/get_kuery_fields.test.ts (100%) rename x-pack/plugins/apm/{server/lib/helpers => common/utils}/get_kuery_fields.ts (100%) create mode 100644 x-pack/plugins/apm/server/routes/alerts/get_source_fields.ts diff --git a/x-pack/plugins/apm/common/service_groups.ts b/x-pack/plugins/apm/common/service_groups.ts index e3a82e7e56b6c7..f922126c32e34e 100644 --- a/x-pack/plugins/apm/common/service_groups.ts +++ b/x-pack/plugins/apm/common/service_groups.ts @@ -5,6 +5,18 @@ * 2.0. */ +import { fromKueryExpression } from '@kbn/es-query'; +import { getKueryFields } from './utils/get_kuery_fields'; +import { + AGENT_NAME, + SERVICE_NAME, + SERVICE_ENVIRONMENT, + SERVICE_LANGUAGE_NAME, +} from './elasticsearch_fieldnames'; +import { i18n } from '@kbn/i18n'; + +const LABELS = 'labels'; // implies labels.* wildcard + export const APM_SERVICE_GROUP_SAVED_OBJECT_TYPE = 'apm-service-group'; export const SERVICE_GROUP_COLOR_DEFAULT = '#D1DAE7'; export const MAX_NUMBER_OF_SERVICE_GROUPS = 500; @@ -20,3 +32,47 @@ export interface SavedServiceGroup extends ServiceGroup { id: string; updatedAt: number; } + +export const SERVICE_GROUP_SUPPORTED_FIELDS = [ + AGENT_NAME, + SERVICE_NAME, + SERVICE_ENVIRONMENT, + SERVICE_LANGUAGE_NAME, + LABELS, +]; + +export function isSupportedField(fieldName: string) { + return ( + fieldName.startsWith(LABELS) || + SERVICE_GROUP_SUPPORTED_FIELDS.includes(fieldName) + ); +} + +export function validateServiceGroupKuery(kuery: string) { + try { + const kueryFields = getKueryFields([fromKueryExpression(kuery)]); + const unsupportedKueryFields = kueryFields.filter( + (fieldName) => !isSupportedField(fieldName) + ); + if (unsupportedKueryFields.length > 0) { + return { + isValid: false, + isParsingError: false, + message: i18n.translate( + 'xpack.apm.serviceGroups.selectServicesForm.title', + { + defaultMessage: + 'Query filter for service group does not support fields [{unsupportedFieldNames}]', + values: { + unsupportedFieldNames: unsupportedKueryFields.join(', '), + }, + } + ), + }; + } else { + return { isValid: true, isParsingError: false, message: '' }; + } + } catch (error) { + return { isValid: false, isParsingError: true, message: error.message }; + } +} diff --git a/x-pack/plugins/apm/server/lib/helpers/get_kuery_fields.test.ts b/x-pack/plugins/apm/common/utils/get_kuery_fields.test.ts similarity index 100% rename from x-pack/plugins/apm/server/lib/helpers/get_kuery_fields.test.ts rename to x-pack/plugins/apm/common/utils/get_kuery_fields.test.ts diff --git a/x-pack/plugins/apm/server/lib/helpers/get_kuery_fields.ts b/x-pack/plugins/apm/common/utils/get_kuery_fields.ts similarity index 100% rename from x-pack/plugins/apm/server/lib/helpers/get_kuery_fields.ts rename to x-pack/plugins/apm/common/utils/get_kuery_fields.ts diff --git a/x-pack/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx b/x-pack/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx index 7a97583bbd4ae2..9143cd4a9a4ac3 100644 --- a/x-pack/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx +++ b/x-pack/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx @@ -27,6 +27,10 @@ import { KueryBar } from '../../../shared/kuery_bar'; import { ServiceListPreview } from './service_list_preview'; import type { StagedServiceGroup } from './save_modal'; import { getDateRange } from '../../../../context/url_params_context/helpers'; +import { + validateServiceGroupKuery, + isSupportedField, +} from '../../../../../common/service_groups'; const CentralizedContainer = styled.div` display: flex; @@ -39,13 +43,6 @@ const MAX_CONTAINER_HEIGHT = 600; const MODAL_HEADER_HEIGHT = 180; const MODAL_FOOTER_HEIGHT = 80; -const suggestedFieldsWhitelist = [ - 'agent.name', - 'service.name', - 'service.language.name', - 'service.environment', -]; - const Container = styled.div` width: 600px; height: ${MAX_CONTAINER_HEIGHT}px; @@ -70,6 +67,7 @@ export function SelectServices({ }: Props) { const [kuery, setKuery] = useState(serviceGroup?.kuery || ''); const [stagedKuery, setStagedKuery] = useState(serviceGroup?.kuery || ''); + const [kueryValidationMessage, setKueryValidationMessage] = useState(''); useEffect(() => { if (isEdit) { @@ -78,6 +76,21 @@ export function SelectServices({ } }, [isEdit, serviceGroup.kuery]); + useEffect(() => { + if (!stagedKuery) { + return; + } + const { isValid, isParsingError, message } = + validateServiceGroupKuery(stagedKuery); + if (isValid || isParsingError) { + if (kueryValidationMessage !== '') { + setKueryValidationMessage(''); + } + } else { + setKueryValidationMessage(message); + } + }, [stagedKuery]); + const { start, end } = useMemo( () => getDateRange({ @@ -122,6 +135,11 @@ export function SelectServices({ } )} + {kueryValidationMessage && ( + + {kueryValidationMessage} + + )} ; + }; + }; +} + +export function getSourceFields(bucket?: AggResultBucket) { + if (!bucket) { + return {}; + } + const sourceDoc: SourceDoc = + bucket?.source_fields?.hits.hits[0]?._source ?? {}; + return flattenSourceDoc(sourceDoc); +} + +export function flattenSourceDoc( + val: SourceDoc | string, + path: string[] = [] +): Record { + if (typeof val !== 'object') { + return { [path.join('.')]: val }; + } + return Object.keys(val).reduce((acc, key) => { + const fieldMap = flattenSourceDoc(val[key], [...path, key]); + return { ...acc, ...fieldMap }; + }, {}); +} diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts index 58e475ced07fb3..34cdb0d2173722 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts @@ -37,6 +37,7 @@ import { getApmIndices } from '../../../settings/apm_indices/get_apm_indices'; import { apmActionVariables } from '../../action_variables'; import { alertingEsClient } from '../../alerting_es_client'; import { RegisterRuleDependencies } from '../../register_apm_rule_types'; +import { getSourceFieldsAgg, getSourceFields } from '../../get_source_fields'; const paramsSchema = schema.object({ windowSize: schema.number(), @@ -124,6 +125,7 @@ export function registerErrorCountRuleType({ ], size: 10000, }, + aggs: getSourceFieldsAgg(), }, }, }, @@ -137,13 +139,19 @@ export function registerErrorCountRuleType({ const errorCountResults = response.aggregations?.error_counts.buckets.map((bucket) => { const [serviceName, environment] = bucket.key; - return { serviceName, environment, errorCount: bucket.doc_count }; + return { + serviceName, + environment, + errorCount: bucket.doc_count, + sourceFields: getSourceFields(bucket), + }; }) ?? []; errorCountResults .filter((result) => result.errorCount >= ruleParams.threshold) .forEach((result) => { - const { serviceName, environment, errorCount } = result; + const { serviceName, environment, errorCount, sourceFields } = + result; const alertReason = formatErrorCountReason({ serviceName, threshold: ruleParams.threshold, @@ -176,6 +184,7 @@ export function registerErrorCountRuleType({ [ALERT_EVALUATION_VALUE]: errorCount, [ALERT_EVALUATION_THRESHOLD]: ruleParams.threshold, [ALERT_REASON]: alertReason, + ...sourceFields, }, }) .scheduleActions(ruleTypeConfig.defaultActionGroupId, { diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts index 0ea099c8d4bc22..38bfa9018e31c3 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts @@ -14,6 +14,7 @@ import { } from '@kbn/rule-data-utils'; import { firstValueFrom } from 'rxjs'; import { asDuration } from '@kbn/observability-plugin/common/utils/formatters'; +import { termQuery } from '@kbn/observability-plugin/server'; import { createLifecycleRuleTypeFactory } from '@kbn/rule-registry-plugin/server'; import { ProcessorEvent } from '@kbn/observability-plugin/common'; import { getAlertUrlTransaction } from '../../../../../common/utils/formatters'; @@ -47,6 +48,7 @@ import { apmActionVariables } from '../../action_variables'; import { alertingEsClient } from '../../alerting_es_client'; import { RegisterRuleDependencies } from '../../register_apm_rule_types'; import { averageOrPercentileAgg } from '../../average_or_percentile_agg'; +import { getSourceFields, getSourceFieldsAgg } from '../../get_source_fields'; const paramsSchema = schema.object({ serviceName: schema.string(), @@ -140,26 +142,32 @@ export function registerTransactionDurationRuleType({ ...getDocumentTypeFilterForTransactions( searchAggregatedTransactions ), - { term: { [SERVICE_NAME]: ruleParams.serviceName } }, - { - term: { - [TRANSACTION_TYPE]: ruleParams.transactionType, - }, - }, + ...termQuery(SERVICE_NAME, ruleParams.serviceName), + ...termQuery(TRANSACTION_TYPE, ruleParams.transactionType), ...environmentQuery(ruleParams.environment), ] as QueryDslQueryContainer[], }, }, aggs: { - environments: { - terms: { - field: SERVICE_ENVIRONMENT, - missing: ENVIRONMENT_NOT_DEFINED.value, + series: { + multi_terms: { + terms: [ + { field: SERVICE_NAME }, + { + field: SERVICE_ENVIRONMENT, + missing: ENVIRONMENT_NOT_DEFINED.value, + }, + { field: TRANSACTION_TYPE }, + ], + size: 10000, + }, + aggs: { + ...averageOrPercentileAgg({ + aggregationType: ruleParams.aggregationType, + transactionDurationField: field, + }), + ...getSourceFieldsAgg(), }, - aggs: averageOrPercentileAgg({ - aggregationType: ruleParams.aggregationType, - transactionDurationField: field, - }), }, }, }, @@ -177,32 +185,40 @@ export function registerTransactionDurationRuleType({ // Converts threshold to microseconds because this is the unit used on transactionDuration const thresholdMicroseconds = ruleParams.threshold * 1000; - const triggeredEnvironmentDurations = - response.aggregations.environments.buckets - .map((bucket) => { - const { key: environment } = bucket; - const transactionDuration = - 'avgLatency' in bucket // only true if ruleParams.aggregationType === 'avg' - ? bucket.avgLatency.value - : bucket.pctLatency.values[0].value; - return { transactionDuration, environment }; - }) - .filter( - ({ transactionDuration }) => - transactionDuration !== null && - transactionDuration > thresholdMicroseconds - ) as Array<{ transactionDuration: number; environment: string }>; + const triggeredBuckets = []; + for (let bucket of response.aggregations.series.buckets) { + const [serviceName, environment, transactionType] = bucket.key; + const transactionDuration = + 'avgLatency' in bucket // only true if ruleParams.aggregationType === 'avg' + ? bucket.avgLatency.value + : bucket.pctLatency.values[0].value; + if ( + transactionDuration !== null && + transactionDuration > thresholdMicroseconds + ) { + triggeredBuckets.push({ + serviceName, + environment, + transactionType, + transactionDuration, + sourceFields: getSourceFields(bucket), + }); + } + } for (const { + serviceName, environment, + transactionType, transactionDuration, - } of triggeredEnvironmentDurations) { + sourceFields, + } of triggeredBuckets) { const durationFormatter = getDurationFormatter(transactionDuration); const transactionDurationFormatted = durationFormatter(transactionDuration).formatted; const reasonMessage = formatTransactionDurationReason({ measured: transactionDuration, - serviceName: ruleParams.serviceName, + serviceName, threshold: thresholdMicroseconds, asDuration, aggregationType: String(ruleParams.aggregationType), @@ -211,9 +227,9 @@ export function registerTransactionDurationRuleType({ }); const relativeViewInAppUrl = getAlertUrlTransaction( - ruleParams.serviceName, + serviceName, getEnvironmentEsField(environment)?.[SERVICE_ENVIRONMENT], - ruleParams.transactionType + transactionType ); const viewInAppUrl = basePath.publicBaseUrl @@ -228,18 +244,19 @@ export function registerTransactionDurationRuleType({ environment )}`, fields: { - [SERVICE_NAME]: ruleParams.serviceName, + [SERVICE_NAME]: serviceName, ...getEnvironmentEsField(environment), - [TRANSACTION_TYPE]: ruleParams.transactionType, + [TRANSACTION_TYPE]: transactionType, [PROCESSOR_EVENT]: ProcessorEvent.transaction, [ALERT_EVALUATION_VALUE]: transactionDuration, [ALERT_EVALUATION_THRESHOLD]: thresholdMicroseconds, [ALERT_REASON]: reasonMessage, + ...sourceFields, }, }) .scheduleActions(ruleTypeConfig.defaultActionGroupId, { - transactionType: ruleParams.transactionType, - serviceName: ruleParams.serviceName, + transactionType, + serviceName, environment: getEnvironmentLabel(environment), threshold: thresholdMicroseconds, triggerValue: transactionDurationFormatted, diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts index 15a5880345ffd1..fbc7f7aa31bc7a 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts @@ -44,6 +44,7 @@ import { alertingEsClient } from '../../alerting_es_client'; import { RegisterRuleDependencies } from '../../register_apm_rule_types'; import { SearchAggregatedTransactionSetting } from '../../../../../common/aggregated_transactions'; import { getDocumentTypeFilterForTransactions } from '../../../../lib/helpers/transactions'; +import { getSourceFields, getSourceFieldsAgg } from '../../get_source_fields'; const paramsSchema = schema.object({ windowSize: schema.number(), @@ -160,6 +161,7 @@ export function registerTransactionErrorRateRuleType({ terms: { field: EVENT_OUTCOME, }, + aggs: getSourceFieldsAgg(), }, }, }, @@ -180,10 +182,10 @@ export function registerTransactionErrorRateRuleType({ for (const bucket of response.aggregations.series.buckets) { const [serviceName, environment, transactionType] = bucket.key; - const failed = - bucket.outcomes.buckets.find( - (outcomeBucket) => outcomeBucket.key === EventOutcome.failure - )?.doc_count ?? 0; + const failedOutcomeBucket = bucket.outcomes.buckets.find( + (outcomeBucket) => outcomeBucket.key === EventOutcome.failure + ); + const failed = failedOutcomeBucket?.doc_count ?? 0; const succesful = bucket.outcomes.buckets.find( (outcomeBucket) => outcomeBucket.key === EventOutcome.success @@ -196,13 +198,19 @@ export function registerTransactionErrorRateRuleType({ environment, transactionType, errorRate, + sourceFields: getSourceFields(failedOutcomeBucket), }); } } results.forEach((result) => { - const { serviceName, environment, transactionType, errorRate } = - result; + const { + serviceName, + environment, + transactionType, + errorRate, + sourceFields, + } = result; const reasonMessage = formatTransactionErrorRateReason({ threshold: ruleParams.threshold, measured: errorRate, @@ -241,6 +249,7 @@ export function registerTransactionErrorRateRuleType({ [ALERT_EVALUATION_VALUE]: errorRate, [ALERT_EVALUATION_THRESHOLD]: ruleParams.threshold, [ALERT_REASON]: reasonMessage, + ...sourceFields, }, }) .scheduleActions(ruleTypeConfig.defaultActionGroupId, { diff --git a/x-pack/plugins/apm/server/routes/service_groups/route.ts b/x-pack/plugins/apm/server/routes/service_groups/route.ts index 4da84e68486968..6207aa2a6aa6bb 100644 --- a/x-pack/plugins/apm/server/routes/service_groups/route.ts +++ b/x-pack/plugins/apm/server/routes/service_groups/route.ts @@ -6,6 +6,7 @@ */ import * as t from 'io-ts'; +import Boom from '@hapi/boom'; import { apmServiceGroupMaxNumberOfServices } from '@kbn/observability-plugin/common'; import { createApmServerRoute } from '../apm_routes/create_apm_server_route'; import { kueryRt, rangeRt } from '../default_api_types'; @@ -14,7 +15,10 @@ import { getServiceGroup } from './get_service_group'; import { saveServiceGroup } from './save_service_group'; import { deleteServiceGroup } from './delete_service_group'; import { lookupServices } from './lookup_services'; -import { SavedServiceGroup } from '../../../common/service_groups'; +import { + validateServiceGroupKuery, + SavedServiceGroup, +} from '../../../common/service_groups'; import { getServicesCounts } from './get_services_counts'; import { getApmEventClient } from '../../lib/helpers/get_apm_event_client'; @@ -120,6 +124,10 @@ const serviceGroupSaveRoute = createApmServerRoute({ const { savedObjects: { client: savedObjectsClient }, } = await context.core; + const { isValid, message } = validateServiceGroupKuery(params.body.kuery); + if (!isValid) { + throw Boom.badRequest(message); + } await saveServiceGroup({ savedObjectsClient, From d5f5a19c293a202d6f239bd1e381f56c9154e4d0 Mon Sep 17 00:00:00 2001 From: Oliver Gupte Date: Tue, 25 Oct 2022 17:29:56 -0400 Subject: [PATCH 02/14] add support to anomaly rule type to store supported service group fields in alert --- .../server/routes/alerts/get_source_fields.ts | 6 +- .../get_anomalous_event_source_fields.ts | 72 +++++++++++++++++++ .../anomaly/register_anomaly_rule_type.ts | 51 +++++++++++-- 3 files changed, 123 insertions(+), 6 deletions(-) create mode 100644 x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts diff --git a/x-pack/plugins/apm/server/routes/alerts/get_source_fields.ts b/x-pack/plugins/apm/server/routes/alerts/get_source_fields.ts index 735c83d162a7c1..cbc70f6ea472d5 100644 --- a/x-pack/plugins/apm/server/routes/alerts/get_source_fields.ts +++ b/x-pack/plugins/apm/server/routes/alerts/get_source_fields.ts @@ -5,13 +5,16 @@ * 2.0. */ +import { AggregationsTopHitsAggregation } from '@elastic/elasticsearch/lib/api/types'; import { SERVICE_GROUP_SUPPORTED_FIELDS } from '../../../common/service_groups'; export interface SourceDoc { [key: string]: string | SourceDoc; } -export function getSourceFieldsAgg() { +export function getSourceFieldsAgg( + topHitsOpts: AggregationsTopHitsAggregation = {} +) { return { source_fields: { top_hits: { @@ -19,6 +22,7 @@ export function getSourceFieldsAgg() { _source: { includes: SERVICE_GROUP_SUPPORTED_FIELDS, }, + ...topHitsOpts, }, }, }; diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts new file mode 100644 index 00000000000000..4a81be7381fb43 --- /dev/null +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts @@ -0,0 +1,72 @@ +/* + * 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 { + SERVICE_ENVIRONMENT, + SERVICE_NAME, + TRANSACTION_TYPE, + TRANSACTION_DURATION, +} from '../../../../../common/elasticsearch_fieldnames'; +import { IScopedClusterClient } from '@kbn/core/server'; +import { alertingEsClient } from '../../alerting_es_client'; +import { getSourceFields, getSourceFieldsAgg } from '../../get_source_fields'; + +export async function getAnomalousEventSourceFields({ + scopedClusterClient, + index, + serviceName, + environment, + transactionType, + timestamp, + bucketSpan, +}: { + scopedClusterClient: IScopedClusterClient; + index: string; + serviceName: string; + environment: string; + transactionType: string; + timestamp: number; + bucketSpan: number; +}) { + const params = { + index, + body: { + size: 0, + query: { + bool: { + filter: [ + { term: { [SERVICE_NAME]: serviceName } }, + { term: { [TRANSACTION_TYPE]: transactionType } }, + { term: { [SERVICE_ENVIRONMENT]: environment } }, + { + range: { + '@timestamp': { + gte: timestamp, + lte: timestamp + bucketSpan * 1000, + format: 'epoch_millis', + }, + }, + }, + ], + }, + }, + aggs: { + ...getSourceFieldsAgg({ + sort: [{ [TRANSACTION_DURATION]: { order: 'desc' as const } }], + }), + }, + }, + }; + + const response = await alertingEsClient({ + scopedClusterClient, + params, + }); + if (!response.aggregations) { + return {}; + } + return getSourceFields(response.aggregations); +} diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts index d82a4997ffe0e6..64a16293c97690 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts @@ -46,6 +46,9 @@ import { getAlertUrlTransaction } from '../../../../../common/utils/formatters'; import { getMLJobs } from '../../../service_map/get_service_anomalies'; import { apmActionVariables } from '../../action_variables'; import { RegisterRuleDependencies } from '../../register_apm_rule_types'; +import { getAnomalousEventSourceFields } from './get_anomalous_event_source_fields'; +import { firstValueFrom } from 'rxjs'; +import { getApmIndices } from '../../../settings/apm_indices/get_apm_indices'; const paramsSchema = schema.object({ serviceName: schema.maybe(schema.string()), @@ -66,6 +69,7 @@ const ruleTypeConfig = RULE_TYPES_CONFIG[ApmRuleType.Anomaly]; export function registerAnomalyRuleType({ logger, ruleDataClient, + config$, alerting, ml, basePath, @@ -102,6 +106,14 @@ export function registerAnomalyRuleType({ if (!ml) { return {}; } + + const config = await firstValueFrom(config$); + const indices = await getApmIndices({ + config, + savedObjectsClient: services.savedObjectsClient, + }); + const { transaction: transactionsIndex } = indices; + const ruleParams = params; const request = {} as KibanaRequest; const { mlAnomalySearch } = ml.mlSystemProvider( @@ -161,8 +173,14 @@ export function registerAnomalyRuleType({ }, }, }, - ...termQuery('partition_field_value', ruleParams.serviceName), - ...termQuery('by_field_value', ruleParams.transactionType), + ...termQuery( + 'partition_field_value', + ruleParams.serviceName || null + ), + ...termQuery( + 'by_field_value', + ruleParams.transactionType || null + ), ...termQuery( 'detector_index', getApmMlDetectorIndex(ApmMlDetectorType.txLatency) @@ -188,6 +206,8 @@ export function registerAnomalyRuleType({ { field: 'partition_field_value' }, { field: 'by_field_value' }, { field: 'job_id' }, + { field: 'timestamp' }, + { field: 'bucket_span' }, ] as const), sort: { timestamp: 'desc' as const, @@ -222,14 +242,34 @@ export function registerAnomalyRuleType({ transactionType: latest.by_field_value as string, environment: job.environment, score: latest.record_score as number, + timestamp: Date.parse(latest.timestamp as string), + bucketSpan: latest.bucket_span as number, }; }) .filter((anomaly) => anomaly ? anomaly.score >= threshold : false ) ?? []; - compact(anomalies).forEach((anomaly) => { - const { serviceName, environment, transactionType, score } = anomaly; + for (const anomaly of compact(anomalies)) { + const { + serviceName, + environment, + transactionType, + score, + timestamp, + bucketSpan, + } = anomaly; + + const eventSourceFields = await getAnomalousEventSourceFields({ + scopedClusterClient: services.scopedClusterClient, + index: transactionsIndex, + serviceName, + environment, + transactionType, + timestamp, + bucketSpan, + }); + const severityLevel = getSeverity(score); const reasonMessage = formatAnomalyReason({ measured: score, @@ -270,6 +310,7 @@ export function registerAnomalyRuleType({ [ALERT_EVALUATION_VALUE]: score, [ALERT_EVALUATION_THRESHOLD]: threshold, [ALERT_REASON]: reasonMessage, + ...eventSourceFields, }, }) .scheduleActions(ruleTypeConfig.defaultActionGroupId, { @@ -281,7 +322,7 @@ export function registerAnomalyRuleType({ reason: reasonMessage, viewInAppUrl, }); - }); + } return {}; }, From 401d3da612b9a9d90755a0fc85b1a2632b4696f8 Mon Sep 17 00:00:00 2001 From: Oliver Gupte Date: Wed, 26 Oct 2022 00:19:51 -0400 Subject: [PATCH 03/14] address PR feedback and fixes checks --- x-pack/plugins/apm/common/service_groups.ts | 2 +- .../app/service_groups/service_group_save/select_services.tsx | 4 +--- .../rule_types/anomaly/get_anomalous_event_source_fields.ts | 1 + .../rule_types/error_count/register_error_count_rule_type.ts | 4 +++- .../register_transaction_duration_rule_type.test.ts | 4 ++-- .../register_transaction_duration_rule_type.ts | 4 +++- .../register_transaction_error_rate_rule_type.ts | 4 +++- 7 files changed, 14 insertions(+), 9 deletions(-) diff --git a/x-pack/plugins/apm/common/service_groups.ts b/x-pack/plugins/apm/common/service_groups.ts index f922126c32e34e..9a7869fddf8740 100644 --- a/x-pack/plugins/apm/common/service_groups.ts +++ b/x-pack/plugins/apm/common/service_groups.ts @@ -59,7 +59,7 @@ export function validateServiceGroupKuery(kuery: string) { isValid: false, isParsingError: false, message: i18n.translate( - 'xpack.apm.serviceGroups.selectServicesForm.title', + 'xpack.apm.serviceGroups.invalidFields.message', { defaultMessage: 'Query filter for service group does not support fields [{unsupportedFieldNames}]', diff --git a/x-pack/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx b/x-pack/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx index 9143cd4a9a4ac3..27a8a7e38de5e2 100644 --- a/x-pack/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx +++ b/x-pack/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx @@ -83,9 +83,7 @@ export function SelectServices({ const { isValid, isParsingError, message } = validateServiceGroupKuery(stagedKuery); if (isValid || isParsingError) { - if (kueryValidationMessage !== '') { - setKueryValidationMessage(''); - } + setKueryValidationMessage(''); } else { setKueryValidationMessage(message); } diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts index 4a81be7381fb43..b972e9d87a4781 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts @@ -35,6 +35,7 @@ export async function getAnomalousEventSourceFields({ index, body: { size: 0, + track_total_hits: false, query: { bool: { filter: [ diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts index 34cdb0d2173722..56aface32d7fb4 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts @@ -31,6 +31,7 @@ import { PROCESSOR_EVENT, SERVICE_ENVIRONMENT, SERVICE_NAME, + TRANSACTION_DURATION, } from '../../../../../common/elasticsearch_fieldnames'; import { environmentQuery } from '../../../../../common/utils/environment_query'; import { getApmIndices } from '../../../settings/apm_indices/get_apm_indices'; @@ -123,7 +124,8 @@ export function registerErrorCountRuleType({ missing: ENVIRONMENT_NOT_DEFINED.value, }, ], - size: 10000, + size: 1000, + order: { [TRANSACTION_DURATION]: 'desc' as const }, }, aggs: getSourceFieldsAgg(), }, diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.test.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.test.ts index 4d8b91636fb6c7..2b159e7acc0d28 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.test.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.test.ts @@ -24,10 +24,10 @@ describe('registerTransactionDurationRuleType', () => { }, }, aggregations: { - environments: { + series: { buckets: [ { - key: 'ENVIRONMENT_NOT_DEFINED', + key: ['opbeans-java', 'ENVIRONMENT_NOT_DEFINED', 'request'], avgLatency: { value: 5500000, }, diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts index 38bfa9018e31c3..467e842b55c202 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts @@ -31,6 +31,7 @@ import { SERVICE_NAME, TRANSACTION_TYPE, SERVICE_ENVIRONMENT, + TRANSACTION_DURATION, } from '../../../../../common/elasticsearch_fieldnames'; import { ENVIRONMENT_NOT_DEFINED, @@ -159,7 +160,8 @@ export function registerTransactionDurationRuleType({ }, { field: TRANSACTION_TYPE }, ], - size: 10000, + size: 1000, + order: { [TRANSACTION_DURATION]: 'desc' as const }, }, aggs: { ...averageOrPercentileAgg({ diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts index fbc7f7aa31bc7a..5514d160f49b76 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts @@ -34,6 +34,7 @@ import { SERVICE_ENVIRONMENT, SERVICE_NAME, TRANSACTION_TYPE, + TRANSACTION_DURATION, } from '../../../../../common/elasticsearch_fieldnames'; import { EventOutcome } from '../../../../../common/event_outcome'; import { asDecimalOrInteger } from '../../../../../common/utils/formatters'; @@ -154,7 +155,8 @@ export function registerTransactionErrorRateRuleType({ }, { field: TRANSACTION_TYPE }, ], - size: 10000, + size: 1000, + order: { [TRANSACTION_DURATION]: 'desc' as const }, }, aggs: { outcomes: { From 516f987d09f0d66ee79eacf6ca2a754a8c6b58f2 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Wed, 26 Oct 2022 04:27:24 +0000 Subject: [PATCH 04/14] [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' --- x-pack/plugins/apm/common/service_groups.ts | 2 +- .../rule_types/anomaly/get_anomalous_event_source_fields.ts | 2 +- .../register_transaction_duration_rule_type.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/apm/common/service_groups.ts b/x-pack/plugins/apm/common/service_groups.ts index 9a7869fddf8740..6a48f7167273d2 100644 --- a/x-pack/plugins/apm/common/service_groups.ts +++ b/x-pack/plugins/apm/common/service_groups.ts @@ -6,6 +6,7 @@ */ import { fromKueryExpression } from '@kbn/es-query'; +import { i18n } from '@kbn/i18n'; import { getKueryFields } from './utils/get_kuery_fields'; import { AGENT_NAME, @@ -13,7 +14,6 @@ import { SERVICE_ENVIRONMENT, SERVICE_LANGUAGE_NAME, } from './elasticsearch_fieldnames'; -import { i18n } from '@kbn/i18n'; const LABELS = 'labels'; // implies labels.* wildcard diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts index b972e9d87a4781..95126bac392ad8 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts @@ -4,13 +4,13 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ +import { IScopedClusterClient } from '@kbn/core/server'; import { SERVICE_ENVIRONMENT, SERVICE_NAME, TRANSACTION_TYPE, TRANSACTION_DURATION, } from '../../../../../common/elasticsearch_fieldnames'; -import { IScopedClusterClient } from '@kbn/core/server'; import { alertingEsClient } from '../../alerting_es_client'; import { getSourceFields, getSourceFieldsAgg } from '../../get_source_fields'; diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts index 467e842b55c202..c54b9bafa222a1 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts @@ -188,7 +188,7 @@ export function registerTransactionDurationRuleType({ const thresholdMicroseconds = ruleParams.threshold * 1000; const triggeredBuckets = []; - for (let bucket of response.aggregations.series.buckets) { + for (const bucket of response.aggregations.series.buckets) { const [serviceName, environment, transactionType] = bucket.key; const transactionDuration = 'avgLatency' in bucket // only true if ruleParams.aggregationType === 'avg' From 0abc855215de800aff72344250fe3c403be3192e Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Wed, 26 Oct 2022 05:02:24 +0000 Subject: [PATCH 05/14] [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' --- .../alerts/rule_types/anomaly/register_anomaly_rule_type.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts index 64a16293c97690..70993dc682a284 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts @@ -19,6 +19,7 @@ import { KibanaRequest } from '@kbn/core/server'; import { termQuery } from '@kbn/observability-plugin/server'; import { createLifecycleRuleTypeFactory } from '@kbn/rule-registry-plugin/server'; import { ProcessorEvent } from '@kbn/observability-plugin/common'; +import { firstValueFrom } from 'rxjs'; import { ApmRuleType, RULE_TYPES_CONFIG, @@ -47,7 +48,6 @@ import { getMLJobs } from '../../../service_map/get_service_anomalies'; import { apmActionVariables } from '../../action_variables'; import { RegisterRuleDependencies } from '../../register_apm_rule_types'; import { getAnomalousEventSourceFields } from './get_anomalous_event_source_fields'; -import { firstValueFrom } from 'rxjs'; import { getApmIndices } from '../../../settings/apm_indices/get_apm_indices'; const paramsSchema = schema.object({ From e4c474338a81639ad930b7a452339d82e40eb381 Mon Sep 17 00:00:00 2001 From: Oliver Gupte Date: Wed, 26 Oct 2022 09:46:49 -0400 Subject: [PATCH 06/14] add API tests for field validation --- .../service_groups/save_service_group.spec.ts | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 x-pack/test/apm_api_integration/tests/service_groups/save_service_group.spec.ts diff --git a/x-pack/test/apm_api_integration/tests/service_groups/save_service_group.spec.ts b/x-pack/test/apm_api_integration/tests/service_groups/save_service_group.spec.ts new file mode 100644 index 00000000000000..53f3732941c7bf --- /dev/null +++ b/x-pack/test/apm_api_integration/tests/service_groups/save_service_group.spec.ts @@ -0,0 +1,88 @@ +/* + * 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 { ApmApiError } from '../../common/apm_api_supertest'; +import { FtrProviderContext } from '../../common/ftr_provider_context'; +import { expectToReject } from '../../common/utils/expect_to_reject'; + +export default function ApiTest({ getService }: FtrProviderContext) { + const registry = getService('registry'); + const apmApiClient = getService('apmApiClient'); + const supertest = getService('supertest'); + + async function callApi({ + serviceGroupId, + groupName, + kuery, + description, + color, + }: { + serviceGroupId?: string; + groupName: string; + kuery: string; + description?: string; + color?: string; + }) { + const response = await apmApiClient.writeUser({ + endpoint: 'POST /internal/apm/service-group', + params: { + query: { + serviceGroupId, + }, + body: { + groupName: groupName, + kuery: kuery, + description, + color, + }, + }, + }); + return response; + } + + type SavedObjectsFindResults = Array<{ + id: string; + type: string; + }>; + + async function deleteServiceGroups() { + const response = await supertest + .get('/api/saved_objects/_find?type=apm-service-group') + .set('kbn-xsrf', 'true'); + const saved_objects: SavedObjectsFindResults = response.body.saved_objects; + const bulkDeleteBody = saved_objects.map(({ id, type }) => ({ id, type })); + return supertest + .post(`/api/saved_objects/_bulk_delete?force=true`) + .set('kbn-xsrf', 'foo') + .send(bulkDeleteBody); + } + + registry.when('Service group create', { config: 'basic', archives: [] }, () => { + afterEach(deleteServiceGroups); + + it('creates a new service group', async () => { + const response = await callApi({ + groupName: 'synthbeans', + kuery: 'service.name: synth*', + }); + expect(response.status).to.be(200); + expect(Object.keys(response.body).length).to.be(0); + }); + + it('handles invalid fields with error response', async () => { + const err = await expectToReject(() => + callApi({ + groupName: 'synthbeans', + kuery: 'service.name: synth* or transaction.type: request', + }) + ); + + expect(err.res.status).to.be(400); + expect(err.res.body.message).to.contain('transaction.type'); + }); + }); +} From 198589b1cb5e0b7e4a0ae64ae0d34e1fd1012357 Mon Sep 17 00:00:00 2001 From: Oliver Gupte Date: Wed, 26 Oct 2022 10:00:22 -0400 Subject: [PATCH 07/14] fixes linting --- .../tests/service_groups/save_service_group.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/test/apm_api_integration/tests/service_groups/save_service_group.spec.ts b/x-pack/test/apm_api_integration/tests/service_groups/save_service_group.spec.ts index 53f3732941c7bf..9d9697577c021c 100644 --- a/x-pack/test/apm_api_integration/tests/service_groups/save_service_group.spec.ts +++ b/x-pack/test/apm_api_integration/tests/service_groups/save_service_group.spec.ts @@ -53,8 +53,8 @@ export default function ApiTest({ getService }: FtrProviderContext) { const response = await supertest .get('/api/saved_objects/_find?type=apm-service-group') .set('kbn-xsrf', 'true'); - const saved_objects: SavedObjectsFindResults = response.body.saved_objects; - const bulkDeleteBody = saved_objects.map(({ id, type }) => ({ id, type })); + const savedObjects: SavedObjectsFindResults = response.body.saved_objects; + const bulkDeleteBody = savedObjects.map(({ id, type }) => ({ id, type })); return supertest .post(`/api/saved_objects/_bulk_delete?force=true`) .set('kbn-xsrf', 'foo') From 9ec7cf392b12d25783b32d165930abcebba811ef Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Wed, 26 Oct 2022 14:07:27 +0000 Subject: [PATCH 08/14] [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' --- .../tests/service_groups/save_service_group.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/test/apm_api_integration/tests/service_groups/save_service_group.spec.ts b/x-pack/test/apm_api_integration/tests/service_groups/save_service_group.spec.ts index 9d9697577c021c..533d6079c1a6dd 100644 --- a/x-pack/test/apm_api_integration/tests/service_groups/save_service_group.spec.ts +++ b/x-pack/test/apm_api_integration/tests/service_groups/save_service_group.spec.ts @@ -34,8 +34,8 @@ export default function ApiTest({ getService }: FtrProviderContext) { serviceGroupId, }, body: { - groupName: groupName, - kuery: kuery, + groupName, + kuery, description, color, }, From 112edf64ffd6db243d78d1486158c15e1177cfe2 Mon Sep 17 00:00:00 2001 From: Oliver Gupte Date: Wed, 26 Oct 2022 16:04:19 -0400 Subject: [PATCH 09/14] fixes multi_terms sort order paths, for each rule type query --- .../server/routes/alerts/average_or_percentile_agg.ts | 10 ++++++++++ .../rule_types/anomaly/register_anomaly_rule_type.ts | 3 ++- .../error_count/register_error_count_rule_type.ts | 3 +-- .../register_transaction_duration_rule_type.ts | 8 +++++--- .../register_transaction_error_rate_rule_type.ts | 3 +-- 5 files changed, 19 insertions(+), 8 deletions(-) diff --git a/x-pack/plugins/apm/server/routes/alerts/average_or_percentile_agg.ts b/x-pack/plugins/apm/server/routes/alerts/average_or_percentile_agg.ts index 0a7b9e29229bbd..d458b45b99d52e 100644 --- a/x-pack/plugins/apm/server/routes/alerts/average_or_percentile_agg.ts +++ b/x-pack/plugins/apm/server/routes/alerts/average_or_percentile_agg.ts @@ -45,3 +45,13 @@ export function averageOrPercentileAgg({ }, }; } + +export function getMultiTermsSortOrder(aggregationType: AggregationType): { + order: { [path: string]: 'desc' }; +} { + if (aggregationType === AggregationType.Avg) { + return { order: { avgLatency: 'desc' } }; + } + const percentsKey = aggregationType === AggregationType.P95 ? 95 : 99; + return { order: { [`pctLatency.${percentsKey}`]: 'desc' } }; +} diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts index 70993dc682a284..d93c0ff09d3ec3 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts @@ -196,7 +196,8 @@ export function registerAnomalyRuleType({ { field: 'by_field_value' }, { field: 'job_id' }, ], - size: 10000, + size: 1000, + order: { 'latest_score.record_score': 'desc' as const }, }, aggs: { latest_score: { diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts index 56aface32d7fb4..d1a4e5418b01fc 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts @@ -31,7 +31,6 @@ import { PROCESSOR_EVENT, SERVICE_ENVIRONMENT, SERVICE_NAME, - TRANSACTION_DURATION, } from '../../../../../common/elasticsearch_fieldnames'; import { environmentQuery } from '../../../../../common/utils/environment_query'; import { getApmIndices } from '../../../settings/apm_indices/get_apm_indices'; @@ -125,7 +124,7 @@ export function registerErrorCountRuleType({ }, ], size: 1000, - order: { [TRANSACTION_DURATION]: 'desc' as const }, + order: { _count: 'desc' as const }, }, aggs: getSourceFieldsAgg(), }, diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts index c54b9bafa222a1..1bee9c526df63d 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts @@ -31,7 +31,6 @@ import { SERVICE_NAME, TRANSACTION_TYPE, SERVICE_ENVIRONMENT, - TRANSACTION_DURATION, } from '../../../../../common/elasticsearch_fieldnames'; import { ENVIRONMENT_NOT_DEFINED, @@ -48,7 +47,10 @@ import { getApmIndices } from '../../../settings/apm_indices/get_apm_indices'; import { apmActionVariables } from '../../action_variables'; import { alertingEsClient } from '../../alerting_es_client'; import { RegisterRuleDependencies } from '../../register_apm_rule_types'; -import { averageOrPercentileAgg } from '../../average_or_percentile_agg'; +import { + averageOrPercentileAgg, + getMultiTermsSortOrder, +} from '../../average_or_percentile_agg'; import { getSourceFields, getSourceFieldsAgg } from '../../get_source_fields'; const paramsSchema = schema.object({ @@ -161,7 +163,7 @@ export function registerTransactionDurationRuleType({ { field: TRANSACTION_TYPE }, ], size: 1000, - order: { [TRANSACTION_DURATION]: 'desc' as const }, + ...getMultiTermsSortOrder(ruleParams.aggregationType), }, aggs: { ...averageOrPercentileAgg({ diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts index 5514d160f49b76..2291ac027b3f3c 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts @@ -34,7 +34,6 @@ import { SERVICE_ENVIRONMENT, SERVICE_NAME, TRANSACTION_TYPE, - TRANSACTION_DURATION, } from '../../../../../common/elasticsearch_fieldnames'; import { EventOutcome } from '../../../../../common/event_outcome'; import { asDecimalOrInteger } from '../../../../../common/utils/formatters'; @@ -156,7 +155,7 @@ export function registerTransactionErrorRateRuleType({ { field: TRANSACTION_TYPE }, ], size: 1000, - order: { [TRANSACTION_DURATION]: 'desc' as const }, + order: { _count: 'desc' as const }, }, aggs: { outcomes: { From 081b2cddd814375023e42ac21b9ca5e960eb74a0 Mon Sep 17 00:00:00 2001 From: Oliver Gupte Date: Wed, 26 Oct 2022 17:57:29 -0400 Subject: [PATCH 10/14] adds unit tests and moves some source files --- .../plugins/apm/common/service_groups.test.ts | 63 +++++++++ .../get_anomalous_event_source_fields.ts | 2 +- .../register_error_count_rule_type.ts | 2 +- .../rule_types/get_source_fields.test.ts | 121 ++++++++++++++++++ .../{ => rule_types}/get_source_fields.ts | 2 +- .../average_or_percentile_agg.ts | 0 .../get_transaction_duration_chart_preview.ts | 2 +- ...register_transaction_duration_rule_type.ts | 4 +- ...gister_transaction_error_rate_rule_type.ts | 2 +- 9 files changed, 191 insertions(+), 7 deletions(-) create mode 100644 x-pack/plugins/apm/common/service_groups.test.ts create mode 100644 x-pack/plugins/apm/server/routes/alerts/rule_types/get_source_fields.test.ts rename x-pack/plugins/apm/server/routes/alerts/{ => rule_types}/get_source_fields.ts (94%) rename x-pack/plugins/apm/server/routes/alerts/{ => rule_types/transaction_duration}/average_or_percentile_agg.ts (100%) diff --git a/x-pack/plugins/apm/common/service_groups.test.ts b/x-pack/plugins/apm/common/service_groups.test.ts new file mode 100644 index 00000000000000..e2e2c0d7b64d18 --- /dev/null +++ b/x-pack/plugins/apm/common/service_groups.test.ts @@ -0,0 +1,63 @@ +/* + * 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 { + isSupportedField, + validateServiceGroupKuery, + SERVICE_GROUP_SUPPORTED_FIELDS, +} from './service_groups'; +import { + TRANSACTION_TYPE, + TRANSACTION_DURATION, + SERVICE_FRAMEWORK_VERSION, +} from './elasticsearch_fieldnames'; + +const unsupportedFields = [ + TRANSACTION_TYPE, + TRANSACTION_DURATION, + SERVICE_FRAMEWORK_VERSION, +]; +const mockFields = [...SERVICE_GROUP_SUPPORTED_FIELDS, ...unsupportedFields]; + +describe('service_groups common utils', () => { + describe('isSupportedField', () => { + it('should only filter supported supported fields for service groups', () => { + const supportedFields = mockFields.filter(isSupportedField); + expect(supportedFields).toEqual(SERVICE_GROUP_SUPPORTED_FIELDS); + }); + }); + describe('validateServiceGroupKuery', () => { + it('should validate supported KQL filter for a service group', () => { + const result = validateServiceGroupKuery( + `service.name: testbeans* or agent.name: "nodejs"` + ); + expect(result).toHaveProperty('isValid', true); + expect(result).toHaveProperty('isParsingError', false); + expect(result).toHaveProperty('message', ''); + }); + it('should return validation error when unsupported fields are used', () => { + const result = validateServiceGroupKuery( + `service.name: testbeans* or agent.name: "nodejs" or transaction.type: request` + ); + expect(result).toHaveProperty('isValid', false); + expect(result).toHaveProperty('isParsingError', false); + expect(result).toHaveProperty( + 'message', + 'Query filter for service group does not support fields [transaction.type]' + ); + }); + it('should return parsing error when KQL is incomplete', () => { + const result = validateServiceGroupKuery( + `service.name: testbeans* or agent.name: "nod` + ); + expect(result).toHaveProperty('isValid', false); + expect(result).toHaveProperty('isParsingError', true); + expect(result).toHaveProperty('message'); + expect(result).not.toBe(''); + }); + }); +}); diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts index 95126bac392ad8..f4e46bca29c8c0 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts @@ -12,7 +12,7 @@ import { TRANSACTION_DURATION, } from '../../../../../common/elasticsearch_fieldnames'; import { alertingEsClient } from '../../alerting_es_client'; -import { getSourceFields, getSourceFieldsAgg } from '../../get_source_fields'; +import { getSourceFields, getSourceFieldsAgg } from '../get_source_fields'; export async function getAnomalousEventSourceFields({ scopedClusterClient, diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts index d1a4e5418b01fc..b3b9b620d501c2 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts @@ -37,7 +37,7 @@ import { getApmIndices } from '../../../settings/apm_indices/get_apm_indices'; import { apmActionVariables } from '../../action_variables'; import { alertingEsClient } from '../../alerting_es_client'; import { RegisterRuleDependencies } from '../../register_apm_rule_types'; -import { getSourceFieldsAgg, getSourceFields } from '../../get_source_fields'; +import { getSourceFieldsAgg, getSourceFields } from '../get_source_fields'; const paramsSchema = schema.object({ windowSize: schema.number(), diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/get_source_fields.test.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/get_source_fields.test.ts new file mode 100644 index 00000000000000..51bdb6580126c5 --- /dev/null +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/get_source_fields.test.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 + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { + getSourceFields, + getSourceFieldsAgg, + flattenSourceDoc, +} from './get_source_fields'; + +const mockSourceObj = { + service: { + name: 'testbeans', + environment: 'testing', + language: { + name: 'typescript', + }, + }, + labels: { + team: 'test', + }, + agent: { + name: 'nodejs', + }, +}; + +const mockBucket = { + source_fields: { + hits: { + hits: [{ _source: mockSourceObj }], + }, + }, +}; + +describe('getSourceFields', () => { + it('should return a flattened record of fields and values for a given bucket', () => { + const result = getSourceFields(mockBucket); + expect(result).toMatchInlineSnapshot(` + Object { + "agent.name": "nodejs", + "labels.team": "test", + "service.environment": "testing", + "service.language.name": "typescript", + "service.name": "testbeans", + } + `); + }); +}); + +describe('getSourceFieldsAgg', () => { + it('should create a agg for specific source fields', () => { + const agg = getSourceFieldsAgg(); + expect(agg).toMatchInlineSnapshot(` + Object { + "source_fields": Object { + "top_hits": Object { + "_source": Object { + "includes": Array [ + "agent.name", + "service.name", + "service.environment", + "service.language.name", + "labels", + ], + }, + "size": 1, + }, + }, + } + `); + }); + + it('should accept options for top_hits options', () => { + const agg = getSourceFieldsAgg({ + sort: [{ 'transaction.duration.us': { order: 'desc' } }], + }); + expect(agg).toMatchInlineSnapshot(` + Object { + "source_fields": Object { + "top_hits": Object { + "_source": Object { + "includes": Array [ + "agent.name", + "service.name", + "service.environment", + "service.language.name", + "labels", + ], + }, + "size": 1, + "sort": Array [ + Object { + "transaction.duration.us": Object { + "order": "desc", + }, + }, + ], + }, + }, + } + `); + }); +}); + +describe('flattenSourceDoc', () => { + it('should flatten a given nested object with dot delim paths as keys', () => { + const result = flattenSourceDoc(mockSourceObj); + expect(result).toMatchInlineSnapshot(` + Object { + "agent.name": "nodejs", + "labels.team": "test", + "service.environment": "testing", + "service.language.name": "typescript", + "service.name": "testbeans", + } + `); + }); +}); diff --git a/x-pack/plugins/apm/server/routes/alerts/get_source_fields.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/get_source_fields.ts similarity index 94% rename from x-pack/plugins/apm/server/routes/alerts/get_source_fields.ts rename to x-pack/plugins/apm/server/routes/alerts/rule_types/get_source_fields.ts index cbc70f6ea472d5..41a7c27e13485c 100644 --- a/x-pack/plugins/apm/server/routes/alerts/get_source_fields.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/get_source_fields.ts @@ -6,7 +6,7 @@ */ import { AggregationsTopHitsAggregation } from '@elastic/elasticsearch/lib/api/types'; -import { SERVICE_GROUP_SUPPORTED_FIELDS } from '../../../common/service_groups'; +import { SERVICE_GROUP_SUPPORTED_FIELDS } from '../../../../common/service_groups'; export interface SourceDoc { [key: string]: string | SourceDoc; diff --git a/x-pack/plugins/apm/server/routes/alerts/average_or_percentile_agg.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/average_or_percentile_agg.ts similarity index 100% rename from x-pack/plugins/apm/server/routes/alerts/average_or_percentile_agg.ts rename to x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/average_or_percentile_agg.ts diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_transaction_duration_chart_preview.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_transaction_duration_chart_preview.ts index 292748f3af16c6..781e9739fdba96 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_transaction_duration_chart_preview.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_transaction_duration_chart_preview.ts @@ -25,7 +25,7 @@ import { ENVIRONMENT_NOT_DEFINED, getEnvironmentLabel, } from '../../../../../common/environment_filter_values'; -import { averageOrPercentileAgg } from '../../average_or_percentile_agg'; +import { averageOrPercentileAgg } from './average_or_percentile_agg'; import { APMConfig } from '../../../..'; import { APMEventClient } from '../../../../lib/helpers/create_es_client/create_apm_event_client'; diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts index 1bee9c526df63d..c0939d5a1f2797 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts @@ -50,8 +50,8 @@ import { RegisterRuleDependencies } from '../../register_apm_rule_types'; import { averageOrPercentileAgg, getMultiTermsSortOrder, -} from '../../average_or_percentile_agg'; -import { getSourceFields, getSourceFieldsAgg } from '../../get_source_fields'; +} from './average_or_percentile_agg'; +import { getSourceFields, getSourceFieldsAgg } from '../get_source_fields'; const paramsSchema = schema.object({ serviceName: schema.string(), diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts index 2291ac027b3f3c..49b8cc9230e440 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts @@ -44,7 +44,7 @@ import { alertingEsClient } from '../../alerting_es_client'; import { RegisterRuleDependencies } from '../../register_apm_rule_types'; import { SearchAggregatedTransactionSetting } from '../../../../../common/aggregated_transactions'; import { getDocumentTypeFilterForTransactions } from '../../../../lib/helpers/transactions'; -import { getSourceFields, getSourceFieldsAgg } from '../../get_source_fields'; +import { getSourceFields, getSourceFieldsAgg } from '../get_source_fields'; const paramsSchema = schema.object({ windowSize: schema.number(), From 9a91b16f91f1fcdd00fee702cefe1a64a252cc75 Mon Sep 17 00:00:00 2001 From: Oliver Gupte Date: Thu, 27 Oct 2022 10:43:03 -0400 Subject: [PATCH 11/14] fixed back import path --- .../transaction_duration/average_or_percentile_agg.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/average_or_percentile_agg.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/average_or_percentile_agg.ts index d458b45b99d52e..2e61108b8a9a08 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/average_or_percentile_agg.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/average_or_percentile_agg.ts @@ -4,8 +4,8 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import { AggregationType } from '../../../common/rules/apm_rule_types'; -import { getDurationFieldForTransactions } from '../../lib/helpers/transactions'; +import { AggregationType } from '../../../../../common/rules/apm_rule_types'; +import { getDurationFieldForTransactions } from '../../../../lib/helpers/transactions'; type TransactionDurationField = ReturnType< typeof getDurationFieldForTransactions From 0940bf3dd73bd641cd084ee9187b74bff33221df Mon Sep 17 00:00:00 2001 From: Oliver Gupte Date: Fri, 28 Oct 2022 02:02:21 -0400 Subject: [PATCH 12/14] PR feedback --- .../plugins/apm/common/service_groups.test.ts | 24 ++++++++------ .../apm/common/utils/environment_query.ts | 2 +- .../components/alerting/utils/fields.tsx | 6 ++-- .../service_group_save/select_services.tsx | 6 ++-- ...> get_service_group_fields_for_anomaly.ts} | 32 +++++++++++++++---- .../anomaly/register_anomaly_rule_type.ts | 28 ++++------------ .../register_error_count_rule_type.ts | 11 ++++--- ...st.ts => get_service_group_fields.test.ts} | 12 +++---- ..._fields.ts => get_service_group_fields.ts} | 4 +-- ...register_transaction_duration_rule_type.ts | 15 +++++---- ...gister_transaction_error_rate_rule_type.ts | 11 ++++--- .../service_map/get_service_anomalies.ts | 6 ++-- 12 files changed, 88 insertions(+), 69 deletions(-) rename x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/{get_anomalous_event_source_fields.ts => get_service_group_fields_for_anomaly.ts} (65%) rename x-pack/plugins/apm/server/routes/alerts/rule_types/{get_source_fields.test.ts => get_service_group_fields.test.ts} (92%) rename x-pack/plugins/apm/server/routes/alerts/rule_types/{get_source_fields.ts => get_service_group_fields.ts} (92%) diff --git a/x-pack/plugins/apm/common/service_groups.test.ts b/x-pack/plugins/apm/common/service_groups.test.ts index e2e2c0d7b64d18..65f0c48dd73b3d 100644 --- a/x-pack/plugins/apm/common/service_groups.test.ts +++ b/x-pack/plugins/apm/common/service_groups.test.ts @@ -16,18 +16,22 @@ import { SERVICE_FRAMEWORK_VERSION, } from './elasticsearch_fieldnames'; -const unsupportedFields = [ - TRANSACTION_TYPE, - TRANSACTION_DURATION, - SERVICE_FRAMEWORK_VERSION, -]; -const mockFields = [...SERVICE_GROUP_SUPPORTED_FIELDS, ...unsupportedFields]; - describe('service_groups common utils', () => { describe('isSupportedField', () => { - it('should only filter supported supported fields for service groups', () => { - const supportedFields = mockFields.filter(isSupportedField); - expect(supportedFields).toEqual(SERVICE_GROUP_SUPPORTED_FIELDS); + it('should allow supported fields', () => { + SERVICE_GROUP_SUPPORTED_FIELDS.map((field) => { + expect(isSupportedField(field)).toBe(true); + }); + }); + it('should reject unsupported fields', () => { + const unsupportedFields = [ + TRANSACTION_TYPE, + TRANSACTION_DURATION, + SERVICE_FRAMEWORK_VERSION, + ]; + unsupportedFields.map((field) => { + expect(isSupportedField(field)).toBe(false); + }); }); }); describe('validateServiceGroupKuery', () => { diff --git a/x-pack/plugins/apm/common/utils/environment_query.ts b/x-pack/plugins/apm/common/utils/environment_query.ts index bc02e4cd2518b2..42744778b861b3 100644 --- a/x-pack/plugins/apm/common/utils/environment_query.ts +++ b/x-pack/plugins/apm/common/utils/environment_query.ts @@ -17,7 +17,7 @@ import { import { SERVICE_NODE_NAME_MISSING } from '../service_nodes'; export function environmentQuery( - environment: string + environment: string | undefined ): QueryDslQueryContainer[] { if (!environment || environment === ENVIRONMENT_ALL.value) { return []; diff --git a/x-pack/plugins/apm/public/components/alerting/utils/fields.tsx b/x-pack/plugins/apm/public/components/alerting/utils/fields.tsx index 129c36e14102cc..4e4038a5275287 100644 --- a/x-pack/plugins/apm/public/components/alerting/utils/fields.tsx +++ b/x-pack/plugins/apm/public/components/alerting/utils/fields.tsx @@ -47,7 +47,7 @@ export function ServiceField({ )} defaultValue={currentValue} fieldName={SERVICE_NAME} - onChange={onChange} + onChange={(value) => onChange(value || undefined)} placeholder={i18n.translate('xpack.apm.serviceNamesSelectPlaceholder', { defaultMessage: 'Select service name', })} @@ -82,7 +82,7 @@ export function EnvironmentField({ )} defaultValue={getEnvironmentLabel(currentValue)} fieldName={SERVICE_ENVIRONMENT} - onChange={onChange} + onChange={(value) => onChange(value || undefined)} placeholder={i18n.translate('xpack.apm.environmentsSelectPlaceholder', { defaultMessage: 'Select environment', })} @@ -115,7 +115,7 @@ export function TransactionTypeField({ )} defaultValue={currentValue} fieldName={TRANSACTION_TYPE} - onChange={onChange} + onChange={(value) => onChange(value || undefined)} placeholder={i18n.translate( 'xpack.apm.transactionTypesSelectPlaceholder', { diff --git a/x-pack/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx b/x-pack/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx index 27a8a7e38de5e2..14c881e162974d 100644 --- a/x-pack/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx +++ b/x-pack/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx @@ -67,7 +67,9 @@ export function SelectServices({ }: Props) { const [kuery, setKuery] = useState(serviceGroup?.kuery || ''); const [stagedKuery, setStagedKuery] = useState(serviceGroup?.kuery || ''); - const [kueryValidationMessage, setKueryValidationMessage] = useState(''); + const [kueryValidationMessage, setKueryValidationMessage] = useState< + string | undefined + >(); useEffect(() => { if (isEdit) { @@ -83,7 +85,7 @@ export function SelectServices({ const { isValid, isParsingError, message } = validateServiceGroupKuery(stagedKuery); if (isValid || isParsingError) { - setKueryValidationMessage(''); + setKueryValidationMessage(undefined); } else { setKueryValidationMessage(message); } diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_service_group_fields_for_anomaly.ts similarity index 65% rename from x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts rename to x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_service_group_fields_for_anomaly.ts index f4e46bca29c8c0..0786cd81aa7f27 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_anomalous_event_source_fields.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/get_service_group_fields_for_anomaly.ts @@ -4,7 +4,11 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import { IScopedClusterClient } from '@kbn/core/server'; +import { firstValueFrom } from 'rxjs'; +import { + IScopedClusterClient, + SavedObjectsClientContract, +} from '@kbn/core/server'; import { SERVICE_ENVIRONMENT, SERVICE_NAME, @@ -12,25 +16,39 @@ import { TRANSACTION_DURATION, } from '../../../../../common/elasticsearch_fieldnames'; import { alertingEsClient } from '../../alerting_es_client'; -import { getSourceFields, getSourceFieldsAgg } from '../get_source_fields'; +import { + getServiceGroupFields, + getServiceGroupFieldsAgg, +} from '../get_service_group_fields'; +import { getApmIndices } from '../../../settings/apm_indices/get_apm_indices'; +import { RegisterRuleDependencies } from '../../register_apm_rule_types'; -export async function getAnomalousEventSourceFields({ +export async function getServiceGroupFieldsForAnomaly({ + config$, scopedClusterClient, - index, + savedObjectsClient, serviceName, environment, transactionType, timestamp, bucketSpan, }: { + config$: RegisterRuleDependencies['config$']; scopedClusterClient: IScopedClusterClient; - index: string; + savedObjectsClient: SavedObjectsClientContract; serviceName: string; environment: string; transactionType: string; timestamp: number; bucketSpan: number; }) { + const config = await firstValueFrom(config$); + const indices = await getApmIndices({ + config, + savedObjectsClient, + }); + const { transaction: index } = indices; + const params = { index, body: { @@ -55,7 +73,7 @@ export async function getAnomalousEventSourceFields({ }, }, aggs: { - ...getSourceFieldsAgg({ + ...getServiceGroupFieldsAgg({ sort: [{ [TRANSACTION_DURATION]: { order: 'desc' as const } }], }), }, @@ -69,5 +87,5 @@ export async function getAnomalousEventSourceFields({ if (!response.aggregations) { return {}; } - return getSourceFields(response.aggregations); + return getServiceGroupFields(response.aggregations); } diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts index d93c0ff09d3ec3..376f6d6614d87e 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts @@ -19,7 +19,6 @@ import { KibanaRequest } from '@kbn/core/server'; import { termQuery } from '@kbn/observability-plugin/server'; import { createLifecycleRuleTypeFactory } from '@kbn/rule-registry-plugin/server'; import { ProcessorEvent } from '@kbn/observability-plugin/common'; -import { firstValueFrom } from 'rxjs'; import { ApmRuleType, RULE_TYPES_CONFIG, @@ -47,15 +46,14 @@ import { getAlertUrlTransaction } from '../../../../../common/utils/formatters'; import { getMLJobs } from '../../../service_map/get_service_anomalies'; import { apmActionVariables } from '../../action_variables'; import { RegisterRuleDependencies } from '../../register_apm_rule_types'; -import { getAnomalousEventSourceFields } from './get_anomalous_event_source_fields'; -import { getApmIndices } from '../../../settings/apm_indices/get_apm_indices'; +import { getServiceGroupFieldsForAnomaly } from './get_service_group_fields_for_anomaly'; const paramsSchema = schema.object({ serviceName: schema.maybe(schema.string()), transactionType: schema.maybe(schema.string()), windowSize: schema.number(), windowUnit: schema.string(), - environment: schema.string(), + environment: schema.maybe(schema.string()), anomalySeverityType: schema.oneOf([ schema.literal(ANOMALY_SEVERITY.CRITICAL), schema.literal(ANOMALY_SEVERITY.MAJOR), @@ -107,13 +105,6 @@ export function registerAnomalyRuleType({ return {}; } - const config = await firstValueFrom(config$); - const indices = await getApmIndices({ - config, - savedObjectsClient: services.savedObjectsClient, - }); - const { transaction: transactionsIndex } = indices; - const ruleParams = params; const request = {} as KibanaRequest; const { mlAnomalySearch } = ml.mlSystemProvider( @@ -173,14 +164,8 @@ export function registerAnomalyRuleType({ }, }, }, - ...termQuery( - 'partition_field_value', - ruleParams.serviceName || null - ), - ...termQuery( - 'by_field_value', - ruleParams.transactionType || null - ), + ...termQuery('partition_field_value', ruleParams.serviceName), + ...termQuery('by_field_value', ruleParams.transactionType), ...termQuery( 'detector_index', getApmMlDetectorIndex(ApmMlDetectorType.txLatency) @@ -261,9 +246,10 @@ export function registerAnomalyRuleType({ bucketSpan, } = anomaly; - const eventSourceFields = await getAnomalousEventSourceFields({ + const eventSourceFields = await getServiceGroupFieldsForAnomaly({ + config$, scopedClusterClient: services.scopedClusterClient, - index: transactionsIndex, + savedObjectsClient: services.savedObjectsClient, serviceName, environment, transactionType, diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts index b3b9b620d501c2..051b707712f19e 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts @@ -37,14 +37,17 @@ import { getApmIndices } from '../../../settings/apm_indices/get_apm_indices'; import { apmActionVariables } from '../../action_variables'; import { alertingEsClient } from '../../alerting_es_client'; import { RegisterRuleDependencies } from '../../register_apm_rule_types'; -import { getSourceFieldsAgg, getSourceFields } from '../get_source_fields'; +import { + getServiceGroupFieldsAgg, + getServiceGroupFields, +} from '../get_service_group_fields'; const paramsSchema = schema.object({ windowSize: schema.number(), windowUnit: schema.string(), threshold: schema.number(), serviceName: schema.maybe(schema.string()), - environment: schema.string(), + environment: schema.maybe(schema.string()), }); const ruleTypeConfig = RULE_TYPES_CONFIG[ApmRuleType.ErrorCount]; @@ -126,7 +129,7 @@ export function registerErrorCountRuleType({ size: 1000, order: { _count: 'desc' as const }, }, - aggs: getSourceFieldsAgg(), + aggs: getServiceGroupFieldsAgg(), }, }, }, @@ -144,7 +147,7 @@ export function registerErrorCountRuleType({ serviceName, environment, errorCount: bucket.doc_count, - sourceFields: getSourceFields(bucket), + sourceFields: getServiceGroupFields(bucket), }; }) ?? []; diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/get_source_fields.test.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/get_service_group_fields.test.ts similarity index 92% rename from x-pack/plugins/apm/server/routes/alerts/rule_types/get_source_fields.test.ts rename to x-pack/plugins/apm/server/routes/alerts/rule_types/get_service_group_fields.test.ts index 51bdb6580126c5..0df590d524f91c 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/get_source_fields.test.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/get_service_group_fields.test.ts @@ -6,10 +6,10 @@ */ import { - getSourceFields, - getSourceFieldsAgg, + getServiceGroupFields, + getServiceGroupFieldsAgg, flattenSourceDoc, -} from './get_source_fields'; +} from './get_service_group_fields'; const mockSourceObj = { service: { @@ -37,7 +37,7 @@ const mockBucket = { describe('getSourceFields', () => { it('should return a flattened record of fields and values for a given bucket', () => { - const result = getSourceFields(mockBucket); + const result = getServiceGroupFields(mockBucket); expect(result).toMatchInlineSnapshot(` Object { "agent.name": "nodejs", @@ -52,7 +52,7 @@ describe('getSourceFields', () => { describe('getSourceFieldsAgg', () => { it('should create a agg for specific source fields', () => { - const agg = getSourceFieldsAgg(); + const agg = getServiceGroupFieldsAgg(); expect(agg).toMatchInlineSnapshot(` Object { "source_fields": Object { @@ -74,7 +74,7 @@ describe('getSourceFieldsAgg', () => { }); it('should accept options for top_hits options', () => { - const agg = getSourceFieldsAgg({ + const agg = getServiceGroupFieldsAgg({ sort: [{ 'transaction.duration.us': { order: 'desc' } }], }); expect(agg).toMatchInlineSnapshot(` diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/get_source_fields.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/get_service_group_fields.ts similarity index 92% rename from x-pack/plugins/apm/server/routes/alerts/rule_types/get_source_fields.ts rename to x-pack/plugins/apm/server/routes/alerts/rule_types/get_service_group_fields.ts index 41a7c27e13485c..2a50b8ba2f31e8 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/get_source_fields.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/get_service_group_fields.ts @@ -12,7 +12,7 @@ export interface SourceDoc { [key: string]: string | SourceDoc; } -export function getSourceFieldsAgg( +export function getServiceGroupFieldsAgg( topHitsOpts: AggregationsTopHitsAggregation = {} ) { return { @@ -36,7 +36,7 @@ interface AggResultBucket { }; } -export function getSourceFields(bucket?: AggResultBucket) { +export function getServiceGroupFields(bucket?: AggResultBucket) { if (!bucket) { return {}; } diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts index c0939d5a1f2797..cfc71166440c9b 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts @@ -51,11 +51,14 @@ import { averageOrPercentileAgg, getMultiTermsSortOrder, } from './average_or_percentile_agg'; -import { getSourceFields, getSourceFieldsAgg } from '../get_source_fields'; +import { + getServiceGroupFields, + getServiceGroupFieldsAgg, +} from '../get_service_group_fields'; const paramsSchema = schema.object({ - serviceName: schema.string(), - transactionType: schema.string(), + serviceName: schema.maybe(schema.string()), + transactionType: schema.maybe(schema.string()), windowSize: schema.number(), windowUnit: schema.string(), threshold: schema.number(), @@ -64,7 +67,7 @@ const paramsSchema = schema.object({ schema.literal(AggregationType.P95), schema.literal(AggregationType.P99), ]), - environment: schema.string(), + environment: schema.maybe(schema.string()), }); const ruleTypeConfig = RULE_TYPES_CONFIG[ApmRuleType.TransactionDuration]; @@ -170,7 +173,7 @@ export function registerTransactionDurationRuleType({ aggregationType: ruleParams.aggregationType, transactionDurationField: field, }), - ...getSourceFieldsAgg(), + ...getServiceGroupFieldsAgg(), }, }, }, @@ -205,7 +208,7 @@ export function registerTransactionDurationRuleType({ environment, transactionType, transactionDuration, - sourceFields: getSourceFields(bucket), + sourceFields: getServiceGroupFields(bucket), }); } } diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts index 49b8cc9230e440..232d18afe801bb 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts @@ -44,7 +44,10 @@ import { alertingEsClient } from '../../alerting_es_client'; import { RegisterRuleDependencies } from '../../register_apm_rule_types'; import { SearchAggregatedTransactionSetting } from '../../../../../common/aggregated_transactions'; import { getDocumentTypeFilterForTransactions } from '../../../../lib/helpers/transactions'; -import { getSourceFields, getSourceFieldsAgg } from '../get_source_fields'; +import { + getServiceGroupFields, + getServiceGroupFieldsAgg, +} from '../get_service_group_fields'; const paramsSchema = schema.object({ windowSize: schema.number(), @@ -52,7 +55,7 @@ const paramsSchema = schema.object({ threshold: schema.number(), transactionType: schema.maybe(schema.string()), serviceName: schema.maybe(schema.string()), - environment: schema.string(), + environment: schema.maybe(schema.string()), }); const ruleTypeConfig = RULE_TYPES_CONFIG[ApmRuleType.TransactionErrorRate]; @@ -162,7 +165,7 @@ export function registerTransactionErrorRateRuleType({ terms: { field: EVENT_OUTCOME, }, - aggs: getSourceFieldsAgg(), + aggs: getServiceGroupFieldsAgg(), }, }, }, @@ -199,7 +202,7 @@ export function registerTransactionErrorRateRuleType({ environment, transactionType, errorRate, - sourceFields: getSourceFields(failedOutcomeBucket), + sourceFields: getServiceGroupFields(failedOutcomeBucket), }); } } diff --git a/x-pack/plugins/apm/server/routes/service_map/get_service_anomalies.ts b/x-pack/plugins/apm/server/routes/service_map/get_service_anomalies.ts index 3c5211528cc91b..e67b2b554df050 100644 --- a/x-pack/plugins/apm/server/routes/service_map/get_service_anomalies.ts +++ b/x-pack/plugins/apm/server/routes/service_map/get_service_anomalies.ts @@ -157,14 +157,14 @@ export async function getServiceAnomalies({ export async function getMLJobs( anomalyDetectors: ReturnType, - environment: string + environment?: string ) { const jobs = await getMlJobsWithAPMGroup(anomalyDetectors); // to filter out legacy jobs we are filtering by the existence of `apm_ml_version` in `custom_settings` // and checking that it is compatable. const mlJobs = jobs.filter((job) => job.version >= 2); - if (environment !== ENVIRONMENT_ALL.value) { + if (environment && environment !== ENVIRONMENT_ALL.value) { const matchingMLJob = mlJobs.find((job) => job.environment === environment); if (!matchingMLJob) { return []; @@ -176,7 +176,7 @@ export async function getMLJobs( export async function getMLJobIds( anomalyDetectors: ReturnType, - environment: string + environment?: string ) { const mlJobs = await getMLJobs(anomalyDetectors, environment); return mlJobs.map((job) => job.jobId); From 51fcde75278fa6fdf0e7843fee1c41a40ce87f12 Mon Sep 17 00:00:00 2001 From: Oliver Gupte Date: Fri, 28 Oct 2022 23:15:53 -0400 Subject: [PATCH 13/14] improvements to kuery validation --- .../plugins/apm/common/service_groups.test.ts | 14 +++---- x-pack/plugins/apm/common/service_groups.ts | 42 ++++++++++--------- .../service_group_save/select_services.tsx | 9 +--- .../apm/server/routes/service_groups/route.ts | 6 ++- 4 files changed, 36 insertions(+), 35 deletions(-) diff --git a/x-pack/plugins/apm/common/service_groups.test.ts b/x-pack/plugins/apm/common/service_groups.test.ts index 65f0c48dd73b3d..856eec4ef2e3f5 100644 --- a/x-pack/plugins/apm/common/service_groups.test.ts +++ b/x-pack/plugins/apm/common/service_groups.test.ts @@ -39,16 +39,16 @@ describe('service_groups common utils', () => { const result = validateServiceGroupKuery( `service.name: testbeans* or agent.name: "nodejs"` ); - expect(result).toHaveProperty('isValid', true); - expect(result).toHaveProperty('isParsingError', false); - expect(result).toHaveProperty('message', ''); + expect(result).toHaveProperty('isValidFields', true); + expect(result).toHaveProperty('isValidSyntax', true); + expect(result).not.toHaveProperty('message'); }); it('should return validation error when unsupported fields are used', () => { const result = validateServiceGroupKuery( `service.name: testbeans* or agent.name: "nodejs" or transaction.type: request` ); - expect(result).toHaveProperty('isValid', false); - expect(result).toHaveProperty('isParsingError', false); + expect(result).toHaveProperty('isValidFields', false); + expect(result).toHaveProperty('isValidSyntax', true); expect(result).toHaveProperty( 'message', 'Query filter for service group does not support fields [transaction.type]' @@ -58,8 +58,8 @@ describe('service_groups common utils', () => { const result = validateServiceGroupKuery( `service.name: testbeans* or agent.name: "nod` ); - expect(result).toHaveProperty('isValid', false); - expect(result).toHaveProperty('isParsingError', true); + expect(result).toHaveProperty('isValidFields', false); + expect(result).toHaveProperty('isValidSyntax', false); expect(result).toHaveProperty('message'); expect(result).not.toBe(''); }); diff --git a/x-pack/plugins/apm/common/service_groups.ts b/x-pack/plugins/apm/common/service_groups.ts index 6a48f7167273d2..4b2ba1288ecaef 100644 --- a/x-pack/plugins/apm/common/service_groups.ts +++ b/x-pack/plugins/apm/common/service_groups.ts @@ -48,31 +48,35 @@ export function isSupportedField(fieldName: string) { ); } -export function validateServiceGroupKuery(kuery: string) { +export function validateServiceGroupKuery(kuery: string): { + isValidFields: boolean; + isValidSyntax: boolean; + message?: string; +} { try { const kueryFields = getKueryFields([fromKueryExpression(kuery)]); const unsupportedKueryFields = kueryFields.filter( (fieldName) => !isSupportedField(fieldName) ); - if (unsupportedKueryFields.length > 0) { - return { - isValid: false, - isParsingError: false, - message: i18n.translate( - 'xpack.apm.serviceGroups.invalidFields.message', - { - defaultMessage: - 'Query filter for service group does not support fields [{unsupportedFieldNames}]', - values: { - unsupportedFieldNames: unsupportedKueryFields.join(', '), - }, - } - ), - }; - } else { - return { isValid: true, isParsingError: false, message: '' }; + if (unsupportedKueryFields.length === 0) { + return { isValidFields: true, isValidSyntax: true }; } + return { + isValidFields: false, + isValidSyntax: true, + message: i18n.translate('xpack.apm.serviceGroups.invalidFields.message', { + defaultMessage: + 'Query filter for service group does not support fields [{unsupportedFieldNames}]', + values: { + unsupportedFieldNames: unsupportedKueryFields.join(', '), + }, + }), + }; } catch (error) { - return { isValid: false, isParsingError: true, message: error.message }; + return { + isValidFields: false, + isValidSyntax: false, + message: error.message, + }; } } diff --git a/x-pack/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx b/x-pack/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx index 14c881e162974d..96b0b47a38a1b0 100644 --- a/x-pack/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx +++ b/x-pack/plugins/apm/public/components/app/service_groups/service_group_save/select_services.tsx @@ -82,13 +82,8 @@ export function SelectServices({ if (!stagedKuery) { return; } - const { isValid, isParsingError, message } = - validateServiceGroupKuery(stagedKuery); - if (isValid || isParsingError) { - setKueryValidationMessage(undefined); - } else { - setKueryValidationMessage(message); - } + const { message } = validateServiceGroupKuery(stagedKuery); + setKueryValidationMessage(message); }, [stagedKuery]); const { start, end } = useMemo( diff --git a/x-pack/plugins/apm/server/routes/service_groups/route.ts b/x-pack/plugins/apm/server/routes/service_groups/route.ts index 6207aa2a6aa6bb..dde307efa7c4b7 100644 --- a/x-pack/plugins/apm/server/routes/service_groups/route.ts +++ b/x-pack/plugins/apm/server/routes/service_groups/route.ts @@ -124,8 +124,10 @@ const serviceGroupSaveRoute = createApmServerRoute({ const { savedObjects: { client: savedObjectsClient }, } = await context.core; - const { isValid, message } = validateServiceGroupKuery(params.body.kuery); - if (!isValid) { + const { isValidFields, isValidSyntax, message } = validateServiceGroupKuery( + params.body.kuery + ); + if (!(isValidFields && isValidSyntax)) { throw Boom.badRequest(message); } From f3bb573e375388193749874654c37360cc792371 Mon Sep 17 00:00:00 2001 From: Oliver Gupte Date: Sat, 29 Oct 2022 00:38:53 -0400 Subject: [PATCH 14/14] fixes selecting 'All' in service.name, transaction.type fields when creating/editing APM Rules (#143861) --- .../public/components/alerting/utils/fields.tsx | 12 +++++++----- .../anomaly/register_anomaly_rule_type.ts | 12 +++++++++--- .../error_count/register_error_count_rule_type.ts | 6 ++++-- .../register_transaction_duration_rule_type.ts | 14 +++++++++----- .../register_transaction_error_rate_rule_type.ts | 10 +++++++--- .../plugins/observability/server/utils/queries.ts | 9 +++++++-- 6 files changed, 43 insertions(+), 20 deletions(-) diff --git a/x-pack/plugins/apm/public/components/alerting/utils/fields.tsx b/x-pack/plugins/apm/public/components/alerting/utils/fields.tsx index 4e4038a5275287..3f028c2ead0022 100644 --- a/x-pack/plugins/apm/public/components/alerting/utils/fields.tsx +++ b/x-pack/plugins/apm/public/components/alerting/utils/fields.tsx @@ -38,7 +38,9 @@ export function ServiceField({ })} > onChange(value || undefined)} + onChange={onChange} placeholder={i18n.translate('xpack.apm.serviceNamesSelectPlaceholder', { defaultMessage: 'Select service name', })} @@ -82,7 +84,7 @@ export function EnvironmentField({ )} defaultValue={getEnvironmentLabel(currentValue)} fieldName={SERVICE_ENVIRONMENT} - onChange={(value) => onChange(value || undefined)} + onChange={onChange} placeholder={i18n.translate('xpack.apm.environmentsSelectPlaceholder', { defaultMessage: 'Select environment', })} @@ -106,7 +108,7 @@ export function TransactionTypeField({ return ( onChange(value || undefined)} + onChange={onChange} placeholder={i18n.translate( 'xpack.apm.transactionTypesSelectPlaceholder', { diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts index 376f6d6614d87e..d85b8df2798fea 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/anomaly/register_anomaly_rule_type.ts @@ -53,7 +53,7 @@ const paramsSchema = schema.object({ transactionType: schema.maybe(schema.string()), windowSize: schema.number(), windowUnit: schema.string(), - environment: schema.maybe(schema.string()), + environment: schema.string(), anomalySeverityType: schema.oneOf([ schema.literal(ANOMALY_SEVERITY.CRITICAL), schema.literal(ANOMALY_SEVERITY.MAJOR), @@ -164,8 +164,14 @@ export function registerAnomalyRuleType({ }, }, }, - ...termQuery('partition_field_value', ruleParams.serviceName), - ...termQuery('by_field_value', ruleParams.transactionType), + ...termQuery( + 'partition_field_value', + ruleParams.serviceName, + { queryEmptyString: false } + ), + ...termQuery('by_field_value', ruleParams.transactionType, { + queryEmptyString: false, + }), ...termQuery( 'detector_index', getApmMlDetectorIndex(ApmMlDetectorType.txLatency) diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts index 051b707712f19e..d9826aae392c8d 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/error_count/register_error_count_rule_type.ts @@ -47,7 +47,7 @@ const paramsSchema = schema.object({ windowUnit: schema.string(), threshold: schema.number(), serviceName: schema.maybe(schema.string()), - environment: schema.maybe(schema.string()), + environment: schema.string(), }); const ruleTypeConfig = RULE_TYPES_CONFIG[ApmRuleType.ErrorCount]; @@ -111,7 +111,9 @@ export function registerErrorCountRuleType({ }, }, { term: { [PROCESSOR_EVENT]: ProcessorEvent.error } }, - ...termQuery(SERVICE_NAME, ruleParams.serviceName), + ...termQuery(SERVICE_NAME, ruleParams.serviceName, { + queryEmptyString: false, + }), ...environmentQuery(ruleParams.environment), ], }, diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts index cfc71166440c9b..b4c7a6212b62dc 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts @@ -57,8 +57,8 @@ import { } from '../get_service_group_fields'; const paramsSchema = schema.object({ - serviceName: schema.maybe(schema.string()), - transactionType: schema.maybe(schema.string()), + serviceName: schema.string(), + transactionType: schema.string(), windowSize: schema.number(), windowUnit: schema.string(), threshold: schema.number(), @@ -67,7 +67,7 @@ const paramsSchema = schema.object({ schema.literal(AggregationType.P95), schema.literal(AggregationType.P99), ]), - environment: schema.maybe(schema.string()), + environment: schema.string(), }); const ruleTypeConfig = RULE_TYPES_CONFIG[ApmRuleType.TransactionDuration]; @@ -148,8 +148,12 @@ export function registerTransactionDurationRuleType({ ...getDocumentTypeFilterForTransactions( searchAggregatedTransactions ), - ...termQuery(SERVICE_NAME, ruleParams.serviceName), - ...termQuery(TRANSACTION_TYPE, ruleParams.transactionType), + ...termQuery(SERVICE_NAME, ruleParams.serviceName, { + queryEmptyString: false, + }), + ...termQuery(TRANSACTION_TYPE, ruleParams.transactionType, { + queryEmptyString: false, + }), ...environmentQuery(ruleParams.environment), ] as QueryDslQueryContainer[], }, diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts index 232d18afe801bb..73f7ccda26401f 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/register_transaction_error_rate_rule_type.ts @@ -55,7 +55,7 @@ const paramsSchema = schema.object({ threshold: schema.number(), transactionType: schema.maybe(schema.string()), serviceName: schema.maybe(schema.string()), - environment: schema.maybe(schema.string()), + environment: schema.string(), }); const ruleTypeConfig = RULE_TYPES_CONFIG[ApmRuleType.TransactionErrorRate]; @@ -140,8 +140,12 @@ export function registerTransactionErrorRateRuleType({ ], }, }, - ...termQuery(SERVICE_NAME, ruleParams.serviceName), - ...termQuery(TRANSACTION_TYPE, ruleParams.transactionType), + ...termQuery(SERVICE_NAME, ruleParams.serviceName, { + queryEmptyString: false, + }), + ...termQuery(TRANSACTION_TYPE, ruleParams.transactionType, { + queryEmptyString: false, + }), ...environmentQuery(ruleParams.environment), ], }, diff --git a/x-pack/plugins/observability/server/utils/queries.ts b/x-pack/plugins/observability/server/utils/queries.ts index 008b8720de7cd4..f6a5a02d8e4156 100644 --- a/x-pack/plugins/observability/server/utils/queries.ts +++ b/x-pack/plugins/observability/server/utils/queries.ts @@ -13,11 +13,16 @@ function isUndefinedOrNull(value: any): value is undefined | null { return value === undefined || value === null; } +interface TermQueryOpts { + queryEmptyString: boolean; +} + export function termQuery( field: T, - value: string | boolean | number | undefined | null + value: string | boolean | number | undefined | null, + opts: TermQueryOpts = { queryEmptyString: true } ): QueryDslQueryContainer[] { - if (isUndefinedOrNull(value)) { + if (isUndefinedOrNull(value) || (!opts.queryEmptyString && value === '')) { return []; }