Skip to content

Commit

Permalink
[ML] Fix a call stack size exception triggered by a negative tickInte…
Browse files Browse the repository at this point in the history
…rval. (#24742)

In certain cases tickInterval mistakenly could end up being negative which made getTickValues() run into a call stack size exception. This PR fixes it by a) adding a check to getTickValues() that interval must not be 0 or smaller and b) changing the way the tickInterval is determined in the Anomaly Explorer Charts.
  • Loading branch information
walterra authored Oct 30, 2018
1 parent 0b74169 commit 8b2ce42
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -239,11 +239,10 @@ export class ExplorerChartDistribution extends React.Component {
timeBuckets.setInterval('auto');
const xAxisTickFormat = timeBuckets.getScaledDateFormat();

const emphasisStart = Math.max(config.selectedEarliest, config.plotEarliest);
const emphasisEnd = Math.min(config.selectedLatest, config.plotLatest);
const tickValuesStart = Math.max(config.selectedEarliest, config.plotEarliest);
// +1 ms to account for the ms that was substracted for query aggregations.
const interval = emphasisEnd - emphasisStart + 1;
const tickValues = getTickValues(emphasisStart, interval, config.plotEarliest, config.plotLatest);
const interval = config.selectedLatest - config.selectedEarliest + 1;
const tickValues = getTickValues(tickValuesStart, interval, config.plotEarliest, config.plotLatest);

const xAxis = d3.svg.axis().scale(lineChartXScale)
.orient('bottom')
Expand Down Expand Up @@ -283,7 +282,7 @@ export class ExplorerChartDistribution extends React.Component {
.call(yAxis);

if (tooManyBuckets === false) {
removeLabelOverlap(gAxis, emphasisStart, interval, vizWidth);
removeLabelOverlap(gAxis, tickValuesStart, interval, vizWidth);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,10 @@ export class ExplorerChartSingleMetric extends React.Component {
timeBuckets.setInterval('auto');
const xAxisTickFormat = timeBuckets.getScaledDateFormat();

const emphasisStart = Math.max(config.selectedEarliest, config.plotEarliest);
const emphasisEnd = Math.min(config.selectedLatest, config.plotLatest);
const tickValuesStart = Math.max(config.selectedEarliest, config.plotEarliest);
// +1 ms to account for the ms that was substracted for query aggregations.
const interval = emphasisEnd - emphasisStart + 1;
const tickValues = getTickValues(emphasisStart, interval, config.plotEarliest, config.plotLatest);
const interval = config.selectedLatest - config.selectedEarliest + 1;
const tickValues = getTickValues(tickValuesStart, interval, config.plotEarliest, config.plotLatest);

const xAxis = d3.svg.axis().scale(lineChartXScale)
.orient('bottom')
Expand Down Expand Up @@ -236,7 +235,7 @@ export class ExplorerChartSingleMetric extends React.Component {
.call(yAxis);

if (tooManyBuckets === false) {
removeLabelOverlap(gAxis, emphasisStart, interval, vizWidth);
removeLabelOverlap(gAxis, tickValuesStart, interval, vizWidth);
}
}

Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/ml/public/util/chart_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,12 @@ const TICK_DIRECTION = {
// the bounds of earliest and latest. This is useful for the Anomaly Explorer Charts
// to align axis ticks with the gray area resembling the swimlane cell selection.
export function getTickValues(startTimeMs, tickInterval, earliest, latest) {
// A tickInterval equal or smaller than 0 would trigger a call stack exception,
// so we're trying to catch that before it happens.
if (tickInterval <= 0) {
throw Error('tickInterval must be larger than 0.');
}

const tickValues = [startTimeMs];

function addTicks(ts, operator) {
Expand Down
9 changes: 9 additions & 0 deletions x-pack/plugins/ml/public/util/chart_utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,15 @@ describe('getTickValues', () => {
1519257600000
]);
});

test('invalid tickIntervals trigger an error', () => {
expect(() => {
getTickValues(1518652800000, 0, 1518274800000, 1519635600000);
}).toThrow();
expect(() => {
getTickValues(1518652800000, -604800000, 1518274800000, 1519635600000);
}).toThrow();
});
});

describe('isLabelLengthAboveThreshold', () => {
Expand Down

0 comments on commit 8b2ce42

Please sign in to comment.