Skip to content

Commit

Permalink
[8.11] [ML] AIOps: Fix Change point embeddable reporting (#169962) (#…
Browse files Browse the repository at this point in the history
…170046)

# Backport

This will backport the following commits from `main` to `8.11`:
- [[ML] AIOps: Fix Change point embeddable reporting
(#169962)](#169962)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Dima
Arnautov","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-10-27T15:13:52Z","message":"[ML]
AIOps: Fix Change point embeddable reporting (#169962)\n\n##
Summary\r\n\r\nFixes #169733\r\n\r\n#### Reporting fix\r\n\r\nChange
point detection embeddable was incorrectly reporting
render\r\ncompletion. It was relying on the `onLoad` callback from the
Lens\r\nembeddable responsible for chart rendering, which only indicates
that\r\ndata fetching is complete, but not the actual rendering.
Current\r\nimplementation relies on the `renderComplete` event from each
child\r\nembeddable. Both PNG and PDF exports tested and work as
expected.\r\n\r\n\r\n![DASHBOARDDDD](https://github.com/elastic/kibana/assets/5236598/fb718f31-5862-43ab-82e3-60ebb795b8eb)\r\n\r\n####
Additional fixes\r\n\r\n- Fixes the metric and split field controls
states when editing existing\r\nChange point embeddable from a
dashboard\r\n- Fixes `filter` query if partitions input is initialized
as an empty\r\narray.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"80d382a22f2adc39a63146d3ffb5cb7763090c2e","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Feature:Reporting",":ml","Team:ML","Feature:ML/AIOps","Feature:Embeddables","v8.11.0","v8.12.0"],"number":169962,"url":"https://github.com/elastic/kibana/pull/169962","mergeCommit":{"message":"[ML]
AIOps: Fix Change point embeddable reporting (#169962)\n\n##
Summary\r\n\r\nFixes #169733\r\n\r\n#### Reporting fix\r\n\r\nChange
point detection embeddable was incorrectly reporting
render\r\ncompletion. It was relying on the `onLoad` callback from the
Lens\r\nembeddable responsible for chart rendering, which only indicates
that\r\ndata fetching is complete, but not the actual rendering.
Current\r\nimplementation relies on the `renderComplete` event from each
child\r\nembeddable. Both PNG and PDF exports tested and work as
expected.\r\n\r\n\r\n![DASHBOARDDDD](https://github.com/elastic/kibana/assets/5236598/fb718f31-5862-43ab-82e3-60ebb795b8eb)\r\n\r\n####
Additional fixes\r\n\r\n- Fixes the metric and split field controls
states when editing existing\r\nChange point embeddable from a
dashboard\r\n- Fixes `filter` query if partitions input is initialized
as an empty\r\narray.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"80d382a22f2adc39a63146d3ffb5cb7763090c2e"}},"sourceBranch":"main","suggestedTargetBranches":["8.11"],"targetPullRequestStates":[{"branch":"8.11","label":"v8.11.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/169962","number":169962,"mergeCommit":{"message":"[ML]
AIOps: Fix Change point embeddable reporting (#169962)\n\n##
Summary\r\n\r\nFixes #169733\r\n\r\n#### Reporting fix\r\n\r\nChange
point detection embeddable was incorrectly reporting
render\r\ncompletion. It was relying on the `onLoad` callback from the
Lens\r\nembeddable responsible for chart rendering, which only indicates
that\r\ndata fetching is complete, but not the actual rendering.
Current\r\nimplementation relies on the `renderComplete` event from each
child\r\nembeddable. Both PNG and PDF exports tested and work as
expected.\r\n\r\n\r\n![DASHBOARDDDD](https://github.com/elastic/kibana/assets/5236598/fb718f31-5862-43ab-82e3-60ebb795b8eb)\r\n\r\n####
Additional fixes\r\n\r\n- Fixes the metric and split field controls
states when editing existing\r\nChange point embeddable from a
dashboard\r\n- Fixes `filter` query if partitions input is initialized
as an empty\r\narray.\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"80d382a22f2adc39a63146d3ffb5cb7763090c2e"}}]}]
BACKPORT-->
  • Loading branch information
darnautov authored Oct 30, 2023
1 parent b757313 commit a9f16eb
Show file tree
Hide file tree
Showing 9 changed files with 205 additions and 25 deletions.
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',
},
{
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

0 comments on commit a9f16eb

Please sign in to comment.