Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: support navigating to discover in alerting popover #316

Merged
merged 4 commits into from
Sep 20, 2024
+354 −4
Merged

feat: support navigating to discover in alerting popover #316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8ae1720
feat: support navigate to discover in alerting popover
raintygao Sep 19, 2024
823b136
feat: support MDS and fix test env error
raintygao Sep 20, 2024
764156f
doc: add changelog
raintygao Sep 20, 2024
3a1c7c6
add tests
raintygao Sep 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- feat: exposed an API to check if a give agent config name has configured with agent id([#307](https://github.com/opensearch-project/dashboards-assistant/pull/307))
- feat: check all required agents before enabling index pattern selection for text to visualization([313](https://github.com/opensearch-project/dashboards-assistant/pull/313))
- fix: pass data source id for alert summary/insight([#321](https://github.com/opensearch-project/dashboards-assistant/pull/321))
- feat: support navigating to discover in alerting popover([#316](https://github.com/opensearch-project/dashboards-assistant/pull/316))

### 📈 Features/Enhancements

Expand Down
135 changes: 135 additions & 0 deletions public/components/incontext_insight/generate_popover_body.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,16 @@ import { GeneratePopoverBody } from './generate_popover_body';
import { HttpSetup } from '../../../../../src/core/public';
import { SUMMARY_ASSISTANT_API } from '../../../common/constants/llm';
import { usageCollectionPluginMock } from '../../../../../src/plugins/usage_collection/public/mocks';
import { coreMock } from '../../../../../src/core/public/mocks';
import { dataPluginMock } from '../../../../../src/plugins/data/public/mocks';

jest.mock('../../services');

jest.mock('../../utils', () => ({
createIndexPatterns: jest.fn().mockResolvedValue('index pattern'),
buildUrlQuery: jest.fn().mockResolvedValue('query'),
}));

const mockToasts = {
addDanger: jest.fn(),
};
Expand All @@ -33,6 +40,36 @@ const mockHttpSetup: HttpSetup = ({
post: mockPost,
} as unknown) as HttpSetup; // Mocking HttpSetup

const mockDSL = `{
"query": {
"bool": {
"filter": [
{
"range": {
"timestamp": {
"from": "2024-09-06T04:02:52||-1h",
"to": "2024-09-06T04:02:52",
"include_lower": true,
"include_upper": true,
"boost": 1
}
}
},
{
"term": {
"FlightDelay": {
"value": "true",
"boost": 1
}
}
}
],
"adjust_pure_negative": true,
"boost": 1
}
}
}`;

describe('GeneratePopoverBody', () => {
const incontextInsightMock = {
contextProvider: jest.fn(),
Expand Down Expand Up @@ -240,4 +277,102 @@ describe('GeneratePopoverBody', () => {
// insight tip icon is not visible for this alert
expect(screen.queryAllByLabelText('How was this generated?')).toHaveLength(0);
});

it('should not display discover link if monitor type is not query_level_monitor or bucket_level_monitor', async () => {
incontextInsightMock.contextProvider = jest.fn().mockResolvedValue({
additionalInfo: {
dsl: mockDSL,
index: 'mock_index',
dataSourceId: `test-data-source-id`,
monitorType: 'mock_type',
},
});
mockPost.mockImplementation((path: string, body) => {
let value;
switch (path) {
case SUMMARY_ASSISTANT_API.SUMMARIZE:
value = {
summary: 'Generated summary content',
insightAgentIdExists: true,
};
break;

case SUMMARY_ASSISTANT_API.INSIGHT:
value = 'Generated insight content';
break;

default:
return null;
}
return Promise.resolve(value);
});

const { queryByText } = render(
<GeneratePopoverBody
incontextInsight={incontextInsightMock}
httpSetup={mockHttpSetup}
closePopover={closePopoverMock}
/>
);

await waitFor(() => {
expect(queryByText('Discover details')).not.toBeInTheDocument();
});
});

it('handle navigate to discover after clicking link', async () => {
incontextInsightMock.contextProvider = jest.fn().mockResolvedValue({
additionalInfo: {
dsl: mockDSL,
index: 'mock_index',
dataSourceId: `test-data-source-id`,
monitorType: 'query_level_monitor',
},
});
mockPost.mockImplementation((path: string, body) => {
let value;
switch (path) {
case SUMMARY_ASSISTANT_API.SUMMARIZE:
value = {
summary: 'Generated summary content',
insightAgentIdExists: true,
};
break;

case SUMMARY_ASSISTANT_API.INSIGHT:
value = 'Generated insight content';
break;

default:
return null;
}
return Promise.resolve(value);
});

const coreStart = coreMock.createStart();
const dataStart = dataPluginMock.createStartContract();
const getStartServices = jest.fn().mockResolvedValue([
coreStart,
{
data: dataStart,
},
]);
const { getByText } = render(
<GeneratePopoverBody
incontextInsight={incontextInsightMock}
httpSetup={mockHttpSetup}
closePopover={closePopoverMock}
getStartServices={getStartServices}
/>
);

await waitFor(() => {
const button = getByText('Discover details');
expect(button).toBeInTheDocument();
fireEvent.click(button);
expect(coreStart.application.navigateToUrl).toHaveBeenCalledWith(
'data-explorer/discover#?query'
);
});
});
});
74 changes: 71 additions & 3 deletions public/components/incontext_insight/generate_popover_body.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* SPDX-License-Identifier: Apache-2.0
*/

import React, { useState } from 'react';
import React, { useEffect, useState } from 'react';
import { i18n } from '@osd/i18n';
import {
EuiFlexGroup,
Expand All @@ -17,24 +17,29 @@ import {
EuiSpacer,
EuiText,
EuiTitle,
EuiButton,
} from '@elastic/eui';
import { useEffectOnce } from 'react-use';
import { METRIC_TYPE } from '@osd/analytics';
import { MessageActions } from '../../tabs/chat/messages/message_action';
import { ContextObj, IncontextInsight as IncontextInsightInput } from '../../types';
import { getNotifications } from '../../services';
import { HttpSetup } from '../../../../../src/core/public';
import { HttpSetup, StartServicesAccessor } from '../../../../../src/core/public';
import { SUMMARY_ASSISTANT_API } from '../../../common/constants/llm';
import shiny_sparkle from '../../assets/shiny_sparkle.svg';
import { UsageCollectionSetup } from '../../../../../src/plugins/usage_collection/public';
import { reportMetric } from '../../utils/report_metric';
import { buildUrlQuery, createIndexPatterns } from '../../utils';
import { AssistantPluginStartDependencies } from '../../types';
import { UI_SETTINGS } from '../../../../../src/plugins/data/public';

export const GeneratePopoverBody: React.FC<{
incontextInsight: IncontextInsightInput;
httpSetup?: HttpSetup;
usageCollection?: UsageCollectionSetup;
closePopover: () => void;
}> = ({ incontextInsight, httpSetup, usageCollection, closePopover }) => {
getStartServices?: StartServicesAccessor<AssistantPluginStartDependencies>;
}> = ({ incontextInsight, httpSetup, usageCollection, closePopover, getStartServices }) => {
const [summary, setSummary] = useState('');
const [insight, setInsight] = useState('');
const [insightAvailable, setInsightAvailable] = useState(false);
Expand All @@ -43,6 +48,20 @@ export const GeneratePopoverBody: React.FC<{

const toasts = getNotifications().toasts;

const [displayDiscoverButton, setDisplayDiscoverButton] = useState(false);

useEffect(() => {
const getMonitorType = async () => {
const context = await incontextInsight.contextProvider?.();
const monitorType = context?.additionalInfo?.monitorType;
// Only this two types from alerting contain DSL and index.
const shoudDisplayDiscoverButton =
monitorType === 'query_level_monitor' || monitorType === 'bucket_level_monitor';
setDisplayDiscoverButton(shoudDisplayDiscoverButton);
};
getMonitorType();
}, [incontextInsight, setDisplayDiscoverButton]);

useEffectOnce(() => {
onGenerateSummary(
incontextInsight.suggestions && incontextInsight.suggestions.length > 0
Expand Down Expand Up @@ -160,6 +179,48 @@ export const GeneratePopoverBody: React.FC<{
return generateInsight();
};

const handleNavigateToDiscover = async () => {
const context = await incontextInsight?.contextProvider?.();
const dsl = context?.additionalInfo?.dsl;
const indexName = context?.additionalInfo?.index;
if (!dsl || !indexName) return;
const dslObject = JSON.parse(dsl);
const filters = dslObject?.query?.bool?.filter;
if (!filters) return;
const timeDslIndex = filters?.findIndex((filter: Record<string, string>) => filter?.range);
const timeDsl = filters[timeDslIndex]?.range;
const timeFieldName = Object.keys(timeDsl)[0];
if (!timeFieldName) return;
filters?.splice(timeDslIndex, 1);
Comment on lines +190 to +194
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there could have multiple range filters in filters and date range is not the first one

{
    "size": 0,
    "query": {
        "bool": {
            "filter": [
                {
                    "range": {
                        "total_quantity": {
                            "from": 1,
                            "to": 5,
                            "include_lower": true,
                            "include_upper": true,
                            "boost": 1
                        }
                    }
                },
                {
                    "range": {
                        "order_date": {
                            "from": "{{period_end}}||-1h",
                            "to": "{{period_end}}",
                            "include_lower": true,
                            "include_upper": true,
                            "format": "epoch_millis",
                            "boost": 1
                        }
                    }
                }
            ],
            "adjust_pure_negative": true,
            "boost": 1
        }
    }
}

Copy link
Collaborator Author

@raintygao raintygao Sep 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which case will this DSL exist? I was told offline there only be one range in DSL, if so, this could be a potential issue.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does period_end mean?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which case will this DSL exist? I was told offline there only be one range in DSL, if so, this could be a potential issue.

This is a manual input query DSL. From create monitor page, if i add more range filters they will not at the first. It will looks like below, and the code logic is find first range filter, work for most cases.

"query": {
   "size": 0,
   "aggregations": {},
   "query": {
      "bool": {
         "filter": [
            {
               "range": {
                  "order_date": {
                     "gte": "{{period_end}}||-1h",
                     "lte": "{{period_end}}",
                     "format": "epoch_millis"
                  }
               }
            },
            {
               "range": {
                  "products.price": {
                     "gte": 10,
                     "lte": 20
                  }
               }
            }
         ]
      }
   }
}

What does period_end mean?

this is placeholder to actual trigger time.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There still have other edge cases, like customized query don't have range filter. For POC we can ignore it.


if (getStartServices) {
const [coreStart, startDeps] = await getStartServices();
const newDiscoverEnabled = coreStart.uiSettings.get(UI_SETTINGS.QUERY_ENHANCEMENTS_ENABLED);
if (!newDiscoverEnabled) {
// Only new discover supports DQL with filters.
coreStart.uiSettings.set(UI_SETTINGS.QUERY_ENHANCEMENTS_ENABLED, true);
}
Hailong-am marked this conversation as resolved.
Show resolved Hide resolved

const indexPattern = await createIndexPatterns(
startDeps.data,
indexName,
timeFieldName,
context?.dataSourceId
);
if (!indexPattern) return;
const query = await buildUrlQuery(
startDeps.data,
coreStart.savedObjects,
indexPattern,
dslObject,
timeDsl[timeFieldName],
context?.dataSourceId
);
Hailong-am marked this conversation as resolved.
Show resolved Hide resolved
// Navigate to new discover with query built to populate
coreStart.application.navigateToUrl(`data-explorer/discover#?${query}`);
}
};

const renderContent = () => {
const content = showInsight && insightAvailable ? insight : summary;
return content ? (
Expand Down Expand Up @@ -251,6 +312,13 @@ export const GeneratePopoverBody: React.FC<{
<>
{renderInnerTitle()}
{renderContent()}
{displayDiscoverButton && (
<EuiButton onClick={handleNavigateToDiscover}>
{i18n.translate('assistantDashboards.incontextInsight.discover', {
defaultMessage: 'Discover details',
})}
</EuiButton>
Hailong-am marked this conversation as resolved.
Show resolved Hide resolved
)}
</>
);
};
6 changes: 5 additions & 1 deletion public/components/incontext_insight/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,24 @@ import { getIncontextInsightRegistry, getNotifications } from '../../services';
// TODO: Replace with getChrome().logos.Chat.url
import chatIcon from '../../assets/chat.svg';
import sparkle from '../../assets/sparkle.svg';
import { HttpSetup } from '../../../../../src/core/public';
import { HttpSetup, StartServicesAccessor } from '../../../../../src/core/public';
import { GeneratePopoverBody } from './generate_popover_body';
import { UsageCollectionSetup } from '../../../../../src/plugins/usage_collection/public/plugin';
import { AssistantPluginStartDependencies } from '../../types';

export interface IncontextInsightProps {
children?: React.ReactNode;
httpSetup?: HttpSetup;
usageCollection?: UsageCollectionSetup;
getStartServices?: StartServicesAccessor<AssistantPluginStartDependencies>;
}

// TODO: add saved objects / config to store seed suggestions
export const IncontextInsight = ({
children,
httpSetup,
usageCollection,
getStartServices,
}: IncontextInsightProps) => {
const anchor = useRef<HTMLDivElement>(null);
const [isVisible, setIsVisible] = useState(false);
Expand Down Expand Up @@ -287,6 +290,7 @@ export const IncontextInsight = ({
httpSetup={httpSetup}
usageCollection={usageCollection}
closePopover={closePopover}
getStartServices={getStartServices}
/>
);
case 'summary':
Expand Down
1 change: 1 addition & 0 deletions public/plugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ export class AssistantPlugin
{...props}
httpSetup={httpSetup}
usageCollection={setupDeps.usageCollection}
getStartServices={core.getStartServices}
/>
);
},
Expand Down
Loading
Loading