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.

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

[8.11] [ML] AIOps: Fix Change point embeddable reporting (#169962) #170046

Merged
merged 1 commit into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import React, { FC } from 'react';
import React, { FC, useCallback, useEffect, useRef } from 'react';
import type { Filter, Query, TimeRange } from '@kbn/es-query';
import { useCommonChartProps } from './use_common_chart_props';
import { useAiopsAppContext } from '../../hooks/use_aiops_app_context';
Expand All @@ -18,6 +18,7 @@ export interface ChartComponentProps {
interval: string;

onLoading?: (isLoading: boolean) => void;
onRenderComplete?: () => void;
}

export interface ChartComponentPropsAll {
Expand All @@ -31,34 +32,59 @@ export interface ChartComponentPropsAll {
}

export const ChartComponent: FC<ChartComponentProps> = React.memo(
({ annotation, fieldConfig, interval, onLoading }) => {
({ annotation, fieldConfig, interval, onLoading, onRenderComplete }) => {
const {
lens: { EmbeddableComponent },
} = useAiopsAppContext();

const chartWrapperRef = useRef<HTMLDivElement>(null);

const renderCompleteListener = useCallback(
(event: Event) => {
if (event.target === chartWrapperRef.current) return;
if (onRenderComplete) {
onRenderComplete();
}
},
[onRenderComplete]
);

useEffect(() => {
if (!chartWrapperRef.current) {
throw new Error('Reference to the chart wrapper is not set');
}
const chartWrapper = chartWrapperRef.current;
chartWrapper.addEventListener('renderComplete', renderCompleteListener);
return () => {
chartWrapper.removeEventListener('renderComplete', renderCompleteListener);
};
}, [renderCompleteListener]);

const { filters, timeRange, query, attributes } = useCommonChartProps({
fieldConfig,
annotation,
bucketInterval: interval,
});

return (
<EmbeddableComponent
id={`changePointChart_${annotation.group ? annotation.group.value : annotation.label}`}
style={{ height: 350 }}
timeRange={timeRange}
query={query}
filters={filters}
// @ts-ignore
attributes={attributes}
renderMode={'view'}
executionContext={{
type: 'aiops_change_point_detection_chart',
name: 'Change point detection',
}}
disableTriggers
onLoad={onLoading}
/>
<div ref={chartWrapperRef}>
<EmbeddableComponent
id={`changePointChart_${annotation.group ? annotation.group.value : annotation.label}`}
style={{ height: 350 }}
timeRange={timeRange}
query={query}
filters={filters}
// @ts-ignore
attributes={attributes}
renderMode={'view'}
executionContext={{
type: 'aiops_change_point_detection_chart',
name: 'Change point detection',
}}
disableTriggers
onLoad={onLoading}
/>
</div>
);
}
);
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ export const ChartsGrid: FC<{
Object.fromEntries(changePoints.map((v, i) => [i, true]))
);

const onLoadCallback = useCallback(
/**
* Callback to track render of each chart component
* to report when all charts are ready.
*/
const onChartRenderCompleteCallback = useCallback(
(chartId: number, isLoading: boolean) => {
if (!onRenderComplete) return;
loadCounter.current[chartId] = isLoading;
Expand Down Expand Up @@ -141,7 +145,12 @@ export const ChartsGrid: FC<{
annotation={v}
interval={interval}
onLoading={(isLoading) => {
onLoadCallback(index, isLoading);
if (isLoading) {
onChartRenderCompleteCallback(index, true);
}
}}
onRenderComplete={() => {
onChartRenderCompleteCallback(index, false);
}}
/>
</EuiPanel>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ const FieldPanel: FC<FieldPanelProps> = ({
}),
icon: 'plusInCircle',
panel: 'attachMainPanel',
'data-test-subj': 'aiopsChangePointDetectionAttachButton',
},
]
: []),
Expand All @@ -248,6 +249,7 @@ const FieldPanel: FC<FieldPanelProps> = ({
disabled: removeDisabled,
},
],
'data=test-subj': 'aiopsChangePointDetectionContextMenuPanel',
Copy link
Contributor

Choose a reason for hiding this comment

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

The = should be - I guess?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed with @darnautov to fix in a separate PR as this needs correcting on main too, and this data-test-subj is not currently being used.

},
{
id: 'attachMainPanel',
Expand All @@ -269,6 +271,7 @@ const FieldPanel: FC<FieldPanelProps> = ({
defaultMessage: 'To dashboard',
}),
panel: 'attachToDashboardPanel',
'data-test-subj': 'aiopsChangePointDetectionAttachToDashboardButton',
},
]
: []),
Expand All @@ -290,6 +293,7 @@ const FieldPanel: FC<FieldPanelProps> = ({
),
}
: {}),
'data-test-subj': 'aiopsChangePointDetectionAttachToCaseButton',
onClick: () => {
openCasesModalCallback({
timeRange,
Expand All @@ -308,6 +312,7 @@ const FieldPanel: FC<FieldPanelProps> = ({
]
: []),
],
'data-test-subj': 'aiopsChangePointDetectionAttachChartPanel',
},
{
id: 'attachToDashboardPanel',
Expand All @@ -318,7 +323,7 @@ const FieldPanel: FC<FieldPanelProps> = ({
content: (
<EuiPanel paddingSize={'s'}>
<EuiSpacer size={'s'} />
<EuiForm>
<EuiForm data-test-subj="aiopsChangePointDetectionDashboardAttachmentForm">
<EuiFormRow fullWidth>
<EuiSwitch
label={i18n.translate('xpack.aiops.changePointDetection.applyTimeRangeLabel', {
Expand All @@ -334,6 +339,7 @@ const FieldPanel: FC<FieldPanelProps> = ({
})
}
compressed
data-test-subj="aiopsChangePointDetectionAttachToDashboardApplyTimeRangeSwitch"
/>
</EuiFormRow>
{isDefined(fieldConfig.splitField) && selectedPartitions.length === 0 ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ export const PartitionsSelector: FC<PartitionsSelectorProps> = ({

useEffect(
function onSplitFieldChange() {
if (splitField !== prevSplitField) {
fetchResults('');
fetchResults('');
if (prevSplitField !== undefined && splitField !== prevSplitField) {
onChange([]);
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ export const FormControls: FC<{
return;
}

if (metricFieldOptions === prevMetricFieldOptions) return;
if (!prevMetricFieldOptions || metricFieldOptions === prevMetricFieldOptions) return;

onChange({
fn: formInput.fn,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ export class EmbeddableChangePointChart extends AbstractEmbeddable<
// required for the export feature to work
this.node.setAttribute('data-shared-item', '');

// test subject selector for functional tests
this.node.setAttribute('data-test-subj', 'aiopsEmbeddableChangePointChart');

const I18nContext = this.deps.i18n.Context;

const datePickerDeps = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export const ChartGridEmbeddableWrapper: FC<
},
});

if (partitions && fieldConfig.splitField) {
if (Array.isArray(partitions) && partitions.length > 0 && fieldConfig.splitField) {
mergedQuery.bool?.filter.push({
terms: {
[fieldConfig.splitField]: partitions,
Expand Down
8 changes: 8 additions & 0 deletions x-pack/test/functional/apps/aiops/change_point_detection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,5 +94,13 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
await aiops.changePointDetectionPage.addChangePointConfig();
await aiops.changePointDetectionPage.assertPanelExist(1);
});

it('attaches change point charts to a dashboard', async () => {
await aiops.changePointDetectionPage.assertPanelExist(0);
await aiops.changePointDetectionPage.attachChartsToDashboard(0, {
applyTimeRange: true,
maxSeries: 1,
});
});
});
}
128 changes: 128 additions & 0 deletions x-pack/test/functional/services/aiops/change_point_detection_page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@ import expect from '@kbn/expect';
import { FtrProviderContext } from '../../ftr_provider_context';
import { MlTableService } from '../ml/common_table_service';

export interface DashboardAttachmentOptions {
applyTimeRange: boolean;
maxSeries: number;
}

export function ChangePointDetectionPageProvider(
{ getService, getPageObject }: FtrProviderContext,
tableService: MlTableService
Expand All @@ -18,6 +23,7 @@ export function ChangePointDetectionPageProvider(
const comboBox = getService('comboBox');
const browser = getService('browser');
const elasticChart = getService('elasticChart');
const dashboardPage = getPageObject('dashboard');

return {
async navigateToIndexPatternSelection() {
Expand Down Expand Up @@ -124,6 +130,128 @@ export function ChangePointDetectionPageProvider(
});
},

async openPanelContextMenu(panelIndex: number) {
await testSubjects.click(
`aiopsChangePointPanel_${panelIndex} > aiopsChangePointDetectionContextMenuButton`
);
await retry.tryForTime(30 * 1000, async () => {
await testSubjects.existOrFail(`aiopsChangePointDetectionAttachButton`);
});
},

async clickAttachChartsButton() {
await testSubjects.click('aiopsChangePointDetectionAttachButton');
await retry.tryForTime(30 * 1000, async () => {
await testSubjects.missingOrFail(`aiopsChangePointDetectionAttachButton`);
await testSubjects.existOrFail(`aiopsChangePointDetectionAttachToDashboardButton`);
});
},

async clickAttachDashboardButton() {
await testSubjects.click('aiopsChangePointDetectionAttachToDashboardButton');
await retry.tryForTime(30 * 1000, async () => {
await testSubjects.existOrFail(`aiopsChangePointDetectionDashboardAttachmentForm`);
});
},

async assertApplyTimeRangeControl(expectedValue: boolean) {
const isChecked = await testSubjects.isEuiSwitchChecked(
`aiopsChangePointDetectionAttachToDashboardApplyTimeRangeSwitch`
);
expect(isChecked).to.eql(
expectedValue,
`Expected apply time range to be ${expectedValue ? 'enabled' : 'disabled'}`
);
},

async assertMaxSeriesControl(expectedValue: number) {
const currentValue = Number(
await testSubjects.getAttribute('aiopsMaxSeriesControlFieldNumber', 'value')
);
expect(currentValue).to.eql(
expectedValue,
`Expected max series control to be ${expectedValue} (got ${currentValue})`
);
},

async toggleApplyTimeRangeControl(isChecked: boolean) {
await testSubjects.setEuiSwitch(
`aiopsChangePointDetectionAttachToDashboardApplyTimeRangeSwitch`,
isChecked ? 'check' : 'uncheck'
);
await this.assertApplyTimeRangeControl(isChecked);
},

async setMaxSeriesControl(value: number) {
await testSubjects.setValue('aiopsMaxSeriesControlFieldNumber', value.toString());
await this.assertMaxSeriesControl(value);
},

async completeDashboardAttachmentForm(attachmentOptions: DashboardAttachmentOptions) {
// assert default values
await this.assertApplyTimeRangeControl(false);
await this.assertMaxSeriesControl(6);

if (attachmentOptions.applyTimeRange) {
await this.toggleApplyTimeRangeControl(attachmentOptions.applyTimeRange);
}

if (attachmentOptions.maxSeries) {
await this.setMaxSeriesControl(attachmentOptions.maxSeries);
}

await testSubjects.click('aiopsChangePointDetectionSubmitDashboardAttachButton');

await retry.tryForTime(30 * 1000, async () => {
// await testSubjects.missingOrFail(`aiopsChangePointDetectionSubmitDashboardAttachButton`);
await testSubjects.existOrFail('savedObjectSaveModal');
});
},

async completeSaveToDashboardForm(options?: { createNew: boolean; dashboardName?: string }) {
await retry.tryForTime(30 * 1000, async () => {
const dashboardSelector = await testSubjects.find('add-to-dashboard-options');

if (options?.createNew) {
const label = await dashboardSelector.findByCssSelector(
`label[for="new-dashboard-option"]`
);
await label.click();
}

await testSubjects.click('confirmSaveSavedObjectButton');
await retry.waitForWithTimeout('Save modal to disappear', 1000, () =>
testSubjects
.missingOrFail('confirmSaveSavedObjectButton')
.then(() => true)
.catch(() => false)
);

// make sure the dashboard page actually loaded
const dashboardItemCount = await dashboardPage.getSharedItemsCount();
expect(dashboardItemCount).to.not.eql(undefined);
});
// changing to the dashboard app might take some time
const embeddable = await testSubjects.find('aiopsEmbeddableChangePointChart', 30 * 1000);
const lensChart = await embeddable.findByClassName('lnsExpressionRenderer');
expect(await lensChart.isDisplayed()).to.eql(
true,
'Change point detection chart should be displayed in dashboard'
);
},

async attachChartsToDashboard(
panelIndex: number,
attachmentOptions: DashboardAttachmentOptions
) {
await this.assertPanelExist(panelIndex);
await this.openPanelContextMenu(panelIndex);
await this.clickAttachChartsButton();
await this.clickAttachDashboardButton();
await this.completeDashboardAttachmentForm(attachmentOptions);
await this.completeSaveToDashboardForm({ createNew: true });
},

getTable(index: number) {
return tableService.getServiceInstance(
'ChangePointResultsTable',
Expand Down
Loading