From aea6d6448a661011b493743c9d35f6fefd79204b Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Fri, 8 May 2020 18:28:05 +0300 Subject: [PATCH 01/10] [Discover] Deangularize the hits counter and create a react component (#65631) * Deangularize the hits counter and create a react component * Add aria-label to button for accessibility * Add icon to the link button and use EuiText * Remove snapshots and test with findTestSubject * Change toString with String() Co-authored-by: Elastic Machine --- .../public/application/_discover.scss | 10 --- .../public/application/angular/discover.html | 24 ++---- .../hits_counter/hits_counter.test.tsx | 69 +++++++++++++++ .../components/hits_counter/hits_counter.tsx | 83 +++++++++++++++++++ .../hits_counter/hits_counter_directive.ts | 27 ++++++ .../components/hits_counter/index.ts | 21 +++++ .../helpers/format_number_with_commas.ts | 27 ++++++ .../public/application/helpers/index.ts | 1 + .../discover/public/get_inner_angular.ts | 2 + 9 files changed, 236 insertions(+), 28 deletions(-) create mode 100644 src/plugins/discover/public/application/components/hits_counter/hits_counter.test.tsx create mode 100644 src/plugins/discover/public/application/components/hits_counter/hits_counter.tsx create mode 100644 src/plugins/discover/public/application/components/hits_counter/hits_counter_directive.ts create mode 100644 src/plugins/discover/public/application/components/hits_counter/index.ts create mode 100644 src/plugins/discover/public/application/helpers/format_number_with_commas.ts diff --git a/src/plugins/discover/public/application/_discover.scss b/src/plugins/discover/public/application/_discover.scss index 8eaa66cf58624..590dbebdf4cfe 100644 --- a/src/plugins/discover/public/application/_discover.scss +++ b/src/plugins/discover/public/application/_discover.scss @@ -38,17 +38,7 @@ discover-app { } .dscResultCount { - text-align: center; padding-top: $euiSizeXS; - padding-left: $euiSizeM; - - .dscResultHits { - padding-left: $euiSizeXS; - } - - > .kuiLink { - padding-left: $euiSizeM; - } } .dscTimechart__header { diff --git a/src/plugins/discover/public/application/angular/discover.html b/src/plugins/discover/public/application/angular/discover.html index b4db89b9275b4..a0f98ea38ef78 100644 --- a/src/plugins/discover/public/application/angular/discover.html +++ b/src/plugins/discover/public/application/angular/discover.html @@ -89,24 +89,12 @@

{{screenTitle}}

-
- {{(hits || 0) | number:0}} - - -
+ +
; + + beforeAll(() => { + props = { + onResetQuery: jest.fn(), + showResetButton: true, + hits: 2, + }; + }); + + it('HitsCounter renders a button by providing the showResetButton property', () => { + component = mountWithIntl(); + expect(findTestSubject(component, 'resetSavedSearch').length).toBe(1); + }); + + it('HitsCounter not renders a button when the showResetButton property is false', () => { + component = mountWithIntl( + + ); + expect(findTestSubject(component, 'resetSavedSearch').length).toBe(0); + }); + + it('expect to render the number of hits', function() { + component = mountWithIntl(); + const hits = findTestSubject(component, 'discoverQueryHits'); + expect(hits.text()).toBe('2'); + }); + + it('expect to render 1,899 hits if 1899 hits given', function() { + component = mountWithIntl( + + ); + const hits = findTestSubject(component, 'discoverQueryHits'); + expect(hits.text()).toBe('1,899'); + }); + + it('should reset query', function() { + component = mountWithIntl(); + findTestSubject(component, 'resetSavedSearch').simulate('click'); + expect(props.onResetQuery).toHaveBeenCalled(); + }); +}); diff --git a/src/plugins/discover/public/application/components/hits_counter/hits_counter.tsx b/src/plugins/discover/public/application/components/hits_counter/hits_counter.tsx new file mode 100644 index 0000000000000..1d2cd12877b1c --- /dev/null +++ b/src/plugins/discover/public/application/components/hits_counter/hits_counter.tsx @@ -0,0 +1,83 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import React from 'react'; +import { EuiButtonEmpty, EuiFlexGroup, EuiFlexItem, EuiText } from '@elastic/eui'; +import { FormattedMessage, I18nProvider } from '@kbn/i18n/react'; +import { i18n } from '@kbn/i18n'; +import { formatNumWithCommas } from '../../helpers'; + +export interface HitsCounterProps { + /** + * the number of query hits + */ + hits: number; + /** + * displays the reset button + */ + showResetButton: boolean; + /** + * resets the query + */ + onResetQuery: () => void; +} + +export function HitsCounter({ hits, showResetButton, onResetQuery }: HitsCounterProps) { + return ( + + + + + {formatNumWithCommas(hits)}{' '} + + + + {showResetButton && ( + + + + + + )} + + + ); +} diff --git a/src/plugins/discover/public/application/components/hits_counter/hits_counter_directive.ts b/src/plugins/discover/public/application/components/hits_counter/hits_counter_directive.ts new file mode 100644 index 0000000000000..8d45e28370cad --- /dev/null +++ b/src/plugins/discover/public/application/components/hits_counter/hits_counter_directive.ts @@ -0,0 +1,27 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { HitsCounter } from './hits_counter'; + +export function createHitsCounterDirective(reactDirective: any) { + return reactDirective(HitsCounter, [ + ['hits', { watchDepth: 'reference' }], + ['showResetButton', { watchDepth: 'reference' }], + ['onResetQuery', { watchDepth: 'reference' }], + ]); +} diff --git a/src/plugins/discover/public/application/components/hits_counter/index.ts b/src/plugins/discover/public/application/components/hits_counter/index.ts new file mode 100644 index 0000000000000..58e7a9eda7f51 --- /dev/null +++ b/src/plugins/discover/public/application/components/hits_counter/index.ts @@ -0,0 +1,21 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export { HitsCounter } from './hits_counter'; +export { createHitsCounterDirective } from './hits_counter_directive'; diff --git a/src/plugins/discover/public/application/helpers/format_number_with_commas.ts b/src/plugins/discover/public/application/helpers/format_number_with_commas.ts new file mode 100644 index 0000000000000..01a010d823d5f --- /dev/null +++ b/src/plugins/discover/public/application/helpers/format_number_with_commas.ts @@ -0,0 +1,27 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +const COMMA_SEPARATOR_RE = /(\d)(?=(\d{3})+(?!\d))/g; + +/** + * Converts a number to a string and adds commas + * as thousands separators + */ +export const formatNumWithCommas = (input: number) => + String(input).replace(COMMA_SEPARATOR_RE, '$1,'); diff --git a/src/plugins/discover/public/application/helpers/index.ts b/src/plugins/discover/public/application/helpers/index.ts index 7196c96989e97..3555d24924e80 100644 --- a/src/plugins/discover/public/application/helpers/index.ts +++ b/src/plugins/discover/public/application/helpers/index.ts @@ -18,3 +18,4 @@ */ export { shortenDottedString } from './shorten_dotted_string'; +export { formatNumWithCommas } from './format_number_with_commas'; diff --git a/src/plugins/discover/public/get_inner_angular.ts b/src/plugins/discover/public/get_inner_angular.ts index e7813c43383f9..8c3f4f030688c 100644 --- a/src/plugins/discover/public/get_inner_angular.ts +++ b/src/plugins/discover/public/get_inner_angular.ts @@ -57,6 +57,7 @@ import { createTopNavHelper, } from '../../kibana_legacy/public'; import { createDiscoverSidebarDirective } from './application/components/sidebar'; +import { createHitsCounterDirective } from '././application/components/hits_counter'; import { DiscoverStartPlugins } from './plugin'; /** @@ -151,6 +152,7 @@ export function initializeInnerAngularModule( .directive('fixedScroll', FixedScrollProvider) .directive('renderComplete', createRenderCompleteDirective) .directive('discoverSidebar', createDiscoverSidebarDirective) + .directive('hitsCounter', createHitsCounterDirective) .service('debounce', ['$timeout', DebounceProviderTimeout]); } From d40387161e16aa9cf8396d669c1f8478ec674442 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Fri, 8 May 2020 11:35:17 -0400 Subject: [PATCH 02/10] Skipping failing tests. #65867 #65866 #65865 --- .../server/models/job_validation/job_validation.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/ml/server/models/job_validation/job_validation.test.ts b/x-pack/plugins/ml/server/models/job_validation/job_validation.test.ts index 9851f80a42d5b..ca127f43d08af 100644 --- a/x-pack/plugins/ml/server/models/job_validation/job_validation.test.ts +++ b/x-pack/plugins/ml/server/models/job_validation/job_validation.test.ts @@ -290,7 +290,8 @@ describe('ML - validateJob', () => { }); }); - it('basic validation passes, extended checks return some messages', () => { + // Failing https://github.com/elastic/kibana/issues/65865 + it.skip('basic validation passes, extended checks return some messages', () => { const payload = getBasicPayload(); return validateJob(callWithRequest, payload).then(messages => { const ids = messages.map(m => m.id); @@ -303,7 +304,8 @@ describe('ML - validateJob', () => { }); }); - it('categorization job using mlcategory passes aggregatable field check', () => { + // Failing https://github.com/elastic/kibana/issues/65866 + it.skip('categorization job using mlcategory passes aggregatable field check', () => { const payload: any = { job: { job_id: 'categorization_test', @@ -369,7 +371,8 @@ describe('ML - validateJob', () => { }); }); - it('script field not reported as non aggregatable', () => { + // Failing https://github.com/elastic/kibana/issues/65867 + it.skip('script field not reported as non aggregatable', () => { const payload: any = { job: { job_id: 'categorization_test', From e79f331fb460c954dcb2d8fcf5704681f26d9f5a Mon Sep 17 00:00:00 2001 From: Andrew Goldstein Date: Fri, 8 May 2020 09:38:38 -0600 Subject: [PATCH 03/10] [SIEM] Fixes a CSS issue with Timeline field truncation (#65789) ## Summary Fixes [a CSS issue where Timeline field truncation](https://github.com/elastic/kibana/issues/65170) wasn't working, per the following screenshots: ### Before before ### After after ## Desk testing * The timeline in the _Before_ and _After_ screenshots above includes columns that typically contain large values (e.g. `process.hash.sha256`). It also contains the `event.module` column, which has special formatting, as detailed below. * You may re-create the timeline shown in the _Before_ and _After_ screenshots, or download the exported timeline from the following link [truncation.ndjson.txt](https://github.com/elastic/kibana/files/4596036/truncation.ndjson.txt) and import it. (Remove the `.txt` extension after downloading it.) * The `event.module` field has special formatting that displays an icon link to the endpoint if it's been configured. To desk test this without configuring an endpoint, edit `x-pack/plugins/siem/public/components/timeline/body/renderers/formatted_field_helpers.tsx`, and change the following line: ``` {endpointRefUrl != null && canYouAddEndpointLogo(moduleName, endpointRefUrl) && ( ``` to ``` {true && ( ``` The above change forces the icon to always appear, even if you don't have an endpoint configured. ### Desk tested in: - Chrome `81.0.4044.138` - Firefox `76.0` - Safari `13.1` --- .../body/renderers/formatted_field_helpers.tsx | 9 +++++++-- .../public/components/with_hover_actions/index.tsx | 13 +++++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/siem/public/components/timeline/body/renderers/formatted_field_helpers.tsx b/x-pack/plugins/siem/public/components/timeline/body/renderers/formatted_field_helpers.tsx index b48cc546fe78c..7c9accd4cef49 100644 --- a/x-pack/plugins/siem/public/components/timeline/body/renderers/formatted_field_helpers.tsx +++ b/x-pack/plugins/siem/public/components/timeline/body/renderers/formatted_field_helpers.tsx @@ -7,6 +7,7 @@ import { EuiLink, EuiFlexGroup, EuiFlexItem, EuiIcon, EuiToolTip } from '@elastic/eui'; import { isString, isEmpty } from 'lodash/fp'; import React from 'react'; +import styled from 'styled-components'; import { DefaultDraggable } from '../../../draggables'; import { getEmptyTagValue } from '../../../empty_value'; @@ -18,6 +19,10 @@ import endPointSvg from '../../../../utils/logo_endpoint/64_color.svg'; import * as i18n from './translations'; +const EventModuleFlexItem = styled(EuiFlexItem)` + width: 100%; +`; + export const renderRuleName = ({ contextId, eventId, @@ -87,7 +92,7 @@ export const renderEventModule = ({ endpointRefUrl != null && !isEmpty(endpointRefUrl) ? 'flexStart' : 'spaceBetween' } > - + {content} - + {endpointRefUrl != null && canYouAddEndpointLogo(moduleName, endpointRefUrl) && ( ( const popover = useMemo(() => { return ( - ( panelPaddingSize={!alwaysShow ? 's' : 'none'} > {isOpen ? hoverContent : null} - + ); }, [content, onMouseLeave, isOpen, alwaysShow, hoverContent]); From 3a6c1ceeddfb51efed5cff163599ab17bcf78701 Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Fri, 8 May 2020 17:56:28 +0200 Subject: [PATCH 04/10] [Discover] Prevent whitespace wrapping of doc table header (#52861) Co-authored-by: Dave Snider --- .../angular/doc_table/components/_table_header.scss | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/plugins/discover/public/application/angular/doc_table/components/_table_header.scss b/src/plugins/discover/public/application/angular/doc_table/components/_table_header.scss index 099286f8c875c..9ea4e21632ace 100644 --- a/src/plugins/discover/public/application/angular/doc_table/components/_table_header.scss +++ b/src/plugins/discover/public/application/angular/doc_table/components/_table_header.scss @@ -1,3 +1,6 @@ +.kbnDocTableHeader { + white-space: nowrap; +} .kbnDocTableHeader button { margin-left: $euiSizeXS; } From 97561d6751a6f864d06697800a8fbf0d39d05966 Mon Sep 17 00:00:00 2001 From: Matthew Kime Date: Fri, 8 May 2020 11:53:42 -0500 Subject: [PATCH 05/10] restore index pattern management data-test-subj's (#64697) * restore index pattern management data-test-subj's --- .../edit_index_pattern/tabs/utils.ts | 3 +++ test/functional/page_objects/settings_page.ts | 16 +++++++--------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/tabs/utils.ts b/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/tabs/utils.ts index bdb1436c37efb..83335a6fabfeb 100644 --- a/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/tabs/utils.ts +++ b/src/legacy/core_plugins/kibana/public/management/sections/index_patterns/edit_index_pattern/tabs/utils.ts @@ -96,18 +96,21 @@ export function getTabs( tabs.push({ name: getTitle('indexed', filteredCount, totalCount), id: TAB_INDEXED_FIELDS, + 'data-test-subj': 'tab-indexedFields', }); if (indexPatternListProvider.areScriptedFieldsEnabled(indexPattern)) { tabs.push({ name: getTitle('scripted', filteredCount, totalCount), id: TAB_SCRIPTED_FIELDS, + 'data-test-subj': 'tab-scriptedFields', }); } tabs.push({ name: getTitle('sourceFilters', filteredCount, totalCount), id: TAB_SOURCE_FILTERS, + 'data-test-subj': 'tab-sourceFilters', }); return tabs; diff --git a/test/functional/page_objects/settings_page.ts b/test/functional/page_objects/settings_page.ts index 8175361ffc842..b8069b31257d3 100644 --- a/test/functional/page_objects/settings_page.ts +++ b/test/functional/page_objects/settings_page.ts @@ -206,17 +206,15 @@ export function SettingsPageProvider({ getService, getPageObjects }: FtrProvider async getFieldsTabCount() { return retry.try(async () => { - const indexedFieldsTab = await find.byCssSelector('#indexedFields .euiTab__content'); - const text = await indexedFieldsTab.getVisibleText(); - return text.split(/[()]/)[1]; + const text = await testSubjects.getVisibleText('tab-indexedFields'); + return text.split(' ')[1].replace(/\((.*)\)/, '$1'); }); } async getScriptedFieldsTabCount() { return await retry.try(async () => { - const scriptedFieldsTab = await find.byCssSelector('#scriptedFields .euiTab__content'); - const text = await scriptedFieldsTab.getVisibleText(); - return text.split(/[()]/)[1]; + const text = await testSubjects.getVisibleText('tab-scriptedFields'); + return text.split(' ')[2].replace(/\((.*)\)/, '$1'); }); } @@ -431,17 +429,17 @@ export function SettingsPageProvider({ getService, getPageObjects }: FtrProvider async clickFieldsTab() { log.debug('click Fields tab'); - await find.clickByCssSelector('#indexedFields'); + await testSubjects.click('tab-indexedFields'); } async clickScriptedFieldsTab() { log.debug('click Scripted Fields tab'); - await find.clickByCssSelector('#scriptedFields'); + await testSubjects.click('tab-scriptedFields'); } async clickSourceFiltersTab() { log.debug('click Source Filters tab'); - await find.clickByCssSelector('#sourceFilters'); + await testSubjects.click('tab-sourceFilters'); } async editScriptedField(name: string) { From d3b155f8437f1cf2dbe09ba30f667231dce386e0 Mon Sep 17 00:00:00 2001 From: Quynh Nguyen <43350163+qn895@users.noreply.github.com> Date: Fri, 8 May 2020 12:06:59 -0500 Subject: [PATCH 06/10] [ML] Add job timing stats to anomaly jobs (#65696) * [ML] Add anomaly job timing stats to Counts & JSON * [ML] Remove roundTo3DecimalPlace and clean up * [ML] Fix format_values to round decimals for time values * [ML] Remove timing_stats and forecast_stats from cloneJob * [ML] Remove timing_stats & forecasts in job_service instead of utils --- .../components/job_details/extract_job_details.js | 10 ++++++++++ .../jobs_list/components/job_details/format_values.js | 6 ++++++ .../jobs_list/components/job_details/job_details.js | 3 ++- .../ml/public/application/services/job_service.js | 2 ++ x-pack/plugins/ml/server/models/job_service/jobs.ts | 6 ++---- 5 files changed, 22 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/job_details/extract_job_details.js b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/job_details/extract_job_details.js index f7b0e726ecc53..fa36a0626d632 100644 --- a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/job_details/extract_job_details.js +++ b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/job_details/extract_job_details.js @@ -165,6 +165,15 @@ export function extractJobDetails(job) { items: filterObjects(job.model_size_stats).map(formatValues), }; + const jobTimingStats = { + id: 'jobTimingStats', + title: i18n.translate('xpack.ml.jobsList.jobDetails.jobTimingStatsTitle', { + defaultMessage: 'Job timing stats', + }), + position: 'left', + items: filterObjects(job.timing_stats).map(formatValues), + }; + const datafeedTimingStats = { id: 'datafeedTimingStats', title: i18n.translate('xpack.ml.jobsList.jobDetails.datafeedTimingStatsTitle', { @@ -192,6 +201,7 @@ export function extractJobDetails(job) { datafeed, counts, modelSizeStats, + jobTimingStats, datafeedTimingStats, }; } diff --git a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/job_details/format_values.js b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/job_details/format_values.js index 9984f3be299ae..246a476517ace 100644 --- a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/job_details/format_values.js +++ b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/job_details/format_values.js @@ -63,6 +63,12 @@ export function formatValues([key, value]) { // numbers rounded to 3 decimal places case 'average_search_time_per_bucket_ms': case 'exponential_average_search_time_per_hour_ms': + case 'total_bucket_processing_time_ms': + case 'minimum_bucket_processing_time_ms': + case 'maximum_bucket_processing_time_ms': + case 'average_bucket_processing_time_ms': + case 'exponential_average_bucket_processing_time_ms': + case 'exponential_average_bucket_processing_time_per_hour_ms': value = typeof value === 'number' ? roundToDecimalPlace(value, 3).toLocaleString() : value; break; diff --git a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/job_details/job_details.js b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/job_details/job_details.js index e3f348ad32b0c..0375997b86bb8 100644 --- a/x-pack/plugins/ml/public/application/jobs/jobs_list/components/job_details/job_details.js +++ b/x-pack/plugins/ml/public/application/jobs/jobs_list/components/job_details/job_details.js @@ -60,6 +60,7 @@ export class JobDetails extends Component { datafeed, counts, modelSizeStats, + jobTimingStats, datafeedTimingStats, } = extractJobDetails(job); @@ -102,7 +103,7 @@ export class JobDetails extends Component { content: ( ), }, diff --git a/x-pack/plugins/ml/public/application/services/job_service.js b/x-pack/plugins/ml/public/application/services/job_service.js index bbfec49ac1388..fb75476c48fa3 100644 --- a/x-pack/plugins/ml/public/application/services/job_service.js +++ b/x-pack/plugins/ml/public/application/services/job_service.js @@ -369,6 +369,8 @@ class JobService { delete tempJob.open_time; delete tempJob.established_model_memory; delete tempJob.calendars; + delete tempJob.timing_stats; + delete tempJob.forecasts_stats; delete tempJob.analysis_config.use_per_partition_normalization; diff --git a/x-pack/plugins/ml/server/models/job_service/jobs.ts b/x-pack/plugins/ml/server/models/job_service/jobs.ts index 6024ecf4925e6..225cd43e411a4 100644 --- a/x-pack/plugins/ml/server/models/job_service/jobs.ts +++ b/x-pack/plugins/ml/server/models/job_service/jobs.ts @@ -328,7 +328,7 @@ export function jobsProvider(callAsCurrentUser: APICaller) { // create jobs objects containing job stats, datafeeds, datafeed stats and calendars if (jobResults && jobResults.jobs) { jobResults.jobs.forEach(job => { - const tempJob = job as CombinedJobWithStats; + let tempJob = job as CombinedJobWithStats; const calendars: string[] = [ ...(calendarsByJobId[tempJob.job_id] || []), @@ -341,9 +341,7 @@ export function jobsProvider(callAsCurrentUser: APICaller) { if (jobStatsResults && jobStatsResults.jobs) { const jobStats = jobStatsResults.jobs.find(js => js.job_id === tempJob.job_id); if (jobStats !== undefined) { - tempJob.state = jobStats.state; - tempJob.data_counts = jobStats.data_counts; - tempJob.model_size_stats = jobStats.model_size_stats; + tempJob = { ...tempJob, ...jobStats }; if (jobStats.node) { tempJob.node = jobStats.node; } From 1c6e6cb7b7a3ac7522691989881cd74fd3b17992 Mon Sep 17 00:00:00 2001 From: Chris Cowan Date: Fri, 8 May 2020 10:55:43 -0700 Subject: [PATCH 07/10] [Metrics UI] Fix p95/p99 charts and alerting error (#65579) * [Metrics UI] Fix p95/p99 charts and alerting error - Fixes #65561 * Fixing open in visualize for percentiles * Adding test for P95; refactoring to use first consitently --- .../components/expression_chart.tsx | 5 +- .../components/expression_row.tsx | 16 ++++++ .../public/alerting/metric_threshold/types.ts | 2 + .../components/helpers/create_tsvb_link.ts | 18 +++++++ .../components/series_chart.tsx | 19 ++++--- .../create_percentile_aggregation.ts | 22 ++++++++ .../metric_threshold_executor.test.ts | 52 +++++++++++++++++++ .../metric_threshold_executor.ts | 18 +++++-- .../alerting/metric_threshold/test_mocks.ts | 8 +-- .../lib/alerting/metric_threshold/types.ts | 2 + 10 files changed, 144 insertions(+), 18 deletions(-) create mode 100644 x-pack/plugins/infra/server/lib/alerting/metric_threshold/create_percentile_aggregation.ts diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_chart.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_chart.tsx index 82e751627a05b..77147d1b3b2b7 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_chart.tsx +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_chart.tsx @@ -118,10 +118,7 @@ export const ExpressionChart: React.FC = ({ const series = { ...firstSeries, rows: firstSeries.rows.map(row => { - const newRow: MetricsExplorerRow = { - timestamp: row.timestamp, - metric_0: row.metric_0 || null, - }; + const newRow: MetricsExplorerRow = { ...row }; thresholds.forEach((thresholdValue, index) => { newRow[`metric_threshold_${index}`] = thresholdValue; }); diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_row.tsx b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_row.tsx index 8801df380b48d..be0f5f88a2b55 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_row.tsx +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/components/expression_row.tsx @@ -234,4 +234,20 @@ export const aggregationType: { [key: string]: any } = { value: AGGREGATION_TYPES.SUM, validNormalizedTypes: ['number'], }, + p95: { + text: i18n.translate('xpack.infra.metrics.alertFlyout.aggregationText.p95', { + defaultMessage: '95th Percentile', + }), + fieldRequired: false, + value: AGGREGATION_TYPES.P95, + validNormalizedTypes: ['number'], + }, + p99: { + text: i18n.translate('xpack.infra.metrics.alertFlyout.aggregationText.p99', { + defaultMessage: '99th Percentile', + }), + fieldRequired: false, + value: AGGREGATION_TYPES.P99, + validNormalizedTypes: ['number'], + }, }; diff --git a/x-pack/plugins/infra/public/alerting/metric_threshold/types.ts b/x-pack/plugins/infra/public/alerting/metric_threshold/types.ts index af3baf191bed2..e421455cf6efd 100644 --- a/x-pack/plugins/infra/public/alerting/metric_threshold/types.ts +++ b/x-pack/plugins/infra/public/alerting/metric_threshold/types.ts @@ -29,6 +29,8 @@ export enum AGGREGATION_TYPES { MAX = 'max', RATE = 'rate', CARDINALITY = 'cardinality', + P95 = 'p95', + P99 = 'p99', } export interface MetricThresholdAlertParams { diff --git a/x-pack/plugins/infra/public/pages/metrics/metrics_explorer/components/helpers/create_tsvb_link.ts b/x-pack/plugins/infra/public/pages/metrics/metrics_explorer/components/helpers/create_tsvb_link.ts index 559422584f579..f773c843d12fd 100644 --- a/x-pack/plugins/infra/public/pages/metrics/metrics_explorer/components/helpers/create_tsvb_link.ts +++ b/x-pack/plugins/infra/public/pages/metrics/metrics_explorer/components/helpers/create_tsvb_link.ts @@ -46,6 +46,24 @@ export const metricsExplorerMetricToTSVBMetric = (metric: MetricsExplorerOptions field: derivativeId, }, ]; + } else if (metric.aggregation === 'p95' || metric.aggregation === 'p99') { + const percentileValue = metric.aggregation === 'p95' ? '95' : '99'; + return [ + { + id: uuid.v1(), + type: 'percentile', + field: metric.field, + percentiles: [ + { + id: uuid.v1(), + value: percentileValue, + mode: 'line', + percentile: '', + shade: 0.2, + }, + ], + }, + ]; } else { return [ { diff --git a/x-pack/plugins/infra/public/pages/metrics/metrics_explorer/components/series_chart.tsx b/x-pack/plugins/infra/public/pages/metrics/metrics_explorer/components/series_chart.tsx index 3b84fcbc34836..223318da8cf46 100644 --- a/x-pack/plugins/infra/public/pages/metrics/metrics_explorer/components/series_chart.tsx +++ b/x-pack/plugins/infra/public/pages/metrics/metrics_explorer/components/series_chart.tsx @@ -20,6 +20,7 @@ import { MetricsExplorerOptionsMetric, MetricsExplorerChartType, } from '../hooks/use_metrics_explorer_options'; +import { getMetricId } from './helpers/get_metric_id'; type NumberOrString = string | number; @@ -45,10 +46,12 @@ export const MetricsExplorerAreaChart = ({ metric, id, series, type, stack, opac colorTransformer(MetricsExplorerColor.color0); const yAccessors = Array.isArray(id) - ? id.map(i => `metric_${i}`).slice(id.length - 1, id.length) - : [`metric_${id}`]; + ? id.map(i => getMetricId(metric, i)).slice(id.length - 1, id.length) + : [getMetricId(metric, id)]; const y0Accessors = - Array.isArray(id) && id.length > 1 ? id.map(i => `metric_${i}`).slice(0, 1) : undefined; + Array.isArray(id) && id.length > 1 + ? id.map(i => getMetricId(metric, i)).slice(0, 1) + : undefined; const chartId = `series-${series.id}-${yAccessors.join('-')}`; const seriesAreaStyle: RecursivePartial = { @@ -85,8 +88,10 @@ export const MetricsExplorerBarChart = ({ metric, id, series, stack }: Props) => (metric.color && colorTransformer(metric.color)) || colorTransformer(MetricsExplorerColor.color0); - const yAccessor = `metric_${id}`; - const chartId = `series-${series.id}-${yAccessor}`; + const yAccessors = Array.isArray(id) + ? id.map(i => getMetricId(metric, i)).slice(id.length - 1, id.length) + : [getMetricId(metric, id)]; + const chartId = `series-${series.id}-${yAccessors.join('-')}`; const seriesBarStyle: RecursivePartial = { rectBorder: { @@ -100,13 +105,13 @@ export const MetricsExplorerBarChart = ({ metric, id, series, stack }: Props) => }; return ( { + const value = type === Aggregators.P95 ? 95 : 99; + return { + aggregatedValue: { + percentiles: { + field, + percents: [value], + keyed: false, + }, + }, + }; +}; diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts index 2531e939792af..ed5efc1473953 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.test.ts @@ -233,6 +233,58 @@ describe('The metric threshold alert type', () => { expect(getState(instanceID).alertState).toBe(AlertStates.OK); }); }); + describe('querying with the p99 aggregator', () => { + const instanceID = 'test-*'; + const execute = (comparator: Comparator, threshold: number[]) => + executor({ + services, + params: { + criteria: [ + { + ...baseCriterion, + comparator, + threshold, + aggType: 'p99', + metric: 'test.metric.2', + }, + ], + }, + }); + test('alerts based on the p99 values', async () => { + await execute(Comparator.GT, [1]); + expect(mostRecentAction(instanceID).id).toBe(FIRED_ACTIONS.id); + expect(getState(instanceID).alertState).toBe(AlertStates.ALERT); + await execute(Comparator.LT, [1]); + expect(mostRecentAction(instanceID)).toBe(undefined); + expect(getState(instanceID).alertState).toBe(AlertStates.OK); + }); + }); + describe('querying with the p95 aggregator', () => { + const instanceID = 'test-*'; + const execute = (comparator: Comparator, threshold: number[]) => + executor({ + services, + params: { + criteria: [ + { + ...baseCriterion, + comparator, + threshold, + aggType: 'p95', + metric: 'test.metric.1', + }, + ], + }, + }); + test('alerts based on the p95 values', async () => { + await execute(Comparator.GT, [0.25]); + expect(mostRecentAction(instanceID).id).toBe(FIRED_ACTIONS.id); + expect(getState(instanceID).alertState).toBe(AlertStates.ALERT); + await execute(Comparator.LT, [0.95]); + expect(mostRecentAction(instanceID)).toBe(undefined); + expect(getState(instanceID).alertState).toBe(AlertStates.OK); + }); + }); describe("querying a metric that hasn't reported data", () => { const instanceID = 'test-*'; const execute = (alertOnNoData: boolean) => diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts index ec9389537835b..71bee3209bf53 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/metric_threshold_executor.ts @@ -3,7 +3,7 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { mapValues } from 'lodash'; +import { mapValues, first } from 'lodash'; import { i18n } from '@kbn/i18n'; import { InfraDatabaseSearchResponse } from '../../adapters/framework/adapter_types'; import { createAfterKeyHandler } from '../../../utils/create_afterkey_handler'; @@ -21,12 +21,16 @@ import { AlertServices, AlertExecutorOptions } from '../../../../../alerting/ser import { getIntervalInSeconds } from '../../../utils/get_interval_in_seconds'; import { getDateHistogramOffset } from '../../snapshot/query_helpers'; import { InfraBackendLibs } from '../../infra_types'; +import { createPercentileAggregation } from './create_percentile_aggregation'; const TOTAL_BUCKETS = 5; interface Aggregation { aggregatedIntervals: { - buckets: Array<{ aggregatedValue: { value: number }; doc_count: number }>; + buckets: Array<{ + aggregatedValue: { value: number; values?: Array<{ key: number; value: number }> }; + doc_count: number; + }>; }; } @@ -47,6 +51,12 @@ const getCurrentValueFromAggregations = ( if (aggType === Aggregators.COUNT) { return mostRecentBucket.doc_count; } + if (aggType === Aggregators.P95 || aggType === Aggregators.P99) { + const values = mostRecentBucket.aggregatedValue?.values || []; + const firstValue = first(values); + if (!firstValue) return null; + return firstValue.value; + } const { value } = mostRecentBucket.aggregatedValue; return value; } catch (e) { @@ -86,6 +96,8 @@ export const getElasticsearchMetricQuery = ( ? {} : aggType === Aggregators.RATE ? networkTraffic('aggregatedValue', metric) + : aggType === Aggregators.P95 || aggType === Aggregators.P99 + ? createPercentileAggregation(aggType, metric) : { aggregatedValue: { [aggType]: { @@ -275,7 +287,7 @@ export const createMetricThresholdExecutor = (libs: InfraBackendLibs, alertId: s ); // Because each alert result has the same group definitions, just grap the groups from the first one. - const groups = Object.keys(alertResults[0]); + const groups = Object.keys(first(alertResults)); for (const group of groups) { const alertInstance = services.alertInstanceFactory(`${alertId}-${group}`); diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/test_mocks.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/test_mocks.ts index fa55f80e472de..25b709d6afc51 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/test_mocks.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/test_mocks.ts @@ -7,22 +7,22 @@ const bucketsA = [ { doc_count: 2, - aggregatedValue: { value: 0.5 }, + aggregatedValue: { value: 0.5, values: [{ key: 95.0, value: 0.5 }] }, }, { doc_count: 3, - aggregatedValue: { value: 1.0 }, + aggregatedValue: { value: 1.0, values: [{ key: 95.0, value: 1.0 }] }, }, ]; const bucketsB = [ { doc_count: 4, - aggregatedValue: { value: 2.5 }, + aggregatedValue: { value: 2.5, values: [{ key: 99.0, value: 2.5 }] }, }, { doc_count: 5, - aggregatedValue: { value: 3.5 }, + aggregatedValue: { value: 3.5, values: [{ key: 99.0, value: 3.5 }] }, }, ]; diff --git a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/types.ts b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/types.ts index 18f5503fe2c9e..76ddd107bd728 100644 --- a/x-pack/plugins/infra/server/lib/alerting/metric_threshold/types.ts +++ b/x-pack/plugins/infra/server/lib/alerting/metric_threshold/types.ts @@ -23,6 +23,8 @@ export enum Aggregators { MAX = 'max', RATE = 'rate', CARDINALITY = 'cardinality', + P95 = 'p95', + P99 = 'p99', } export enum AlertStates { From 808e02564b67a89b9098c098eb57af841afd7152 Mon Sep 17 00:00:00 2001 From: Christos Nasikas Date: Fri, 8 May 2020 21:24:44 +0300 Subject: [PATCH 08/10] [SIEM][CASE] Moves functional tests from "legacyEs" to "Es" (#65851) --- .../basic/tests/configure/get_configure.ts | 2 +- .../basic/tests/configure/patch_configure.ts | 2 +- .../basic/tests/configure/post_configure.ts | 2 +- x-pack/test/case_api_integration/common/lib/utils.ts | 7 ++++--- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/x-pack/test/case_api_integration/basic/tests/configure/get_configure.ts b/x-pack/test/case_api_integration/basic/tests/configure/get_configure.ts index a9fc2706a6ba2..930cf42b9efc5 100644 --- a/x-pack/test/case_api_integration/basic/tests/configure/get_configure.ts +++ b/x-pack/test/case_api_integration/basic/tests/configure/get_configure.ts @@ -18,7 +18,7 @@ import { // eslint-disable-next-line import/no-default-export export default ({ getService }: FtrProviderContext): void => { const supertest = getService('supertest'); - const es = getService('legacyEs'); + const es = getService('es'); describe('get_configure', () => { afterEach(async () => { diff --git a/x-pack/test/case_api_integration/basic/tests/configure/patch_configure.ts b/x-pack/test/case_api_integration/basic/tests/configure/patch_configure.ts index d66baa2a2eee2..5f7d7c4c5c346 100644 --- a/x-pack/test/case_api_integration/basic/tests/configure/patch_configure.ts +++ b/x-pack/test/case_api_integration/basic/tests/configure/patch_configure.ts @@ -18,7 +18,7 @@ import { // eslint-disable-next-line import/no-default-export export default ({ getService }: FtrProviderContext): void => { const supertest = getService('supertest'); - const es = getService('legacyEs'); + const es = getService('es'); describe('post_configure', () => { afterEach(async () => { diff --git a/x-pack/test/case_api_integration/basic/tests/configure/post_configure.ts b/x-pack/test/case_api_integration/basic/tests/configure/post_configure.ts index c2284492e5b77..86214a2bd3187 100644 --- a/x-pack/test/case_api_integration/basic/tests/configure/post_configure.ts +++ b/x-pack/test/case_api_integration/basic/tests/configure/post_configure.ts @@ -18,7 +18,7 @@ import { // eslint-disable-next-line import/no-default-export export default ({ getService }: FtrProviderContext): void => { const supertest = getService('supertest'); - const es = getService('legacyEs'); + const es = getService('es'); describe('post_configure', () => { afterEach(async () => { diff --git a/x-pack/test/case_api_integration/common/lib/utils.ts b/x-pack/test/case_api_integration/common/lib/utils.ts index 6d0db69309b90..df768ff09b368 100644 --- a/x-pack/test/case_api_integration/common/lib/utils.ts +++ b/x-pack/test/case_api_integration/common/lib/utils.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { Client } from '@elastic/elasticsearch'; import { CasesConfigureRequest, CasesConfigureResponse } from '../../../../plugins/case/common/api'; export const getConfiguration = (connector_id: string = 'connector-1'): CasesConfigureRequest => { @@ -29,12 +30,12 @@ export const removeServerGeneratedPropertiesFromConfigure = ( return rest; }; -export const deleteConfiguration = async (es: any): Promise => { +export const deleteConfiguration = async (es: Client): Promise => { await es.deleteByQuery({ index: '.kibana', q: 'type:cases-configure', - waitForCompletion: true, - refresh: 'wait_for', + wait_for_completion: true, + refresh: true, body: {}, }); }; From 4d3261083b30567ff81b3fa132259872c27eaab9 Mon Sep 17 00:00:00 2001 From: Frank Hassanabad Date: Fri, 8 May 2020 13:26:31 -0600 Subject: [PATCH 09/10] [SIEM][Detection Engine] Increases the template limit for ECS mappings ## Summary Increases the template limit for ECS mappings from default of 1k to 10k. This mirrors auditbeat, winlogbeat, filebeat, etc.. - [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios ### For maintainers --- .../routes/index/get_signals_template.test.ts | 17 +++++++++++++++-- .../routes/index/get_signals_template.ts | 5 +++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/index/get_signals_template.test.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/index/get_signals_template.test.ts index 80594ca74a353..30362392898d1 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/index/get_signals_template.test.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/index/get_signals_template.test.ts @@ -7,10 +7,18 @@ import { getSignalsTemplate } from './get_signals_template'; describe('get_signals_template', () => { - test('it should set the lifecycle name and the rollover alias to be the name of the index passed in', () => { + test('it should set the lifecycle "name" and "rollover_alias" to be the name of the index passed in', () => { const template = getSignalsTemplate('test-index'); expect(template.settings).toEqual({ - index: { lifecycle: { name: 'test-index', rollover_alias: 'test-index' } }, + index: { + lifecycle: { + name: 'test-index', + rollover_alias: 'test-index', + }, + }, + mapping: { + total_fields: { limit: 10000 }, + }, }); }); @@ -28,4 +36,9 @@ describe('get_signals_template', () => { const template = getSignalsTemplate('test-index'); expect(typeof template.mappings.properties.signal).toEqual('object'); }); + + test('it should have a "total_fields" section that is at least 10k in size', () => { + const template = getSignalsTemplate('test-index'); + expect(template.settings.mapping.total_fields.limit).toBeGreaterThanOrEqual(10000); + }); }); diff --git a/x-pack/plugins/siem/server/lib/detection_engine/routes/index/get_signals_template.ts b/x-pack/plugins/siem/server/lib/detection_engine/routes/index/get_signals_template.ts index c6580f0bdda42..01d7182e253ce 100644 --- a/x-pack/plugins/siem/server/lib/detection_engine/routes/index/get_signals_template.ts +++ b/x-pack/plugins/siem/server/lib/detection_engine/routes/index/get_signals_template.ts @@ -17,6 +17,11 @@ export const getSignalsTemplate = (index: string) => { rollover_alias: index, }, }, + mapping: { + total_fields: { + limit: 10000, + }, + }, }, index_patterns: [`${index}-*`], mappings: ecsMapping.mappings, From 04f37364fd228ab429bede7eb13768e4ed7f264b Mon Sep 17 00:00:00 2001 From: Nathan L Smith Date: Fri, 8 May 2020 16:16:15 -0500 Subject: [PATCH 10/10] Fix anomalies display on focused APM service map (#65882) The map anomlies rings display was working on the global map and on the focused service of a focused map, but not on the other services on a focused map. This is because we were adding the anomlies to the list of services from the initial query, but not to the list of services derived from the connections data. Make the transformation that add anomalies happen after the derived services nodes are added. This is done in the function that was called `dedupeConnections`, but since it does much more than dedupe connections has been renamed to `transformServiceMapResponses`. Also make the node types extend `cytoscape.NodeDataDefinition` in order to simplify the types in the transformation (we were adding `& { id: string }` in some places which this replaces.) Fixes #65403. --- x-pack/plugins/apm/common/service_map.ts | 9 +-- .../server/lib/service_map/get_service_map.ts | 17 ++--- .../server/lib/service_map/ml_helpers.test.ts | 12 ++-- .../apm/server/lib/service_map/ml_helpers.ts | 9 +-- ...> transform_service_map_responses.test.ts} | 37 +++++++---- ....ts => transform_service_map_responses.ts} | 66 +++++++++++-------- 6 files changed, 85 insertions(+), 65 deletions(-) rename x-pack/plugins/apm/server/lib/service_map/{dedupe_connections/index.test.ts => transform_service_map_responses.test.ts} (83%) rename x-pack/plugins/apm/server/lib/service_map/{dedupe_connections/index.ts => transform_service_map_responses.ts} (76%) diff --git a/x-pack/plugins/apm/common/service_map.ts b/x-pack/plugins/apm/common/service_map.ts index dc457c38a52af..87db0005fb656 100644 --- a/x-pack/plugins/apm/common/service_map.ts +++ b/x-pack/plugins/apm/common/service_map.ts @@ -5,22 +5,23 @@ */ import { i18n } from '@kbn/i18n'; +import cytoscape from 'cytoscape'; import { ILicense } from '../../licensing/public'; import { AGENT_NAME, SERVICE_ENVIRONMENT, SERVICE_NAME, + SPAN_DESTINATION_SERVICE_RESOURCE, SPAN_SUBTYPE, - SPAN_TYPE, - SPAN_DESTINATION_SERVICE_RESOURCE + SPAN_TYPE } from './elasticsearch_fieldnames'; -export interface ServiceConnectionNode { +export interface ServiceConnectionNode extends cytoscape.NodeDataDefinition { [SERVICE_NAME]: string; [SERVICE_ENVIRONMENT]: string | null; [AGENT_NAME]: string; } -export interface ExternalConnectionNode { +export interface ExternalConnectionNode extends cytoscape.NodeDataDefinition { [SPAN_DESTINATION_SERVICE_RESOURCE]: string; [SPAN_TYPE]: string; [SPAN_SUBTYPE]: string; diff --git a/x-pack/plugins/apm/server/lib/service_map/get_service_map.ts b/x-pack/plugins/apm/server/lib/service_map/get_service_map.ts index 7d5f0a75d2208..8fb44b70bc081 100644 --- a/x-pack/plugins/apm/server/lib/service_map/get_service_map.ts +++ b/x-pack/plugins/apm/server/lib/service_map/get_service_map.ts @@ -9,16 +9,15 @@ import { SERVICE_ENVIRONMENT, SERVICE_NAME } from '../../../common/elasticsearch_fieldnames'; +import { getMlIndex } from '../../../common/ml_job_constants'; import { getServicesProjection } from '../../../common/projections/services'; import { mergeProjection } from '../../../common/projections/util/merge_projection'; import { PromiseReturnType } from '../../../typings/common'; +import { rangeFilter } from '../helpers/range_filter'; import { Setup, SetupTimeRange } from '../helpers/setup_request'; -import { dedupeConnections } from './dedupe_connections'; +import { transformServiceMapResponses } from './transform_service_map_responses'; import { getServiceMapFromTraceIds } from './get_service_map_from_trace_ids'; import { getTraceSampleIds } from './get_trace_sample_ids'; -import { addAnomaliesToServicesData } from './ml_helpers'; -import { getMlIndex } from '../../../common/ml_job_constants'; -import { rangeFilter } from '../helpers/range_filter'; export interface IEnvOptions { setup: Setup & SetupTimeRange; @@ -179,13 +178,9 @@ export async function getServiceMap(options: IEnvOptions) { getAnomaliesData(options) ]); - const servicesDataWithAnomalies = addAnomaliesToServicesData( - servicesData, - anomaliesData - ); - - return dedupeConnections({ + return transformServiceMapResponses({ ...connectionData, - services: servicesDataWithAnomalies + anomalies: anomaliesData, + services: servicesData }); } diff --git a/x-pack/plugins/apm/server/lib/service_map/ml_helpers.test.ts b/x-pack/plugins/apm/server/lib/service_map/ml_helpers.test.ts index c80ba8dba01ea..908dbe6df4636 100644 --- a/x-pack/plugins/apm/server/lib/service_map/ml_helpers.test.ts +++ b/x-pack/plugins/apm/server/lib/service_map/ml_helpers.test.ts @@ -5,11 +5,11 @@ */ import { AnomaliesResponse } from './get_service_map'; -import { addAnomaliesToServicesData } from './ml_helpers'; +import { addAnomaliesDataToNodes } from './ml_helpers'; -describe('addAnomaliesToServicesData', () => { - it('adds anomalies to services data', () => { - const servicesData = [ +describe('addAnomaliesDataToNodes', () => { + it('adds anomalies to nodes', () => { + const nodes = [ { 'service.name': 'opbeans-ruby', 'agent.name': 'ruby', @@ -89,8 +89,8 @@ describe('addAnomaliesToServicesData', () => { ]; expect( - addAnomaliesToServicesData( - servicesData, + addAnomaliesDataToNodes( + nodes, (anomaliesResponse as unknown) as AnomaliesResponse ) ).toEqual(result); diff --git a/x-pack/plugins/apm/server/lib/service_map/ml_helpers.ts b/x-pack/plugins/apm/server/lib/service_map/ml_helpers.ts index 9789911660bd0..fae9e7d4cb1c6 100644 --- a/x-pack/plugins/apm/server/lib/service_map/ml_helpers.ts +++ b/x-pack/plugins/apm/server/lib/service_map/ml_helpers.ts @@ -9,10 +9,11 @@ import { getMlJobServiceName, getSeverity } from '../../../common/ml_job_constants'; -import { AnomaliesResponse, ServicesResponse } from './get_service_map'; +import { ConnectionNode } from '../../../common/service_map'; +import { AnomaliesResponse } from './get_service_map'; -export function addAnomaliesToServicesData( - servicesData: ServicesResponse, +export function addAnomaliesDataToNodes( + nodes: ConnectionNode[], anomaliesResponse: AnomaliesResponse ) { const anomaliesMap = ( @@ -52,7 +53,7 @@ export function addAnomaliesToServicesData( }; }, {}); - const servicesDataWithAnomalies = servicesData.map(service => { + const servicesDataWithAnomalies = nodes.map(service => { const serviceAnomalies = anomaliesMap[service[SERVICE_NAME]]; if (serviceAnomalies) { const maxScore = serviceAnomalies.max_score; diff --git a/x-pack/plugins/apm/server/lib/service_map/dedupe_connections/index.test.ts b/x-pack/plugins/apm/server/lib/service_map/transform_service_map_responses.test.ts similarity index 83% rename from x-pack/plugins/apm/server/lib/service_map/dedupe_connections/index.test.ts rename to x-pack/plugins/apm/server/lib/service_map/transform_service_map_responses.test.ts index 4af8a54139204..45b64c1ad03a4 100644 --- a/x-pack/plugins/apm/server/lib/service_map/dedupe_connections/index.test.ts +++ b/x-pack/plugins/apm/server/lib/service_map/transform_service_map_responses.test.ts @@ -4,16 +4,19 @@ * you may not use this file except in compliance with the Elastic License. */ -import { ServiceMapResponse } from './'; import { - SPAN_DESTINATION_SERVICE_RESOURCE, - SERVICE_NAME, - SERVICE_ENVIRONMENT, AGENT_NAME, - SPAN_TYPE, - SPAN_SUBTYPE -} from '../../../../common/elasticsearch_fieldnames'; -import { dedupeConnections } from './'; + SERVICE_ENVIRONMENT, + SERVICE_NAME, + SPAN_DESTINATION_SERVICE_RESOURCE, + SPAN_SUBTYPE, + SPAN_TYPE +} from '../../../common/elasticsearch_fieldnames'; +import { AnomaliesResponse } from './get_service_map'; +import { + transformServiceMapResponses, + ServiceMapResponse +} from './transform_service_map_responses'; const nodejsService = { [SERVICE_NAME]: 'opbeans-node', @@ -33,9 +36,14 @@ const javaService = { [AGENT_NAME]: 'java' }; -describe('dedupeConnections', () => { +const anomalies = ({ + aggregations: { jobs: { buckets: [] } } +} as unknown) as AnomaliesResponse; + +describe('transformServiceMapResponses', () => { it('maps external destinations to internal services', () => { const response: ServiceMapResponse = { + anomalies, services: [nodejsService, javaService], discoveredServices: [ { @@ -51,7 +59,7 @@ describe('dedupeConnections', () => { ] }; - const { elements } = dedupeConnections(response); + const { elements } = transformServiceMapResponses(response); const connection = elements.find( element => 'source' in element.data && 'target' in element.data @@ -67,6 +75,7 @@ describe('dedupeConnections', () => { it('collapses external destinations based on span.destination.resource.name', () => { const response: ServiceMapResponse = { + anomalies, services: [nodejsService, javaService], discoveredServices: [ { @@ -89,7 +98,7 @@ describe('dedupeConnections', () => { ] }; - const { elements } = dedupeConnections(response); + const { elements } = transformServiceMapResponses(response); const connections = elements.filter(element => 'source' in element.data); @@ -102,6 +111,7 @@ describe('dedupeConnections', () => { it('picks the first span.type/subtype in an alphabetically sorted list', () => { const response: ServiceMapResponse = { + anomalies, services: [javaService], discoveredServices: [], connections: [ @@ -126,7 +136,7 @@ describe('dedupeConnections', () => { ] }; - const { elements } = dedupeConnections(response); + const { elements } = transformServiceMapResponses(response); const nodes = elements.filter(element => !('source' in element.data)); @@ -140,6 +150,7 @@ describe('dedupeConnections', () => { it('processes connections without a matching "service" aggregation', () => { const response: ServiceMapResponse = { + anomalies, services: [javaService], discoveredServices: [], connections: [ @@ -150,7 +161,7 @@ describe('dedupeConnections', () => { ] }; - const { elements } = dedupeConnections(response); + const { elements } = transformServiceMapResponses(response); expect(elements.length).toBe(3); }); diff --git a/x-pack/plugins/apm/server/lib/service_map/dedupe_connections/index.ts b/x-pack/plugins/apm/server/lib/service_map/transform_service_map_responses.ts similarity index 76% rename from x-pack/plugins/apm/server/lib/service_map/dedupe_connections/index.ts rename to x-pack/plugins/apm/server/lib/service_map/transform_service_map_responses.ts index e5d7c0b2de10c..8b91bb98b5200 100644 --- a/x-pack/plugins/apm/server/lib/service_map/dedupe_connections/index.ts +++ b/x-pack/plugins/apm/server/lib/service_map/transform_service_map_responses.ts @@ -10,14 +10,19 @@ import { SPAN_DESTINATION_SERVICE_RESOURCE, SPAN_TYPE, SPAN_SUBTYPE -} from '../../../../common/elasticsearch_fieldnames'; +} from '../../../common/elasticsearch_fieldnames'; import { Connection, ConnectionNode, ServiceConnectionNode, ExternalConnectionNode -} from '../../../../common/service_map'; -import { ConnectionsResponse, ServicesResponse } from '../get_service_map'; +} from '../../../common/service_map'; +import { + ConnectionsResponse, + ServicesResponse, + AnomaliesResponse +} from './get_service_map'; +import { addAnomaliesDataToNodes } from './ml_helpers'; function getConnectionNodeId(node: ConnectionNode): string { if ('span.destination.service.resource' in node) { @@ -34,13 +39,16 @@ function getConnectionId(connection: Connection) { } export type ServiceMapResponse = ConnectionsResponse & { + anomalies: AnomaliesResponse; services: ServicesResponse; }; -export function dedupeConnections(response: ServiceMapResponse) { - const { discoveredServices, services, connections } = response; +export function transformServiceMapResponses(response: ServiceMapResponse) { + const { anomalies, discoveredServices, services, connections } = response; - const allNodes = connections + // Derive the rest of the map nodes from the connections and add the services + // from the services data query + const allNodes: ConnectionNode[] = connections .flatMap(connection => [connection.source, connection.destination]) .map(node => ({ ...node, id: getConnectionNodeId(node) })) .concat( @@ -50,25 +58,21 @@ export function dedupeConnections(response: ServiceMapResponse) { })) ); - const serviceNodes = allNodes.filter(node => SERVICE_NAME in node) as Array< - ServiceConnectionNode & { - id: string; - } - >; + // List of nodes that are services + const serviceNodes = allNodes.filter( + node => SERVICE_NAME in node + ) as ServiceConnectionNode[]; + // List of nodes that are externals const externalNodes = allNodes.filter( node => SPAN_DESTINATION_SERVICE_RESOURCE in node - ) as Array< - ExternalConnectionNode & { - id: string; - } - >; + ) as ExternalConnectionNode[]; - // 1. maps external nodes to internal services - // 2. collapses external nodes into one node based on span.destination.service.resource - // 3. picks the first available span.type/span.subtype in an alphabetically sorted list + // 1. Map external nodes to internal services + // 2. Collapse external nodes into one node based on span.destination.service.resource + // 3. Pick the first available span.type/span.subtype in an alphabetically sorted list const nodeMap = allNodes.reduce((map, node) => { - if (map[node.id]) { + if (!node.id || map[node.id]) { return map; } @@ -119,14 +123,14 @@ export function dedupeConnections(response: ServiceMapResponse) { .sort()[0] } }; - }, {} as Record); + }, {} as Record); - // maps destination.address to service.name if possible + // Map destination.address to service.name if possible function getConnectionNode(node: ConnectionNode) { return nodeMap[getConnectionNodeId(node)]; } - // build connections with mapped nodes + // Build connections with mapped nodes const mappedConnections = connections .map(connection => { const sourceData = getConnectionNode(connection.source); @@ -166,7 +170,7 @@ export function dedupeConnections(response: ServiceMapResponse) { {} as Record ); - // instead of adding connections in two directions, + // Instead of adding connections in two directions, // we add a `bidirectional` flag to use in styling const dedupedConnections = (sortBy( Object.values(connectionsById), @@ -192,10 +196,18 @@ export function dedupeConnections(response: ServiceMapResponse) { return prev.concat(connection); }, []); + // Add anomlies data + const dedupedNodesWithAnomliesData = addAnomaliesDataToNodes( + dedupedNodes, + anomalies + ); + // Put everything together in elements, with everything in the "data" property - const elements = [...dedupedConnections, ...dedupedNodes].map(element => ({ - data: element - })); + const elements = [...dedupedConnections, ...dedupedNodesWithAnomliesData].map( + element => ({ + data: element + }) + ); return { elements }; }