From f4276af2a9d7efbece28fef9852eb794056e0479 Mon Sep 17 00:00:00 2001 From: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Date: Wed, 21 Jul 2021 07:26:05 -0400 Subject: [PATCH 1/2] [data.search] Set default expiration to 1m if search sessions are disabled (#105329) (#106367) Co-authored-by: Anton Dosov Co-authored-by: Liza K Co-authored-by: Lukas Olson Co-authored-by: Anton Dosov Co-authored-by: Liza K --- .../data/server/search/session/mocks.ts | 3 +- .../eql_search/eql_search_strategy.ts | 4 +- .../ese_search/ese_search_strategy.test.ts | 2 +- .../ese_search/ese_search_strategy.ts | 2 +- .../ese_search/request_utils.test.ts | 153 ++++++++++++++++++ .../strategies/ese_search/request_utils.ts | 40 +++-- 6 files changed, 184 insertions(+), 20 deletions(-) create mode 100644 src/plugins/data/server/search/strategies/ese_search/request_utils.test.ts diff --git a/src/plugins/data/server/search/session/mocks.ts b/src/plugins/data/server/search/session/mocks.ts index 4deaecbf8056d2..ec99853088f784 100644 --- a/src/plugins/data/server/search/session/mocks.ts +++ b/src/plugins/data/server/search/session/mocks.ts @@ -27,7 +27,8 @@ export function createSearchSessionsClientMock(): jest.Mocked< getConfig: jest.fn( () => (({ - defaultExpiration: moment.duration('1', 'm'), + defaultExpiration: moment.duration('1', 'w'), + enabled: true, } as unknown) as SearchSessionsConfigSchema) ), }; diff --git a/src/plugins/data/server/search/strategies/eql_search/eql_search_strategy.ts b/src/plugins/data/server/search/strategies/eql_search/eql_search_strategy.ts index 91de0fca3674c9..4c75d62f121901 100644 --- a/src/plugins/data/server/search/strategies/eql_search/eql_search_strategy.ts +++ b/src/plugins/data/server/search/strategies/eql_search/eql_search_strategy.ts @@ -45,11 +45,11 @@ export const eqlSearchStrategyProvider = ( uiSettingsClient ); const params = id - ? getDefaultAsyncGetParams(options) + ? getDefaultAsyncGetParams(null, options) : { ...(await getIgnoreThrottled(uiSettingsClient)), ...defaultParams, - ...getDefaultAsyncGetParams(options), + ...getDefaultAsyncGetParams(null, options), ...request.params, }; const promise = id diff --git a/src/plugins/data/server/search/strategies/ese_search/ese_search_strategy.test.ts b/src/plugins/data/server/search/strategies/ese_search/ese_search_strategy.test.ts index 56b26a7ebe02cc..7a1ef2fe0a48b5 100644 --- a/src/plugins/data/server/search/strategies/ese_search/ese_search_strategy.test.ts +++ b/src/plugins/data/server/search/strategies/ese_search/ese_search_strategy.test.ts @@ -171,7 +171,7 @@ describe('ES search strategy', () => { expect(request.index).toEqual(params.index); expect(request.body).toEqual(params.body); - expect(request).toHaveProperty('keep_alive', '60000ms'); + expect(request).toHaveProperty('keep_alive', '604800000ms'); }); it('makes a GET request to async search without keepalive', async () => { diff --git a/src/plugins/data/server/search/strategies/ese_search/ese_search_strategy.ts b/src/plugins/data/server/search/strategies/ese_search/ese_search_strategy.ts index d6af00ada80fa6..271032a9e1e270 100644 --- a/src/plugins/data/server/search/strategies/ese_search/ese_search_strategy.ts +++ b/src/plugins/data/server/search/strategies/ese_search/ese_search_strategy.ts @@ -59,7 +59,7 @@ export const enhancedEsSearchStrategyProvider = ( const search = async () => { const params = id - ? getDefaultAsyncGetParams(options) + ? getDefaultAsyncGetParams(searchSessionsClient.getConfig(), options) : { ...(await getDefaultAsyncSubmitParams( uiSettingsClient, diff --git a/src/plugins/data/server/search/strategies/ese_search/request_utils.test.ts b/src/plugins/data/server/search/strategies/ese_search/request_utils.test.ts new file mode 100644 index 00000000000000..272e41e8bf82d4 --- /dev/null +++ b/src/plugins/data/server/search/strategies/ese_search/request_utils.test.ts @@ -0,0 +1,153 @@ +/* + * 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 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { + getDefaultAsyncSubmitParams, + getDefaultAsyncGetParams, + getIgnoreThrottled, +} from './request_utils'; +import { IUiSettingsClient } from 'kibana/server'; +import { UI_SETTINGS } from '../../../../common'; +import moment from 'moment'; +import { SearchSessionsConfigSchema } from '../../../../config'; + +const getMockUiSettingsClient = (config: Record) => { + return { get: async (key: string) => config[key] } as IUiSettingsClient; +}; + +const getMockSearchSessionsConfig = ({ + enabled = true, + defaultExpiration = moment.duration(7, 'd'), +} = {}) => + ({ + enabled, + defaultExpiration, + } as SearchSessionsConfigSchema); + +describe('request utils', () => { + describe('getIgnoreThrottled', () => { + test('returns `ignore_throttled` as `true` when `includeFrozen` is `false`', async () => { + const mockUiSettingsClient = getMockUiSettingsClient({ + [UI_SETTINGS.SEARCH_INCLUDE_FROZEN]: false, + }); + const result = await getIgnoreThrottled(mockUiSettingsClient); + expect(result.ignore_throttled).toBe(true); + }); + + test('returns `ignore_throttled` as `false` when `includeFrozen` is `true`', async () => { + const mockUiSettingsClient = getMockUiSettingsClient({ + [UI_SETTINGS.SEARCH_INCLUDE_FROZEN]: true, + }); + const result = await getIgnoreThrottled(mockUiSettingsClient); + expect(result.ignore_throttled).toBe(false); + }); + }); + + describe('getDefaultAsyncSubmitParams', () => { + test('Uses `keep_alive` from default params if no `sessionId` is provided', async () => { + const mockUiSettingsClient = getMockUiSettingsClient({ + [UI_SETTINGS.SEARCH_INCLUDE_FROZEN]: false, + }); + const mockConfig = getMockSearchSessionsConfig({ + defaultExpiration: moment.duration(3, 'd'), + }); + const params = await getDefaultAsyncSubmitParams(mockUiSettingsClient, mockConfig, {}); + expect(params).toHaveProperty('keep_alive', '1m'); + }); + + test('Uses `keep_alive` from config if enabled', async () => { + const mockUiSettingsClient = getMockUiSettingsClient({ + [UI_SETTINGS.SEARCH_INCLUDE_FROZEN]: false, + }); + const mockConfig = getMockSearchSessionsConfig({ + defaultExpiration: moment.duration(3, 'd'), + }); + const params = await getDefaultAsyncSubmitParams(mockUiSettingsClient, mockConfig, { + sessionId: 'foo', + }); + expect(params).toHaveProperty('keep_alive', '259200000ms'); + }); + + test('Uses `keepAlive` of `1m` if disabled', async () => { + const mockUiSettingsClient = getMockUiSettingsClient({ + [UI_SETTINGS.SEARCH_INCLUDE_FROZEN]: false, + }); + const mockConfig = getMockSearchSessionsConfig({ + defaultExpiration: moment.duration(3, 'd'), + enabled: false, + }); + const params = await getDefaultAsyncSubmitParams(mockUiSettingsClient, mockConfig, { + sessionId: 'foo', + }); + expect(params).toHaveProperty('keep_alive', '1m'); + }); + + test('Uses `keep_on_completion` if enabled', async () => { + const mockUiSettingsClient = getMockUiSettingsClient({ + [UI_SETTINGS.SEARCH_INCLUDE_FROZEN]: false, + }); + const mockConfig = getMockSearchSessionsConfig({}); + const params = await getDefaultAsyncSubmitParams(mockUiSettingsClient, mockConfig, { + sessionId: 'foo', + }); + expect(params).toHaveProperty('keep_on_completion', true); + }); + + test('Does not use `keep_on_completion` if disabled', async () => { + const mockUiSettingsClient = getMockUiSettingsClient({ + [UI_SETTINGS.SEARCH_INCLUDE_FROZEN]: false, + }); + const mockConfig = getMockSearchSessionsConfig({ + defaultExpiration: moment.duration(3, 'd'), + enabled: false, + }); + const params = await getDefaultAsyncSubmitParams(mockUiSettingsClient, mockConfig, { + sessionId: 'foo', + }); + expect(params).toHaveProperty('keep_on_completion', false); + }); + }); + + describe('getDefaultAsyncGetParams', () => { + test('Uses `wait_for_completion_timeout`', async () => { + const mockConfig = getMockSearchSessionsConfig({ + defaultExpiration: moment.duration(3, 'd'), + enabled: true, + }); + const params = getDefaultAsyncGetParams(mockConfig, {}); + expect(params).toHaveProperty('wait_for_completion_timeout'); + }); + + test('Uses `keep_alive` if `sessionId` is not provided', async () => { + const mockConfig = getMockSearchSessionsConfig({ + defaultExpiration: moment.duration(3, 'd'), + enabled: true, + }); + const params = getDefaultAsyncGetParams(mockConfig, {}); + expect(params).toHaveProperty('keep_alive', '1m'); + }); + + test('Has no `keep_alive` if `sessionId` is provided', async () => { + const mockConfig = getMockSearchSessionsConfig({ + defaultExpiration: moment.duration(3, 'd'), + enabled: true, + }); + const params = getDefaultAsyncGetParams(mockConfig, { sessionId: 'foo' }); + expect(params).not.toHaveProperty('keep_alive'); + }); + + test('Uses `keep_alive` if `sessionId` is provided but sessions disabled', async () => { + const mockConfig = getMockSearchSessionsConfig({ + defaultExpiration: moment.duration(3, 'd'), + enabled: false, + }); + const params = getDefaultAsyncGetParams(mockConfig, { sessionId: 'foo' }); + expect(params).toHaveProperty('keep_alive', '1m'); + }); + }); +}); diff --git a/src/plugins/data/server/search/strategies/ese_search/request_utils.ts b/src/plugins/data/server/search/strategies/ese_search/request_utils.ts index 70da0ba2edcc33..8bf4473355ccf1 100644 --- a/src/plugins/data/server/search/strategies/ese_search/request_utils.ts +++ b/src/plugins/data/server/search/strategies/ese_search/request_utils.ts @@ -46,21 +46,26 @@ export async function getDefaultAsyncSubmitParams( | 'keep_on_completion' > > { + const useSearchSessions = searchSessionsConfig?.enabled && !!options.sessionId; + + // TODO: searchSessionsConfig could be "null" if we are running without x-pack which happens only in tests. + // This can be cleaned up when we completely stop separating basic and oss + const keepAlive = useSearchSessions + ? `${searchSessionsConfig!.defaultExpiration.asMilliseconds()}ms` + : '1m'; + return { + // TODO: adjust for partial results batched_reduce_size: 64, - keep_on_completion: !!options.sessionId, // Always return an ID, even if the request completes quickly - ...getDefaultAsyncGetParams(options), + // Wait up to 100ms for the response to return + wait_for_completion_timeout: '100ms', + // If search sessions are used, store and get an async ID even for short running requests. + keep_on_completion: useSearchSessions, + // The initial keepalive is as defined in defaultExpiration if search sessions are used or 1m otherwise. + keep_alive: keepAlive, ...(await getIgnoreThrottled(uiSettingsClient)), ...(await getDefaultSearchParams(uiSettingsClient)), - ...(options.sessionId - ? { - // TODO: searchSessionsConfig could be "null" if we are running without x-pack which happens only in tests. - // This can be cleaned up when we completely stop separating basic and oss - keep_alive: searchSessionsConfig - ? `${searchSessionsConfig.defaultExpiration.asMilliseconds()}ms` - : '1m', - } - : {}), + // If search sessions are used, set the initial expiration time. }; } @@ -68,15 +73,20 @@ export async function getDefaultAsyncSubmitParams( @internal */ export function getDefaultAsyncGetParams( + searchSessionsConfig: SearchSessionsConfigSchema | null, options: ISearchOptions ): Pick { + const useSearchSessions = searchSessionsConfig?.enabled && !!options.sessionId; + return { - wait_for_completion_timeout: '100ms', // Wait up to 100ms for the response to return - ...(options.sessionId - ? undefined + // Wait up to 100ms for the response to return + wait_for_completion_timeout: '100ms', + ...(useSearchSessions + ? // Don't change the expiration of search requests that are tracked in a search session + undefined : { + // We still need to do polling for searches not within the context of a search session or when search session disabled keep_alive: '1m', - // We still need to do polling for searches not within the context of a search session }), }; } From 5270b831536c413a5ed1b43e835153be444ec842 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Wed, 21 Jul 2021 14:52:09 +0300 Subject: [PATCH 2/2] [XY axis] Fixes the values inside bar charts (#106198) (#106370) * [XY axis] Fixes the values inside bar charts * Apply PR comments --- .../public/components/xy_settings.tsx | 18 +++++------------- .../public/utils/render_all_series.tsx | 7 ++++++- 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/plugins/vis_type_xy/public/components/xy_settings.tsx b/src/plugins/vis_type_xy/public/components/xy_settings.tsx index 8d6a7eecdfe522..6d1425f488a47a 100644 --- a/src/plugins/vis_type_xy/public/components/xy_settings.tsx +++ b/src/plugins/vis_type_xy/public/components/xy_settings.tsx @@ -60,23 +60,15 @@ type XYSettingsProps = Pick< legendPosition: Position; }; -function getValueLabelsStyling(isHorizontal: boolean) { - const VALUE_LABELS_MAX_FONTSIZE = 15; +function getValueLabelsStyling() { + const VALUE_LABELS_MAX_FONTSIZE = 12; const VALUE_LABELS_MIN_FONTSIZE = 10; - const VALUE_LABELS_VERTICAL_OFFSET = -10; - const VALUE_LABELS_HORIZONTAL_OFFSET = 10; return { displayValue: { fontSize: { min: VALUE_LABELS_MIN_FONTSIZE, max: VALUE_LABELS_MAX_FONTSIZE }, - fill: { textInverted: true, textBorder: 2 }, - alignment: isHorizontal - ? { - vertical: VerticalAlignment.Middle, - } - : { horizontal: HorizontalAlignment.Center }, - offsetX: isHorizontal ? VALUE_LABELS_HORIZONTAL_OFFSET : 0, - offsetY: isHorizontal ? 0 : VALUE_LABELS_VERTICAL_OFFSET, + fill: { textInverted: false, textContrast: true }, + alignment: { horizontal: HorizontalAlignment.Center, vertical: VerticalAlignment.Middle }, }, }; } @@ -103,7 +95,7 @@ export const XYSettings: FC = ({ const theme = themeService.useChartsTheme(); const baseTheme = themeService.useChartsBaseTheme(); const dimmingOpacity = getUISettings().get('visualization:dimmingOpacity'); - const valueLabelsStyling = getValueLabelsStyling(rotation === 90 || rotation === -90); + const valueLabelsStyling = getValueLabelsStyling(); const themeOverrides: PartialTheme = { markSizeRatio, diff --git a/src/plugins/vis_type_xy/public/utils/render_all_series.tsx b/src/plugins/vis_type_xy/public/utils/render_all_series.tsx index ebf36944959819..d186617fef2ae5 100644 --- a/src/plugins/vis_type_xy/public/utils/render_all_series.tsx +++ b/src/plugins/vis_type_xy/public/utils/render_all_series.tsx @@ -131,7 +131,12 @@ export const renderAllSeries = ( minBarHeight={2} displayValueSettings={{ showValueLabel, - overflowConstraints: [LabelOverflowConstraint.ChartEdges], + isValueContainedInElement: false, + isAlternatingValueLabel: false, + overflowConstraints: [ + LabelOverflowConstraint.ChartEdges, + LabelOverflowConstraint.BarGeometry, + ], }} /> );