diff --git a/x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.test.tsx b/x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.test.tsx index 76eea019ac83e..54d455519329c 100644 --- a/x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.test.tsx +++ b/x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.test.tsx @@ -168,11 +168,12 @@ describe('useFailedTransactionsCorrelations', () => { ); try { + // Each simulated request takes 100ms. After an inital 50ms + // we track the internal requests the hook is running and + // check the expected progress after these requests. jest.advanceTimersByTime(50); await waitFor(() => expect(result.current.progress.loaded).toBe(0)); jest.advanceTimersByTime(100); - await waitFor(() => expect(result.current.progress.loaded).toBe(0)); - jest.advanceTimersByTime(100); await waitFor(() => expect(result.current.progress.loaded).toBe(0.05)); expect(result.current.progress).toEqual({ @@ -180,6 +181,28 @@ describe('useFailedTransactionsCorrelations', () => { isRunning: true, loaded: 0.05, }); + expect(result.current.response).toEqual({ + ccsWarning: false, + fieldStats: undefined, + errorHistogram: undefined, + failedTransactionsCorrelations: undefined, + overallHistogram: [ + { + doc_count: 1234, + key: 'the-key', + }, + ], + percentileThresholdValue: 1.234, + }); + + jest.advanceTimersByTime(100); + await waitFor(() => expect(result.current.progress.loaded).toBe(0.1)); + + expect(result.current.progress).toEqual({ + error: undefined, + isRunning: true, + loaded: 0.1, + }); expect(result.current.response).toEqual({ ccsWarning: false, fieldStats: undefined, @@ -200,23 +223,23 @@ describe('useFailedTransactionsCorrelations', () => { }); jest.advanceTimersByTime(100); - await waitFor(() => expect(result.current.progress.loaded).toBe(0.1)); + await waitFor(() => expect(result.current.progress.loaded).toBe(0.15)); // field candidates are an implementation detail and - // will not be exposed, it will just set loaded to 0.1. + // will not be exposed, it will just set loaded to 0.15. expect(result.current.progress).toEqual({ error: undefined, isRunning: true, - loaded: 0.1, + loaded: 0.15, }); jest.advanceTimersByTime(100); - await waitFor(() => expect(result.current.progress.loaded).toBe(1)); + await waitFor(() => expect(result.current.progress.loaded).toBe(0.9)); expect(result.current.progress).toEqual({ error: undefined, isRunning: true, - loaded: 1, + loaded: 0.9, }); expect(result.current.response).toEqual({ @@ -252,9 +275,7 @@ describe('useFailedTransactionsCorrelations', () => { }); jest.advanceTimersByTime(100); - await waitFor(() => - expect(result.current.response.fieldStats).toBeDefined() - ); + await waitFor(() => expect(result.current.progress.loaded).toBe(1)); expect(result.current.progress).toEqual({ error: undefined, diff --git a/x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.ts b/x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.ts index 4119d9e2e4dae..f9c365c539e0f 100644 --- a/x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.ts +++ b/x-pack/plugins/apm/public/components/app/correlations/use_failed_transactions_correlations.ts @@ -37,9 +37,10 @@ import { useFetchParams } from './use_fetch_params'; // Overall progress is a float from 0 to 1. const LOADED_OVERALL_HISTOGRAM = 0.05; -const LOADED_FIELD_CANDIDATES = LOADED_OVERALL_HISTOGRAM + 0.05; +const LOADED_ERROR_HISTOGRAM = LOADED_OVERALL_HISTOGRAM + 0.05; +const LOADED_FIELD_CANDIDATES = LOADED_ERROR_HISTOGRAM + 0.05; const LOADED_DONE = 1; -const PROGRESS_STEP_P_VALUES = 0.9; +const PROGRESS_STEP_P_VALUES = 0.9 - LOADED_FIELD_CANDIDATES; export function useFailedTransactionsCorrelations() { const fetchParams = useFetchParams(); @@ -85,61 +86,78 @@ export function useFailedTransactionsCorrelations() { fallbackResult: undefined, }; - const [overallHistogramResponse, errorHistogramRespone] = - await Promise.all([ - // Initial call to fetch the overall distribution for the log-log plot. - callApmApi( - 'POST /internal/apm/latency/overall_distribution/transactions', - { - signal: abortCtrl.current.signal, - params: { - body: { - ...fetchParams, - percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD, - chartType: - LatencyDistributionChartType.failedTransactionsCorrelations, - }, - }, - } - ), - callApmApi( - 'POST /internal/apm/latency/overall_distribution/transactions', - { - signal: abortCtrl.current.signal, - params: { - body: { - ...fetchParams, - percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD, - termFilters: [ - { - fieldName: EVENT_OUTCOME, - fieldValue: EventOutcome.failure, - }, - ], - chartType: - LatencyDistributionChartType.failedTransactionsCorrelations, - }, - }, - } - ), - ]); + // Initial call to fetch the overall distribution for the log-log plot. + const overallHistogramResponse = await callApmApi( + 'POST /internal/apm/latency/overall_distribution/transactions', + { + signal: abortCtrl.current.signal, + params: { + body: { + ...fetchParams, + percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD, + chartType: + LatencyDistributionChartType.failedTransactionsCorrelations, + }, + }, + } + ); + + if (abortCtrl.current.signal.aborted) { + return; + } - const { overallHistogram, totalDocCount, percentileThresholdValue } = - overallHistogramResponse; - const { overallHistogram: errorHistogram } = errorHistogramRespone; + const { + overallHistogram, + totalDocCount, + percentileThresholdValue, + durationMin, + durationMax, + } = overallHistogramResponse; - responseUpdate.errorHistogram = errorHistogram; responseUpdate.overallHistogram = overallHistogram; responseUpdate.totalDocCount = totalDocCount; responseUpdate.percentileThresholdValue = percentileThresholdValue; + setResponse({ + ...responseUpdate, + loaded: LOADED_OVERALL_HISTOGRAM, + }); + setResponse.flush(); + + const errorHistogramResponse = await callApmApi( + 'POST /internal/apm/latency/overall_distribution/transactions', + { + signal: abortCtrl.current.signal, + params: { + body: { + ...fetchParams, + percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD, + termFilters: [ + { + fieldName: EVENT_OUTCOME, + fieldValue: EventOutcome.failure, + }, + ], + durationMin, + durationMax, + chartType: + LatencyDistributionChartType.failedTransactionsCorrelations, + }, + }, + } + ); + if (abortCtrl.current.signal.aborted) { return; } + const { overallHistogram: errorHistogram } = errorHistogramResponse; + + responseUpdate.errorHistogram = errorHistogram; + setResponse({ ...responseUpdate, - loaded: LOADED_OVERALL_HISTOGRAM, + loaded: LOADED_ERROR_HISTOGRAM, }); setResponse.flush(); @@ -179,7 +197,12 @@ export function useFailedTransactionsCorrelations() { { signal: abortCtrl.current.signal, params: { - body: { ...fetchParams, fieldCandidates: fieldCandidatesChunk }, + body: { + ...fetchParams, + fieldCandidates: fieldCandidatesChunk, + durationMin, + durationMax, + }, }, } ); @@ -284,7 +307,7 @@ export function useFailedTransactionsCorrelations() { const progress = useMemo( () => ({ error, - loaded, + loaded: Math.round(loaded * 100) / 100, isRunning, }), [error, loaded, isRunning] diff --git a/x-pack/plugins/apm/public/components/app/correlations/use_latency_correlations.ts b/x-pack/plugins/apm/public/components/app/correlations/use_latency_correlations.ts index b46aaf815d6ad..b2ba6fc4c68b1 100644 --- a/x-pack/plugins/apm/public/components/app/correlations/use_latency_correlations.ts +++ b/x-pack/plugins/apm/public/components/app/correlations/use_latency_correlations.ts @@ -86,20 +86,25 @@ export function useLatencyCorrelations() { }; // Initial call to fetch the overall distribution for the log-log plot. - const { overallHistogram, totalDocCount, percentileThresholdValue } = - await callApmApi( - 'POST /internal/apm/latency/overall_distribution/transactions', - { - signal: abortCtrl.current.signal, - params: { - body: { - ...fetchParams, - percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD, - chartType: LatencyDistributionChartType.latencyCorrelations, - }, + const { + overallHistogram, + totalDocCount, + percentileThresholdValue, + durationMin, + durationMax, + } = await callApmApi( + 'POST /internal/apm/latency/overall_distribution/transactions', + { + signal: abortCtrl.current.signal, + params: { + body: { + ...fetchParams, + percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD, + chartType: LatencyDistributionChartType.latencyCorrelations, }, - } - ); + }, + } + ); responseUpdate.overallHistogram = overallHistogram; responseUpdate.totalDocCount = totalDocCount; responseUpdate.percentileThresholdValue = percentileThresholdValue; @@ -192,7 +197,12 @@ export function useLatencyCorrelations() { { signal: abortCtrl.current.signal, params: { - body: { ...fetchParams, fieldValuePairs: fieldValuePairChunk }, + body: { + ...fetchParams, + durationMin, + durationMax, + fieldValuePairs: fieldValuePairChunk, + }, }, } ); diff --git a/x-pack/plugins/apm/public/components/app/transaction_details/distribution/use_transaction_distribution_chart_data.ts b/x-pack/plugins/apm/public/components/app/transaction_details/distribution/use_transaction_distribution_chart_data.ts index 42febda7a186f..77f284e0ffee7 100644 --- a/x-pack/plugins/apm/public/components/app/transaction_details/distribution/use_transaction_distribution_chart_data.ts +++ b/x-pack/plugins/apm/public/components/app/transaction_details/distribution/use_transaction_distribution_chart_data.ts @@ -86,7 +86,9 @@ export const useTransactionDistributionChartData = () => { params.serviceName && params.environment && params.start && - params.end + params.end && + overallLatencyData.durationMin && + overallLatencyData.durationMax ) { return callApmApi( 'POST /internal/apm/latency/overall_distribution/transactions', @@ -94,6 +96,8 @@ export const useTransactionDistributionChartData = () => { params: { body: { ...params, + durationMin: overallLatencyData.durationMin, + durationMax: overallLatencyData.durationMax, percentileThreshold: DEFAULT_PERCENTILE_THRESHOLD, termFilters: [ { @@ -108,7 +112,7 @@ export const useTransactionDistributionChartData = () => { ); } }, - [params] + [params, overallLatencyData.durationMin, overallLatencyData.durationMax] ); useEffect(() => { diff --git a/x-pack/plugins/apm/public/components/shared/charts/duration_distribution_chart/index.test.tsx b/x-pack/plugins/apm/public/components/shared/charts/duration_distribution_chart/index.test.tsx index 6af1c89fd1991..05cedbdc1e391 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/duration_distribution_chart/index.test.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/duration_distribution_chart/index.test.tsx @@ -7,11 +7,11 @@ import type { HistogramItem } from '../../../../../common/correlations/types'; -import { replaceHistogramDotsWithBars } from '.'; +import { replaceHistogramZerosWithMinimumDomainValue } from '.'; describe('TransactionDistributionChart', () => { - describe('replaceHistogramDotsWithBars', () => { - it('does the thing', () => { + describe('replaceHistogramZerosWithMinimumDomainValue', () => { + it('replaces zeroes', () => { const mockHistogram = [ { doc_count: 10 }, { doc_count: 10 }, @@ -25,7 +25,9 @@ describe('TransactionDistributionChart', () => { { doc_count: 10 }, ] as HistogramItem[]; - expect(replaceHistogramDotsWithBars(mockHistogram)).toEqual([ + expect( + replaceHistogramZerosWithMinimumDomainValue(mockHistogram) + ).toEqual([ { doc_count: 10 }, { doc_count: 10 }, { doc_count: 0.0001 }, diff --git a/x-pack/plugins/apm/public/components/shared/charts/duration_distribution_chart/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/duration_distribution_chart/index.tsx index 03e2af308a798..e1ca9e7f90b3e 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/duration_distribution_chart/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/duration_distribution_chart/index.tsx @@ -5,7 +5,7 @@ * 2.0. */ -import React from 'react'; +import React, { useMemo } from 'react'; import { flatten } from 'lodash'; import { @@ -83,18 +83,21 @@ const getAnnotationsStyle = (color = 'gray'): LineAnnotationStyle => ({ }, }); -// TODO Revisit this approach since it actually manipulates the numbers -// showing in the chart and its tooltips. -const CHART_PLACEHOLDER_VALUE = 0.0001; +// With a log based y axis in combination with the `CURVE_STEP_AFTER` style, +// the line of an area would not go down to 0 but end on the y axis at the last value >0. +// By replacing the 0s with a small value >0 the line will be drawn as intended. +// This is just to visually fix the line, for tooltips, that number will be again rounded down to 0. +// Note this workaround is only safe to use for this type of chart because it works with +// count based values and not a float based metric for example on the y axis. +const Y_AXIS_MIN_DOMAIN = 0.5; +const Y_AXIS_MIN_VALUE = 0.0001; -// Elastic charts will show any lone bin (i.e. a populated bin followed by empty bin) -// as a circular marker instead of a bar -// This provides a workaround by making the next bin not empty -// TODO Find a way to get rid of this workaround since it alters original values of the data. -export const replaceHistogramDotsWithBars = (histogramItems: HistogramItem[]) => +export const replaceHistogramZerosWithMinimumDomainValue = ( + histogramItems: HistogramItem[] +) => histogramItems.reduce((histogramItem, _, i) => { if (histogramItem[i].doc_count === 0) { - histogramItem[i].doc_count = CHART_PLACEHOLDER_VALUE; + histogramItem[i].doc_count = Y_AXIS_MIN_VALUE; } return histogramItem; }, histogramItems); @@ -140,9 +143,10 @@ export function DurationDistributionChart({ ...flatten(data.map((d) => d.histogram)).map((d) => d.doc_count) ) ?? 0; const yTicks = Math.max(1, Math.ceil(Math.log10(yMax))); + const yAxisMaxDomain = Math.pow(10, yTicks); const yAxisDomain = { - min: 0.5, - max: Math.pow(10, yTicks), + min: Y_AXIS_MIN_DOMAIN, + max: yAxisMaxDomain, }; const selectionAnnotation = @@ -153,13 +157,22 @@ export function DurationDistributionChart({ x0: selection[0], x1: selection[1], y0: 0, - y1: 100000, + y1: yAxisMaxDomain, }, details: 'selection', }, ] : undefined; + const chartData = useMemo( + () => + data.map((d) => ({ + ...d, + histogram: replaceHistogramZerosWithMinimumDomainValue(d.histogram), + })), + [data] + ); + return (
- {data.map((d, i) => ( + {chartData.map((d) => (