From 9950af0d886634799f83936b545c474436ed5605 Mon Sep 17 00:00:00 2001 From: rshen91 Date: Thu, 16 Nov 2023 12:16:27 -0700 Subject: [PATCH] code review --- .../kbn-generate-csv/src/generate_csv.test.ts | 42 +++---------------- packages/kbn-generate-csv/src/generate_csv.ts | 23 +++++----- .../src/get_export_settings.test.ts | 2 +- .../src/get_export_settings.ts | 4 +- packages/kbn-generate-csv/types.ts | 1 - .../sections/anomalies/log_entry_example.tsx | 6 ++- .../metric_control/custom_metric_form.tsx | 6 ++- 7 files changed, 28 insertions(+), 56 deletions(-) diff --git a/packages/kbn-generate-csv/src/generate_csv.test.ts b/packages/kbn-generate-csv/src/generate_csv.test.ts index 2ff1ab4b187e8a..372ff808398605 100644 --- a/packages/kbn-generate-csv/src/generate_csv.test.ts +++ b/packages/kbn-generate-csv/src/generate_csv.test.ts @@ -117,8 +117,8 @@ describe('CsvGenerator', () => { maxSizeBytes: 180000, useByteOrderMarkEncoding: false, scroll: { size: 500, duration: '30s' }, - maxConcurrentShardRequests: 5, enablePanelActionDownload: true, + maxConcurrentShardRequests: 5, }; searchSourceMock.getField = jest.fn((key: string) => { @@ -245,8 +245,8 @@ describe('CsvGenerator', () => { maxSizeBytes: TEST_MAX_SIZE, useByteOrderMarkEncoding: false, scroll: { size: 500, duration: '30s' }, - maxConcurrentShardRequests: 5, enablePanelActionDownload: true, + maxConcurrentShardRequests: 5, }; mockDataClient.search = jest.fn().mockImplementation(() => @@ -347,7 +347,7 @@ describe('CsvGenerator', () => { expect(mockDataClient.search).toHaveBeenCalledTimes(10); expect(mockDataClient.search).toBeCalledWith( - { params: { body: {}, ignore_throttled: undefined } }, + { params: { body: {}, ignore_throttled: undefined }, maxConcurrentShardRequests: 5 }, { strategy: 'es', transport: { maxRetries: 0, requestTimeout: '30s' } } ); @@ -760,8 +760,8 @@ describe('CsvGenerator', () => { maxSizeBytes: 180000, useByteOrderMarkEncoding: false, scroll: { size: 500, duration: '30s' }, - maxConcurrentShardRequests: 5, enablePanelActionDownload: true, + maxConcurrentShardRequests: 5, }; mockDataClient.search = jest.fn().mockImplementation(() => Rx.of({ @@ -847,6 +847,7 @@ describe('CsvGenerator', () => { expect(mockDataClient.search).toBeCalledWith( { + maxConcurrentShardRequests: 5, params: { body: {}, }, @@ -1054,37 +1055,4 @@ describe('CsvGenerator', () => { `); }); }); - - it('handles max concurrent shards settings set to a number', async () => { - const mockShardsConfig = { - ...mockConfig, - maxConcurrentShardRequests: 6, - }; - const generateCsv = new CsvGenerator( - createMockJob({ columns: ['date', 'ip', 'message'] }), - mockShardsConfig, - { - es: mockEsClient, - data: mockDataClient, - uiSettings: uiSettingsClient, - }, - { - searchSourceStart: mockSearchSourceService, - fieldFormatsRegistry: mockFieldFormatsRegistry, - }, - new CancellationToken(), - mockLogger, - stream - ); - await generateCsv.generateData(); - await expect(mockEsClient.asCurrentUser.openPointInTime).toHaveBeenCalledWith( - { - ignore_unavailable: true, - ignore_throttled: undefined, - index: 'logstash-*', - keep_alive: '30s', - }, - { maxConcurrentShardRequests: 6, maxRetries: 0, requestTimeout: '30s' } - ); - }); }); diff --git a/packages/kbn-generate-csv/src/generate_csv.ts b/packages/kbn-generate-csv/src/generate_csv.ts index df16ec1f753931..63a883e999a125 100644 --- a/packages/kbn-generate-csv/src/generate_csv.ts +++ b/packages/kbn-generate-csv/src/generate_csv.ts @@ -61,12 +61,12 @@ export class CsvGenerator { private stream: Writable ) {} - private async openPointInTime( - indexPatternTitle: string, - settings: CsvExportSettings, - maxConcurrentShardRequests: number - ) { - const { duration } = settings.scroll; + private async openPointInTime(indexPatternTitle: string, settings: CsvExportSettings) { + const { + includeFrozen, + maxConcurrentShardRequests, + scroll: { duration }, + } = settings; let pitId: string | undefined; this.logger.debug(`Requesting PIT for: [${indexPatternTitle}]...`); try { @@ -77,7 +77,7 @@ export class CsvGenerator { keep_alive: duration, ignore_unavailable: true, // @ts-expect-error ignore_throttled is not in the type definition, but it is accepted by es - ignore_throttled: settings.includeFrozen ? false : undefined, // "true" will cause deprecation warnings logged in ES + ignore_throttled: includeFrozen ? false : undefined, // "true" will cause deprecation warnings logged in ES }, { requestTimeout: duration, @@ -104,7 +104,7 @@ export class CsvGenerator { settings: CsvExportSettings, searchAfter?: estypes.SortResults ) { - const { scroll: scrollSettings } = settings; + const { scroll: scrollSettings, maxConcurrentShardRequests } = settings; searchSource.setField('size', scrollSettings.size); if (searchAfter) { @@ -126,6 +126,7 @@ export class CsvGenerator { params: { body: searchBody, }, + maxConcurrentShardRequests, }; let results: estypes.SearchResponse | undefined; @@ -335,11 +336,7 @@ export class CsvGenerator { let totalRelation = 'eq'; let searchAfter: estypes.SortResults | undefined; - let pitId = await this.openPointInTime( - indexPatternTitle, - settings, - this.config.maxConcurrentShardRequests - ); + let pitId = await this.openPointInTime(indexPatternTitle, settings); // apply timezone from the job to all date field formatters try { diff --git a/packages/kbn-generate-csv/src/get_export_settings.test.ts b/packages/kbn-generate-csv/src/get_export_settings.test.ts index 2cf716417437f9..05b83d5978add8 100644 --- a/packages/kbn-generate-csv/src/get_export_settings.test.ts +++ b/packages/kbn-generate-csv/src/get_export_settings.test.ts @@ -63,7 +63,7 @@ describe('getExportSettings', () => { "escapeFormulaValues": false, "escapeValue": [Function], "includeFrozen": false, - "maxConcurrentShardSize": 5, + "maxConcurrentShardRequests": 5, "maxSizeBytes": 180000, "scroll": Object { "duration": "30s", diff --git a/packages/kbn-generate-csv/src/get_export_settings.ts b/packages/kbn-generate-csv/src/get_export_settings.ts index a63ca5d5dd30d0..ac8ab57a19ca7c 100644 --- a/packages/kbn-generate-csv/src/get_export_settings.ts +++ b/packages/kbn-generate-csv/src/get_export_settings.ts @@ -31,7 +31,7 @@ export interface CsvExportSettings { escapeFormulaValues: boolean; escapeValue: (value: string) => string; includeFrozen: boolean; - maxConcurrentShardSize: number; + maxConcurrentShardRequests: number; } export const getExportSettings = async ( @@ -83,6 +83,6 @@ export const getExportSettings = async ( checkForFormulas: config.checkForFormulas, escapeFormulaValues, escapeValue, - maxConcurrentShardSize: config.maxConcurrentShardRequests, + maxConcurrentShardRequests: config.maxConcurrentShardRequests, }; }; diff --git a/packages/kbn-generate-csv/types.ts b/packages/kbn-generate-csv/types.ts index 74ed96db419456..6d7461609afe24 100644 --- a/packages/kbn-generate-csv/types.ts +++ b/packages/kbn-generate-csv/types.ts @@ -15,5 +15,4 @@ export interface JobParamsCSV { browserTimezone?: string; searchSource: SerializedSearchSourceFields; columns?: string[]; - maxConcurrentShardRequests: number; } diff --git a/x-pack/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/log_entry_example.tsx b/x-pack/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/log_entry_example.tsx index 174abbbe60bc42..93f709a2826831 100644 --- a/x-pack/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/log_entry_example.tsx +++ b/x-pack/plugins/infra/public/pages/logs/log_entry_rate/sections/anomalies/log_entry_example.tsx @@ -6,6 +6,7 @@ */ import React, { useMemo, useCallback, useState } from 'react'; +import { FormattedMessage } from '@kbn/i18n-react'; import moment from 'moment'; import { encode } from '@kbn/rison'; import { i18n } from '@kbn/i18n'; @@ -301,7 +302,10 @@ export const LogEntryExampleMessageHeaders: React.FunctionComponent<{ data-test-subj="logColumnHeader messageLogColumnHeader" key={columnConfiguration.messageColumn.id} > - Message + ); } else if (isFieldLogColumnConfiguration(columnConfiguration)) { diff --git a/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/custom_metric_form.tsx b/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/custom_metric_form.tsx index c3cd96ca5ad431..eb6bca7da19c5e 100644 --- a/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/custom_metric_form.tsx +++ b/x-pack/plugins/infra/public/pages/metrics/inventory_view/components/waffle/metric_control/custom_metric_form.tsx @@ -181,7 +181,11 @@ export const CustomMetricForm = withTheme( - of + + {i18n.translate('xpack.infra.customMetricForm.span.ofLabel', { + defaultMessage: 'of', + })} +