Skip to content

Commit

Permalink
[8.3] [APM] Add ML expected model bounds as an option to Comparison c…
Browse files Browse the repository at this point in the history
…ontrols (#132456) | [APM] Fix ML expected bounds missing data in chart, option showing when not neccessary (#132975) (#133826)

* [APM] Add ML expected model bounds as an option to Comparison controls (#132456)

* [ML] Add bounds options

* [ML] Renable anomalyChartTimeseries boundaries

* [ML] Make it into string

* [ML] Make it into string

* Match colors

* [ML] Add comparisonEnabledRt

* [ML] Fix types, tests

* [ML] Revert json bucket span change

* [ML] Add bounds options

* [ML] Renable anomalyChartTimeseries boundaries

* [ML] Make it into string

* [ML] Make it into string

* Match colors

* [ML] Add comparisonEnabledRt

* [ML] Fix types, tests

* [ML] Revert json bucket span change

* Refactor to use offset with TimeRangeComparisonEnum.ExpectedBounds

* Change comparisonColor to be part of anomalySeries

* Fix i18n

* Add Comparison text to replace 'Previous period'

* Fix expected bounds legend default to first

* Hide values that are N/A in the tooltips

* Fix i18n, color, and null values in tooltips

* Refactor to use preferredEnvironment

* Don't disable expected bounds by default

* Match color of expected bounds with time comparison color

* Fix tests

* Use bucket_span as minBucketSize for anomaly results

* Fix previousPeriodColor undefined for latency chart

* Remove 'Comparison:' in legend

* Change anomalyTimeseriesColor to use currentPeriod to match stuff

* Fix type error

* Fix lower model bounds

* Add comments

* Remove fit

* Remove comparisonEnabledRt

* Change text

Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit c1d0ec5)

* [APM] Fix ML expected bounds missing data in chart, option showing when not neccessary (#132975)

* Only show expected bounds option in Overview/Transactions subpage of Services

* Bucket span

* Fix bucket span didn't show up in chart

* Change fixed interval to 60s, add elastic-charts ref

* ui settings remembering

* Address comments

* Address comments

* Fix linting

* Move data point down instead of extending range

* Change to last, add test

* Fix to use xValuesExpectedBounds

* Revert change to to add extra point as es ml already handled it

Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit 017c8e1)

Co-authored-by: Quynh Nguyen <[email protected]>
Co-authored-by: Dario Gieselaar <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
4 people authored Jun 10, 2022
1 parent a6b3372 commit 2bf5891
Show file tree
Hide file tree
Showing 58 changed files with 621 additions and 254 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/apm/common/anomaly_detection/apm_ml_job.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,5 @@ export interface ApmMlJob {
jobState?: JOB_STATE;
datafeedId?: string;
datafeedState?: DATAFEED_STATE;
bucketSpan?: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -62,17 +62,14 @@ describe('getPreferredServiceAnomalyTimeseries', () => {
];

describe('with one environment', () => {
const environments = [PROD];

describe('and all being selected', () => {
const environment = ENVIRONMENT_ALL.value;
const preferredEnvironment = PROD;
it('returns the series for prod', () => {
expect(
getPreferredServiceAnomalyTimeseries({
allAnomalyTimeseries,
detectorType: ApmMlDetectorType.txLatency,
environment,
environments,
preferredEnvironment,
fallbackToTransactions: false,
})?.environment
).toBe(PROD);
Expand All @@ -81,18 +78,15 @@ describe('getPreferredServiceAnomalyTimeseries', () => {
});

describe('with multiple environments', () => {
const environments = [PROD, DEV];

describe('and all being selected', () => {
const environment = ENVIRONMENT_ALL.value;
const preferredEnvironment = ENVIRONMENT_ALL.value;

it('returns no series', () => {
expect(
getPreferredServiceAnomalyTimeseries({
allAnomalyTimeseries,
detectorType: ApmMlDetectorType.txLatency,
environment,
environments,
preferredEnvironment,
fallbackToTransactions: false,
})
).toBeUndefined();
Expand All @@ -101,23 +95,21 @@ describe('getPreferredServiceAnomalyTimeseries', () => {
getPreferredServiceAnomalyTimeseries({
allAnomalyTimeseries,
detectorType: ApmMlDetectorType.txLatency,
environment,
environments,
preferredEnvironment,
fallbackToTransactions: true,
})
).toBeUndefined();
});
});

describe('and production being selected', () => {
const environment = PROD;
const preferredEnvironment = PROD;

it('returns the series for production', () => {
const series = getPreferredServiceAnomalyTimeseries({
allAnomalyTimeseries,
detectorType: ApmMlDetectorType.txFailureRate,
environment,
environments,
preferredEnvironment,
fallbackToTransactions: false,
});

Expand All @@ -143,15 +135,13 @@ describe('getPreferredServiceAnomalyTimeseries', () => {
}),
];

const environments = [PROD];
const environment = ENVIRONMENT_ALL.value;
const preferredEnvironment = PROD;

it('selects the most recent version when transaction metrics are being used', () => {
const series = getPreferredServiceAnomalyTimeseries({
allAnomalyTimeseries,
detectorType: ApmMlDetectorType.txLatency,
environment,
environments,
preferredEnvironment,
fallbackToTransactions: false,
});

Expand All @@ -162,8 +152,7 @@ describe('getPreferredServiceAnomalyTimeseries', () => {
const series = getPreferredServiceAnomalyTimeseries({
allAnomalyTimeseries,
detectorType: ApmMlDetectorType.txLatency,
environment,
environments,
preferredEnvironment,
fallbackToTransactions: true,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,17 @@
* 2.0.
*/

import { ENVIRONMENT_ALL } from '../environment_filter_values';
import { Environment } from '../environment_rt';
import { ApmMlDetectorType } from './apm_ml_detectors';
import { ServiceAnomalyTimeseries } from './service_anomaly_timeseries';

export function getPreferredServiceAnomalyTimeseries({
environment,
environments,
preferredEnvironment,
detectorType,
allAnomalyTimeseries,
fallbackToTransactions,
}: {
environment: Environment;
environments: Environment[];
preferredEnvironment: Environment;
detectorType: ApmMlDetectorType;
allAnomalyTimeseries: ServiceAnomalyTimeseries[];
fallbackToTransactions: boolean;
Expand All @@ -27,11 +24,6 @@ export function getPreferredServiceAnomalyTimeseries({
(serie) => serie.type === detectorType
);

const preferredEnvironment =
environment === ENVIRONMENT_ALL.value && environments.length === 1
? environments[0]
: environment;

return seriesForType.find(
(serie) =>
serie.environment === preferredEnvironment &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,6 @@

import * as t from 'io-ts';

export const offsetRt = t.partial({ offset: t.string });
export const offsetRt = t.partial({
offset: t.string,
});
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ describe.skip('Service overview: Time Comparison', () => {
cy.get('[data-test-subj="throughput"]')
.get('#echHighlighterClipPath__throughput')
.realHover({ position: 'center' });
cy.contains('Previous period');
cy.contains('Week before');
cy.contains('0 tpm');

cy.contains('Throughput');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import { i18n } from '@kbn/i18n';
import React from 'react';
import { isTimeComparison } from '../../shared/time_comparison/get_comparison_options';
import { getNodeName, NodeType } from '../../../../common/connections';
import { useApmParams } from '../../../hooks/use_apm_params';
import { useFetcher } from '../../../hooks/use_fetcher';
Expand Down Expand Up @@ -43,7 +44,10 @@ export function BackendDetailDependenciesTable() {
end,
environment,
numBuckets: 20,
offset: comparisonEnabled ? offset : undefined,
offset:
comparisonEnabled && isTimeComparison(offset)
? offset
: undefined,
kuery,
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
*/
import React, { useMemo } from 'react';
import { i18n } from '@kbn/i18n';
import { usePreviousPeriodLabel } from '../../../hooks/use_previous_period_text';
import { isTimeComparison } from '../../shared/time_comparison/get_comparison_options';
import { asPercent } from '../../../../common/utils/formatters';
import { useFetcher } from '../../../hooks/use_fetcher';
import { useTimeRange } from '../../../hooks/use_time_range';
Expand Down Expand Up @@ -55,7 +57,10 @@ export function BackendFailedTransactionRateChart({
backendName,
start,
end,
offset: comparisonEnabled ? offset : undefined,
offset:
comparisonEnabled && isTimeComparison(offset)
? offset
: undefined,
kuery,
environment,
},
Expand All @@ -68,6 +73,8 @@ export function BackendFailedTransactionRateChart({
const { currentPeriodColor, previousPeriodColor } = getTimeSeriesColor(
ChartType.FAILED_TRANSACTION_RATE
);

const previousPeriodLabel = usePreviousPeriodLabel();
const timeseries = useMemo(() => {
const specs: Array<TimeSeries<Coordinate>> = [];

Expand All @@ -87,15 +94,12 @@ export function BackendFailedTransactionRateChart({
data: data.comparisonTimeseries,
type: 'area',
color: previousPeriodColor,
title: i18n.translate(
'xpack.apm.backendErrorRateChart.previousPeriodLabel',
{ defaultMessage: 'Previous period' }
),
title: previousPeriodLabel,
});
}

return specs;
}, [data, currentPeriodColor, previousPeriodColor]);
}, [data, currentPeriodColor, previousPeriodColor, previousPeriodLabel]);

return (
<TimeseriesChart
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
*/
import React, { useMemo } from 'react';
import { i18n } from '@kbn/i18n';
import { usePreviousPeriodLabel } from '../../../hooks/use_previous_period_text';
import { isTimeComparison } from '../../shared/time_comparison/get_comparison_options';
import { getDurationFormatter } from '../../../../common/utils/formatters';
import { useFetcher } from '../../../hooks/use_fetcher';
import { useTimeRange } from '../../../hooks/use_time_range';
Expand Down Expand Up @@ -51,7 +53,10 @@ export function BackendLatencyChart({ height }: { height: number }) {
backendName,
start,
end,
offset: comparisonEnabled ? offset : undefined,
offset:
comparisonEnabled && isTimeComparison(offset)
? offset
: undefined,
kuery,
environment,
},
Expand All @@ -65,6 +70,8 @@ export function BackendLatencyChart({ height }: { height: number }) {
ChartType.LATENCY_AVG
);

const previousPeriodLabel = usePreviousPeriodLabel();

const timeseries = useMemo(() => {
const specs: Array<TimeSeries<Coordinate>> = [];

Expand All @@ -84,15 +91,12 @@ export function BackendLatencyChart({ height }: { height: number }) {
data: data.comparisonTimeseries,
type: 'area',
color: previousPeriodColor,
title: i18n.translate(
'xpack.apm.backendLatencyChart.previousPeriodLabel',
{ defaultMessage: 'Previous period' }
),
title: previousPeriodLabel,
});
}

return specs;
}, [data, currentPeriodColor, previousPeriodColor]);
}, [data, currentPeriodColor, previousPeriodColor, previousPeriodLabel]);

const maxY = getMaxY(timeseries);
const latencyFormatter = getDurationFormatter(maxY);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
*/
import React, { useMemo } from 'react';
import { i18n } from '@kbn/i18n';
import { usePreviousPeriodLabel } from '../../../hooks/use_previous_period_text';
import { isTimeComparison } from '../../shared/time_comparison/get_comparison_options';
import { asTransactionRate } from '../../../../common/utils/formatters';
import { useFetcher } from '../../../hooks/use_fetcher';
import { useTimeRange } from '../../../hooks/use_time_range';
Expand Down Expand Up @@ -47,7 +49,10 @@ export function BackendThroughputChart({ height }: { height: number }) {
backendName,
start,
end,
offset: comparisonEnabled ? offset : undefined,
offset:
comparisonEnabled && isTimeComparison(offset)
? offset
: undefined,
kuery,
environment,
},
Expand All @@ -61,6 +66,8 @@ export function BackendThroughputChart({ height }: { height: number }) {
ChartType.THROUGHPUT
);

const previousPeriodLabel = usePreviousPeriodLabel();

const timeseries = useMemo(() => {
const specs: Array<TimeSeries<Coordinate>> = [];

Expand All @@ -80,15 +87,12 @@ export function BackendThroughputChart({ height }: { height: number }) {
data: data.comparisonTimeseries,
type: 'area',
color: previousPeriodColor,
title: i18n.translate(
'xpack.apm.backendThroughputChart.previousPeriodLabel',
{ defaultMessage: 'Previous period' }
),
title: previousPeriodLabel,
});
}

return specs;
}, [data, currentPeriodColor, previousPeriodColor]);
}, [data, currentPeriodColor, previousPeriodColor, previousPeriodLabel]);

return (
<TimeseriesChart
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { METRIC_TYPE } from '@kbn/analytics';
import { i18n } from '@kbn/i18n';
import React from 'react';
import { useUiTracker } from '@kbn/observability-plugin/public';
import { isTimeComparison } from '../../../shared/time_comparison/get_comparison_options';
import { getNodeName, NodeType } from '../../../../../common/connections';
import { useApmParams } from '../../../../hooks/use_apm_params';
import { useFetcher } from '../../../../hooks/use_fetcher';
Expand Down Expand Up @@ -45,7 +46,10 @@ export function BackendInventoryDependenciesTable() {
end,
environment,
numBuckets: 20,
offset: comparisonEnabled ? offset : undefined,
offset:
comparisonEnabled && isTimeComparison(offset)
? offset
: undefined,
kuery,
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
import { EuiTitle } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
import { usePreviousPeriodLabel } from '../../../../hooks/use_previous_period_text';
import { useApmPluginContext } from '../../../../context/apm_plugin/use_apm_plugin_context';
import { useLegacyUrlParams } from '../../../../context/url_params_context/use_url_params';
import { FETCH_STATUS } from '../../../../hooks/use_fetcher';
Expand All @@ -41,6 +42,7 @@ export function ErrorDistribution({ distribution, title, fetchStatus }: Props) {
const { urlParams } = useLegacyUrlParams();
const { comparisonEnabled } = urlParams;

const previousPeriodLabel = usePreviousPeriodLabel();
const timeseries = [
{
data: distribution.currentPeriod,
Expand All @@ -54,10 +56,7 @@ export function ErrorDistribution({ distribution, title, fetchStatus }: Props) {
{
data: distribution.previousPeriod,
color: theme.eui.euiColorMediumShade,
title: i18n.translate(
'xpack.apm.errorGroup.chart.ocurrences.previousPeriodLabel',
{ defaultMessage: 'Previous period' }
),
title: previousPeriodLabel,
},
]
: []),
Expand Down
Loading

0 comments on commit 2bf5891

Please sign in to comment.