-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[ML] Anomaly Detection: Adds popover links menu to anomaly explorer charts. #186587
Changes from 6 commits
7b10f35
c003f34
a934e95
cb9ee0f
0ae1576
f07cdb4
db0bd6f
e35c568
6cff8ba
7829012
aea101d
2ffbaa4
19ba311
79ec10b
4959864
4e1e8af
270b75f
5ac1cbb
5117507
8e8185b
d7488d2
2b7d099
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,8 @@ import React from 'react'; | |
import d3 from 'd3'; | ||
import moment from 'moment'; | ||
|
||
import { EuiPopover } from '@elastic/eui'; | ||
|
||
import { i18n } from '@kbn/i18n'; | ||
import { | ||
getFormattedSeverityScore, | ||
|
@@ -25,6 +27,8 @@ import { | |
import { formatHumanReadableDateTime } from '@kbn/ml-date-utils'; | ||
import { context } from '@kbn/kibana-react-plugin/public'; | ||
|
||
import { LinksMenuUI } from '../../components/anomalies_table/links_menu'; | ||
import { RuleEditorFlyout } from '../../components/rule_editor'; | ||
import { formatValue } from '../../formatters/format_value'; | ||
import { | ||
LINE_CHART_ANOMALY_RADIUS, | ||
|
@@ -43,6 +47,7 @@ import { CHART_HEIGHT, TRANSPARENT_BACKGROUND } from './constants'; | |
import { filter } from 'rxjs'; | ||
import { drawCursor } from './utils/draw_anomaly_explorer_charts_cursor'; | ||
|
||
const popoverMenuOffset = 0; | ||
const CONTENT_WRAPPER_HEIGHT = 215; | ||
const CONTENT_WRAPPER_CLASS = 'ml-explorer-chart-content-wrapper'; | ||
|
||
|
@@ -52,6 +57,7 @@ export class ExplorerChartSingleMetric extends React.Component { | |
tooManyBuckets: PropTypes.bool, | ||
seriesConfig: PropTypes.object, | ||
severity: PropTypes.number.isRequired, | ||
tableData: PropTypes.object, | ||
tooltipService: PropTypes.object.isRequired, | ||
timeBuckets: PropTypes.object.isRequired, | ||
onPointerUpdate: PropTypes.func.isRequired, | ||
|
@@ -63,7 +69,9 @@ export class ExplorerChartSingleMetric extends React.Component { | |
constructor(props) { | ||
super(props); | ||
this.chartScales = undefined; | ||
this.state = { popoverData: null, popoverCoords: [0, 0], showRuleEditorFlyout: () => {} }; | ||
} | ||
|
||
componentDidMount() { | ||
this.renderChart(); | ||
|
||
|
@@ -351,6 +359,8 @@ export class ExplorerChartSingleMetric extends React.Component { | |
.attr('d', lineChartValuesLine(data)); | ||
} | ||
|
||
const that = this; | ||
|
||
function drawLineChartMarkers(data) { | ||
// Render circle markers for the points. | ||
// These are used for displaying tooltips on mouseover. | ||
|
@@ -375,9 +385,17 @@ export class ExplorerChartSingleMetric extends React.Component { | |
.enter() | ||
.append('circle') | ||
.attr('r', LINE_CHART_ANOMALY_RADIUS) | ||
.on('click', function (d) { | ||
d3.event.preventDefault(); | ||
if (d.anomalyScore === undefined) return; | ||
showAnomalyPopover(d, this); | ||
peteharverson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}) | ||
// Don't use an arrow function since we need access to `this`. | ||
.on('mouseover', function (d) { | ||
showLineChartTooltip(d, this); | ||
// Show the tooltip only if the actions menu isn't active | ||
if (that.state.popoverData === null) { | ||
showLineChartTooltip(d, this); | ||
} | ||
}) | ||
.on('mouseout', () => tooltipService.hide()); | ||
|
||
|
@@ -448,6 +466,37 @@ export class ExplorerChartSingleMetric extends React.Component { | |
.attr('y', (d) => lineChartYScale(d.value) - SCHEDULED_EVENT_SYMBOL_HEIGHT / 2); | ||
} | ||
|
||
function showAnomalyPopover(marker, circle) { | ||
const anomalyTime = marker.date; | ||
|
||
// The table items could be aggregated, so we have to find the item | ||
// that has the closest timestamp to the selected anomaly from the chart. | ||
const tableItem = that.props.tableData.anomalies.reduce((closestItem, currentItem) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like this is the same logic used in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 moved this to a util function including unit tests in 5ac1cbb. |
||
const closestItemDelta = Math.abs(anomalyTime - closestItem.source.timestamp); | ||
const currentItemDelta = Math.abs(anomalyTime - currentItem.source.timestamp); | ||
return currentItemDelta < closestItemDelta ? currentItem : closestItem; | ||
}, that.props.tableData.anomalies[0]); | ||
|
||
if (tableItem) { | ||
// Overwrite the timestamp of the possibly aggregated table item with the | ||
// timestamp of the anomaly clicked in the chart so we're able to pick | ||
// the right baseline and deviation time ranges for Log Rate Analysis. | ||
tableItem.source.timestamp = anomalyTime; | ||
|
||
// Calculate the relative coordinates of the clicked anomaly marker | ||
// so we're able to position the popover actions menu above it. | ||
const dotRect = circle.getBoundingClientRect(); | ||
const rootRect = that.rootNode.getBoundingClientRect(); | ||
const x = Math.round(dotRect.x + dotRect.width / 2 - rootRect.x); | ||
const y = Math.round(dotRect.y + dotRect.height / 2 - rootRect.y) - popoverMenuOffset; | ||
|
||
// Hide any active tooltip | ||
that.props.tooltipService.hide(); | ||
// Set the popover state to enable the actions menu | ||
that.setState({ popoverData: tableItem, popoverCoords: [x, y] }); | ||
} | ||
} | ||
|
||
function showLineChartTooltip(marker, circle) { | ||
// Show the time and metric values in the tooltip. | ||
// Uses date, value, upper, lower and anomalyScore (optional) marker properties. | ||
|
@@ -589,6 +638,22 @@ export class ExplorerChartSingleMetric extends React.Component { | |
this.rootNode = componentNode; | ||
} | ||
|
||
closePopover() { | ||
this.setState({ popoverData: null, popoverCoords: [0, 0] }); | ||
} | ||
|
||
setShowRuleEditorFlyoutFunction = (func) => { | ||
this.setState({ | ||
showRuleEditorFlyout: func, | ||
}); | ||
}; | ||
|
||
unsetShowRuleEditorFlyoutFunction = () => { | ||
this.setState({ | ||
showRuleEditorFlyout: () => {}, | ||
}); | ||
}; | ||
|
||
render() { | ||
const { seriesConfig } = this.props; | ||
|
||
|
@@ -601,10 +666,44 @@ export class ExplorerChartSingleMetric extends React.Component { | |
const isLoading = seriesConfig.loading; | ||
|
||
return ( | ||
<div className="ml-explorer-chart" ref={this.setRef.bind(this)}> | ||
{isLoading && <LoadingIndicator height={CONTENT_WRAPPER_HEIGHT} />} | ||
{!isLoading && <div className={CONTENT_WRAPPER_CLASS} />} | ||
</div> | ||
<> | ||
<RuleEditorFlyout | ||
setShowFunction={this.setShowRuleEditorFlyoutFunction} | ||
unsetShowFunction={this.unsetShowRuleEditorFlyoutFunction} | ||
/> | ||
{this.state.popoverData !== null && ( | ||
<div | ||
style={{ | ||
position: 'absolute', | ||
marginLeft: this.state.popoverCoords[0], | ||
marginTop: this.state.popoverCoords[1], | ||
}} | ||
> | ||
<EuiPopover | ||
isOpen={true} | ||
closePopover={() => this.closePopover()} | ||
panelPaddingSize="none" | ||
anchorPosition="upLeft" | ||
> | ||
<LinksMenuUI | ||
anomaly={this.state.popoverData} | ||
bounds={this.props.bounds} | ||
showMapsLink={false} | ||
showViewSeriesLink={true} | ||
isAggregatedData={this.props.tableData.interval !== 'second'} | ||
interval={this.props.tableData.interval} | ||
showRuleEditorFlyout={this.state.showRuleEditorFlyout} | ||
onItemClick={() => this.closePopover()} | ||
sourceIndicesWithGeoFields={this.props.sourceIndicesWithGeoFields} | ||
/> | ||
</EuiPopover> | ||
</div> | ||
)} | ||
<div className="ml-explorer-chart" ref={this.setRef.bind(this)}> | ||
{isLoading && <LoadingIndicator height={CONTENT_WRAPPER_HEIGHT} />} | ||
{!isLoading && <div className={CONTENT_WRAPPER_CLASS} />} | ||
</div> | ||
</> | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not to be addressed in this PR but I think we should move away from this pattern of having a child set a parent component setter since it's an antipattern. I know this is old code and this is the way it's done in the Rule editor flyout currently but just think we should add this to the list of things to improve.