From 76e4cf416c977bebe21f5f35466002bfa0d826b1 Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Wed, 9 Oct 2024 19:02:17 +0800 Subject: [PATCH 1/7] BugFix: [AlertInsight] Remove the cache of insight id in node server Signed-off-by: Heng Qian --- .../generate_popover_body.test.tsx | 10 ++--- .../generate_popover_body.tsx | 6 ++- server/routes/summary_routes.ts | 38 +++++-------------- 3 files changed, 19 insertions(+), 35 deletions(-) diff --git a/public/components/incontext_insight/generate_popover_body.test.tsx b/public/components/incontext_insight/generate_popover_body.test.tsx index 4b3c1302..b7341a79 100644 --- a/public/components/incontext_insight/generate_popover_body.test.tsx +++ b/public/components/incontext_insight/generate_popover_body.test.tsx @@ -87,7 +87,7 @@ describe('GeneratePopoverBody', () => { case SUMMARY_ASSISTANT_API.SUMMARIZE: value = { summary: 'Generated summary content', - insightAgentIdExists: true, + insightAgentId: 'insightAgentId', }; break; @@ -168,7 +168,7 @@ describe('GeneratePopoverBody', () => { case SUMMARY_ASSISTANT_API.SUMMARIZE: value = { summary: 'Generated summary content', - insightAgentIdExists: false, + insightAgentId: undefined, }; break; @@ -245,7 +245,7 @@ describe('GeneratePopoverBody', () => { case SUMMARY_ASSISTANT_API.SUMMARIZE: value = { summary: 'Generated summary content', - insightAgentIdExists: true, + insightAgentId: 'insightAgentId', }; break; @@ -293,7 +293,7 @@ describe('GeneratePopoverBody', () => { case SUMMARY_ASSISTANT_API.SUMMARIZE: value = { summary: 'Generated summary content', - insightAgentIdExists: true, + insightAgentId: 'insightAgentId', }; break; @@ -335,7 +335,7 @@ describe('GeneratePopoverBody', () => { case SUMMARY_ASSISTANT_API.SUMMARIZE: value = { summary: 'Generated summary content', - insightAgentIdExists: true, + insightAgentId: 'insightAgentId', }; break; diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index e3326aa3..c278517b 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -122,10 +122,12 @@ export const GeneratePopoverBody: React.FC<{ .then((response) => { const summaryContent = response.summary; setSummary(summaryContent); - const insightAgentIdExists = insightType !== undefined && response.insightAgentIdExists; + const insightAgentId = response.insightAgentId; + const insightAgentIdExists = !!insightType && !!insightAgentId; setInsightAvailable(insightAgentIdExists); if (insightAgentIdExists) { onGenerateInsightBasedOnSummary( + insightAgentId, dataSourceQuery, summaryType, insightType, @@ -150,6 +152,7 @@ export const GeneratePopoverBody: React.FC<{ }; const onGenerateInsightBasedOnSummary = ( + insightAgentId: string, dataSourceQuery: {}, summaryType: string, insightType: string, @@ -161,6 +164,7 @@ export const GeneratePopoverBody: React.FC<{ httpSetup ?.post(SUMMARY_ASSISTANT_API.INSIGHT, { body: JSON.stringify({ + insightAgentId, summaryType, insightType, summary: summaryContent, diff --git a/server/routes/summary_routes.ts b/server/routes/summary_routes.ts index f5c9c058..3d8f5d4f 100644 --- a/server/routes/summary_routes.ts +++ b/server/routes/summary_routes.ts @@ -14,8 +14,6 @@ const SUMMARY_AGENT_CONFIG_ID = 'os_summary'; const LOG_PATTERN_SUMMARY_AGENT_CONFIG_ID = 'os_summary_with_logPattern'; const OS_INSIGHT_AGENT_CONFIG_ID = 'os_insight'; const DATA2SUMMARY_AGENT_CONFIG_ID = 'os_data2summary'; -let osInsightAgentId: string | undefined; -let userInsightAgentId: string | undefined; export function registerSummaryAssistantRoutes( router: IRouter, @@ -57,33 +55,26 @@ export function registerSummaryAssistantRoutes( topNLogPatternData: req.body.topNLogPatternData, }); let summary; - let insightAgentIdExists = false; + let insightAgentId; try { if (req.body.insightType) { // We have separate agent for os_insight and user_insight. And for user_insight, we can // only get it by searching on name since it is not stored in agent config. if (req.body.insightType === 'os_insight') { - if (!osInsightAgentId) { - osInsightAgentId = await getAgentIdByConfigName(OS_INSIGHT_AGENT_CONFIG_ID, client); - } - insightAgentIdExists = !!osInsightAgentId; - } else if (req.body.insightType === 'user_insight') { - if (req.body.type === 'alerts') { - if (!userInsightAgentId) { - userInsightAgentId = await searchAgent({ name: 'KB_For_Alert_Insight' }, client); - } - } - insightAgentIdExists = !!userInsightAgentId; + insightAgentId = await getAgentIdByConfigName(OS_INSIGHT_AGENT_CONFIG_ID, client); + } else if (req.body.insightType === 'user_insight' && req.body.type === 'alerts') { + insightAgentId = await searchAgent({ name: 'KB_For_Alert_Insight' }, client); } } } catch (e) { context.assistant_plugin.logger.info( - `Cannot find insight agent for ${req.body.insightType}` + `Cannot find insight agent for ${req.body.insightType}`, + e ); } try { summary = response.body.inference_results[0].output[0].result; - return res.ok({ body: { summary, insightAgentIdExists } }); + return res.ok({ body: { summary, insightAgentId } }); } catch (e) { return res.internalError(); } @@ -94,6 +85,7 @@ export function registerSummaryAssistantRoutes( path: SUMMARY_ASSISTANT_API.INSIGHT, validate: { body: schema.object({ + insightAgentId: schema.string(), summaryType: schema.string(), insightType: schema.string(), summary: schema.string(), @@ -110,16 +102,8 @@ export function registerSummaryAssistantRoutes( context, dataSourceId: req.query.dataSourceId, }); - const insightAgentId = - req.body.insightType === 'os_insight' ? osInsightAgentId : userInsightAgentId; - if (!insightAgentId) { - context.assistant_plugin.logger.info( - `Cannot find insight agent for ${req.body.insightType}` - ); - return res.internalError(); - } const assistantClient = assistantService.getScopedClient(req, context); - const response = await assistantClient.executeAgent(insightAgentId, { + const response = await assistantClient.executeAgent(req.body.insightAgentId, { context: req.body.context, summary: req.body.summary, question: req.body.question, @@ -128,10 +112,6 @@ export function registerSummaryAssistantRoutes( return res.ok({ body: response.body.inference_results[0].output[0].result }); } catch (e) { return res.internalError(); - } finally { - // Reset both agents id in case of update - userInsightAgentId = undefined; - osInsightAgentId = undefined; } }) ); From d1ee519e57491c6d505a239e74b7ceda9892afb2 Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Wed, 9 Oct 2024 19:21:40 +0800 Subject: [PATCH 2/7] Add CHANGELOG.md Signed-off-by: Heng Qian --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11928319..8037607c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - feat: report metrics for text to visualization([#312](https://github.com/opensearch-project/dashboards-assistant/pull/312)) - fix: Fix dynamic uses of i18n([#335](https://github.com/opensearch-project/dashboards-assistant/pull/335)) - fix: Fix unrecognized creating index pattern duplicate cases([#337](https://github.com/opensearch-project/dashboards-assistant/pull/337)) +- fix: Remove the cache of insight agent id in node server([#343](https://github.com/opensearch-project/dashboards-assistant/pull/343)) ### 📈 Features/Enhancements From ae4db7376900f9dce19dbdbbaa256ac3cc0b4a88 Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Wed, 9 Oct 2024 19:27:34 +0800 Subject: [PATCH 3/7] Change log level to debug if not finding insight agent id Signed-off-by: Heng Qian --- server/routes/summary_routes.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/routes/summary_routes.ts b/server/routes/summary_routes.ts index 3d8f5d4f..7e9a6e0d 100644 --- a/server/routes/summary_routes.ts +++ b/server/routes/summary_routes.ts @@ -11,7 +11,7 @@ import { getAgentIdByConfigName, searchAgent } from './get_agent'; import { AssistantServiceSetup } from '../services/assistant_service'; const SUMMARY_AGENT_CONFIG_ID = 'os_summary'; -const LOG_PATTERN_SUMMARY_AGENT_CONFIG_ID = 'os_summary_with_logPattern'; +const LOG_PATTERN_SUMMARY_AGENT_CONFIG_ID = 'os_summary_with_log_pattern'; const OS_INSIGHT_AGENT_CONFIG_ID = 'os_insight'; const DATA2SUMMARY_AGENT_CONFIG_ID = 'os_data2summary'; @@ -67,7 +67,7 @@ export function registerSummaryAssistantRoutes( } } } catch (e) { - context.assistant_plugin.logger.info( + context.assistant_plugin.logger.debug( `Cannot find insight agent for ${req.body.insightType}`, e ); From 507f29bfed15639c758bba66a4135b6aa02ae689 Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Thu, 10 Oct 2024 11:24:15 +0800 Subject: [PATCH 4/7] Hide insight agent id in response Signed-off-by: Heng Qian --- .../generate_popover_body.test.tsx | 10 ++--- .../generate_popover_body.tsx | 7 +--- server/routes/summary_routes.ts | 42 +++++++++++++------ 3 files changed, 36 insertions(+), 23 deletions(-) diff --git a/public/components/incontext_insight/generate_popover_body.test.tsx b/public/components/incontext_insight/generate_popover_body.test.tsx index b7341a79..4b3c1302 100644 --- a/public/components/incontext_insight/generate_popover_body.test.tsx +++ b/public/components/incontext_insight/generate_popover_body.test.tsx @@ -87,7 +87,7 @@ describe('GeneratePopoverBody', () => { case SUMMARY_ASSISTANT_API.SUMMARIZE: value = { summary: 'Generated summary content', - insightAgentId: 'insightAgentId', + insightAgentIdExists: true, }; break; @@ -168,7 +168,7 @@ describe('GeneratePopoverBody', () => { case SUMMARY_ASSISTANT_API.SUMMARIZE: value = { summary: 'Generated summary content', - insightAgentId: undefined, + insightAgentIdExists: false, }; break; @@ -245,7 +245,7 @@ describe('GeneratePopoverBody', () => { case SUMMARY_ASSISTANT_API.SUMMARIZE: value = { summary: 'Generated summary content', - insightAgentId: 'insightAgentId', + insightAgentIdExists: true, }; break; @@ -293,7 +293,7 @@ describe('GeneratePopoverBody', () => { case SUMMARY_ASSISTANT_API.SUMMARIZE: value = { summary: 'Generated summary content', - insightAgentId: 'insightAgentId', + insightAgentIdExists: true, }; break; @@ -335,7 +335,7 @@ describe('GeneratePopoverBody', () => { case SUMMARY_ASSISTANT_API.SUMMARIZE: value = { summary: 'Generated summary content', - insightAgentId: 'insightAgentId', + insightAgentIdExists: true, }; break; diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index c278517b..549c03c9 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -109,7 +109,7 @@ export const GeneratePopoverBody: React.FC<{ await httpSetup ?.post(SUMMARY_ASSISTANT_API.SUMMARIZE, { body: JSON.stringify({ - type: summaryType, + summaryType, insightType, question: summarizationQuestion, context: contextContent, @@ -123,11 +123,10 @@ export const GeneratePopoverBody: React.FC<{ const summaryContent = response.summary; setSummary(summaryContent); const insightAgentId = response.insightAgentId; - const insightAgentIdExists = !!insightType && !!insightAgentId; + const insightAgentIdExists = !!insightType && response.insightAgentIdExists; setInsightAvailable(insightAgentIdExists); if (insightAgentIdExists) { onGenerateInsightBasedOnSummary( - insightAgentId, dataSourceQuery, summaryType, insightType, @@ -152,7 +151,6 @@ export const GeneratePopoverBody: React.FC<{ }; const onGenerateInsightBasedOnSummary = ( - insightAgentId: string, dataSourceQuery: {}, summaryType: string, insightType: string, @@ -164,7 +162,6 @@ export const GeneratePopoverBody: React.FC<{ httpSetup ?.post(SUMMARY_ASSISTANT_API.INSIGHT, { body: JSON.stringify({ - insightAgentId, summaryType, insightType, summary: summaryContent, diff --git a/server/routes/summary_routes.ts b/server/routes/summary_routes.ts index 7e9a6e0d..a4d6e241 100644 --- a/server/routes/summary_routes.ts +++ b/server/routes/summary_routes.ts @@ -4,7 +4,7 @@ */ import { schema } from '@osd/config-schema'; -import { IRouter } from '../../../../src/core/server'; +import { IRouter, OpenSearchClient, RequestHandlerContext } from '../../../../src/core/server'; import { SUMMARY_ASSISTANT_API } from '../../common/constants/llm'; import { getOpenSearchClientTransport } from '../utils/get_opensearch_client_transport'; import { getAgentIdByConfigName, searchAgent } from './get_agent'; @@ -24,7 +24,7 @@ export function registerSummaryAssistantRoutes( path: SUMMARY_ASSISTANT_API.SUMMARIZE, validate: { body: schema.object({ - type: schema.string(), + summaryType: schema.string(), insightType: schema.maybe(schema.string()), question: schema.string(), context: schema.maybe(schema.string()), @@ -55,16 +55,14 @@ export function registerSummaryAssistantRoutes( topNLogPatternData: req.body.topNLogPatternData, }); let summary; - let insightAgentId; + let insightAgentIdExists = false; try { if (req.body.insightType) { - // We have separate agent for os_insight and user_insight. And for user_insight, we can - // only get it by searching on name since it is not stored in agent config. - if (req.body.insightType === 'os_insight') { - insightAgentId = await getAgentIdByConfigName(OS_INSIGHT_AGENT_CONFIG_ID, client); - } else if (req.body.insightType === 'user_insight' && req.body.type === 'alerts') { - insightAgentId = await searchAgent({ name: 'KB_For_Alert_Insight' }, client); - } + insightAgentIdExists = !!(await detectInsightAgentId( + req.body.insightType, + req.body.summaryType, + client + )); } } catch (e) { context.assistant_plugin.logger.debug( @@ -74,7 +72,7 @@ export function registerSummaryAssistantRoutes( } try { summary = response.body.inference_results[0].output[0].result; - return res.ok({ body: { summary, insightAgentId } }); + return res.ok({ body: { summary, insightAgentIdExists } }); } catch (e) { return res.internalError(); } @@ -85,7 +83,6 @@ export function registerSummaryAssistantRoutes( path: SUMMARY_ASSISTANT_API.INSIGHT, validate: { body: schema.object({ - insightAgentId: schema.string(), summaryType: schema.string(), insightType: schema.string(), summary: schema.string(), @@ -103,7 +100,12 @@ export function registerSummaryAssistantRoutes( dataSourceId: req.query.dataSourceId, }); const assistantClient = assistantService.getScopedClient(req, context); - const response = await assistantClient.executeAgent(req.body.insightAgentId, { + const insightAgentId = await detectInsightAgentId( + req.body.insightType, + req.body.summaryType, + client + ); + const response = await assistantClient.executeAgent(insightAgentId, { context: req.body.context, summary: req.body.summary, question: req.body.question, @@ -117,6 +119,20 @@ export function registerSummaryAssistantRoutes( ); } +function detectInsightAgentId( + insightType: string, + appType: string, + client: OpenSearchClient['transport'] +) { + // We have separate agent for os_insight and user_insight. And for user_insight, we can + // only get it by searching on name since it is not stored in agent config. + if (insightType === 'os_insight') { + return getAgentIdByConfigName(OS_INSIGHT_AGENT_CONFIG_ID, client); + } else if (insightType === 'user_insight' && appType === 'alerts') { + return searchAgent({ name: 'KB_For_Alert_Insight' }, client); + } +} + export function registerData2SummaryRoutes( router: IRouter, assistantService: AssistantServiceSetup From 4923fa0c034243ed734e7a1d3cbca590a89926e0 Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Thu, 10 Oct 2024 11:26:28 +0800 Subject: [PATCH 5/7] Minor fix Signed-off-by: Heng Qian --- public/components/incontext_insight/generate_popover_body.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/public/components/incontext_insight/generate_popover_body.tsx b/public/components/incontext_insight/generate_popover_body.tsx index 549c03c9..a24b0120 100644 --- a/public/components/incontext_insight/generate_popover_body.tsx +++ b/public/components/incontext_insight/generate_popover_body.tsx @@ -122,7 +122,6 @@ export const GeneratePopoverBody: React.FC<{ .then((response) => { const summaryContent = response.summary; setSummary(summaryContent); - const insightAgentId = response.insightAgentId; const insightAgentIdExists = !!insightType && response.insightAgentIdExists; setInsightAvailable(insightAgentIdExists); if (insightAgentIdExists) { From b7cefcf7dac8ff7c2adf60bd3ef7977d2161e917 Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Thu, 10 Oct 2024 11:27:51 +0800 Subject: [PATCH 6/7] Minor fix Signed-off-by: Heng Qian --- server/routes/summary_routes.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/routes/summary_routes.ts b/server/routes/summary_routes.ts index a4d6e241..45eb3110 100644 --- a/server/routes/summary_routes.ts +++ b/server/routes/summary_routes.ts @@ -121,14 +121,14 @@ export function registerSummaryAssistantRoutes( function detectInsightAgentId( insightType: string, - appType: string, + summaryType: string, client: OpenSearchClient['transport'] ) { // We have separate agent for os_insight and user_insight. And for user_insight, we can // only get it by searching on name since it is not stored in agent config. if (insightType === 'os_insight') { return getAgentIdByConfigName(OS_INSIGHT_AGENT_CONFIG_ID, client); - } else if (insightType === 'user_insight' && appType === 'alerts') { + } else if (insightType === 'user_insight' && summaryType === 'alerts') { return searchAgent({ name: 'KB_For_Alert_Insight' }, client); } } From 8054998fdc4c82dbc936f9b810d74cf5b8d83f60 Mon Sep 17 00:00:00 2001 From: Heng Qian Date: Thu, 10 Oct 2024 15:52:02 +0800 Subject: [PATCH 7/7] Address comments Signed-off-by: Heng Qian --- server/routes/summary_routes.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/server/routes/summary_routes.ts b/server/routes/summary_routes.ts index 45eb3110..4300246c 100644 --- a/server/routes/summary_routes.ts +++ b/server/routes/summary_routes.ts @@ -74,7 +74,7 @@ export function registerSummaryAssistantRoutes( summary = response.body.inference_results[0].output[0].result; return res.ok({ body: { summary, insightAgentIdExists } }); } catch (e) { - return res.internalError(); + return res.badRequest({ body: e }); } }) ); @@ -113,7 +113,7 @@ export function registerSummaryAssistantRoutes( try { return res.ok({ body: response.body.inference_results[0].output[0].result }); } catch (e) { - return res.internalError(); + return res.badRequest({ body: e }); } }) ); @@ -131,6 +131,7 @@ function detectInsightAgentId( } else if (insightType === 'user_insight' && summaryType === 'alerts') { return searchAgent({ name: 'KB_For_Alert_Insight' }, client); } + return undefined; } export function registerData2SummaryRoutes( @@ -169,7 +170,7 @@ export function registerData2SummaryRoutes( const result = response.body.inference_results[0].output[0].result; return res.ok({ body: result }); } catch (e) { - return res.internalError(); + return res.badRequest({ body: e }); } }) );