Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.7] [APM] Fix comparison option showing in individual transaction page (#155915) #156613

Merged
merged 1 commit into from
May 3, 2023
Merged

[8.7] [APM] Fix comparison option showing in individual transaction page (#155915) #156613

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions x-pack/plugins/apm/public/components/shared/time_comparison/get_comparison_options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function formatDate({
)} - ${previousPeriodEnd.format(dateFormat)}`;
}

function isDefined<T>(argument: T | undefined | null): argument is T {
export function isDefined<T>(argument: T | undefined | null): argument is T {
return argument !== undefined && argument !== null;
}

Expand Down Expand Up @@ -143,11 +143,9 @@ export function getComparisonOptions({
comparisonTypes = [TimeRangeComparisonEnum.PeriodBefore];
}

const hasMLJobsMatchingEnv =
Array.isArray(anomalyDetectionJobsData?.jobs) &&
anomalyDetectionJobsData?.jobs.some(
(j) => j.environment === preferredEnvironment
);
const hasMLJob =
isDefined(anomalyDetectionJobsData) &&
anomalyDetectionJobsData.jobs.length > 0;

const comparisonOptions = getSelectOptions({
comparisonTypes,
Expand All @@ -156,9 +154,12 @@ export function getComparisonOptions({
msDiff,
});

if (showSelectedBoundsOption) {
if (showSelectedBoundsOption && hasMLJob) {
const disabled =
anomalyDetectionJobsStatus === 'success' && !hasMLJobsMatchingEnv;
anomalyDetectionJobsStatus === 'success' &&
!anomalyDetectionJobsData.jobs.some(
(j) => j.environment === preferredEnvironment
);
comparisonOptions.push({
value: TimeRangeComparisonEnum.ExpectedBounds,
text: disabled
Expand Down
239 changes: 236 additions & 3 deletions x-pack/plugins/apm/public/components/shared/time_comparison/index.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,30 +16,71 @@ import {
import { TimeComparison } from '.';
import * as urlHelpers from '../links/url_helpers';
import moment from 'moment';
import { MockApmPluginContextWrapper } from '../../../context/apm_plugin/mock_apm_plugin_context';
import {
mockApmPluginContextValue,
MockApmPluginContextWrapper,
} from '../../../context/apm_plugin/mock_apm_plugin_context';
import { ENVIRONMENT_ALL } from '../../../../common/environment_filter_values';
import * as useAnomalyDetectionJobsContextModule from '../../../context/anomaly_detection_jobs/use_anomaly_detection_jobs_context';
import * as useEnvironmentContextModule from '../../../context/environments_context/use_environments_context';
import type { ApmPluginContextValue } from '../../../context/apm_plugin/apm_plugin_context';
import { merge } from 'lodash';
import type { ApmMlJob } from '../../../../common/anomaly_detection/apm_ml_job';
import { FETCH_STATUS } from '../../../hooks/use_fetcher';

const ML_AD_JOBS = {
jobs: [
{
jobId: 'apm-prod-9f5f-apm_tx_metrics',
jobState: 'opened',
datafeedId: 'datafeed-apm-prod-9f5f-apm_tx_metrics',
datafeedState: 'started',
version: 3,
environment: 'prod',
bucketSpan: '15m',
},
{
jobId: 'apm-staging-4fec-apm_tx_metrics',
jobState: 'opened',
datafeedId: 'datafeed-apm-staging-4fec-apm_tx_metrics',
datafeedState: 'started',
version: 3,
environment: 'staging',
bucketSpan: '15m',
},
] as ApmMlJob[],
hasLegacyJobs: false,
};

const NO_ML_AD_JOBS = { jobs: [] as ApmMlJob[], hasLegacyJobs: false };

function getWrapper({
rangeFrom,
rangeTo,
offset,
comparisonEnabled,
environment = ENVIRONMENT_ALL.value,
url = '/services',
mockPluginContext = undefined,
params = '',
}: {
rangeFrom: string;
rangeTo: string;
offset?: string;
comparisonEnabled?: boolean;
environment?: string;
url?: string;
params?: string;
mockPluginContext?: ApmPluginContextValue;
}) {
return ({ children }: { children?: ReactNode }) => {
return (
<MemoryRouter
initialEntries={[
`/services?rangeFrom=${rangeFrom}&rangeTo=${rangeTo}&environment=${environment}&offset=${offset}&comparisonEnabled=${comparisonEnabled}`,
`${url}?rangeFrom=${rangeFrom}&rangeTo=${rangeTo}&environment=${environment}&offset=${offset}&comparisonEnabled=${comparisonEnabled}${params}`,
]}
>
<MockApmPluginContextWrapper>
<MockApmPluginContextWrapper value={mockPluginContext}>
<EuiThemeProvider>{children}</EuiThemeProvider>
</MockApmPluginContextWrapper>
</MemoryRouter>
Expand All @@ -48,6 +89,27 @@ function getWrapper({
}

describe('TimeComparison component', () => {
const mockMLJobs = () => {
jest
.spyOn(
useAnomalyDetectionJobsContextModule,
'useAnomalyDetectionJobsContext'
)
.mockReturnValue(
// @ts-ignore mocking only partial data
{
anomalyDetectionJobsStatus: FETCH_STATUS.SUCCESS,
anomalyDetectionJobsData: ML_AD_JOBS,
}
);

jest
.spyOn(useEnvironmentContextModule, 'useEnvironmentsContext')
.mockReturnValue({
// @ts-ignore mocking only partial data
preferredEnvironment: 'prod',
});
};
beforeAll(() => {
moment.tz.setDefault('Europe/Amsterdam');
});
Expand All @@ -56,8 +118,179 @@ describe('TimeComparison component', () => {
const spy = jest.spyOn(urlHelpers, 'replace');
beforeEach(() => {
jest.resetAllMocks();
mockMLJobs();
});

describe('ML expected model bounds', () => {
const pluginContextCanGetMlJobs = merge({}, mockApmPluginContextValue, {
core: {
application: { capabilities: { ml: { canGetJobs: true } } },
},
}) as unknown as ApmPluginContextValue;

it('shows disabled option for expected bounds when there are ML jobs available with sufficient permission', () => {
jest
.spyOn(useEnvironmentContextModule, 'useEnvironmentsContext')
.mockReturnValueOnce(
// @ts-ignore mocking only partial data
{
preferredEnvironment: ENVIRONMENT_ALL.value,
}
);

const Wrapper = getWrapper({
url: '/services/frontend/transactions',
rangeFrom: '2020-05-27T16:32:46.747Z',
rangeTo: '2021-06-04T16:32:46.747Z',
comparisonEnabled: true,
offset: '32227200000ms',
mockPluginContext: pluginContextCanGetMlJobs,
});
const component = render(<TimeComparison />, {
wrapper: Wrapper,
});
expect(spy).not.toHaveBeenCalled();
expectTextsInDocument(component, [
'20/05/19 18:32 - 27/05/20 18:32',
'Expected bounds (Anomaly detection must be enabled for env)',
]);
expect(
(component.getByTestId('comparisonSelect') as HTMLSelectElement)
.selectedIndex
).toEqual(0);
});

it('shows enabled option for expected bounds when there are ML jobs available matching the preferred environment', () => {
jest
.spyOn(useEnvironmentContextModule, 'useEnvironmentsContext')
.mockReturnValueOnce({
// @ts-ignore mocking only partial data
preferredEnvironment: 'prod',
});

const Wrapper = getWrapper({
url: '/services/frontend/overview',
rangeFrom: '2020-05-27T16:32:46.747Z',
rangeTo: '2021-06-04T16:32:46.747Z',
comparisonEnabled: true,
offset: '32227200000ms',
mockPluginContext: pluginContextCanGetMlJobs,
environment: 'prod',
});
const component = render(<TimeComparison />, {
wrapper: Wrapper,
});
expect(spy).not.toHaveBeenCalled();
expectTextsInDocument(component, [
'20/05/19 18:32 - 27/05/20 18:32',
'Expected bounds',
]);
expect(
(component.getByTestId('comparisonSelect') as HTMLSelectElement)
.selectedIndex
).toEqual(0);
});

it('does not show option for expected bounds when there are no ML jobs available', () => {
jest
.spyOn(
useAnomalyDetectionJobsContextModule,
'useAnomalyDetectionJobsContext'
)
.mockReturnValue(
// @ts-ignore mocking only partial data
{
anomalyDetectionJobsStatus: FETCH_STATUS.SUCCESS,
anomalyDetectionJobsData: NO_ML_AD_JOBS,
}
);

const Wrapper = getWrapper({
url: '/services/frontend/transactions',
rangeFrom: '2020-05-27T16:32:46.747Z',
rangeTo: '2021-06-04T16:32:46.747Z',
comparisonEnabled: true,
offset: '32227200000ms',
mockPluginContext: pluginContextCanGetMlJobs,
});
const component = render(<TimeComparison />, {
wrapper: Wrapper,
});
expect(spy).not.toHaveBeenCalled();
expectTextsNotInDocument(component, [
'Expected bounds',
'Expected bounds (Anomaly detection must be enabled for env)',
]);
expectTextsInDocument(component, ['20/05/19 18:32 - 27/05/20 18:32']);
expect(
(component.getByTestId('comparisonSelect') as HTMLSelectElement)
.selectedIndex
).toEqual(0);
});

it('does not show option for expected bounds for pages other than overall transactions and overview', () => {
const urlsWithoutExpectedBoundsOptions = [
'/services/frontend/dependencies',
'/services/frontend/transactions/view',
];

urlsWithoutExpectedBoundsOptions.forEach((url) => {
const Wrapper = getWrapper({
url,
rangeFrom: '2020-05-27T16:32:46.747Z',
rangeTo: '2021-06-04T16:32:46.747Z',
comparisonEnabled: true,
offset: '32227200000ms',
mockPluginContext: pluginContextCanGetMlJobs,
params: '&transactionName=createOrder&transactionType=request',
});
const component = render(<TimeComparison />, {
wrapper: Wrapper,
});
expect(spy).not.toHaveBeenCalled();
expectTextsNotInDocument(component, [
'Expected bounds',
'Expected bounds (Anomaly detection must be enabled for env)',
]);
});
});

it('does not show option for expected bounds if user does not have access to ML jobs', () => {
jest
.spyOn(useEnvironmentContextModule, 'useEnvironmentsContext')
.mockReturnValueOnce(
// @ts-ignore mocking only partial data
{
preferredEnvironment: ENVIRONMENT_ALL.value,
}
);

const Wrapper = getWrapper({
url: '/services/frontend/transactions',
rangeFrom: '2020-05-27T16:32:46.747Z',
rangeTo: '2021-06-04T16:32:46.747Z',
comparisonEnabled: true,
offset: '32227200000ms',
mockPluginContext: merge({}, mockApmPluginContextValue, {
core: {
application: { capabilities: { ml: { canGetJobs: false } } },
},
}) as unknown as ApmPluginContextValue,
});
const component = render(<TimeComparison />, {
wrapper: Wrapper,
});
expect(spy).not.toHaveBeenCalled();
expectTextsNotInDocument(component, [
'Expected bounds',
'Expected bounds (Anomaly detection must be enabled for env)',
]);
expect(
(component.getByTestId('comparisonSelect') as HTMLSelectElement)
.selectedIndex
).toEqual(0);
});
});
describe('Time range is between 0 - 25 hours', () => {
it('sets default values', () => {
const Wrapper = getWrapper({
Expand Down
14 changes: 9 additions & 5 deletions x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,15 @@ export function TimeComparison() {
const comparisonOptions = useMemo(() => {
const matchingRoutes = apmRouter.getRoutesToMatch(location.pathname);
// Only show the "Expected bounds" option in Overview and Transactions tabs
const showExpectedBoundsForThisTab = matchingRoutes.some(
(d) =>
d.path === '/services/{serviceName}/overview' ||
d.path === '/services/{serviceName}/transactions'
);
const showExpectedBoundsForThisTab =
!matchingRoutes.some(
(d) => d.path === '/services/{serviceName}/transactions/view'
) &&
matchingRoutes.some(
(d) =>
d.path === '/services/{serviceName}/overview' ||
d.path === '/services/{serviceName}/transactions'
);

const timeComparisonOptions = getComparisonOptions({
start,
Expand Down