Skip to content

Commit

Permalink
[ML] Fixes missing y-axis description for rare chart. (elastic#24823) (
Browse files Browse the repository at this point in the history
…elastic#24836)

Fixes the missing y-axis description for rare charts in the form of y-axis event distribution split by {{fieldName}}. The intention is to briefly clarify that the type of chart is different than the other ones which have a count or metric based y-axis. This text is in addition to the text provided (but hidden without hovering) in the info icon tooltip.
  • Loading branch information
walterra authored Oct 30, 2018
1 parent eee5bdd commit a659fc3
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 3 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
{
"jobId": "ffb-rare-url-0921",
"detectorIndex": 0,
"metricFunction": "count",
"timeField": "@timestamp",
"interval": "15m",
"datafeedConfig": {
"datafeed_id": "datafeed-ffb-rare-url-0921",
"job_id": "ffb-rare-url-0921",
"query_delay": "115433ms",
"indices": [
"filebeat-6.0.0-2017-nginx-elasticco-anon"
],
"types": [],
"query": {
"match_all": {
"boost": 1
}
},
"scroll_size": 1000,
"chunking_config": {
"mode": "auto"
},
"state": "stopped"
},
"functionDescription": "rare",
"bucketSpanSeconds": 900,
"detectorLabel": "rare by \"nginx.access.url\"",
"entityFields": [
{
"fieldName": "nginx.access.url",
"fieldValue": "/?node=4.1.1,5,7",
"fieldType": "by"
}
],
"infoTooltip": {
"jobId": "ffb-rare-url-0921",
"aggregationInterval": "15m",
"chartFunction": "count",
"entityFields": [
{
"fieldName": "nginx.access.url",
"fieldValue": "/?node=4.1.1,5,7"
}
]
},
"loading": true,
"chartData": null
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export function ExplorerChartLabel({ detectorLabel, entityFields, infoTooltip, w
);
}
ExplorerChartLabel.propTypes = {
detectorLabel: PropTypes.string.isRequired,
detectorLabel: PropTypes.object.isRequired,
entityFields: PropTypes.arrayOf(ExplorerChartLabelBadge.propTypes.entity),
infoTooltip: PropTypes.object.isRequired,
wrapLabel: PropTypes.bool
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,25 @@ function ExplorerChartContainer({
} = series;

const chartType = getChartType(series);
let DetectorLabel = <React.Fragment>{detectorLabel}</React.Fragment>;

if (chartType === CHART_TYPE.EVENT_DISTRIBUTION) {
const byField = series.entityFields.find(d => d.fieldType === 'by');
if (typeof byField !== 'undefined') {
DetectorLabel = (
<React.Fragment>
{detectorLabel}<br />y-axis event distribution split by &quot;{byField.fieldName}&quot;
</React.Fragment>
);
}
}

return (
<React.Fragment>
<EuiFlexGroup justifyContent="spaceBetween">
<EuiFlexItem grow={false}>
<ExplorerChartLabel
detectorLabel={detectorLabel}
detectorLabel={DetectorLabel}
entityFields={entityFields}
infoTooltip={{ ...series.infoTooltip, chartType }}
wrapLabel={wrapLabel}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import { chartData } from './__mocks__/mock_chart_data';
import seriesConfig from './__mocks__/mock_series_config_filebeat.json';
import seriesConfigRare from './__mocks__/mock_series_config_rare.json';

// Mock TimeBuckets and mlFieldFormatService, they don't play well
// with the jest based test setup yet.
Expand Down Expand Up @@ -80,6 +81,7 @@ describe('ExplorerChartsContainer', () => {

const mockedGetBBox = { x: 0, y: -11.5, width: 12.1875, height: 14.5 };
const originalGetBBox = SVGElement.prototype.getBBox;
const rareChartUniqueString = 'y-axis event distribution split by';
beforeEach(() => SVGElement.prototype.getBBox = () => mockedGetBBox);
afterEach(() => (SVGElement.prototype.getBBox = originalGetBBox));

Expand Down Expand Up @@ -111,5 +113,29 @@ describe('ExplorerChartsContainer', () => {
// We test child components with snapshots separately
// so we just do some high level sanity check here.
expect(wrapper.find('.ml-explorer-chart-container').children()).toHaveLength(2);

// Check if the additional y-axis information for rare charts is not part of the chart
expect(wrapper.html().search(rareChartUniqueString)).toBe(-1);
});

test('Initialization with rare detector', () => {
const wrapper = mount(<ExplorerChartsContainer
seriesToPlot={[{
...seriesConfigRare,
chartData,
chartLimits: chartLimits(chartData)
}]}
chartsPerRow={1}
tooManyBuckets={false}
mlSelectSeverityService={mlSelectSeverityServiceMock}
mlChartTooltipService={mlChartTooltipService}
/>);

// We test child components with snapshots separately
// so we just do some high level sanity check here.
expect(wrapper.find('.ml-explorer-chart-container').children()).toHaveLength(2);

// Check if the additional y-axis information for rare charts is part of the chart
expect(wrapper.html().search(rareChartUniqueString)).toBeGreaterThan(0);
});
});
2 changes: 1 addition & 1 deletion x-pack/plugins/ml/public/util/chart_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ export function removeLabelOverlap(axis, startTimeMs, tickInterval, width) {
const fn = function (ts) {
const filteredTicks = axis.selectAll('.tick').filter(d => d === ts);

if (filteredTicks[0].length === 0) {
if (filteredTicks.length === 0 || filteredTicks[0].length === 0) {
return false;
}

Expand Down

0 comments on commit a659fc3

Please sign in to comment.