From 6976988b01f7d970b9b5005f3bf8bba12b05d75d Mon Sep 17 00:00:00 2001 From: Pete Harverson Date: Mon, 12 Jul 2021 11:31:54 +0100 Subject: [PATCH] [ML] Fixes unnecessary too many buckets warning on anomaly chart embeddable (#105043) * [ML] Fixes unnecessary too many buckets warning on anomaly chart embeddable * [ML] Update jest tests for number of axis ticks. Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../explorer_chart_distribution.js | 24 ++++++++++--------- .../explorer_chart_distribution.test.js | 2 +- .../explorer_chart_single_metric.js | 24 ++++++++++--------- .../explorer_chart_single_metric.test.js | 2 +- .../anomaly_explorer_charts_service.ts | 11 +++++++-- 5 files changed, 37 insertions(+), 26 deletions(-) diff --git a/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_distribution.js b/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_distribution.js index 7efd36bbe57c61..27a934fa841fe0 100644 --- a/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_distribution.js +++ b/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_distribution.js @@ -270,12 +270,6 @@ export class ExplorerChartDistribution extends React.Component { const tickValuesStart = Math.max(config.selectedEarliest, config.plotEarliest); // +1 ms to account for the ms that was subtracted for query aggregations. const interval = config.selectedLatest - config.selectedEarliest + 1; - const tickValues = getTickValues( - tickValuesStart, - interval, - config.plotEarliest, - config.plotLatest - ); const xAxis = d3.svg .axis() @@ -286,10 +280,18 @@ export class ExplorerChartDistribution extends React.Component { .tickPadding(10) .tickFormat((d) => moment(d).format(xAxisTickFormat)); - // With tooManyBuckets the chart would end up with no x-axis labels - // because the ticks are based on the span of the emphasis section, - // and the highlighted area spans the whole chart. - if (tooManyBuckets === false) { + // With tooManyBuckets, or when the chart is used as an embeddable, + // the chart would end up with no x-axis labels because the ticks are based on the span of the + // emphasis section, and the selected area spans the whole chart. + const useAutoTicks = + tooManyBuckets === true || interval >= config.plotLatest - config.plotEarliest; + if (useAutoTicks === false) { + const tickValues = getTickValues( + tickValuesStart, + interval, + config.plotEarliest, + config.plotLatest + ); xAxis.tickValues(tickValues); } else { xAxis.ticks(numTicksForDateFormat(vizWidth, xAxisTickFormat)); @@ -327,7 +329,7 @@ export class ExplorerChartDistribution extends React.Component { }); } - if (tooManyBuckets === false) { + if (useAutoTicks === false) { removeLabelOverlap(gAxis, tickValuesStart, interval, vizWidth); } } diff --git a/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_distribution.test.js b/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_distribution.test.js index 11a15b192fc520..8d2f66a870c75c 100644 --- a/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_distribution.test.js +++ b/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_distribution.test.js @@ -139,7 +139,7 @@ describe('ExplorerChart', () => { expect(+selectedInterval.getAttribute('height')).toBe(166); const xAxisTicks = wrapper.getDOMNode().querySelector('.x').querySelectorAll('.tick'); - expect([...xAxisTicks]).toHaveLength(0); + expect([...xAxisTicks]).toHaveLength(1); const yAxisTicks = wrapper.getDOMNode().querySelector('.y').querySelectorAll('.tick'); expect([...yAxisTicks]).toHaveLength(5); const emphasizedAxisLabel = wrapper diff --git a/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_single_metric.js b/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_single_metric.js index dd07a7d6cbdee0..19390017244a8c 100644 --- a/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_single_metric.js +++ b/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_single_metric.js @@ -196,12 +196,6 @@ export class ExplorerChartSingleMetric extends React.Component { const tickValuesStart = Math.max(config.selectedEarliest, config.plotEarliest); // +1 ms to account for the ms that was subtracted for query aggregations. const interval = config.selectedLatest - config.selectedEarliest + 1; - const tickValues = getTickValues( - tickValuesStart, - interval, - config.plotEarliest, - config.plotLatest - ); const xAxis = d3.svg .axis() @@ -212,10 +206,18 @@ export class ExplorerChartSingleMetric extends React.Component { .tickPadding(10) .tickFormat((d) => moment(d).format(xAxisTickFormat)); - // With tooManyBuckets the chart would end up with no x-axis labels - // because the ticks are based on the span of the emphasis section, - // and the highlighted area spans the whole chart. - if (tooManyBuckets === false) { + // With tooManyBuckets, or when the chart is used as an embeddable, + // the chart would end up with no x-axis labels because the ticks are based on the span of the + // emphasis section, and the selected area spans the whole chart. + const useAutoTicks = + tooManyBuckets === true || interval >= config.plotLatest - config.plotEarliest; + if (useAutoTicks === false) { + const tickValues = getTickValues( + tickValuesStart, + interval, + config.plotEarliest, + config.plotLatest + ); xAxis.tickValues(tickValues); } else { xAxis.ticks(numTicksForDateFormat(vizWidth, xAxisTickFormat)); @@ -243,7 +245,7 @@ export class ExplorerChartSingleMetric extends React.Component { axes.append('g').attr('class', 'y axis').call(yAxis); - if (tooManyBuckets === false) { + if (useAutoTicks === false) { removeLabelOverlap(gAxis, tickValuesStart, interval, vizWidth); } } diff --git a/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_single_metric.test.js b/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_single_metric.test.js index 981f7515d3d706..00172965bc2162 100644 --- a/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_single_metric.test.js +++ b/x-pack/plugins/ml/public/application/explorer/explorer_charts/explorer_chart_single_metric.test.js @@ -144,7 +144,7 @@ describe('ExplorerChart', () => { expect(+selectedInterval.getAttribute('height')).toBe(166); const xAxisTicks = wrapper.getDOMNode().querySelector('.x').querySelectorAll('.tick'); - expect([...xAxisTicks]).toHaveLength(0); + expect([...xAxisTicks]).toHaveLength(1); const yAxisTicks = wrapper.getDOMNode().querySelector('.y').querySelectorAll('.tick'); expect([...yAxisTicks]).toHaveLength(10); diff --git a/x-pack/plugins/ml/public/application/services/anomaly_explorer_charts_service.ts b/x-pack/plugins/ml/public/application/services/anomaly_explorer_charts_service.ts index afad043fcc4d13..97ddefac860f2d 100644 --- a/x-pack/plugins/ml/public/application/services/anomaly_explorer_charts_service.ts +++ b/x-pack/plugins/ml/public/application/services/anomaly_explorer_charts_service.ts @@ -225,13 +225,20 @@ export class AnomalyExplorerChartsService { chartRange.min = chartRange.min + maxBucketSpanMs; } + // When used as an embeddable, selectedEarliestMs is the start date on the time picker, + // which may be earlier than the time of the first point plotted in the chart (as we plot + // the first full bucket with a start date no earlier than the start). + const selectedEarliestBucketCeil = boundsMin + ? Math.ceil(Math.max(selectedEarliestMs, boundsMin) / maxBucketSpanMs) * maxBucketSpanMs + : Math.ceil(selectedEarliestMs / maxBucketSpanMs) * maxBucketSpanMs; + const selectedLatestBucketStart = boundsMax ? Math.floor(Math.min(selectedLatestMs, boundsMax) / maxBucketSpanMs) * maxBucketSpanMs : Math.floor(selectedLatestMs / maxBucketSpanMs) * maxBucketSpanMs; if ( - (chartRange.min > selectedEarliestMs || chartRange.max < selectedLatestBucketStart) && - chartRange.max - chartRange.min < selectedLatestBucketStart - selectedEarliestMs + (chartRange.min > selectedEarliestBucketCeil || chartRange.max < selectedLatestBucketStart) && + chartRange.max - chartRange.min < selectedLatestBucketStart - selectedEarliestBucketCeil ) { tooManyBuckets = true; }