From 33500e6d74ed45737907e8d18ebfcb2a6426e7f5 Mon Sep 17 00:00:00 2001 From: Almudena Sanz Date: Thu, 1 Feb 2024 20:12:43 +0100 Subject: [PATCH 1/5] Observability AI Assistant evaluation framework new scenarios (#173818) ## Summary Retaled to: [elastic/obs-ai-assistant-team/#127](https://github.com/elastic/obs-ai-assistant-team/issues/127) ### Observability AI Assistant Evaluation - Added new scenarios for alerts, apm, elasticsearch, esql and kb - KB is installed by default for the evaluation framework - Modified `x-pack/plugins/observability_ai_assistant/scripts/evaluation/evaluation.ts` to include a final summary of the Number of tests and scenarios that failed, and the details of the tests failed and their Reasoning - Slightly modified evaluation prompt adding `and describing and quoting what the assistant did wrong`, because sometimes it hallucinated some evaluations, asking for quotes improves this - In ESQL system prompt, add good/bad examples of common mistakes - Modified `x-pack/plugins/observability_ai_assistant/server/functions/esql/index.ts` to send post-processed ESQL to ESQL function - Adds new post-processing functions for ESQL `x-pack/plugins/observability_ai_assistant/server/functions/esql/correct_common_esql_mistakes.ts` and tests: - modifies `replaceSingleQuotesWithDoubleQuotes` : replaces single quotes by double, unless they are inside double quotes - new `verifySortColumnInKeep` (to fix use column in SORT after a KEEP command that does not include it): verifies if the SORT key is in KEEP, and if it's not, it will include it - new `replaceFunctionsinSortWithStats` (to fix using formula in sort SORT COUNT(*) DESC) : if a formula is used in SORT, it will include a previous STATS command creating the alias for the formula and will use the alias in SORT - Some scenarios also execute the following pre-requisites: - alerts: - create custom threshold rule - create apm transaction_error_rate rule - generate APM data with synthtrace to trigger both rules - apm: - create apm error_rate rule - use synthtrace to generate two services with dependencies (ai-assistant-service-front and ai-assistant-service-back) - use synthtrace to generate services and index transactions - esql: - created indices for esql queries - use synthtrace to generate services and index transactions to execute queries on `metrics-apm*`, `traces-apm*` and `logs-apm*` - kb: - summarize a document and delete if from the KB --------- Co-authored-by: Dario Gieselaar Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../evaluation/alert_templates/templates.ts | 162 +++++++++++++ .../scripts/evaluation/evaluation.ts | 46 +++- .../scripts/evaluation/kibana_client.ts | 88 ++++++- .../evaluation/scenarios/alerts/index.spec.ts | 132 ++++++++++ .../evaluation/scenarios/apm/index.spec.ts | 172 +++++++++++++ .../scenarios/elasticsearch/index.spec.ts | 140 ++++++++++- .../evaluation/scenarios/esql/index.spec.ts | 228 +++++++++++++----- .../evaluation/scenarios/kb/index.spec.ts | 51 ++++ .../scripts/evaluation/services/index.ts | 4 + .../scripts/evaluation/types.ts | 1 + .../esql/correct_common_esql_mistakes.test.ts | 25 ++ .../esql/correct_common_esql_mistakes.ts | 179 ++++++++++---- .../server/functions/esql/index.ts | 4 +- .../server/functions/esql/system_message.txt | 22 +- .../server/service/client/index.ts | 2 + 15 files changed, 1127 insertions(+), 129 deletions(-) create mode 100644 x-pack/plugins/observability_ai_assistant/scripts/evaluation/alert_templates/templates.ts create mode 100644 x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/alerts/index.spec.ts create mode 100644 x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/apm/index.spec.ts create mode 100644 x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/kb/index.spec.ts diff --git a/x-pack/plugins/observability_ai_assistant/scripts/evaluation/alert_templates/templates.ts b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/alert_templates/templates.ts new file mode 100644 index 0000000000000..42d63dced13fd --- /dev/null +++ b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/alert_templates/templates.ts @@ -0,0 +1,162 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +export enum Comparator { + GT = '>', + LT = '<', + GT_OR_EQ = '>=', + LT_OR_EQ = '<=', + BETWEEN = 'between', + OUTSIDE_RANGE = 'outside', +} + +export enum Aggregators { + COUNT = 'count', + AVERAGE = 'avg', + SUM = 'sum', + MIN = 'min', + MAX = 'max', + CARDINALITY = 'cardinality', +} + +export const customThresholdAIAssistantLogCount = { + dataViewParams: { + contentTypeId: 'index-pattern', + data: { + fieldAttrs: '{}', + title: '.ds-logs-apm.*', + timeFieldName: '@timestamp', + sourceFilters: '[]', + fields: '[]', + fieldFormatMap: '{}', + typeMeta: '{}', + runtimeFieldMap: '{}', + name: 'logs_synth', + }, + options: { id: 'logs_synth' }, + version: 1, + }, + + ruleParams: { + tags: ['observability'], + consumer: 'logs', + name: 'Threshold surpassed in AI Assistant eval', + rule_type_id: 'observability.rules.custom_threshold', + params: { + criteria: [ + { + comparator: Comparator.GT, + threshold: [10], + timeSize: 2, + timeUnit: 'h', + metrics: [{ name: 'A', filter: '', aggType: Aggregators.COUNT }], + }, + ], + groupBy: ['service.name'], + alertOnNoData: true, + alertOnGroupDisappear: true, + searchConfiguration: { + query: { + query: '', + language: 'kuery', + }, + index: 'logs_synth', + }, + }, + actions: [], + schedule: { + interval: '1m', + }, + }, +}; + +export const customThresholdAIAssistantMetricAvg = { + ruleParams: { + consumer: 'logs', + name: 'metric_synth', + rule_type_id: 'observability.rules.custom_threshold', + params: { + criteria: [ + { + comparator: Comparator.GT, + threshold: [0.5], + timeSize: 2, + timeUnit: 'h', + metrics: [ + { name: 'A', field: 'system.cpu.total.norm.pct', aggType: Aggregators.AVERAGE }, + ], + }, + ], + groupBy: ['service.name'], + searchConfiguration: { + query: { + query: '', + }, + }, + }, + actions: [], + schedule: { + interval: '1m', + }, + }, +}; + +export const apmTransactionRateAIAssistant = { + ruleParams: { + consumer: 'apm', + name: 'apm_transaction_rate_AIAssistant', + rule_type_id: 'apm.transaction_error_rate', + params: { + threshold: 10, + windowSize: 1, + windowUnit: 'h', + transactionType: undefined, + serviceName: undefined, + environment: 'production', + searchConfiguration: { + query: { + query: ``, + language: 'kuery', + }, + }, + groupBy: ['service.name', 'service.environment'], + useKqlFilter: true, + }, + actions: [], + schedule: { + interval: '1m', + }, + }, +}; + +export const apmErrorCountAIAssistant = { + ruleParams: { + consumer: 'apm', + name: 'apm_error_count_AIAssistant', + rule_type_id: 'apm.error_rate', + params: { + threshold: 5, + windowSize: 1, + windowUnit: 'h', + transactionType: undefined, + serviceName: undefined, + environment: 'test', + searchConfiguration: { + query: { + query: ``, + language: 'kuery', + }, + }, + groupBy: ['service.name', 'service.environment'], + useKqlFilter: true, + }, + actions: [], + schedule: { + interval: '1m', + }, + }, +}; diff --git a/x-pack/plugins/observability_ai_assistant/scripts/evaluation/evaluation.ts b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/evaluation.ts index c39087249285c..4e3664b11fbbd 100644 --- a/x-pack/plugins/observability_ai_assistant/scripts/evaluation/evaluation.ts +++ b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/evaluation.ts @@ -24,6 +24,7 @@ import { getServiceUrls } from './get_service_urls'; import { KibanaClient } from './kibana_client'; import { initServices } from './services'; import { setupSynthtrace } from './setup_synthtrace'; +import { EvaluationResult } from './types'; function runEvaluations() { yargs(process.argv.slice(2)) @@ -36,7 +37,7 @@ function runEvaluations() { kibana: argv.kibana, }); - const kibanaClient = new KibanaClient(serviceUrls.kibanaUrl, argv.spaceId); + const kibanaClient = new KibanaClient(log, serviceUrls.kibanaUrl, argv.spaceId); const esClient = new Client({ node: serviceUrls.esUrl, }); @@ -69,6 +70,8 @@ function runEvaluations() { log.info(`Using connector ${connector.id}`); + await kibanaClient.installKnowledgeBase(); + const scenarios = (argv.files !== undefined && castArray(argv.files).map((file) => Path.join(process.cwd(), file))) || @@ -86,9 +89,15 @@ function runEvaluations() { log, }); + const mocha = new Mocha({ + grep: argv.grep, + timeout: '5m', + }); + const chatClient = kibanaClient.createChatClient({ connectorId: connector.id!, persist: argv.persist, + suite: mocha.suite, }); const header: string[][] = [ @@ -128,7 +137,14 @@ function runEvaluations() { ], }; + const results: EvaluationResult[] = []; + const failedScenarios: string[][] = [ + ['Failed Tests', '', ''], + ['Scenario, Scores, Reasoning', '', ''], + ]; + chatClient.onResult((result) => { + results.push(result); log.debug(`Result:`, JSON.stringify(result)); const output: string[][] = [ [ @@ -151,11 +167,30 @@ function runEvaluations() { result.scores.forEach((score) => { output.push([ score.criterion, - score.score === 0 ? chalk.redBright('failed') : chalk.greenBright('passed'), + score.score < 1 + ? chalk.redBright(String(score.score)) + : chalk.greenBright(String(score.score)), score.reasoning, ]); }); log.write(table.table(output, tableConfig)); + + const totalResults = result.scores.length; + const failedResults = result.scores.filter((score) => score.score < 1).length; + + if (failedResults / totalResults > 0) { + const reasoningConcat = result.scores.map((score) => score.reasoning).join(' '); + failedScenarios.push([ + `${result.name} : ${format(omit(parse(serviceUrls.kibanaUrl), 'auth'))}/${ + argv.spaceId ? `s/${argv.spaceId}/` : '' + }app/observabilityAIAssistant/conversations/${result.conversationId}`, + `Average score ${Math.round( + (result.scores.reduce((total, next) => total + next.score, 0) * 100) / + totalResults + )}. Failed ${failedResults} tests out of ${totalResults}`, + `Reasoning: ${reasoningConcat}`, + ]); + } }); initServices({ @@ -163,11 +198,7 @@ function runEvaluations() { esClient, chatClient, synthtraceEsClients, - }); - - const mocha = new Mocha({ - grep: argv.grep, - timeout: '5m', + logger: log, }); mocha.suite.beforeAll(async () => { @@ -190,6 +221,7 @@ function runEvaluations() { return new Promise((resolve, reject) => { mocha.run((failures: any) => { if (failures) { + log.write(table.table(failedScenarios, tableConfig)); reject(new Error(`Some tests failed`)); return; } diff --git a/x-pack/plugins/observability_ai_assistant/scripts/evaluation/kibana_client.ts b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/kibana_client.ts index 8fea2862e3faa..7ed447c6c907a 100644 --- a/x-pack/plugins/observability_ai_assistant/scripts/evaluation/kibana_client.ts +++ b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/kibana_client.ts @@ -9,6 +9,8 @@ import axios, { AxiosInstance, AxiosResponse } from 'axios'; import { pick, remove } from 'lodash'; import { filter, lastValueFrom, map, toArray } from 'rxjs'; import { format, parse, UrlObject } from 'url'; +import { ToolingLog } from '@kbn/tooling-log'; +import pRetry from 'p-retry'; import { Message, MessageRole } from '../../common'; import { ChatCompletionChunkEvent, @@ -26,6 +28,9 @@ import { getAssistantSetupMessage } from '../../public/service/get_assistant_set import { streamIntoObservable } from '../../server/service/util/stream_into_observable'; import { EvaluationResult } from './types'; +// eslint-disable-next-line spaced-comment +/// + type InnerMessage = Message['message']; type StringOrMessageList = string | InnerMessage[]; @@ -45,7 +50,11 @@ export interface ChatClient { export class KibanaClient { axios: AxiosInstance; - constructor(private readonly url: string, private readonly spaceId?: string) { + constructor( + private readonly log: ToolingLog, + private readonly url: string, + private readonly spaceId?: string + ) { this.axios = axios.create({ headers: { 'kbn-xsrf': 'foo', @@ -71,12 +80,63 @@ export class KibanaClient { return url; } + callKibana( + method: string, + props: { query?: UrlObject['query']; pathname: string }, + data?: any + ) { + const url = this.getUrl(props); + return this.axios({ + method, + url, + data: data || {}, + headers: { + 'kbn-xsrf': 'true', + 'x-elastic-internal-origin': 'foo', + }, + }); + } + + async installKnowledgeBase() { + this.log.debug('Checking to see whether knowledge base is installed'); + + const { + data: { ready }, + } = await this.callKibana<{ ready: boolean }>('GET', { + pathname: '/internal/observability_ai_assistant/kb/status', + }); + + if (ready) { + this.log.info('Knowledge base is installed'); + return; + } + + if (!ready) { + this.log.info('Installing knowledge base'); + } + + await pRetry( + async () => { + const response = await this.callKibana<{}>('POST', { + pathname: '/internal/observability_ai_assistant/kb/setup', + }); + this.log.info('Knowledge base is ready'); + return response.data; + }, + { retries: 10 } + ); + + this.log.info('Knowledge base installed'); + } + createChatClient({ connectorId, persist, + suite, }: { connectorId: string; persist: boolean; + suite: Mocha.Suite; }): ChatClient { function getMessages(message: string | Array): Array { if (typeof message === 'string') { @@ -103,6 +163,24 @@ export class KibanaClient { return { functionDefinitions, contextDefinitions }; } + let currentTitle: string = ''; + + suite.beforeEach(function () { + const currentTest: Mocha.Test = this.currentTest; + const titles: string[] = []; + titles.push(this.currentTest.title); + let parent = currentTest.parent; + while (parent) { + titles.push(parent.title); + parent = parent.parent; + } + currentTitle = titles.reverse().join(' '); + }); + + suite.afterEach(function () { + currentTitle = ''; + }); + const onResultCallbacks: Array<{ callback: (result: EvaluationResult) => void; unregister: () => void; @@ -131,7 +209,7 @@ export class KibanaClient { pathname: '/internal/observability_ai_assistant/chat', }), params, - { responseType: 'stream' } + { responseType: 'stream', timeout: NaN } ) ).data ).pipe( @@ -182,8 +260,9 @@ export class KibanaClient { messages, connectorId, persist, + title: currentTitle, }, - { responseType: 'stream' } + { responseType: 'stream', timeout: NaN } ) ).data ).pipe( @@ -230,7 +309,7 @@ export class KibanaClient { Your goal is to verify whether a conversation between the user and the assistant matches the given criteria. - For each criterion, calculate a score. Explain your score, by describing what the assistant did right, and what the + For each criterion, calculate a score. Explain your score, by describing what the assistant did right, and describing and quoting what the assistant did wrong, where it could improve, and what the root cause was in case of a failure.`, }, }, @@ -296,6 +375,7 @@ export class KibanaClient { ).criteria; const result: EvaluationResult = { + name: currentTitle, conversationId, messages, passed: scoredCriteria.every(({ score }) => score >= 1), diff --git a/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/alerts/index.spec.ts b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/alerts/index.spec.ts new file mode 100644 index 0000000000000..304851e2b1e61 --- /dev/null +++ b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/alerts/index.spec.ts @@ -0,0 +1,132 @@ +/* + * 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 { RuleResponse } from '@kbn/alerting-plugin/common/routes/rule/response/types/v1'; +import moment from 'moment'; +import { apm, timerange } from '@kbn/apm-synthtrace-client'; +import { chatClient, kibanaClient, synthtraceEsClients, logger } from '../../services'; +import { + apmTransactionRateAIAssistant, + customThresholdAIAssistantLogCount, +} from '../../alert_templates/templates'; +import { MessageRole } from '../../../../common'; + +describe('alert function', () => { + const ruleIds: any[] = []; + + before(async () => { + logger.info('Creating APM rule'); + const responseApmRule = await kibanaClient.callKibana( + 'post', + { pathname: '/api/alerting/rule' }, + apmTransactionRateAIAssistant.ruleParams + ); + ruleIds.push(responseApmRule.data.id); + + logger.info('Creating dataview'); + await kibanaClient.callKibana( + 'post', + { pathname: '/api/content_management/rpc/create' }, + customThresholdAIAssistantLogCount.dataViewParams + ); + + logger.info('Creating logs rule'); + const responseLogsRule = await kibanaClient.callKibana( + 'post', + { pathname: '/api/alerting/rule' }, + customThresholdAIAssistantLogCount.ruleParams + ); + ruleIds.push(responseLogsRule.data.id); + + logger.debug('Cleaning APM indices'); + + await synthtraceEsClients.apmSynthtraceEsClient.clean(); + + const myServiceInstance = apm.service('my-service', 'production', 'go').instance('my-instance'); + + logger.debug('Indexing synthtrace data'); + + await synthtraceEsClients.apmSynthtraceEsClient.index( + timerange(moment().subtract(15, 'minutes'), moment()) + .interval('1m') + .rate(10) + .generator((timestamp) => [ + myServiceInstance + .transaction('GET /api') + .timestamp(timestamp) + .duration(50) + .failure() + .errors( + myServiceInstance + .error({ message: 'errorMessage', type: 'My Type' }) + .timestamp(timestamp) + ), + myServiceInstance + .transaction('GET /api') + .timestamp(timestamp) + .duration(50) + .outcome('success'), + ]) + ); + }); + + it('summary of active alerts', async () => { + const conversation = await chatClient.complete('Are there any active alerts?'); + + const result = await chatClient.evaluate(conversation, [ + 'Uses alerts function to retrieve active alerts', + 'Responds with a summary of the current active alerts', + ]); + + expect(result.passed).to.be(true); + }); + + it('filtered alerts', async () => { + let conversation = await chatClient.complete( + 'Do I have any active alerts related to "Threshold surpassed in AI Assistant eval"?' + ); + + conversation = await chatClient.complete( + conversation.conversationId!, + conversation.messages.concat({ + content: 'Do I have any alerts on the service my-service?', + role: MessageRole.User, + }) + ); + + const result = await chatClient.evaluate(conversation, [ + 'Uses alerts function to retrieve active alerts for "Threshold surpassed in AI Assistant eval", uses "filter": "Threshold surpassed in AI Assistant eval" in the alert function', + 'Returns one or more alerts related to "Threshold surpassed in AI Assistant eval", does not return no active alerts related to "Threshold surpassed in AI Assistant eval"', + 'Uses alerts function to filtering on service.name my-service to retrieve active alerts for that service', + 'Returns one or more alerts related to my-service', + ]); + + expect(result.passed).to.be(true); + }); + + after(async () => { + await synthtraceEsClients.apmSynthtraceEsClient.clean(); + + for (const ruleId of ruleIds) { + await kibanaClient.callKibana('delete', { pathname: `/api/alerting/rule/${ruleId}` }); + } + + await kibanaClient.callKibana( + 'post', + { pathname: `/api/content_management/rpc/delete` }, + { + contentTypeId: 'index-pattern', + id: customThresholdAIAssistantLogCount.dataViewParams.options.id, + options: { force: true }, + version: 1, + } + ); + }); +}); diff --git a/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/apm/index.spec.ts b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/apm/index.spec.ts new file mode 100644 index 0000000000000..24a1b21f95237 --- /dev/null +++ b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/apm/index.spec.ts @@ -0,0 +1,172 @@ +/* + * 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 moment from 'moment'; +import { apm, timerange, serviceMap } from '@kbn/apm-synthtrace-client'; +import { RuleResponse } from '@kbn/alerting-plugin/common/routes/rule/response/types/v1'; +import { chatClient, kibanaClient, synthtraceEsClients } from '../../services'; +import { MessageRole } from '../../../../common'; +import { apmErrorCountAIAssistant } from '../../alert_templates/templates'; + +describe('apm', () => { + const ruleIds: any[] = []; + before(async () => { + const responseApmRule = await kibanaClient.callKibana( + 'post', + { pathname: '/api/alerting/rule' }, + apmErrorCountAIAssistant.ruleParams + ); + ruleIds.push(responseApmRule.data.id); + + await synthtraceEsClients.apmSynthtraceEsClient.clean(); + + const serviceMapCallback = serviceMap({ + services: [{ 'ai-assistant-service-front': 'go' }, { 'ai-assistant-service-back': 'python' }], + environment: 'test', + definePaths([main, reco]) { + return [ + [ + [main, 'fetchReco'], + [reco, 'GET /api'], + ], + [reco], + ]; + }, + }); + + const aiAssistantService = apm + .service('ai-assistant-service', 'test', 'go') + .instance('my-instance'); + + const aiAssistantServicePython = apm + .service('ai-assistant-service-reco', 'test', 'python') + .instance('my-instance'); + + await synthtraceEsClients.apmSynthtraceEsClient.index( + timerange(moment().subtract(15, 'minutes'), moment()) + .interval('1m') + .rate(10) + .generator((timestamp) => { + return [ + ...serviceMapCallback(timestamp), + aiAssistantService + .transaction('getReco') + .timestamp(timestamp) + .duration(50) + .outcome('success'), + aiAssistantService + .transaction('removeReco') + .timestamp(timestamp) + .duration(50) + .failure() + .errors( + aiAssistantService + .error({ message: 'ERROR removeReco not suported', type: 'My Type' }) + .timestamp(timestamp) + ), + aiAssistantService + .transaction('GET /api_v1') + .timestamp(timestamp) + .duration(50) + .outcome('success'), + aiAssistantServicePython + .transaction('GET /api_v2') + .timestamp(timestamp) + .duration(50) + .failure() + .errors( + aiAssistantServicePython + .error({ message: 'ERROR api_v2 not supported', type: 'My Type' }) + .timestamp(timestamp) + ), + ]; + }) + ); + }); + + it('service summary, troughput, dependencies and errors', async () => { + let conversation = await chatClient.complete( + 'What is the status of the service ai-assistant-service in the test environment?' + ); + + conversation = await chatClient.complete( + conversation.conversationId!, + conversation.messages.concat({ + content: + 'What is the average throughput for the ai-assistant-service service over the past 4 hours?', + role: MessageRole.User, + }) + ); + + conversation = await chatClient.complete( + conversation.conversationId!, + conversation.messages.concat({ + content: 'What are the downstream dependencies of the ai-assistant-service-front service?', + role: MessageRole.User, + }) + ); + + const result = await chatClient.evaluate(conversation, [ + 'Uses get_apm_service_summary to obtain the status of the ai-assistant-service service', + 'Executes get_apm_timeseries to obtain the throughput of the services ai-assistant-service for the last 4 hours', + 'Gives a summary of the throughput stats for ai-assistant-service', + 'Provides the downstream dependencies of ai-assistant-service-front', + ]); + + expect(result.passed).to.be(true); + }); + it('services in environment', async () => { + let conversation = await chatClient.complete( + 'What are the active services in the environment "test"?' + ); + + conversation = await chatClient.complete( + conversation.conversationId!, + conversation.messages.concat({ + content: 'What is the average error rate per service over the past 4 hours?', + role: MessageRole.User, + }) + ); + + conversation = await chatClient.complete( + conversation.conversationId!, + conversation.messages.concat({ + content: + 'What are the top 2 most frequent errors in the services in the test environment in the last hour?', + role: MessageRole.User, + }) + ); + + conversation = await chatClient.complete( + conversation.conversationId!, + conversation.messages.concat({ + content: 'Are there any alert for those services?', + role: MessageRole.User, + }) + ); + + const result = await chatClient.evaluate(conversation, [ + 'Responds with the active services in the environment "test"', + 'Executes get_apm_timeseries to obtain the error rate of the services for the last 4 hours, for the specified services in test environment', + 'Obtains the top 2 frequent errors of the services in the last hour, for the specified services in test environment', + 'Returns the current alerts for the services, for the specified services in test environment', + ]); + + expect(result.passed).to.be(true); + }); + + after(async () => { + await synthtraceEsClients.apmSynthtraceEsClient.clean(); + + for (const ruleId of ruleIds) { + await kibanaClient.callKibana('delete', { pathname: `/api/alerting/rule/${ruleId}` }); + } + }); +}); diff --git a/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/elasticsearch/index.spec.ts b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/elasticsearch/index.spec.ts index 6f7ff7e333215..1dba311a64495 100644 --- a/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/elasticsearch/index.spec.ts +++ b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/elasticsearch/index.spec.ts @@ -8,19 +8,137 @@ /// import expect from '@kbn/expect'; -import { chatClient } from '../../services'; +import { chatClient, esClient } from '../../services'; +import { MessageRole } from '../../../../common'; -describe('health', () => { - it('returns the cluster health state', async () => { - const conversation = await chatClient.complete( - 'Can you tell me what the state of my Elasticsearch cluster is?' - ); +describe('elasticsearch functions', () => { + describe('health', () => { + it('returns the cluster health state', async () => { + const conversation = await chatClient.complete( + 'Can you tell me what the state of my Elasticsearch cluster is?' + ); - const result = await chatClient.evaluate(conversation, [ - 'Calls the Elasticsearch function with method: GET and path: _cluster/health', - 'Describes the cluster status based on the response from the Elasticsearch function', - ]); + const result = await chatClient.evaluate(conversation, [ + 'Calls the Elasticsearch function with method: GET and path: _cluster/health', + 'Describes the cluster status based on the response from the Elasticsearch function', + ]); - expect(result.passed).to.be(true); + expect(result.passed).to.be(true); + }); + }); + + describe('index management', () => { + describe('existing index', () => { + before(async () => { + await esClient.indices.create({ + index: 'kb', + mappings: { + properties: { + date: { + type: 'date', + }, + kb_doc: { + type: 'keyword', + }, + user: { + type: 'keyword', + }, + }, + }, + }); + + await esClient.index({ + index: 'kb', + document: { + date: '2024-01-23T12:30:00.000Z', + kb_doc: 'document_1', + user: 'user1', + }, + }); + }); + + it('returns the count of docs in the KB', async () => { + const conversation = await chatClient.complete('How many documents are in the index kb?'); + + const result = await chatClient.evaluate(conversation, [ + 'Calls the Elasticsearch function', + 'Finds how many documents are in that index', + ]); + + expect(result.passed).to.be(true); + }); + + it('returns store and refresh stats of an index', async () => { + let conversation = await chatClient.complete('What are the store stats of the index kb?'); + + conversation = await chatClient.complete( + conversation.conversationId!, + conversation.messages.concat({ + content: 'What are the the refresh stats of the index?', + role: MessageRole.User, + }) + ); + + const result = await chatClient.evaluate(conversation, [ + 'Calls the Elasticsearch function with method: kb/_stats/store', + 'Returns the index store stats', + 'Calls the Elasticsearch function with method: kb/_stats/refresh', + 'Returns the index refresh stats', + ]); + + expect(result.passed).to.be(true); + }); + + after(async () => { + await esClient.indices.delete({ + index: 'kb', + }); + }); + }); + + describe('assistant created index', () => { + it('creates index, adds documents and deletes index', async () => { + let conversation = await chatClient.complete( + 'Create a new index called testing_ai_assistant what will have two documents, one for the test_suite alerts with message "This test is for alerts" and another one for the test_suite esql with the message "This test is for esql"' + ); + + conversation = await chatClient.complete( + conversation.conversationId!, + conversation.messages.concat({ + content: 'What are the fields types for the index testing_ai_assistant?', + role: MessageRole.User, + }) + ); + + conversation = await chatClient.complete( + conversation.conversationId!, + conversation.messages.concat({ + content: 'Delete the testing_ai_assistant index', + role: MessageRole.User, + }) + ); + + const result = await chatClient.evaluate(conversation, [ + 'Calls the Elasticsearch function to create the index testing_ai_assistant and add the documents to it', + 'Successfully created index and adds two documents to it', + 'Calls get_dataset_info and retrieves the field types of the index', + 'Deletes the testing_ai_assistant index', + ]); + + expect(result.passed).to.be(true); + }); + }); + }); + describe('other', () => { + it('returns clusters license', async () => { + const conversation = await chatClient.complete('What is my clusters license?'); + + const result = await chatClient.evaluate(conversation, [ + 'Calls the Elasticsearch function', + 'Returns the cluster license based on the response from the Elasticsearch function', + ]); + + expect(result.passed).to.be(true); + }); }); }); diff --git a/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/esql/index.spec.ts b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/esql/index.spec.ts index 4c9b724686c0f..471ccd5448e33 100644 --- a/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/esql/index.spec.ts +++ b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/esql/index.spec.ts @@ -15,10 +15,12 @@ import { chatClient, esClient, synthtraceEsClients } from '../../services'; async function evaluateEsqlQuery({ question, expected, + execute, criteria = [], }: { question: string; expected?: string; + execute?: boolean; criteria?: string[]; }): Promise { const conversation = await chatClient.complete(question); @@ -30,7 +32,9 @@ async function evaluateEsqlQuery({ ${expected}`, ] : []), - ...(expected ? [`The query successfully executed without an error`] : []), + ...(execute + ? [`The query successfully executed without an error`] + : [`The query was not executed`]), ...criteria, ]); @@ -69,16 +73,29 @@ describe('ES|QL query generation', () => { }, }, }); + await esClient.index({ + index: 'packetbeat-8.11.3', + document: { + '@timestamp': '2024-01-23T12:30:00.000Z', + destination: { + domain: 'observability.ai.assistant', + }, + url: { + domain: 'elastic.co', + }, + }, + }); }); it('top 10 unique domains', async () => { await evaluateEsqlQuery({ question: - 'For standard Elastic ECS compliant packetbeat data view, create an ES|QL query that shows the top 10 unique domains by doc count', + 'For standard Elastic ECS compliant packetbeat data view, shows the top 10 unique destination.domain with more docs', expected: `FROM packetbeat-* - | STATS doc_count = COUNT(destination.domain) BY destination.domain + | STATS doc_count = COUNT(*) BY destination.domain | SORT doc_count DESC | LIMIT 10`, + execute: true, }); }); @@ -108,17 +125,27 @@ describe('ES|QL query generation', () => { }, }, }); + + await esClient.index({ + index: 'employees', + document: { + hire_date: '2024-01-23T12:30:00.000Z', + emp_no: 1, + salary: 100, + }, + }); }); - it('five earliest employees', async () => { + it('employees five earliest', async () => { await evaluateEsqlQuery({ question: 'From employees, I want to see the 5 earliest employees (hire_date), I want to display only the month and the year that they were hired in and their employee number (emp_no). Format the date as e.g. "September 2019".', expected: `FROM employees - | EVAL hire_date_formatted = DATE_FORMAT(hire_date, ""MMMM yyyy"") + | EVAL hire_date_formatted = DATE_FORMAT("MMMM YYYY", hire_date) | SORT hire_date | KEEP emp_no, hire_date_formatted | LIMIT 5`, + execute: true, }); }); @@ -132,6 +159,17 @@ describe('ES|QL query generation', () => { }); }); + it('employee hire date', async () => { + await evaluateEsqlQuery({ + question: + 'From employees, extract the year from hire_date and show 10 employees hired in 2024', + expected: `FROM employees + | WHERE DATE_EXTRACT("year", hire_date) == 2024 + | LIMIT 10`, + execute: true, + }); + }); + after(async () => { await esClient.indices.delete({ index: 'employees', @@ -142,54 +180,37 @@ describe('ES|QL query generation', () => { it('logs avg cpu', async () => { await evaluateEsqlQuery({ question: - 'My logs data (ECS) is in `logs-*`. Show me a query that gets the average CPU per host, limit it to the top 10 results, in 1m buckets, and only include the last 15m. ', - expected: `FROM logs-* + 'My metric data (ECS) is in .ds-metrics-apm* Show me a query that gets the average CPU per service, limit it to the top 10 results, in 1m buckets, and only include the last 15m. ', + expected: `FROM .ds-metrics-apm* | WHERE @timestamp >= NOW() - 15 minutes | EVAL bucket = DATE_TRUNC(1 minute, @timestamp) - | STATS avg_cpu = AVG(system.cpu.total.norm.pct) BY bucket, host.name + | STATS avg_cpu = AVG(system.cpu.total.norm.pct) BY bucket, service.name + | SORT avg_cpu DESC | LIMIT 10`, + execute: false, }); }); it('metricbeat avg cpu', async () => { await evaluateEsqlQuery({ - question: `from \`metricbeat*\`, using ES|QL, I want to see the percentage of CPU time normalized by the number of CPU cores, broken down by hostname. the fields are system.cpu.user.pct, system.cpu.system.pct, and system.cpu.cores`, + question: `From metricbeat*, using ES|QL, show me a query to see the percentage of CPU time (system.cpu.system.pct) normalized by the number of CPU cores (system.cpu.cores), broken down by hostname`, expected: `FROM metricbeat* - | EVAL cpu_pct_normalized = (system.cpu.user.pct + system.cpu.system.pct) / system.cpu.cores - | STATS AVG(cpu_pct_normalized) BY host.name`, + | EVAL system_pct_normalized = TO_DOUBLE(system.cpu.system.pct) / system.cpu.cores + | STATS avg_system_pct_normalized = AVG(system_pct_normalized) BY host.name + | SORT host.name ASC`, + execute: false, }); }); - it('postgres avg duration', async () => { + it('postgres avg duration dissect', async () => { await evaluateEsqlQuery({ question: - 'extract the query duration from postgres log messages in postgres-logs*, using ECS fields, and calculate the avg', - expected: `FROM postgres-logs - | DISSECT message "%{} duration: %{query_duration} ms" - | EVAL query_duration_num = TO_DOUBLE(query_duration) - | STATS avg_duration = AVG(query_duration_num)`, - }); - }); - - it('high cardinality logs', async () => { - await evaluateEsqlQuery({ - question: `i have logs in high-cardinality-data-fake_stack.admin-console-* . errors are found when log.level contais the value ERROR. generate a query to obtain the error rate as a percetage of the total logs per day for the last 7 days`, - expected: `FROM high-cardinality-data-fake_stack.admin-console-* - | WHERE @timestamp >= NOW() - 7 days - | EVAL error = CASE(log.level == "ERROR", 1, 0), total = 1 - | EVAL bucket = DATE_TRUNC(1 day, @timestamp) - | STATS total_errors = SUM(error), total_logs = SUM(total) BY bucket - | EVAL error_rate = (total_errors / total_logs) * 100`, - }); - }); - - it('nyc taxis dropoff time', async () => { - await evaluateEsqlQuery({ - question: - 'From `nyc_taxis`, give me a query that shows the top 10 results where the drop off time was between 6am and 10am.', - expected: `FROM nyc_taxis - | WHERE DATE_EXTRACT(drop_off_time, "hour") >= 6 AND DATE_EXTRACT(drop_off_time, "hour") < 10 - | LIMIT 10`, + 'Show me an ESQL query to extract the query duration from postgres log messages in postgres-logs*, with this format "2021-01-01 00:00:00 UTC [12345]: [1-1] user=postgres,db=mydb,app=[unknown],client=127.0.0.1 LOG: duration: 123.456 ms statement: SELECT * FROM my_table", using ECS fields, and calculate the avg', + expected: `FROM postgres-logs* + | DISSECT message "%{}: duration: %{query_duration} ms %{}" + | EVAL duration_double = TO_DOUBLE(duration) + | STATS AVG(duration_double)`, + execute: false, }); }); }); @@ -212,36 +233,60 @@ describe('ES|QL query generation', () => { .outcome('success') ) ); + + await synthtraceEsClients.apmSynthtraceEsClient.index( + timerange(moment().subtract(15, 'minutes'), moment()) + .interval('1m') + .rate(10) + .generator((timestamp) => + myServiceInstance + .transaction('GET /api') + .timestamp(timestamp) + .duration(50) + .failure() + .errors( + myServiceInstance + .error({ + message: '2024-11-15T13:12:00 - ERROR - duration: 12ms', + type: 'My Type', + }) + .timestamp(timestamp) + ) + ) + ); }); it('metrics avg duration', async () => { await evaluateEsqlQuery({ question: - 'I want to see a query for metrics-apm*, filtering on metricset.name:transaction and metricset.interval:1m, showing the average duration (via transaction.duration.histogram), in 50 buckets.', - expected: `FROM metrics-apm* - | WHERE metricset.name == "transaction" AND metricset.interval == "1m" - | EVAL bucket = AUTO_BUCKET(@timestamp, 50, , ) - | STATS avg_duration = AVG(transaction.duration.histogram) BY bucket`, + 'Execute a query for metrics-apm*, filtering on metricset.name:service_transaction and metricset.interval:1m, the average duration (via transaction.duration.histogram), in 50 buckets.', + execute: true, + criteria: [ + 'The assistant know that transaction.duration.histogram cannot be used in ESQL and proposes an alertative solution', + ], }); }); it('service inventory', async () => { await evaluateEsqlQuery({ question: - 'I want to show a list of services with APM data. My data is in `traces-apm*`. I want to show the average transaction duration, the success rate (by dividing event.outcome:failure by event.outcome:failure+success), and total amount of requests. As a time range, select the last 24 hours. Use ES|QL.', + 'I want to see a list of services with APM data. My data is in `traces-apm*`. I want to show the average transaction duration, the success rate (by dividing event.outcome:failure by event.outcome:failure+success), and total amount of requests. As a time range, select the last 24 hours. Use ES|QL.', expected: `FROM traces-apm* - | WHERE @timestamp >= NOW() - 24 hours - | EVAL successful = CASE(event.outcome == "success", 1, 0), - failed = CASE(event.outcome == "failure", 1, 0) - | STATS success_rate = AVG(successful), - avg_duration = AVG(transaction.duration), - total_requests = COUNT(transaction.id) BY service.name`, + | WHERE @timestamp >= NOW() - 24 hours + | EVAL is_failure = CASE(event.outcome == "failure", 1, 0), is_success = CASE(event.outcome == "success", 1, 0) + | STATS total_requests = COUNT(*), avg_duration = AVG(transaction.duration.us), total_failures = SUM(is_failure), total_success = SUM(is_success) BY service.name + | EVAL success_rate = total_success / (total_failures + total_success) + | KEEP service.name, avg_duration, success_rate, total_requests`, + execute: true, + criteria: [ + 'The query provided by the Assistant does NOT include mathematical operations (/, +, -) in STATS aggregations', + ], }); }); it('exit span', async () => { await evaluateEsqlQuery({ - question: `I've got APM data in \`metrics-apm\`. Filter on \`metricset.name:service_destination\` and the last 24 hours. Break down by span.destination.service.resource. Each document contains the count of total events (span.destination.service.response_time.count) for that document's interval and the total amount of latency (span.destination.service.response_time.sum.us). A document either contains an aggregate of failed events (event.outcome:success) or failed events (event.outcome:failure). A single document might represent multiple failures or successes, depending on the value of span.destination.service.response_time.count. For each value of span.destination.service.resource, give me the average throughput, latency per request, and failure rate, as a value between 0 and 1. Just show me the query.`, + question: `I've got APM data in metrics-apm*. Show me a query that filters on metricset.name:service_destination and the last 24 hours. Break down by span.destination.service.resource. Each document contains the count of total events (span.destination.service.response_time.count) for that document's interval and the total amount of latency (span.destination.service.response_time.sum.us). A document either contains an aggregate of failed events (event.outcome:success) or failed events (event.outcome:failure). A single document might represent multiple failures or successes, depending on the value of span.destination.service.response_time.count. For each value of span.destination.service.resource, give me the average throughput, latency per request, and failure rate, as a value between 0 and 1. Just show me the query.`, expected: `FROM metrics-apm | WHERE metricset.name == "service_destination" AND @timestamp >= NOW() - 24 hours | EVAL total_response_time = span.destination.service.response_time.sum.us / span.destination.service.response_time.count, total_failures = CASE(event.outcome == "failure", 1, 0) * span.destination.service.response_time.count @@ -250,16 +295,51 @@ describe('ES|QL query generation', () => { avg_latency = AVG(total_response_time), failure_rate = AVG(total_failures) BY span.destination.service.resource`, + execute: false, + criteria: [ + 'The query provided by the Assistant does NOT include mathematical operations (/, +, -) in STATS aggregations', + ], }); }); it('trace duration', async () => { await evaluateEsqlQuery({ question: - 'My APM data is in `traces-apm*`. What’s the average for `transaction.duration.us` per service over the last hour?', - expected: `FROM traces-apm* + 'My APM data is in .ds-traces-apm-default-*. Execute a query to find the average for `transaction.duration.us` per service over the last hour', + expected: `FROM .ds-traces-apm-default-* | WHERE @timestamp > NOW() - 1 hour | STATS AVG(transaction.duration.us) BY service.name`, + execute: true, + }); + }); + + it('error logs rate', async () => { + await evaluateEsqlQuery({ + question: `i have logs in logs-apm*. Using ESQL, show me the error rate as a percetage of the error logs (identified as processor.event containing the value error) vs the total logs per day for the last 7 days `, + expected: `FROM logs-apm* + | WHERE @timestamp >= NOW() - 7 days + | EVAL day = DATE_TRUNC(1 day, @timestamp) + | EVAL error = CASE(processor.event == "error", 1, 0) + | STATS total_logs = COUNT(*), total_errors = SUM(is_error) BY day + | EVAL error_rate = total_errors / total_logs * 100 + | SORT day ASC`, + execute: true, + criteria: [ + 'The query provided by the Assistant does NOT include mathematical operations (/, +, -) in STATS aggregations', + ], + }); + }); + + it('error message and date', async () => { + await evaluateEsqlQuery({ + question: + 'From logs-apm*, I want to see the 5 latest messages using ESQL, I want to display only the date that they were indexed, processor.event and message. Format the date as e.g. "10:30 AM, 1 of September 2019".', + expected: `FROM logs-apm* + | SORT @timestamp DESC + | EVAL formatted_date = DATE_FORMAT("hh:mm a, d 'of' MMMM yyyy", @timestamp) + | KEEP formatted_date, log.level, message + | LIMIT 5`, + execute: true, }); }); @@ -267,4 +347,44 @@ describe('ES|QL query generation', () => { await synthtraceEsClients.apmSynthtraceEsClient.clean(); }); }); + + describe('SPL queries', () => { + it('network_firewall count by', async () => { + await evaluateEsqlQuery({ + question: `can you convert this SPL query to ESQL? index=network_firewall "SYN Timeout" | stats count by dest`, + expected: `FROM network_firewall + | WHERE _raw == "SYN Timeout" + | STATS count = count(*) by dest`, + execute: false, + }); + }); + it('prod_web length', async () => { + await evaluateEsqlQuery({ + question: `can you convert this SPL query to ESQL? index=prod_web | eval length=len(message) | eval k255=if((length>255),1,0) | eval k2=if((length>2048),1,0) | eval k4=if((length>4096),1,0) |eval k16=if((length>16384),1,0) | stats count, sum(k255), sum(k2),sum(k4),sum(k16), sum(length)`, + expected: `from prod_web + | EVAL length = length(message), k255 = CASE(length > 255, 1, 0), k2 = CASE(length > 2048, 1, 0), k4 = CASE(length > 4096, 1, 0), k16 = CASE(length > 16384, 1, 0) + | STATS COUNT(*), SUM(k255), SUM(k2), SUM(k4), SUM(k16), SUM(length)`, + criteria: [ + 'The query provided by the Assistant uses the ESQL functions LENGTH and CASE, not the SPL functions len and if', + ], + execute: false, + }); + }); + it('prod_web filter message and host', async () => { + await evaluateEsqlQuery({ + question: `can you convert this SPL query to ESQL? index=prod_web NOT "Connection reset" NOT "[acm-app] created a ThreadLocal" sourcetype!=prod_urlf_east_logs sourcetype!=prod_urlf_west_logs host!="dbs-tools-*" NOT "Public] in context with path [/global] " host!="*dev*" host!="*qa*" host!="*uat*"`, + expected: `FROM prod_web + | WHERE _raw NOT LIKE "Connection reset" + AND _raw NOT LIKE "[acm-app] created a ThreadLocal" + AND sourcetype != "prod_urlf_east_logs" + AND sourcetype != "prod_urlf_west_logs" + AND host NOT LIKE "dbs-tools-*" + AND _raw NOT LIKE "Public] in context with path [/global]" + AND host NOT LIKE "*dev*" + AND host NOT LIKE "*qa*" + AND host NOT LIKE "*uat*"`, + execute: false, + }); + }); + }); }); diff --git a/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/kb/index.spec.ts b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/kb/index.spec.ts new file mode 100644 index 0000000000000..f0fb64dc035bb --- /dev/null +++ b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/scenarios/kb/index.spec.ts @@ -0,0 +1,51 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +/// + +import expect from '@kbn/expect'; +import { chatClient, esClient } from '../../services'; +import { MessageRole } from '../../../../common'; + +describe('kb functions', () => { + it('summarizes and recalls information', async () => { + let conversation = await chatClient.complete( + 'Remember that this cluster is used to test the AI Assistant using the Observability AI Evaluation Framework' + ); + + conversation = await chatClient.complete( + conversation.conversationId!, + conversation.messages.concat({ + content: 'What is this cluster used for?', + role: MessageRole.User, + }) + ); + + const result = await chatClient.evaluate(conversation, [ + 'Calls the summarize function', + 'Effectively summarizes and remembers that this cluster is used to test the AI Assistant using the Observability AI Evaluation Framework', + 'Calls the recall function to respond to What is this cluster used for?', + 'Effectively recalls that this cluster is used to test the AI Assistant using the Observability AI Evaluation Framework', + ]); + + expect(result.passed).to.be(true); + }); + + after(async () => { + await esClient.deleteByQuery({ + index: '.kibana-observability-ai-assistant-kb-*', + ignore_unavailable: true, + query: { + match: { + text: { + query: '*Observability AI Evaluation Framework*', + }, + }, + }, + }); + }); +}); diff --git a/x-pack/plugins/observability_ai_assistant/scripts/evaluation/services/index.ts b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/services/index.ts index 5ecf0c48ddec3..2016abc065077 100644 --- a/x-pack/plugins/observability_ai_assistant/scripts/evaluation/services/index.ts +++ b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/services/index.ts @@ -6,6 +6,7 @@ */ import type { Client } from '@elastic/elasticsearch'; +import { ToolingLog } from '@kbn/tooling-log'; import type { ChatClient, KibanaClient } from '../kibana_client'; import type { SynthtraceEsClients } from '../setup_synthtrace'; @@ -26,6 +27,7 @@ function createErrorThrowingProxy(name: string): any { export let chatClient: ChatClient = createErrorThrowingProxy('ChatClient'); export let esClient: Client = createErrorThrowingProxy('esClient'); export let kibanaClient: KibanaClient = createErrorThrowingProxy('kibanaClient'); +export let logger: ToolingLog = createErrorThrowingProxy('logger'); export let synthtraceEsClients: SynthtraceEsClients = createErrorThrowingProxy('synthtraceEsClients'); @@ -35,9 +37,11 @@ export const initServices = (services: { esClient: Client; kibanaClient: KibanaClient; synthtraceEsClients: SynthtraceEsClients; + logger: ToolingLog; }) => { chatClient = services.chatClient; esClient = services.esClient; kibanaClient = services.kibanaClient; synthtraceEsClients = services.synthtraceEsClients; + logger = services.logger; }; diff --git a/x-pack/plugins/observability_ai_assistant/scripts/evaluation/types.ts b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/types.ts index 88182c480ff21..20000b322d911 100644 --- a/x-pack/plugins/observability_ai_assistant/scripts/evaluation/types.ts +++ b/x-pack/plugins/observability_ai_assistant/scripts/evaluation/types.ts @@ -18,6 +18,7 @@ export interface ScenarioOptions { } export interface EvaluationResult { + name: string; conversationId?: string; messages: Array; passed: boolean; diff --git a/x-pack/plugins/observability_ai_assistant/server/functions/esql/correct_common_esql_mistakes.test.ts b/x-pack/plugins/observability_ai_assistant/server/functions/esql/correct_common_esql_mistakes.test.ts index ded551bd84171..33e401bf11c7d 100644 --- a/x-pack/plugins/observability_ai_assistant/server/functions/esql/correct_common_esql_mistakes.test.ts +++ b/x-pack/plugins/observability_ai_assistant/server/functions/esql/correct_common_esql_mistakes.test.ts @@ -56,5 +56,30 @@ describe('correctCommonEsqlMistakes', () => { | WHERE DATE_EXTRACT("hour", dropoff_datetime) >= 6 AND DATE_EXTRACT("hour", dropoff_datetime) < 10 | LIMIT 10` ); + expectQuery( + `FROM nyc_taxis + | WHERE DATE_EXTRACT('hour', "hh:mm a, 'of' d MMMM yyyy") >= 6 AND DATE_EXTRACT('hour', dropoff_datetime) < 10 + | LIMIT 10`, + `FROM nyc_taxis + | WHERE DATE_EXTRACT("hour", "hh:mm a, 'of' d MMMM yyyy") >= 6 AND DATE_EXTRACT("hour", dropoff_datetime) < 10 + | LIMIT 10` + ); + }); + + it(`verifies if the SORT key is in KEEP, and if it's not, it will include it`, () => { + expectQuery( + 'FROM logs-* \n| KEEP date \n| SORT @timestamp DESC', + 'FROM logs-*\n| KEEP date, @timestamp\n| SORT @timestamp DESC' + ); + + expectQuery( + `FROM logs-* | KEEP date, whatever | EVAL my_truncated_date_field = DATE_TRUNC(1 year, date) | SORT @timestamp, my_truncated_date_field DESC`, + 'FROM logs-*\n| KEEP date, whatever, @timestamp\n| EVAL my_truncated_date_field = DATE_TRUNC(1 year, date)\n| SORT @timestamp, my_truncated_date_field DESC' + ); + + expectQuery( + `FROM logs-* | KEEP date, whatever | RENAME whatever AS forever | SORT forever DESC`, + `FROM logs-*\n| KEEP date, whatever\n| RENAME whatever AS forever\n| SORT forever DESC` + ); }); }); diff --git a/x-pack/plugins/observability_ai_assistant/server/functions/esql/correct_common_esql_mistakes.ts b/x-pack/plugins/observability_ai_assistant/server/functions/esql/correct_common_esql_mistakes.ts index 62add102ae58b..11fcd3928695b 100644 --- a/x-pack/plugins/observability_ai_assistant/server/functions/esql/correct_common_esql_mistakes.ts +++ b/x-pack/plugins/observability_ai_assistant/server/functions/esql/correct_common_esql_mistakes.ts @@ -5,56 +5,75 @@ * 2.0. */ -import { Logger } from '@kbn/logging'; +import { isArray } from 'lodash'; +import type { Logger } from '@kbn/logging'; -const DELIMITER_TOKENS = ['`', "'", '"']; +const DELIMITER_TOKENS = ['`', "'", '"', ['(', ')']]; const ESCAPE_TOKEN = '\\\\'; -function splitIntoCommands(query: string) { - const commands: string[] = []; +// this function splits statements by a certain token, +// and takes into account escaping, or function calls + +function split(value: string, splitToken: string) { + const statements: string[] = []; let delimiterToken: string | undefined; - let currentCommand: string = ''; + let currentStatement: string = ''; - const trimmed = query.trim(); + const trimmed = value.trim().split(''); - // @ts-expect-error - // eslint-disable-next-line guard-for-in - for (const indexAsString in trimmed) { - const index = Number(indexAsString); + for (let index = 0; index < trimmed.length; index++) { const char = trimmed[index]; - if (!delimiterToken && char === '|') { - commands.push(currentCommand.trim()); - currentCommand = ''; + if ( + !delimiterToken && + trimmed.slice(index, index + splitToken.length).join('') === splitToken + ) { + index += splitToken.length - 1; + statements.push(currentStatement.trim()); + currentStatement = ''; continue; } - currentCommand += char; + currentStatement += char; if (delimiterToken === char) { // end identifier delimiterToken = undefined; - } else if ( - !delimiterToken && - DELIMITER_TOKENS.includes(char) && - trimmed.substring(index - 2, index - 1) !== ESCAPE_TOKEN - ) { - // start identifier - delimiterToken = char; - continue; + } else if (!delimiterToken && trimmed[index - 1] !== ESCAPE_TOKEN) { + const applicableToken = DELIMITER_TOKENS.find( + (token) => token === char || (isArray(token) && token[0] === char) + ); + if (applicableToken) { + // start identifier + delimiterToken = isArray(applicableToken) ? applicableToken[1] : applicableToken; + continue; + } } } - if (currentCommand) { - commands.push(currentCommand.trim()); + if (currentStatement) { + statements.push(currentStatement.trim()); } - return commands; + return statements; +} + +function splitIntoCommands(query: string) { + const commands: string[] = split(query, '|'); + + return commands.map((command) => { + const commandName = command.match(/^([A-Za-z]+)/)?.[1]; + + return { + name: commandName, + command, + }; + }); } function replaceSingleQuotesWithDoubleQuotes(command: string) { - const regex = /(? { - const commands = splitIntoCommands(query); +function isValidColumnName(column: string) { + return column.match(/^`.*`$/) || column.match(/^[@A-Za-z\._\-]+$/); +} - const correctedFormattedQuery = commands - .map((command) => { - const commandName = command.match(/^([A-Za-z]+)/)?.[1]; +function verifyKeepColumns( + keepCommand: string, + nextCommands: Array<{ name?: string; command: string }> +) { + const columnsInKeep = split(keepCommand.replace(/^KEEP\s*/, ''), ',').map((statement) => + split(statement, '=')?.[0].trim() + ); - let formattedCommand = command; + const availableColumns = columnsInKeep.concat(); - switch (commandName) { - case 'FROM': - formattedCommand = formattedCommand - .replaceAll(/FROM "(.*)"/g, 'FROM `$1`') - .replaceAll(/FROM '(.*)'/g, 'FROM `$1`'); - break; + for (const { name, command } of nextCommands) { + if (['STATS', 'KEEP', 'DROP', 'DISSECT', 'GROK', 'ENRICH'].includes(name || '')) { + // these operations alter columns in a way that is hard to analyze, so we abort + break; + } + + const commandBody = command.replace(/^[A-Za-z]+\s*/, ''); - case 'WHERE': - case 'EVAL': - formattedCommand = replaceSingleQuotesWithDoubleQuotes(formattedCommand); - break; + if (name === 'EVAL') { + // EVAL creates new columns, make them available + const columnsInEval = split(commandBody, ',').map((column) => + split(column.trim(), '=')[0].trim() + ); - case 'STATS': - formattedCommand = replaceAsKeywordWithAssignments(formattedCommand); - break; + columnsInEval.forEach((column) => { + availableColumns.push(column); + }); + } + + if (name === 'RENAME') { + // RENAME creates and removes columns + split(commandBody, ',').forEach((statement) => { + const [prevName, newName] = split(statement, 'AS').map((side) => side.trim()); + availableColumns.push(newName); + if (!availableColumns.includes(prevName)) { + columnsInKeep.push(prevName); } - return formattedCommand; - }) - .join('\n| '); + }); + } + + if (name === 'SORT') { + const columnsInSort = split(commandBody, ',').map((column) => + split(column.trim(), ' ')[0].trim() + ); + + columnsInSort.forEach((column) => { + if (isValidColumnName(column) && !availableColumns.includes(column)) { + columnsInKeep.push(column); + } + }); + } + } + + return `KEEP ${columnsInKeep.join(', ')}`; +} + +export function correctCommonEsqlMistakes(content: string, log: Logger) { + return content.replaceAll(/```esql\n(.*?)\n```/gms, (_, query: string) => { + const commands = splitIntoCommands(query); + + const formattedCommands: string[] = commands.map(({ name, command }, index) => { + let formattedCommand = command; + + switch (name) { + case 'FROM': + formattedCommand = formattedCommand + .replaceAll(/FROM "(.*)"/g, 'FROM `$1`') + .replaceAll(/FROM '(.*)'/g, 'FROM `$1`'); + break; + + case 'WHERE': + case 'EVAL': + formattedCommand = replaceSingleQuotesWithDoubleQuotes(formattedCommand); + break; + + case 'STATS': + formattedCommand = replaceAsKeywordWithAssignments(formattedCommand); + break; + + case 'KEEP': + formattedCommand = verifyKeepColumns(formattedCommand, commands.slice(index + 1)); + break; + } + return formattedCommand; + }); + + const correctedFormattedQuery = formattedCommands.join('\n| '); const originalFormattedQuery = commands.join('\n| '); diff --git a/x-pack/plugins/observability_ai_assistant/server/functions/esql/index.ts b/x-pack/plugins/observability_ai_assistant/server/functions/esql/index.ts index 7e20d1f78c871..a44bf0f5f235c 100644 --- a/x-pack/plugins/observability_ai_assistant/server/functions/esql/index.ts +++ b/x-pack/plugins/observability_ai_assistant/server/functions/esql/index.ts @@ -254,7 +254,9 @@ export function registerEsqlFunction({ return esqlResponse$.pipe( emitWithConcatenatedMessage((msg) => { - const esqlQuery = msg.message.content.match(/```esql([\s\S]*?)```/)?.[1]; + const esqlQuery = correctCommonEsqlMistakes(msg.message.content, resources.logger).match( + /```esql([\s\S]*?)```/ + )?.[1]; return { ...msg, diff --git a/x-pack/plugins/observability_ai_assistant/server/functions/esql/system_message.txt b/x-pack/plugins/observability_ai_assistant/server/functions/esql/system_message.txt index d9ce66038f170..32064d21da4ae 100644 --- a/x-pack/plugins/observability_ai_assistant/server/functions/esql/system_message.txt +++ b/x-pack/plugins/observability_ai_assistant/server/functions/esql/system_message.txt @@ -54,14 +54,21 @@ pattern. Aggregation functions are not supported for EVAL. - GROK: extracts structured data out of a string, using a grok pattern - KEEP: keeps one or more columns, drop the ones that are not kept + only the colums in the KEEP command can be used after a KEEP command - LIMIT: returns the first n number of rows. The maximum value for this is 10000. - MV_EXPAND: expands multi-value columns into a single row per value - RENAME: renames a column -- SORT: sorts the row in a table - STATS ... BY: groups rows according to a common value and calculates -one or more aggregated values over the grouped rows. This commands only - supports aggregation functions, and no other functions or operators. + one or more aggregated values over the grouped rows. This commands only + supports single aggregation functions declared below using an alias, e.g. `STATS count = COUNT(*) by col`. + It does NOT support mathematical operations (+, - , /) of aggregations (e.g. not correct: STATS rate = sum(col1)/sum(col2)), these combinations + must be calculated in a separate step using EVAL, (e.g. correct: STATS sum1 = sum(col1), sum2 = sum(col2) | EVAL rate = sum1/sum2) + It does NOT support other functions not declared below, operators, nor any mathematical operations (+, - or /) +- SORT: sorts the row in a table. Does NOT support sorting by functions. NEVER USE: SORT COUNT(*) by col . INSTEAD USE: STATS count = COUNT(*) by col | SORT count + only supports sorting by column names or aliases, to sort by a function it must first assigned to an alias with STATS + If SORT is used right after a KEEP command, make sure it only uses column names in KEEP, + or move the SORT before the KEEP (e.g. not correct: KEEP date | SORT @timestamp, correct: SORT @timestamp | KEEP date) - WHERE: produces a table that contains all the rows from the input table for which the provided condition returns true. WHERE supports the same functions as EVAL. @@ -198,6 +205,8 @@ ROW a = "2023-01-23T12:15:00.000Z - some text - 127.0.0.1" ```esql FROM employees | WHERE first_name LIKE "?b*" +| STATS doc_count = COUNT(*) by first_name, last_name +| SORT doc_count DESC | KEEP first_name, last_name ``` @@ -232,3 +241,10 @@ FROM logs-* | EVAL failure_rate_per_host = total_failures / total_events | DROP total_events, total_failures ``` + +```esql +FROM logs-* +| WHERE @timestamp <= NOW() - 24 hours +| STATS count = COUNT(*) by log.level +| SORT count DESC +``` \ No newline at end of file diff --git a/x-pack/plugins/observability_ai_assistant/server/service/client/index.ts b/x-pack/plugins/observability_ai_assistant/server/service/client/index.ts index 81399e330ac5c..c1841948c18d9 100644 --- a/x-pack/plugins/observability_ai_assistant/server/service/client/index.ts +++ b/x-pack/plugins/observability_ai_assistant/server/service/client/index.ts @@ -460,6 +460,8 @@ export class ObservabilityAIAssistantClient { }, }); + this.dependencies.logger.debug(`Received action client response: ${executeResult.status}`); + if (executeResult.status === 'error' && executeResult?.serviceMessage) { const tokenLimitRegex = /This model's maximum context length is (\d+) tokens\. However, your messages resulted in (\d+) tokens/g; From 38872c6e94663197913b25437029e2ddfb32b65b Mon Sep 17 00:00:00 2001 From: Nick Peihl Date: Thu, 1 Feb 2024 14:40:30 -0500 Subject: [PATCH 2/5] [Links] Set empty default title for Links saved to library (#176021) Fixes #176016 ## Summary Fixes form validation when saving Links to library. --- src/plugins/links/public/content_management/save_to_library.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/links/public/content_management/save_to_library.tsx b/src/plugins/links/public/content_management/save_to_library.tsx index 6bb00217224cb..e9dba65a532f5 100644 --- a/src/plugins/links/public/content_management/save_to_library.tsx +++ b/src/plugins/links/public/content_management/save_to_library.tsx @@ -75,7 +75,7 @@ export const runSaveToLibrary = async ( resolve(undefined)} - title={newAttributes.title} + title={newAttributes.title ?? ''} customModalTitle={modalTitle} description={newAttributes.description} showDescription From 415b9257bc2ac85a1c70c500260c24f263e03cee Mon Sep 17 00:00:00 2001 From: Matthias Wilhelm Date: Thu, 1 Feb 2024 20:48:43 +0100 Subject: [PATCH 3/5] [Discover] Fix overwriting data view id when loading a saved search with ES|QL (#175769) This commit prevents the propagation of the AppState index (which is the id of the data view in the URL), to the state of a loaded saved search containing a ES|QL query. Since the id of the data view used in this case is retrieved from the text base language statement, there's no reason for mutating the saved search state --- .../public/__mocks__/data_view_esql.ts | 33 +++++++++++++++++++ .../discover/public/__mocks__/saved_search.ts | 6 ++-- .../main/services/discover_state.test.ts | 15 +++++++++ .../main/services/load_saved_search.ts | 6 +++- 4 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 src/plugins/discover/public/__mocks__/data_view_esql.ts diff --git a/src/plugins/discover/public/__mocks__/data_view_esql.ts b/src/plugins/discover/public/__mocks__/data_view_esql.ts new file mode 100644 index 0000000000000..dc8d141a31496 --- /dev/null +++ b/src/plugins/discover/public/__mocks__/data_view_esql.ts @@ -0,0 +1,33 @@ +/* + * 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 { buildDataViewMock } from '@kbn/discover-utils/src/__mocks__'; +import { DataView } from '@kbn/data-views-plugin/common'; + +const fields = [ + { + name: '@timestamp', + displayName: 'timestamp', + type: 'date', + scripted: false, + filterable: true, + aggregatable: true, + sortable: true, + }, + { + name: 'message', + displayName: 'message', + type: 'string', + scripted: false, + filterable: false, + }, +] as DataView['fields']; + +export const dataViewEsql = buildDataViewMock({ + name: 'index-pattern-esql', + fields, +}); diff --git a/src/plugins/discover/public/__mocks__/saved_search.ts b/src/plugins/discover/public/__mocks__/saved_search.ts index 90a2a9c680825..de9a6c199d35a 100644 --- a/src/plugins/discover/public/__mocks__/saved_search.ts +++ b/src/plugins/discover/public/__mocks__/saved_search.ts @@ -11,6 +11,7 @@ import { createSearchSourceMock } from '@kbn/data-plugin/public/mocks'; import { dataViewMock } from '@kbn/discover-utils/src/__mocks__'; import { dataViewWithTimefieldMock } from './data_view_with_timefield'; import { dataViewAdHoc } from './data_view_complex'; +import { dataViewEsql } from './data_view_esql'; export const savedSearchMock = { id: 'the-saved-search-id', @@ -30,9 +31,10 @@ export const savedSearchMockWithTimeFieldNew = { export const savedSearchMockWithESQL = { id: 'the-saved-search-id-esql', searchSource: createSearchSourceMock({ - index: dataViewWithTimefieldMock, - query: { esql: 'FROM "the-saved-search-id-esql"' }, + index: dataViewEsql, + query: { esql: 'FROM "index-pattern-esql"' }, }), + isTextBasedQuery: true, } as unknown as SavedSearch; export const savedSearchAdHoc = { diff --git a/src/plugins/discover/public/application/main/services/discover_state.test.ts b/src/plugins/discover/public/application/main/services/discover_state.test.ts index 2cf1e2c71880d..4662aa8252b02 100644 --- a/src/plugins/discover/public/application/main/services/discover_state.test.ts +++ b/src/plugins/discover/public/application/main/services/discover_state.test.ts @@ -19,6 +19,7 @@ import { savedSearchMock, savedSearchMockWithTimeField, savedSearchMockWithTimeFieldNew, + savedSearchMockWithESQL, } from '../../../__mocks__/saved_search'; import { discoverServiceMock } from '../../../__mocks__/services'; import { dataViewMock } from '@kbn/discover-utils/src/__mocks__'; @@ -650,6 +651,20 @@ describe('Test discover state actions', () => { expect(state.internalState.getState().adHocDataViews[0].id).toBe(adHocDataViewId); }); + test('loadSavedSearch with ES|QL, data view index is not overwritten by URL ', async () => { + const savedSearchMockWithESQLCopy = copySavedSearch(savedSearchMockWithESQL); + const persistedDataViewId = savedSearchMockWithESQLCopy?.searchSource.getField('index')!.id; + const url = "/#?_a=(index:'the-data-view-id')&_g=()"; + const { state } = await getState(url, { + savedSearch: savedSearchMockWithESQLCopy, + isEmptyUrl: false, + }); + const nextSavedSearch = await state.actions.loadSavedSearch({ + savedSearchId: savedSearchMockWithESQL.id, + }); + expect(persistedDataViewId).toBe(nextSavedSearch?.searchSource.getField('index')!.id); + }); + test('onChangeDataView', async () => { const { state, getCurrentUrl } = await getState('/', { savedSearch: savedSearchMock }); const { actions, savedSearchState, dataState, appState } = state; diff --git a/src/plugins/discover/public/application/main/services/load_saved_search.ts b/src/plugins/discover/public/application/main/services/load_saved_search.ts index 8b8dcc2beb2f4..a921e7a69e58c 100644 --- a/src/plugins/discover/public/application/main/services/load_saved_search.ts +++ b/src/plugins/discover/public/application/main/services/load_saved_search.ts @@ -78,7 +78,11 @@ export const loadSavedSearch = async ( savedSearch: nextSavedSearch, }); const dataViewDifferentToAppState = stateDataView.id !== savedSearchDataViewId; - if (stateDataView && (dataViewDifferentToAppState || !savedSearchDataViewId)) { + if ( + !nextSavedSearch.isTextBasedQuery && + stateDataView && + (dataViewDifferentToAppState || !savedSearchDataViewId) + ) { nextSavedSearch.searchSource.setField('index', stateDataView); } } From 585630d0601011e5730392e340690502ba78ce5c Mon Sep 17 00:00:00 2001 From: Elena Stoeva <59341489+ElenaStoeva@users.noreply.github.com> Date: Thu, 1 Feb 2024 19:49:54 +0000 Subject: [PATCH 4/5] [Ingest Pipelines] Fix gsub processor's replacement field serialization (#175832) Fixes https://github.com/elastic/kibana/issues/170904 ## Summary This PR adds serialization to the `replacement` field in the Gsub processor edit form so that the field renders the text the way it is provided in the Es request, with unescaped special characters, if any exist. For example, we want `\n` to render the same, not to render as a new line (empty string) in the `replacement` field in the UI. ### Testing **Deserialization:** 1. Open Dev tools and create an ingest pipeline: ``` PUT _ingest/pipeline/my_pipeline_1 { "description": "My test pipeline", "processors": [ { "gsub": { "field": "message", "pattern": "\\\\n", "replacement": "\n" } }] } ``` 3. Run `GET _ingest/pipeline/my_pipeline_1` and observe the `replacement` fields (should be displayed as a new line). 4. Go to Stack Management -> Ingest Pipelines and start editing `my_pipeline_1`. The `replacement` field in the edit form should render as `\n` instead of an empty string. **Serialization:** 1. Go to Stack Management -> Ingest Pipelines and create a new pipeline `my_pipeline_2` with a Gsub processor on the field `message`, pattern: `\\\\n`, and replacement: `\n`. Save the pipeline 2. Go to Dev tools and get the pipeline: `GET _ingest/pipeline/my_pipeline_2`. The `replacement` field in the response should be displayed as a new line: ``` { "my_pipeline": { "description": "My test pipeline", "processors": [ { "gsub": { "field": "message", "pattern": """\\n""", "replacement": """ """ } } ] } } ``` 3. Edit the pipeline in the UI and change the `replacement` field to `\\n`. 4. Now the response in Dev tools should display `\n` (the new line character was escaped). You can also test with other escape characters. For example, creating a pipeline with a `my\ttab` replacement field in the UI should display `my tab` in the response. --- .../components/processor_form/processors/gsub.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/processor_form/processors/gsub.tsx b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/processor_form/processors/gsub.tsx index 11d06f3cca6fb..7e72848485c11 100644 --- a/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/processor_form/processors/gsub.tsx +++ b/x-pack/plugins/ingest_pipelines/public/application/components/pipeline_editor/components/processor_form/processors/gsub.tsx @@ -52,6 +52,8 @@ const fieldsConfig: FieldsConfig = { label: i18n.translate('xpack.ingestPipelines.pipelineEditor.gsubForm.replacementFieldLabel', { defaultMessage: 'Replacement', }), + deserializer: flow(String, to.escapeBackslashes), + serializer: from.unescapeBackslashes, helpText: i18n.translate( 'xpack.ingestPipelines.pipelineEditor.gsubForm.replacementFieldHelpText', { From 1fe7833a23fa6de8b3a5c3e1912c2dd9c858d8f7 Mon Sep 17 00:00:00 2001 From: Luke G <11671118+lgestc@users.noreply.github.com> Date: Thu, 1 Feb 2024 20:58:14 +0100 Subject: [PATCH 5/5] [Security Solution][Flyout] Use kbn url storage provider, housekeeping (#175882) ## Summary **This does not change any of the api's defined previously.** Changes: - replace custom hook for url serialization with off the shelf solution from public plugins - cleanup the codebase futher - remove internal context based hook ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- packages/kbn-expandable-flyout/README.md | 10 +- packages/kbn-expandable-flyout/index.ts | 7 +- packages/kbn-expandable-flyout/kibana.jsonc | 2 +- packages/kbn-expandable-flyout/src/actions.ts | 7 ++ .../src/components/preview_section.test.tsx | 26 ++--- .../kbn-expandable-flyout/src/context.tsx | 57 --------- .../src/context/url_state_provider.tsx | 109 ------------------ .../use_expandable_flyout_api.ts} | 50 ++------ .../src/hooks/use_expandable_flyout_state.ts | 16 +++ .../src/hooks/use_left_panel.ts | 23 ---- .../src/hooks/use_preview_panel.ts | 23 ---- .../src/hooks/use_right_panel.ts | 23 ---- .../src/index.stories.tsx | 98 ++++++++-------- .../kbn-expandable-flyout/src/index.test.tsx | 1 - packages/kbn-expandable-flyout/src/index.tsx | 8 +- .../kbn-expandable-flyout/src/provider.tsx | 71 ++++++++++-- .../kbn-expandable-flyout/src/reducer.test.ts | 29 ++++- packages/kbn-expandable-flyout/src/reducer.ts | 19 +++ packages/kbn-expandable-flyout/src/redux.ts | 29 +++++ packages/kbn-expandable-flyout/src/state.ts | 8 ++ .../src/test/provider.tsx | 11 +- packages/kbn-expandable-flyout/src/types.ts | 6 - packages/kbn-expandable-flyout/tsconfig.json | 2 +- .../right/components/header_actions.test.tsx | 9 +- .../components/investigation_guide.test.tsx | 3 +- .../shared/mocks/mock_flyout_context.ts | 11 +- .../components/flyout_navigation.test.tsx | 7 +- .../body/renderers/host_name.test.tsx | 8 +- .../alert_details_url_sync.cy.ts | 8 +- 29 files changed, 264 insertions(+), 417 deletions(-) delete mode 100644 packages/kbn-expandable-flyout/src/context.tsx delete mode 100644 packages/kbn-expandable-flyout/src/context/url_state_provider.tsx rename packages/kbn-expandable-flyout/src/{context/memory_state_provider.tsx => hooks/use_expandable_flyout_api.ts} (66%) create mode 100644 packages/kbn-expandable-flyout/src/hooks/use_expandable_flyout_state.ts delete mode 100644 packages/kbn-expandable-flyout/src/hooks/use_left_panel.ts delete mode 100644 packages/kbn-expandable-flyout/src/hooks/use_preview_panel.ts delete mode 100644 packages/kbn-expandable-flyout/src/hooks/use_right_panel.ts create mode 100644 packages/kbn-expandable-flyout/src/redux.ts diff --git a/packages/kbn-expandable-flyout/README.md b/packages/kbn-expandable-flyout/README.md index 0f07b0679c94a..ceac69e20722f 100644 --- a/packages/kbn-expandable-flyout/README.md +++ b/packages/kbn-expandable-flyout/README.md @@ -27,7 +27,11 @@ The expandable-flyout is making some strict UI design decisions: The ExpandableFlyout [React component](https://github.com/elastic/kibana/tree/main/packages/kbn-expandable-flyout/src/index.tsx) renders the UI, leveraging an [EuiFlyout](https://eui.elastic.co/#/layout/flyout). -The ExpandableFlyout [hooks](https://github.com/elastic/kibana/blob/main/packages/kbn-expandable-flyout/src/context.tsx) expose the state and the following api: +To retrieve the flyout's layout (left, right and preview panels), you can utilize [useExpandableFlyoutState](https://github.com/elastic/kibana/blob/main/packages/kbn-expandable-flyout/src/hooks/use_expandable_flyout_state.ts). + +To control (or mutate) flyout's layout, you can utilize [useExpandableFlyoutApi](https://github.com/elastic/kibana/blob/main/packages/kbn-expandable-flyout/src/hooks/use_expandable_flyout_api.ts). + +**Expandable Flyout API** exposes the following methods: - **openFlyout**: open the flyout with a set of panels - **openRightPanel**: open a right panel - **openLeftPanel**: open a left panel @@ -38,10 +42,6 @@ The ExpandableFlyout [hooks](https://github.com/elastic/kibana/blob/main/package - **previousPreviewPanel**: navigate to the previous preview panel - **closeFlyout**: close the flyout -To retrieve the flyout's layout (left, right and preview panels), you can use the **useExpandableFlyoutState** from the same [React context](https://github.com/elastic/kibana/blob/main/packages/kbn-expandable-flyout/src/context.tsx). - -To control (or mutate) flyout's layout, you can use the **useExpandableFlyoutApi** from the same [React context](https://github.com/elastic/kibana/blob/main/packages/kbn-expandable-flyout/src/context.tsx). - ## Usage To use the expandable flyout in your plugin, first you need wrap your code with the [context provider](https://github.com/elastic/kibana/blob/main/packages/kbn-expandable-flyout/src/context.tsx) at a high enough level as follows: diff --git a/packages/kbn-expandable-flyout/index.ts b/packages/kbn-expandable-flyout/index.ts index 90849391fe4a6..d4299d676667f 100644 --- a/packages/kbn-expandable-flyout/index.ts +++ b/packages/kbn-expandable-flyout/index.ts @@ -8,11 +8,8 @@ export { ExpandableFlyout } from './src'; -export { - type ExpandableFlyoutContext, - useExpandableFlyoutState, - useExpandableFlyoutApi, -} from './src/context'; +export { useExpandableFlyoutApi } from './src/hooks/use_expandable_flyout_api'; +export { useExpandableFlyoutState } from './src/hooks/use_expandable_flyout_state'; export { type State as ExpandableFlyoutState } from './src/state'; diff --git a/packages/kbn-expandable-flyout/kibana.jsonc b/packages/kbn-expandable-flyout/kibana.jsonc index b4f63cca6bf91..ae15fc604a1d8 100644 --- a/packages/kbn-expandable-flyout/kibana.jsonc +++ b/packages/kbn-expandable-flyout/kibana.jsonc @@ -1,5 +1,5 @@ { - "type": "shared-common", + "type": "shared-browser", "id": "@kbn/expandable-flyout", "owner": "@elastic/security-threat-hunting-investigations" } diff --git a/packages/kbn-expandable-flyout/src/actions.ts b/packages/kbn-expandable-flyout/src/actions.ts index ce3dd7e208b4c..56b6317032bc6 100644 --- a/packages/kbn-expandable-flyout/src/actions.ts +++ b/packages/kbn-expandable-flyout/src/actions.ts @@ -19,6 +19,7 @@ export enum ActionType { closePreviewPanel = 'close_preview_panel', previousPreviewPanel = 'previous_preview_panel', closeFlyout = 'close_flyout', + urlChanged = 'urlChanged', } export const openPanelsAction = createAction<{ @@ -37,3 +38,9 @@ export const closeLeftPanelAction = createAction(ActionType.closeLeftPanel); export const closePreviewPanelAction = createAction(ActionType.closePreviewPanel); export const previousPreviewPanelAction = createAction(ActionType.previousPreviewPanel); + +export const urlChangedAction = createAction<{ + right?: FlyoutPanelProps; + left?: FlyoutPanelProps; + preview?: FlyoutPanelProps; +}>(ActionType.urlChanged); diff --git a/packages/kbn-expandable-flyout/src/components/preview_section.test.tsx b/packages/kbn-expandable-flyout/src/components/preview_section.test.tsx index d3bd510266084..223d73fa5cf43 100644 --- a/packages/kbn-expandable-flyout/src/components/preview_section.test.tsx +++ b/packages/kbn-expandable-flyout/src/components/preview_section.test.tsx @@ -14,21 +14,19 @@ import { PREVIEW_SECTION_CLOSE_BUTTON_TEST_ID, PREVIEW_SECTION_TEST_ID, } from './test_ids'; -import { ExpandableFlyoutContextValue } from '../context'; import { TestProvider } from '../test/provider'; +import { State } from '../state'; describe('PreviewSection', () => { const context = { - panels: { - right: {}, - left: {}, - preview: [ - { - id: 'key', - }, - ], - }, - } as unknown as ExpandableFlyoutContextValue; + right: {}, + left: {}, + preview: [ + { + id: 'key', + }, + ], + } as unknown as State; const component =
{'component'}
; const left = 500; @@ -37,7 +35,7 @@ describe('PreviewSection', () => { const showBackButton = false; const { getByTestId } = render( - + ); @@ -49,7 +47,7 @@ describe('PreviewSection', () => { const showBackButton = true; const { getByTestId } = render( - + ); @@ -67,7 +65,7 @@ describe('PreviewSection', () => { }; const { getByTestId, getByText } = render( - + ( - undefined -); - -/** - * Retrieve Flyout's api and state - * @deprecated - */ -export const useExpandableFlyoutContext = (): ExpandableFlyoutApi => { - const contextValue = useContext(ExpandableFlyoutContext); - - if (!contextValue) { - throw new Error( - 'ExpandableFlyoutContext can only be used within ExpandableFlyoutContext provider' - ); - } - - const memoryState = useFlyoutMemoryState(); - const urlState = useFlyoutUrlState(); - - return contextValue === 'memory' ? memoryState : urlState; -}; - -/** - * This hook allows you to interact with the flyout, open panels and previews etc. - */ -export const useExpandableFlyoutApi = () => { - const { panels, ...api } = useExpandableFlyoutContext(); - - return api; -}; - -/** - * This hook allows you to access the flyout state, read open panels and previews. - */ -export const useExpandableFlyoutState = () => { - const expandableFlyoutApiAndState = useExpandableFlyoutContext(); - - return expandableFlyoutApiAndState.panels; -}; diff --git a/packages/kbn-expandable-flyout/src/context/url_state_provider.tsx b/packages/kbn-expandable-flyout/src/context/url_state_provider.tsx deleted file mode 100644 index c26525cf0e594..0000000000000 --- a/packages/kbn-expandable-flyout/src/context/url_state_provider.tsx +++ /dev/null @@ -1,109 +0,0 @@ -/* - * 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 { useCallback, useMemo } from 'react'; -import { FlyoutPanelProps, ExpandableFlyoutApi } from '../types'; -import { useRightPanel } from '../hooks/use_right_panel'; -import { useLeftPanel } from '../hooks/use_left_panel'; -import { usePreviewPanel } from '../hooks/use_preview_panel'; -import { State } from '../state'; - -export const useFlyoutUrlState = (): ExpandableFlyoutApi => { - const { setRightPanelState, rightPanelState } = useRightPanel(); - const { setLeftPanelState, leftPanelState } = useLeftPanel(); - const { previewState, setPreviewState } = usePreviewPanel(); - - const panels: State = useMemo( - () => ({ - left: leftPanelState, - right: rightPanelState, - preview: previewState || [], - }), - [leftPanelState, previewState, rightPanelState] - ); - - const openPanels = useCallback( - ({ - right, - left, - preview, - }: { - right?: FlyoutPanelProps; - left?: FlyoutPanelProps; - preview?: FlyoutPanelProps; - }) => { - setRightPanelState(right); - setLeftPanelState(left); - setPreviewState(preview ? [preview] : []); - }, - [setRightPanelState, setLeftPanelState, setPreviewState] - ); - - const openRightPanel = useCallback( - (panel: FlyoutPanelProps) => { - setRightPanelState(panel); - }, - [setRightPanelState] - ); - - const openLeftPanel = useCallback( - (panel: FlyoutPanelProps) => setLeftPanelState(panel), - [setLeftPanelState] - ); - - const openPreviewPanel = useCallback( - (panel: FlyoutPanelProps) => setPreviewState([...(previewState ?? []), panel]), - [previewState, setPreviewState] - ); - - const closeRightPanel = useCallback(() => setRightPanelState(undefined), [setRightPanelState]); - - const closeLeftPanel = useCallback(() => setLeftPanelState(undefined), [setLeftPanelState]); - - const closePreviewPanel = useCallback(() => setPreviewState([]), [setPreviewState]); - - const previousPreviewPanel = useCallback( - () => setPreviewState(previewState?.slice(0, previewState.length - 1)), - [previewState, setPreviewState] - ); - - const closePanels = useCallback(() => { - setRightPanelState(undefined); - setLeftPanelState(undefined); - setPreviewState([]); - }, [setRightPanelState, setLeftPanelState, setPreviewState]); - - const contextValue: ExpandableFlyoutApi = useMemo( - () => ({ - panels, - openFlyout: openPanels, - openRightPanel, - openLeftPanel, - openPreviewPanel, - closeRightPanel, - closeLeftPanel, - closePreviewPanel, - closeFlyout: closePanels, - previousPreviewPanel, - }), - [ - panels, - openPanels, - openRightPanel, - openLeftPanel, - openPreviewPanel, - closeRightPanel, - closeLeftPanel, - closePreviewPanel, - closePanels, - previousPreviewPanel, - ] - ); - - return contextValue; -}; diff --git a/packages/kbn-expandable-flyout/src/context/memory_state_provider.tsx b/packages/kbn-expandable-flyout/src/hooks/use_expandable_flyout_api.ts similarity index 66% rename from packages/kbn-expandable-flyout/src/context/memory_state_provider.tsx rename to packages/kbn-expandable-flyout/src/hooks/use_expandable_flyout_api.ts index a4981c5a2122b..9f42870a31c0f 100644 --- a/packages/kbn-expandable-flyout/src/context/memory_state_provider.tsx +++ b/packages/kbn-expandable-flyout/src/hooks/use_expandable_flyout_api.ts @@ -6,18 +6,7 @@ * Side Public License, v 1. */ -import React, { createContext, FC, useCallback, useMemo } from 'react'; -import { - createDispatchHook, - createSelectorHook, - Provider as ReduxProvider, - ReactReduxContextValue, -} from 'react-redux'; -import { configureStore } from '@reduxjs/toolkit'; - -import { reducer } from '../reducer'; -import { initialState, State } from '../state'; -import type { ExpandableFlyoutApi, FlyoutPanelProps } from '../types'; +import { useCallback, useMemo } from 'react'; import { closeLeftPanelAction, closePanelsAction, @@ -29,24 +18,15 @@ import { openRightPanelAction, previousPreviewPanelAction, } from '../actions'; +import { useDispatch } from '../redux'; +import { FlyoutPanelProps, type ExpandableFlyoutApi } from '../types'; -export const store = configureStore({ - reducer, - devTools: process.env.NODE_ENV !== 'production', - preloadedState: {}, - enhancers: [], -}); - -export const Context = createContext>({ - store, - storeState: initialState, -}); +export type { ExpandableFlyoutApi }; -const useDispatch = createDispatchHook(Context); -const useSelector = createSelectorHook(Context); - -export const useFlyoutMemoryState = (): ExpandableFlyoutApi => { - const state = useSelector((s) => s); +/** + * This hook allows you to interact with the flyout, open panels and previews etc. + */ +export const useExpandableFlyoutApi = () => { const dispatch = useDispatch(); const openPanels = useCallback( @@ -94,7 +74,6 @@ export const useFlyoutMemoryState = (): ExpandableFlyoutApi => { const api: ExpandableFlyoutApi = useMemo( () => ({ - panels: state, openFlyout: openPanels, openRightPanel, openLeftPanel, @@ -106,7 +85,6 @@ export const useFlyoutMemoryState = (): ExpandableFlyoutApi => { previousPreviewPanel, }), [ - state, openPanels, openRightPanel, openLeftPanel, @@ -121,15 +99,3 @@ export const useFlyoutMemoryState = (): ExpandableFlyoutApi => { return api; }; - -/** - * In-memory state provider for the expandable flyout, for cases when we don't want changes to be persisted - * in the url. - */ -export const MemoryStateProvider: FC = ({ children }) => { - return ( - - {children} - - ); -}; diff --git a/packages/kbn-expandable-flyout/src/hooks/use_expandable_flyout_state.ts b/packages/kbn-expandable-flyout/src/hooks/use_expandable_flyout_state.ts new file mode 100644 index 0000000000000..3ae9d69c4d0bc --- /dev/null +++ b/packages/kbn-expandable-flyout/src/hooks/use_expandable_flyout_state.ts @@ -0,0 +1,16 @@ +/* + * 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 { stateSelector, useSelector } from '../redux'; + +/** + * This hook allows you to access the flyout state, read open panels and previews. + */ +export const useExpandableFlyoutState = () => { + return useSelector(stateSelector); +}; diff --git a/packages/kbn-expandable-flyout/src/hooks/use_left_panel.ts b/packages/kbn-expandable-flyout/src/hooks/use_left_panel.ts deleted file mode 100644 index 2a3e4212a06fe..0000000000000 --- a/packages/kbn-expandable-flyout/src/hooks/use_left_panel.ts +++ /dev/null @@ -1,23 +0,0 @@ -/* - * 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 { useUrlState } from '@kbn/url-state'; -import { EXPANDABLE_FLYOUT_URL_KEY } from '../constants'; -import { FlyoutPanelProps } from '../types'; - -/** - * This hook stores state in the URL - */ -export const useLeftPanel = () => { - const [leftPanelState, setLeftPanelState] = useUrlState( - EXPANDABLE_FLYOUT_URL_KEY, - 'leftPanel' - ); - - return { leftPanelState, setLeftPanelState } as const; -}; diff --git a/packages/kbn-expandable-flyout/src/hooks/use_preview_panel.ts b/packages/kbn-expandable-flyout/src/hooks/use_preview_panel.ts deleted file mode 100644 index 5e9cfddb93ba4..0000000000000 --- a/packages/kbn-expandable-flyout/src/hooks/use_preview_panel.ts +++ /dev/null @@ -1,23 +0,0 @@ -/* - * 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 { useUrlState } from '@kbn/url-state'; -import { EXPANDABLE_FLYOUT_URL_KEY } from '../constants'; -import { FlyoutPanelProps } from '../types'; - -/** - * This hook stores state in the URL - */ -export const usePreviewPanel = () => { - const [previewState, setPreviewState] = useUrlState( - EXPANDABLE_FLYOUT_URL_KEY, - 'preview' - ); - - return { previewState, setPreviewState } as const; -}; diff --git a/packages/kbn-expandable-flyout/src/hooks/use_right_panel.ts b/packages/kbn-expandable-flyout/src/hooks/use_right_panel.ts deleted file mode 100644 index 2bce75d65f23e..0000000000000 --- a/packages/kbn-expandable-flyout/src/hooks/use_right_panel.ts +++ /dev/null @@ -1,23 +0,0 @@ -/* - * 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 { useUrlState } from '@kbn/url-state'; -import { EXPANDABLE_FLYOUT_URL_KEY } from '../constants'; -import { FlyoutPanelProps } from '../types'; - -/** - * This hook stores state in the URL - */ -export const useRightPanel = () => { - const [rightPanelState, setRightPanelState] = useUrlState( - EXPANDABLE_FLYOUT_URL_KEY, - 'rightPanel' - ); - - return { rightPanelState, setRightPanelState } as const; -}; diff --git a/packages/kbn-expandable-flyout/src/index.stories.tsx b/packages/kbn-expandable-flyout/src/index.stories.tsx index 8c6bacb20adf6..e02e3de8b791a 100644 --- a/packages/kbn-expandable-flyout/src/index.stories.tsx +++ b/packages/kbn-expandable-flyout/src/index.stories.tsx @@ -19,8 +19,8 @@ import { EuiTitle, } from '@elastic/eui'; import { ExpandableFlyout } from '.'; -import { ExpandableFlyoutContextValue } from './context'; import { TestProvider } from './test/provider'; +import { State } from './state'; export default { component: ExpandableFlyout, @@ -101,89 +101,81 @@ const registeredPanels = [ ]; export const Right: Story = () => { - const context = { - panels: { - right: { - id: 'right', - }, - left: {}, - preview: [], + const state = { + right: { + id: 'right', }, - } as unknown as ExpandableFlyoutContextValue; + left: {}, + preview: [], + } as unknown as State; return ( - + ); }; export const Left: Story = () => { - const context = { - panels: { - right: { - id: 'right', - }, - left: { - id: 'left', - }, - preview: [], + const state = { + right: { + id: 'right', }, - } as unknown as ExpandableFlyoutContextValue; + left: { + id: 'left', + }, + preview: [], + } as unknown as State; return ( - + ); }; export const Preview: Story = () => { - const context = { - panels: { - right: { - id: 'right', - }, - left: { - id: 'left', - }, - preview: [ - { - id: 'preview1', - }, - ], + const state = { + right: { + id: 'right', }, - } as unknown as ExpandableFlyoutContextValue; + left: { + id: 'left', + }, + preview: [ + { + id: 'preview1', + }, + ], + } as unknown as State; return ( - + ); }; export const MultiplePreviews: Story = () => { - const context = { - panels: { - right: { - id: 'right', + const state = { + right: { + id: 'right', + }, + left: { + id: 'left', + }, + preview: [ + { + id: 'preview1', }, - left: { - id: 'left', + { + id: 'preview2', }, - preview: [ - { - id: 'preview1', - }, - { - id: 'preview2', - }, - ], - }, - } as unknown as ExpandableFlyoutContextValue; + ], + } as unknown as State; return ( - + ); diff --git a/packages/kbn-expandable-flyout/src/index.test.tsx b/packages/kbn-expandable-flyout/src/index.test.tsx index a2265adfa5eea..0b8b62ce7187a 100644 --- a/packages/kbn-expandable-flyout/src/index.test.tsx +++ b/packages/kbn-expandable-flyout/src/index.test.tsx @@ -18,7 +18,6 @@ import { } from './components/test_ids'; import { type State } from './state'; import { TestProvider } from './test/provider'; -jest.mock('./context/url_state_provider'); const registeredPanels: Panel[] = [ { diff --git a/packages/kbn-expandable-flyout/src/index.tsx b/packages/kbn-expandable-flyout/src/index.tsx index e7f6c3fcafb23..28dd6a92ff952 100644 --- a/packages/kbn-expandable-flyout/src/index.tsx +++ b/packages/kbn-expandable-flyout/src/index.tsx @@ -11,7 +11,8 @@ import { EuiFlyoutProps } from '@elastic/eui'; import { EuiFlexGroup, EuiFlyout } from '@elastic/eui'; import { useSectionSizes } from './hooks/use_sections_sizes'; import { useWindowSize } from './hooks/use_window_size'; -import { useExpandableFlyoutContext } from './context'; +import { useExpandableFlyoutState } from './hooks/use_expandable_flyout_state'; +import { useExpandableFlyoutApi } from './hooks/use_expandable_flyout_api'; import { PreviewSection } from './components/preview_section'; import { RightSection } from './components/right_section'; import type { FlyoutPanelProps, Panel } from './types'; @@ -40,9 +41,8 @@ export const ExpandableFlyout: React.FC = ({ }) => { const windowWidth = useWindowSize(); - const { closeFlyout, panels } = useExpandableFlyoutContext(); - - const { left, right, preview } = panels; + const { left, right, preview } = useExpandableFlyoutState(); + const { closeFlyout } = useExpandableFlyoutApi(); const leftSection = useMemo( () => registeredPanels.find((panel) => panel.key === left?.id), diff --git a/packages/kbn-expandable-flyout/src/provider.tsx b/packages/kbn-expandable-flyout/src/provider.tsx index 63c8aec6b9bf4..ba18bd189f6e4 100644 --- a/packages/kbn-expandable-flyout/src/provider.tsx +++ b/packages/kbn-expandable-flyout/src/provider.tsx @@ -6,16 +6,70 @@ * Side Public License, v 1. */ -import React, { FC, PropsWithChildren } from 'react'; -import { ExpandableFlyoutContext } from './context'; -import { MemoryStateProvider } from './context/memory_state_provider'; +import { createKbnUrlStateStorage } from '@kbn/kibana-utils-plugin/public'; +import React, { FC, PropsWithChildren, useEffect, useMemo } from 'react'; +import { Provider as ReduxProvider } from 'react-redux'; + +import { useHistory } from 'react-router-dom'; +import { State } from './state'; +import { useExpandableFlyoutState } from './hooks/use_expandable_flyout_state'; +import { EXPANDABLE_FLYOUT_URL_KEY } from './constants'; +import { Context, store, useDispatch } from './redux'; +import { urlChangedAction } from './actions'; + +export type ExpandableFlyoutStorageMode = 'memory' | 'url'; + +/** + * Dispatches actions when url state changes and initializes the state when the app is loaded with flyout url parameters + */ +const UrlSynchronizer = () => { + const state = useExpandableFlyoutState(); + const dispatch = useDispatch(); + + const history = useHistory(); + + const urlStorage = useMemo( + () => + createKbnUrlStateStorage({ + history, + useHash: false, + useHashQuery: false, + }), + [history] + ); + + useEffect(() => { + const currentValue = urlStorage.get(EXPANDABLE_FLYOUT_URL_KEY); + + // Dispatch current value to redux store as it does not happen automatically + if (currentValue) { + dispatch(urlChangedAction({ ...currentValue, preview: currentValue?.preview[0] })); + } + + const subscription = urlStorage.change$(EXPANDABLE_FLYOUT_URL_KEY).subscribe((value) => { + dispatch(urlChangedAction({ ...value, preview: value?.preview?.[0] })); + }); + + return () => subscription.unsubscribe(); + }, [dispatch, urlStorage]); + + useEffect(() => { + const { needsSync, ...stateToSync } = state; + + if (needsSync) { + urlStorage.set(EXPANDABLE_FLYOUT_URL_KEY, stateToSync); + } + }, [urlStorage, state]); + + return null; +}; interface ExpandableFlyoutProviderProps { /** * This allows the user to choose how the flyout storage is handled. * Url storage syncs current values straight to the browser query string. */ - storage?: 'url' | 'memory'; + storage?: ExpandableFlyoutStorageMode; } /** @@ -30,8 +84,11 @@ export const ExpandableFlyoutProvider: FC { return ( - - {children} - + + <> + {storage === 'url' ? : null} + {children} + + ); }; diff --git a/packages/kbn-expandable-flyout/src/reducer.test.ts b/packages/kbn-expandable-flyout/src/reducer.test.ts index 92a9d2d8309e5..db18fbee3e2d7 100644 --- a/packages/kbn-expandable-flyout/src/reducer.test.ts +++ b/packages/kbn-expandable-flyout/src/reducer.test.ts @@ -61,6 +61,7 @@ describe('reducer', () => { left: leftPanel1, right: rightPanel1, preview: [previewPanel1], + needsSync: true, }); }); @@ -81,6 +82,7 @@ describe('reducer', () => { left: leftPanel2, right: rightPanel2, preview: [previewPanel2], + needsSync: true, }); }); @@ -99,6 +101,7 @@ describe('reducer', () => { left: undefined, right: rightPanel2, preview: [], + needsSync: true, }); }); }); @@ -113,6 +116,7 @@ describe('reducer', () => { left: undefined, right: rightPanel1, preview: [], + needsSync: true, }); }); @@ -129,6 +133,7 @@ describe('reducer', () => { left: leftPanel1, right: rightPanel2, preview: [previewPanel1], + needsSync: true, }); }); }); @@ -143,6 +148,7 @@ describe('reducer', () => { left: leftPanel1, right: undefined, preview: [], + needsSync: true, }); }); @@ -159,6 +165,7 @@ describe('reducer', () => { left: leftPanel2, right: rightPanel1, preview: [previewPanel1], + needsSync: true, }); }); }); @@ -173,6 +180,7 @@ describe('reducer', () => { left: undefined, right: undefined, preview: [previewPanel1], + needsSync: true, }); }); @@ -189,6 +197,7 @@ describe('reducer', () => { left: leftPanel1, right: rightPanel1, preview: [previewPanel1, previewPanel2], + needsSync: true, }); }); }); @@ -199,7 +208,7 @@ describe('reducer', () => { const action = closeRightPanelAction(); const newState: State = reducer(state, action); - expect(newState).toEqual(state); + expect(newState).toEqual({ ...state, needsSync: true }); }); it(`should return unmodified state when removing right panel when no right panel exist`, () => { @@ -207,6 +216,7 @@ describe('reducer', () => { left: leftPanel1, right: undefined, preview: [previewPanel1], + needsSync: true, }; const action = closeRightPanelAction(); const newState: State = reducer(state, action); @@ -228,6 +238,7 @@ describe('reducer', () => { left: leftPanel1, right: undefined, preview: [previewPanel1], + needsSync: true, }); }); }); @@ -238,7 +249,7 @@ describe('reducer', () => { const action = closeLeftPanelAction(); const newState: State = reducer(state, action); - expect(newState).toEqual(state); + expect(newState).toEqual({ ...state, needsSync: true }); }); it(`should return unmodified state when removing left panel when no left panel exist`, () => { @@ -246,6 +257,7 @@ describe('reducer', () => { left: undefined, right: rightPanel1, preview: [], + needsSync: true, }; const action = closeLeftPanelAction(); const newState: State = reducer(state, action); @@ -266,13 +278,14 @@ describe('reducer', () => { left: undefined, right: rightPanel1, preview: [previewPanel1], + needsSync: true, }); }); }); describe('should handle closePreviewPanel action', () => { it('should return empty state when removing preview panel on empty state', () => { - const state: State = initialState; + const state: State = { ...initialState, needsSync: true }; const action = closePreviewPanelAction(); const newState: State = reducer(state, action); @@ -288,7 +301,7 @@ describe('reducer', () => { const action = closePreviewPanelAction(); const newState: State = reducer(state, action); - expect(newState).toEqual(state); + expect(newState).toEqual({ ...state, needsSync: true }); }); it('should remove all preview panels', () => { @@ -304,6 +317,7 @@ describe('reducer', () => { left: rightPanel1, right: leftPanel1, preview: [], + needsSync: true, }); }); }); @@ -314,7 +328,7 @@ describe('reducer', () => { const action = previousPreviewPanelAction(); const newState: State = reducer(state, action); - expect(newState).toEqual(state); + expect(newState).toEqual({ ...initialState, needsSync: true }); }); it(`should return unmodified state when previous preview panel when no preview panel exist`, () => { @@ -322,6 +336,7 @@ describe('reducer', () => { left: leftPanel1, right: rightPanel1, preview: [], + needsSync: true, }; const action = previousPreviewPanelAction(); const newState: State = reducer(state, action); @@ -342,6 +357,7 @@ describe('reducer', () => { left: leftPanel1, right: rightPanel1, preview: [previewPanel1], + needsSync: true, }); }); }); @@ -352,7 +368,7 @@ describe('reducer', () => { const action = closePanelsAction(); const newState: State = reducer(state, action); - expect(newState).toEqual(initialState); + expect(newState).toEqual({ ...initialState, needsSync: true }); }); it('should remove all panels', () => { @@ -368,6 +384,7 @@ describe('reducer', () => { left: undefined, right: undefined, preview: [], + needsSync: true, }); }); }); diff --git a/packages/kbn-expandable-flyout/src/reducer.ts b/packages/kbn-expandable-flyout/src/reducer.ts index fe3d40ce9a797..198f99d2785a6 100644 --- a/packages/kbn-expandable-flyout/src/reducer.ts +++ b/packages/kbn-expandable-flyout/src/reducer.ts @@ -17,6 +17,7 @@ import { closeRightPanelAction, previousPreviewPanelAction, openPreviewPanelAction, + urlChangedAction, } from './actions'; import { initialState } from './state'; @@ -25,39 +26,57 @@ export const reducer = createReducer(initialState, (builder) => { state.preview = preview ? [preview] : []; state.right = right; state.left = left; + state.needsSync = true; }); builder.addCase(openLeftPanelAction, (state, { payload }) => { state.left = payload; + state.needsSync = true; }); builder.addCase(openRightPanelAction, (state, { payload }) => { state.right = payload; + state.needsSync = true; }); builder.addCase(openPreviewPanelAction, (state, { payload }) => { state.preview.push(payload); + state.needsSync = true; }); builder.addCase(previousPreviewPanelAction, (state) => { state.preview.pop(); + state.needsSync = true; }); builder.addCase(closePanelsAction, (state) => { state.preview = []; state.right = undefined; state.left = undefined; + state.needsSync = true; }); builder.addCase(closeLeftPanelAction, (state) => { state.left = undefined; + state.needsSync = true; }); builder.addCase(closeRightPanelAction, (state) => { state.right = undefined; + state.needsSync = true; }); builder.addCase(closePreviewPanelAction, (state) => { state.preview = []; + state.needsSync = true; + }); + + builder.addCase(urlChangedAction, (state, { payload: { preview, left, right } }) => { + state.needsSync = false; + state.preview = preview ? [preview] : []; + state.left = left; + state.right = right; + + return state; }); }); diff --git a/packages/kbn-expandable-flyout/src/redux.ts b/packages/kbn-expandable-flyout/src/redux.ts new file mode 100644 index 0000000000000..d1f9d63c8a1a7 --- /dev/null +++ b/packages/kbn-expandable-flyout/src/redux.ts @@ -0,0 +1,29 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 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 { createContext } from 'react'; +import { createDispatchHook, createSelectorHook, ReactReduxContextValue } from 'react-redux'; +import { configureStore } from '@reduxjs/toolkit'; +import { reducer } from './reducer'; +import { initialState, State } from './state'; + +export const store = configureStore({ + reducer, + devTools: process.env.NODE_ENV !== 'production', + enhancers: [], +}); + +export const Context = createContext>({ + store, + storeState: initialState, +}); + +export const useDispatch = createDispatchHook(Context); +export const useSelector = createSelectorHook(Context); + +export const stateSelector = (state: State) => state; diff --git a/packages/kbn-expandable-flyout/src/state.ts b/packages/kbn-expandable-flyout/src/state.ts index 75f8269a82962..2724194759b2b 100644 --- a/packages/kbn-expandable-flyout/src/state.ts +++ b/packages/kbn-expandable-flyout/src/state.ts @@ -21,10 +21,18 @@ export interface State { * Panels to render in the preview section */ preview: FlyoutPanelProps[]; + + /** + * Is the flyout in sync with external storage (eg. url)? + * This value can be used in useEffect for example, to control whether we should + * call an external state sync method. + */ + needsSync?: boolean; } export const initialState: State = { left: undefined, right: undefined, preview: [], + needsSync: false, }; diff --git a/packages/kbn-expandable-flyout/src/test/provider.tsx b/packages/kbn-expandable-flyout/src/test/provider.tsx index eab1d94fc0bbd..448b36e0b3d30 100644 --- a/packages/kbn-expandable-flyout/src/test/provider.tsx +++ b/packages/kbn-expandable-flyout/src/test/provider.tsx @@ -10,8 +10,7 @@ import { Provider as ReduxProvider } from 'react-redux'; import { configureStore } from '@reduxjs/toolkit'; import React, { FC, PropsWithChildren } from 'react'; import { reducer } from '../reducer'; -import { Context } from '../context/memory_state_provider'; -import { ExpandableFlyoutContext } from '../context'; +import { Context } from '../redux'; import { initialState, State } from '../state'; interface TestProviderProps { @@ -30,10 +29,8 @@ export const TestProvider: FC> = ({ }); return ( - - - {children} - - + + {children} + ); }; diff --git a/packages/kbn-expandable-flyout/src/types.ts b/packages/kbn-expandable-flyout/src/types.ts index 2d95426abddec..33b9701ba8831 100644 --- a/packages/kbn-expandable-flyout/src/types.ts +++ b/packages/kbn-expandable-flyout/src/types.ts @@ -7,14 +7,8 @@ */ import React from 'react'; -import { State } from './state'; export interface ExpandableFlyoutApi { - /** - * Right, left and preview panels - */ - panels: State; - /** * Open the flyout with left, right and/or preview panels */ diff --git a/packages/kbn-expandable-flyout/tsconfig.json b/packages/kbn-expandable-flyout/tsconfig.json index 9a5dcbaf03048..2b6e518016871 100644 --- a/packages/kbn-expandable-flyout/tsconfig.json +++ b/packages/kbn-expandable-flyout/tsconfig.json @@ -20,6 +20,6 @@ ], "kbn_references": [ "@kbn/i18n", - "@kbn/url-state" + "@kbn/kibana-utils-plugin" ] } diff --git a/x-pack/plugins/security_solution/public/flyout/document_details/right/components/header_actions.test.tsx b/x-pack/plugins/security_solution/public/flyout/document_details/right/components/header_actions.test.tsx index 8f564b8266332..48460409afac0 100644 --- a/x-pack/plugins/security_solution/public/flyout/document_details/right/components/header_actions.test.tsx +++ b/x-pack/plugins/security_solution/public/flyout/document_details/right/components/header_actions.test.tsx @@ -17,7 +17,6 @@ import { mockDataFormattedForFieldBrowser } from '../../shared/mocks/mock_data_f import { TestProvidersComponent } from '../../../../common/mock'; import { useGetAlertDetailsFlyoutLink } from '../../../../timelines/components/side_panel/event_details/use_get_alert_details_flyout_link'; import { URL_PARAM_KEY } from '../../../../common/hooks/use_url_state'; -import { ExpandableFlyoutProvider } from '@kbn/expandable-flyout'; jest.mock('../../../../common/lib/kibana'); jest.mock('../hooks/use_assistant'); @@ -40,11 +39,9 @@ const mockContextValue = { const renderHeaderActions = (contextValue: RightPanelContext) => render( - - - - - + + + ); diff --git a/x-pack/plugins/security_solution/public/flyout/document_details/right/components/investigation_guide.test.tsx b/x-pack/plugins/security_solution/public/flyout/document_details/right/components/investigation_guide.test.tsx index b27a70c325824..d57492990b882 100644 --- a/x-pack/plugins/security_solution/public/flyout/document_details/right/components/investigation_guide.test.tsx +++ b/x-pack/plugins/security_solution/public/flyout/document_details/right/components/investigation_guide.test.tsx @@ -16,7 +16,6 @@ import { INVESTIGATION_GUIDE_TEST_ID, } from './test_ids'; import { mockContextValue } from '../mocks/mock_context'; -import { mockFlyoutContextValue } from '../../shared/mocks/mock_flyout_context'; import type { ExpandableFlyoutApi } from '@kbn/expandable-flyout'; import { useExpandableFlyoutApi } from '@kbn/expandable-flyout'; import { useInvestigationGuide } from '../../shared/hooks/use_investigation_guide'; @@ -25,6 +24,8 @@ import { LeftPanelInvestigationTab, DocumentDetailsLeftPanelKey } from '../../le jest.mock('../../shared/hooks/use_investigation_guide'); jest.mock('@kbn/expandable-flyout', () => ({ useExpandableFlyoutApi: jest.fn() })); +const mockFlyoutContextValue = { openLeftPanel: jest.fn() }; + const NO_DATA_MESSAGE = 'Investigation guideThere’s no investigation guide for this rule.'; const PREVIEW_MESSAGE = 'Investigation guide is not available in alert preview.'; diff --git a/x-pack/plugins/security_solution/public/flyout/document_details/shared/mocks/mock_flyout_context.ts b/x-pack/plugins/security_solution/public/flyout/document_details/shared/mocks/mock_flyout_context.ts index 419047bc30f77..177022260c770 100644 --- a/x-pack/plugins/security_solution/public/flyout/document_details/shared/mocks/mock_flyout_context.ts +++ b/x-pack/plugins/security_solution/public/flyout/document_details/shared/mocks/mock_flyout_context.ts @@ -5,12 +5,12 @@ * 2.0. */ -import type { ExpandableFlyoutContextValue } from '@kbn/expandable-flyout/src/context'; +import type { ExpandableFlyoutApi } from '@kbn/expandable-flyout'; /** - * Mock flyout context + * Mock flyout api */ -export const mockFlyoutContextValue: ExpandableFlyoutContextValue = { +export const mockFlyoutApi: ExpandableFlyoutApi = { openFlyout: jest.fn(), openRightPanel: jest.fn(), openLeftPanel: jest.fn(), @@ -20,9 +20,4 @@ export const mockFlyoutContextValue: ExpandableFlyoutContextValue = { closePreviewPanel: jest.fn(), previousPreviewPanel: jest.fn(), closeFlyout: jest.fn(), - panels: { - left: undefined, - right: undefined, - preview: [], - }, }; diff --git a/x-pack/plugins/security_solution/public/flyout/shared/components/flyout_navigation.test.tsx b/x-pack/plugins/security_solution/public/flyout/shared/components/flyout_navigation.test.tsx index 86d4ee811b0d7..321245ccde86e 100644 --- a/x-pack/plugins/security_solution/public/flyout/shared/components/flyout_navigation.test.tsx +++ b/x-pack/plugins/security_solution/public/flyout/shared/components/flyout_navigation.test.tsx @@ -19,18 +19,13 @@ import type { ExpandableFlyoutState } from '@kbn/expandable-flyout'; import { useExpandableFlyoutApi, type ExpandableFlyoutApi, - ExpandableFlyoutProvider, useExpandableFlyoutState, } from '@kbn/expandable-flyout'; const expandDetails = jest.fn(); const ExpandableFlyoutTestProviders: FC> = ({ children }) => { - return ( - - {children} - - ); + return {children}; }; jest.mock('@kbn/expandable-flyout', () => ({ diff --git a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/renderers/host_name.test.tsx b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/renderers/host_name.test.tsx index c865e349c0470..994416f4c0c4b 100644 --- a/x-pack/plugins/security_solution/public/timelines/components/timeline/body/renderers/host_name.test.tsx +++ b/x-pack/plugins/security_solution/public/timelines/components/timeline/body/renderers/host_name.test.tsx @@ -4,7 +4,7 @@ * 2.0; you may not use this file except in compliance with the Elastic License * 2.0. */ -import React from 'react'; +import React, { type PropsWithChildren } from 'react'; import { mount } from 'enzyme'; import { waitFor } from '@testing-library/react'; @@ -23,14 +23,12 @@ jest.mock('../../../../../common/hooks/use_experimental_features', () => ({ useIsExperimentalFeatureEnabled: () => mockUseIsExperimentalFeatureEnabled, })); -jest.mock('@kbn/expandable-flyout/src/context', () => { - const original = jest.requireActual('@kbn/expandable-flyout/src/context'); - +jest.mock('@kbn/expandable-flyout', () => { return { - ...original, useExpandableFlyoutApi: () => ({ openRightPanel: mockOpenRightPanel, }), + TestProvider: ({ children }: PropsWithChildren<{}>) => <>{children}, }; }); diff --git a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/expandable_flyout/alert_details_url_sync.cy.ts b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/expandable_flyout/alert_details_url_sync.cy.ts index 0cd12d9381ad9..2cca4a6e5feb6 100644 --- a/x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/expandable_flyout/alert_details_url_sync.cy.ts +++ b/x-pack/test/security_solution_cypress/cypress/e2e/investigations/alerts/expandable_flyout/alert_details_url_sync.cy.ts @@ -26,13 +26,13 @@ describe('Expandable flyout state sync', { tags: ['@ess', '@serverless'] }, () = }); it('should test flyout url sync', () => { - cy.url().should('not.include', 'rightPanel'); + cy.url().should('not.include', 'right'); expandFirstAlertExpandableFlyout(); cy.log('should serialize its state to url'); - cy.url().should('include', 'rightPanel'); + cy.url().should('include', 'right'); cy.get(DOCUMENT_DETAILS_FLYOUT_HEADER_TITLE).should('have.text', rule.name); cy.log('should reopen the flyout after browser refresh'); @@ -40,13 +40,13 @@ describe('Expandable flyout state sync', { tags: ['@ess', '@serverless'] }, () = cy.reload(); waitForAlertsToPopulate(); - cy.url().should('include', 'rightPanel'); + cy.url().should('include', 'right'); cy.get(DOCUMENT_DETAILS_FLYOUT_HEADER_TITLE).should('have.text', rule.name); cy.log('should clear the url state when flyout is closed'); closeFlyout(); - cy.url().should('not.include', 'rightPanel'); + cy.url().should('not.include', 'right'); }); });