Skip to content

Commit

Permalink
[Synthetics] Overview - standardize queries for monitor duration metr…
Browse files Browse the repository at this point in the history
…ic item (#145916)

## Summary

Resolves #145922
Resolves #145270

Fixes a regression from #143309

Standardizes the way the monitor duration metric is queried, ensuring
that both monitor types, UI and Project, are queries in the same way.

The changes were also carried into changes to the Monitor Flyout

Before (duration for UI monitor shows, but not the duration for project
monitors)

![image](https://user-images.githubusercontent.com/11356435/203143905-4564b14e-a2af-402c-a2c1-5b95d22ad1e3.png)

After (duration shows for both monitor types)
<img width="1366" alt="Screen Shot 2022-11-21 at 2 39 09 PM"
src="https://user-images.githubusercontent.com/11356435/203144056-d74617e5-6745-4c17-9a68-e5164f4be689.png">

### Testing
1. Create at least one project monitor and one UI monitor
2. Ensure that both monitor types display the duration metric on the
Overview page after running
3. In the actions popover, click edit monitor. Ensure the page redirects
to the monitor edit page appropriately
4. In the actions popover, click go to monitor. Ensure that the page
redirects to the monitor details page appropriately
5. In the actions popover, disable the monitor. Ensure it is successful.
6. In the actions popover, click inspect. Ensure the monitor flyout
appears, then close the flyout.
7. Click on both monitor types, ensure the monitor flyout appears
8. Ensure the monitor duration metric appears in the monitor flyout
9. Ensure the last test run appears in the monitor flyout
10. Ensure both enable toggles work in the monitor flyout (in the body
of the flyout and actions popover next to the flyout title)
11. Ensure the go to monitor link works in the monitor flyout actions
popover next to the monitor title
12. Ensure the edit monitor link works in the monitor flyout actions
popover next to the monitor title

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
dominiqueclarke and kibanamachine authored Nov 22, 2022
1 parent cd8dc11 commit e8d77b3
Show file tree
Hide file tree
Showing 25 changed files with 221 additions and 75 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ describe('checking migration metadata changes on all registered SO types', () =>
"siem-ui-timeline-pinned-event": "e2697b38751506c7fce6e8b7207a830483dc4283",
"space": "c4a0acce1bd4b9cce85154f2a350624a53111c59",
"spaces-usage-stats": "922d3235bbf519e3fb3b260e27248b1df8249b79",
"synthetics-monitor": "111811218f7e34f40980665a4eb99976f457bb23",
"synthetics-monitor": "d784b64a3def47d3f3d1f367df71ae41ef33cb3c",
"synthetics-privates-locations": "dd00385f4a27ef062c3e57312eeb3799872fa4af",
"tag": "39413f4578cc2128c9a0fda97d0acd1c8862c47a",
"task": "ef53d0f070bd54957b8fe22fae3b1ff208913f76",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ export type MonitorManagementListResult = t.TypeOf<typeof MonitorManagementListR
export const MonitorOverviewItemCodec = t.interface({
name: t.string,
id: t.string,
configId: t.string,
location: MonitorServiceLocationCodec,
isEnabled: t.boolean,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ journey('Overview Sorting', async ({ page, params }) => {
await page.waitForSelector(`[data-test-subj="syntheticsOverviewGridItem"]`);
await page.click('[data-test-subj="syntheticsOverviewSortButton"]');
await page.click('button:has-text("Z -> A")');
await page.waitForSelector('text=Loading');
await page.waitForSelector(`text=${testMonitor1}`);
await page.waitForSelector(`text=${testMonitor2}`);
await page.waitForSelector(`text=${testMonitor3}`);
Expand All @@ -85,7 +84,6 @@ journey('Overview Sorting', async ({ page, params }) => {
await page.waitForSelector(`[data-test-subj="syntheticsOverviewGridItem"]`);
await page.click('[data-test-subj="syntheticsOverviewSortButton"]');
await page.click('button:has-text("Last modified")');
await page.waitForSelector('text=Loading');
await page.waitForSelector(`text=${testMonitor1}`);
await page.waitForSelector(`text=${testMonitor2}`);
await page.waitForSelector(`text=${testMonitor3}`);
Expand All @@ -99,14 +97,12 @@ journey('Overview Sorting', async ({ page, params }) => {
expect(await correctFirstMonitor.count()).toBe(1);
expect(await correctSecondMonitor.count()).toBe(1);
expect(await correctThirdMonitor.count()).toBe(1);
await page.waitForTimeout(30000);
});

step('sort last updated desc', async () => {
await page.waitForSelector(`[data-test-subj="syntheticsOverviewGridItem"]`);
await page.click('[data-test-subj="syntheticsOverviewSortButton"]');
await page.click('button:has-text("Oldest first")');
await page.waitForSelector('text=Loading');
await page.waitForSelector(`text=${testMonitor1}`);
await page.waitForSelector(`text=${testMonitor2}`);
await page.waitForSelector(`text=${testMonitor3}`);
Expand All @@ -120,7 +116,6 @@ journey('Overview Sorting', async ({ page, params }) => {
expect(await correctFirstMonitor.count()).toBe(1);
expect(await correctSecondMonitor.count()).toBe(1);
expect(await correctThirdMonitor.count()).toBe(1);
await page.waitForTimeout(30000);
});

step('delete monitors', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@

import { syntheticsEditMonitorLocatorID } from '@kbn/observability-plugin/common';

async function navigate({ monitorId }: { monitorId: string }) {
async function navigate({ configId }: { configId: string }) {
return {
app: 'synthetics',
path: `/edit-monitor/${monitorId}`,
path: `/edit-monitor/${configId}`,
state: {},
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@

import { syntheticsMonitorDetailLocatorID } from '@kbn/observability-plugin/common';

async function navigate({ monitorId, locationId }: { monitorId: string; locationId?: string }) {
async function navigate({ configId, locationId }: { configId: string; locationId?: string }) {
const locationUrlQueryParam = locationId ? `?locationId=${locationId}` : '';
return {
app: 'synthetics',
path: `/monitor/${monitorId}${locationUrlQueryParam}`,
path: `/monitor/${configId}${locationUrlQueryParam}`,
state: {},
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ export const MonitorDetailsPanel = () => {
const { euiTheme } = useEuiTheme();
const { latestPing } = useMonitorLatestPing();

const { monitorId } = useParams<{ monitorId: string }>();
const { monitorId: configId } = useParams<{ monitorId: string }>();

const dispatch = useDispatch();

const { monitor, loading } = useSelectedMonitor();

if (
(latestPing && latestPing?.config_id !== monitorId) ||
(monitor && monitor[ConfigKey.CONFIG_ID] !== monitorId)
(latestPing && latestPing?.config_id !== configId) ||
(monitor && monitor[ConfigKey.CONFIG_ID] !== configId)
) {
return <EuiLoadingContent lines={6} />;
}
Expand All @@ -69,10 +69,10 @@ export const MonitorDetailsPanel = () => {
{monitor && (
<MonitorEnabled
initialLoading={loading}
id={monitorId}
configId={configId}
monitor={monitor}
reloadPage={() => {
dispatch(getMonitorAction.get({ monitorId }));
dispatch(getMonitorAction.get({ monitorId: configId }));
}}
/>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { useEffect, useState } from 'react';
import { syntheticsEditMonitorLocatorID } from '@kbn/observability-plugin/common';
import { useSyntheticsStartPlugins } from '../../../contexts';

export function useEditMonitorLocator({ monitorId }: { monitorId: string }) {
export function useEditMonitorLocator({ configId }: { configId: string }) {
const [editUrl, setEditUrl] = useState<string | undefined>(undefined);
const locator = useSyntheticsStartPlugins()?.share?.url.locators.get(
syntheticsEditMonitorLocatorID
Expand All @@ -19,12 +19,12 @@ export function useEditMonitorLocator({ monitorId }: { monitorId: string }) {
useEffect(() => {
async function generateUrl() {
const url = await locator?.getUrl({
monitorId,
configId,
});
setEditUrl(url);
}
generateUrl();
}, [locator, monitorId]);
}, [locator, configId]);

return editUrl;
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import { syntheticsMonitorDetailLocatorID } from '@kbn/observability-plugin/comm
import { useSyntheticsStartPlugins } from '../../../contexts';

export function useMonitorDetailLocator({
monitorId,
configId,
locationId,
}: {
monitorId: string;
configId: string;
locationId?: string;
}) {
const [monitorUrl, setMonitorUrl] = useState<string | undefined>(undefined);
Expand All @@ -24,13 +24,13 @@ export function useMonitorDetailLocator({
useEffect(() => {
async function generateUrl() {
const url = await locator?.getUrl({
monitorId,
configId,
locationId,
});
setMonitorUrl(url);
}
generateUrl();
}, [locator, monitorId, locationId]);
}, [locator, configId, locationId]);

return monitorUrl;
}
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ export function getMonitorListColumns({
}),
render: (_enabled: boolean, monitor: EncryptedSyntheticsSavedMonitor) => (
<MonitorEnabled
id={monitor.id}
configId={monitor[ConfigKey.CONFIG_ID]}
monitor={monitor}
reloadPage={reloadPage}
isSwitchable={!loading}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const MonitorDetailsLink = ({
lastSelectedLocationId && monitorHasLocation ? lastSelectedLocationId : firstMonitorLocationId;

const monitorDetailLinkUrl = useMonitorDetailLocator({
monitorId: monitor[ConfigKey.CONFIG_ID],
configId: monitor[ConfigKey.CONFIG_ID],
locationId,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ import * as labels from './labels';
import { useMonitorEnableHandler } from '../../../../hooks/use_monitor_enable_handler';

interface Props {
id: string;
configId: string;
monitor: EncryptedSyntheticsMonitor;
reloadPage: () => void;
initialLoading?: boolean;
isSwitchable?: boolean;
}

export const MonitorEnabled = ({
id,
configId,
monitor,
reloadPage,
initialLoading = false,
Expand All @@ -41,7 +41,7 @@ export const MonitorEnabled = ({
}, [monitorName]);

const { isEnabled, updateMonitorEnabledState, status } = useMonitorEnableHandler({
id,
configId,
isEnabled: monitor[ConfigKey.ENABLED],
reloadPage,
labels: statusLabels,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe('ActionsPopover', () => {
isEnabled: true,
name: 'Monitor 1',
id: 'somelongstring',
configId: '1lkjelre',
};
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ export function ActionsPopover({
const locationName = useLocationName({ locationId: monitor.location.id });

const detailUrl = useMonitorDetailLocator({
monitorId: monitor.id,
configId: monitor.configId,
locationId: monitor.location.id,
});
const editUrl = useEditMonitorLocator({ monitorId: monitor.id });
const editUrl = useEditMonitorLocator({ configId: monitor.configId });

const labels = useMemo(
() => ({
Expand All @@ -101,7 +101,7 @@ export function ActionsPopover({
[monitor.name]
);
const { status, isEnabled, updateMonitorEnabledState } = useMonitorEnableHandler({
id: monitor.id,
configId: monitor.configId,
isEnabled: monitor.isEnabled,
labels,
});
Expand All @@ -125,7 +125,9 @@ export function ActionsPopover({
disabled: !locationName,
onClick: () => {
if (locationName) {
dispatch(setFlyoutConfig({ monitorId: monitor.id, location: locationName }));
dispatch(
setFlyoutConfig({ configId: monitor.configId, location: locationName, id: monitor.id })
);
setIsPopoverOpen(false);
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ export const MetricItem = ({
data: Array<{ x: number; y: number }>;
averageDuration: number;
loaded: boolean;
onClick: (id: string, location: string) => void;
onClick: (params: { id: string; configId: string; location: string }) => void;
}) => {
const [isMouseOver, setIsMouseOver] = useState(false);
const [isPopoverOpen, setIsPopoverOpen] = useState(false);
const locationName = useLocationName({ locationId: monitor.location?.id });
const { locations } = useStatusByLocation(monitor.id);
const { locations } = useStatusByLocation(monitor.configId);
const ping = locations.find((loc) => loc.observer?.geo?.name === locationName);
const theme = useTheme();

Expand All @@ -67,11 +67,15 @@ export const MetricItem = ({
>
<Chart>
<Settings
onElementClick={() => monitor.id && locationName && onClick(monitor.id, locationName)}
onElementClick={() =>
monitor.configId &&
locationName &&
onClick({ configId: monitor.configId, id: monitor.id, location: locationName })
}
baseTheme={DARK_THEME}
/>
<Metric
id={`${monitor.id}-${monitor.location?.id}`}
id={`${monitor.configId}-${monitor.location?.id}`}
data={[
[
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ describe('Monitor Detail Flyout', () => {
const onCloseMock = jest.fn();
const { getByLabelText } = render(
<MonitorDetailFlyout
configId="test-id"
id="test-id"
location="US East"
onClose={onCloseMock}
Expand All @@ -72,6 +73,7 @@ describe('Monitor Detail Flyout', () => {

const { getByText } = render(
<MonitorDetailFlyout
configId="test-id"
id="test-id"
location="US East"
onClose={jest.fn()}
Expand All @@ -90,6 +92,7 @@ describe('Monitor Detail Flyout', () => {

const { getByRole } = render(
<MonitorDetailFlyout
configId="test-id"
id="test-id"
location="US East"
onClose={jest.fn()}
Expand Down Expand Up @@ -124,6 +127,7 @@ describe('Monitor Detail Flyout', () => {

const { getByRole, getByText, getAllByRole } = render(
<MonitorDetailFlyout
configId="test-id"
id="test-id"
location="US East"
onClose={jest.fn()}
Expand Down
Loading

0 comments on commit e8d77b3

Please sign in to comment.