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.

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

[Uptime] Annotate waterfall chart with additional metrics #103642

Merged
merged 30 commits into from
Oct 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
2468721
annotate waterfall chart
shahzad31 Jun 29, 2021
ef718ec
fix types
shahzad31 Jun 29, 2021
3127bb3
Merge branch 'master' into waterfall-annotations
shahzad31 Jun 29, 2021
3f7e7e2
add translations
shahzad31 Jun 29, 2021
f9d9cac
check if nav start has value
shahzad31 Jun 29, 2021
72a644d
improve logic
shahzad31 Jun 29, 2021
8768d00
Merge branch 'master' into waterfall-annotations
shahzad31 Jul 1, 2021
5d89ca0
add test
shahzad31 Jul 1, 2021
1d8290d
Merge branch 'master' into waterfall-annotations
shahzad31 Jul 7, 2021
5248cd3
Merge branch 'master' into waterfall-annotations
shahzad31 Jul 13, 2021
6bf295d
Merge branch 'master' into waterfall-annotations
shahzad31 Jul 14, 2021
abc2c4f
display metrics only when navigation true
shahzad31 Jul 15, 2021
ba51295
fix type
shahzad31 Jul 15, 2021
bd6bb5e
update unit test
shahzad31 Jul 15, 2021
eba7398
Merge branch 'master' into waterfall-annotations
shahzad31 Jul 15, 2021
e349d42
fix test
shahzad31 Jul 15, 2021
ed8b98b
Merge branch 'master' into waterfall-annotations
kibanamachine Jul 19, 2021
1fedde6
Merge branch 'master' into waterfall-annotations
kibanamachine Jul 28, 2021
2e30d48
Merge branch 'master' of github.com:elastic/kibana into waterfall-ann…
shahzad31 Sep 28, 2021
8c3de17
fix type
shahzad31 Sep 28, 2021
3cf399d
Merge branch 'master' of github.com:elastic/kibana into waterfall-ann…
shahzad31 Sep 28, 2021
cc3f0cb
update to step/metrics
shahzad31 Sep 28, 2021
db60bd4
Fix waterfall chart top axis
shahzad31 Sep 28, 2021
a1b00db
Merge branch 'fix-waterfall-chart-axis' into waterfall-annotations
shahzad31 Sep 28, 2021
0b91c4e
update test
shahzad31 Sep 28, 2021
a8f8d5b
Merge branch 'master' of github.com:elastic/kibana into waterfall-ann…
shahzad31 Sep 30, 2021
3021ffe
PR feedback
shahzad31 Sep 30, 2021
334f015
Merge branch 'master' of github.com:elastic/kibana into waterfall-ann…
shahzad31 Oct 1, 2021
9a272ea
Merge branch 'master' of github.com:elastic/kibana into waterfall-ann…
shahzad31 Oct 4, 2021
019185c
update test
shahzad31 Oct 4, 2021
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
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export const SyntheticsNetworkEventsApiResponseType = t.type({
events: t.array(NetworkEventType),
total: t.number,
isWaterfallSupported: t.boolean,
hasNavigationRequest: t.boolean,
});

export type SyntheticsNetworkEventsApiResponse = t.TypeOf<
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { renderHook } from '@testing-library/react-hooks';
import {
BROWSER_TRACE_NAME,
BROWSER_TRACE_START,
BROWSER_TRACE_TYPE,
useStepWaterfallMetrics,
} from './use_step_waterfall_metrics';
import * as reduxHooks from 'react-redux';
import * as searchHooks from '../../../../../../observability/public/hooks/use_es_search';

describe('useStepWaterfallMetrics', () => {
jest
.spyOn(reduxHooks, 'useSelector')
.mockReturnValue({ settings: { heartbeatIndices: 'heartbeat-*' } });

it('returns result as expected', () => {
// @ts-ignore
const searchHook = jest.spyOn(searchHooks, 'useEsSearch').mockReturnValue({
loading: false,
data: {
hits: {
total: { value: 2, relation: 'eq' },
hits: [
{
fields: {
[BROWSER_TRACE_TYPE]: ['mark'],
[BROWSER_TRACE_NAME]: ['navigationStart'],
[BROWSER_TRACE_START]: [3456789],
},
},
{
fields: {
[BROWSER_TRACE_TYPE]: ['mark'],
[BROWSER_TRACE_NAME]: ['domContentLoaded'],
[BROWSER_TRACE_START]: [4456789],
},
},
],
},
} as any,
});

const { result } = renderHook(
(props) =>
useStepWaterfallMetrics({
checkGroup: '44D-444FFF-444-FFF-3333',
hasNavigationRequest: true,
stepIndex: 1,
}),
{}
);

expect(searchHook).toHaveBeenCalledWith(
{
body: {
_source: false,
fields: ['browser.*'],
query: {
bool: {
filter: [
{
term: {
'synthetics.step.index': 1,
},
},
{
term: {
'monitor.check_group': '44D-444FFF-444-FFF-3333',
},
},
{
term: {
'synthetics.type': 'step/metrics',
},
},
],
},
},
size: 1000,
},
index: 'heartbeat-*',
},
['heartbeat-*', '44D-444FFF-444-FFF-3333', true]
);
expect(result.current).toEqual({
loading: false,
metrics: [
{
id: 'domContentLoaded',
offset: 1000,
},
],
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

import { useSelector } from 'react-redux';
import { createEsParams, useEsSearch } from '../../../../../../observability/public';
import { selectDynamicSettings } from '../../../../state/selectors';
import { MarkerItems } from '../waterfall/context/waterfall_chart';

export interface Props {
checkGroup: string;
stepIndex: number;
hasNavigationRequest?: boolean;
}
export const BROWSER_TRACE_TYPE = 'browser.relative_trace.type';
export const BROWSER_TRACE_NAME = 'browser.relative_trace.name';
export const BROWSER_TRACE_START = 'browser.relative_trace.start.us';
export const NAVIGATION_START = 'navigationStart';

export const useStepWaterfallMetrics = ({ checkGroup, hasNavigationRequest, stepIndex }: Props) => {
const { settings } = useSelector(selectDynamicSettings);

const heartbeatIndices = settings?.heartbeatIndices || '';

const { data, loading } = useEsSearch(
hasNavigationRequest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this whole hook depends on hasNavigation request, can be exit early if this is false and return an empty metrics array and loading false?

? createEsParams({
index: heartbeatIndices!,
body: {
query: {
bool: {
filter: [
{
term: {
'synthetics.step.index': stepIndex,
},
},
{
term: {
'monitor.check_group': checkGroup,
},
},
{
term: {
'synthetics.type': 'step/metrics',
},
},
],
},
},
fields: ['browser.*'],
size: 1000,
_source: false,
},
})
: {},
[heartbeatIndices, checkGroup, hasNavigationRequest]
);

if (!hasNavigationRequest) {
return { metrics: [], loading: false };
}

const metrics: MarkerItems = [];

if (data && hasNavigationRequest) {
const metricDocs = data.hits.hits as unknown as Array<{ fields: any }>;
let navigationStart = 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, This is prone to errors and wold result in absurd values when navigationStart is not present in the doc.

I would change this logic to this.

  • Find the navigationStart from the metricDocs and if not present don't calculate the flyout metrics.
  • If present, calculate the offset metrics as already done.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improved it to make sure check for existence explicitly.

let navigationStartExist = false;

metricDocs.forEach(({ fields }) => {
if (fields[BROWSER_TRACE_TYPE]?.[0] === 'mark') {
const { [BROWSER_TRACE_NAME]: metricType, [BROWSER_TRACE_START]: metricValue } = fields;
if (metricType?.[0] === NAVIGATION_START) {
navigationStart = metricValue?.[0];
navigationStartExist = true;
}
}
});

if (navigationStartExist) {
metricDocs.forEach(({ fields }) => {
if (fields[BROWSER_TRACE_TYPE]?.[0] === 'mark') {
const { [BROWSER_TRACE_NAME]: metricType, [BROWSER_TRACE_START]: metricValue } = fields;
if (metricType?.[0] !== NAVIGATION_START) {
metrics.push({
id: metricType?.[0],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can skip doing the undefined check here?

offset: (metricValue?.[0] - navigationStart) / 1000,
});
}
}
});
}
}

return { metrics, loading };
};
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { getNetworkEvents } from '../../../../../state/actions/network_events';
import { networkEventsSelector } from '../../../../../state/selectors';
import { WaterfallChartWrapper } from './waterfall_chart_wrapper';
import { extractItems } from './data_formatting';
import { useStepWaterfallMetrics } from '../use_step_waterfall_metrics';

export const NO_DATA_TEXT = i18n.translate('xpack.uptime.synthetics.stepDetail.waterfallNoData', {
defaultMessage: 'No waterfall data could be found for this step',
Expand Down Expand Up @@ -44,6 +45,12 @@ export const WaterfallChartContainer: React.FC<Props> = ({ checkGroup, stepIndex
const isWaterfallSupported = networkEvents?.isWaterfallSupported;
const hasEvents = networkEvents?.events?.length > 0;

const { metrics } = useStepWaterfallMetrics({
checkGroup,
stepIndex,
hasNavigationRequest: networkEvents?.hasNavigationRequest,
});

return (
<>
{!waterfallLoaded && (
Expand All @@ -70,6 +77,7 @@ export const WaterfallChartContainer: React.FC<Props> = ({ checkGroup, stepIndex
{waterfallLoaded && hasEvents && isWaterfallSupported && (
<WaterfallChartWrapper
data={extractItems(networkEvents.events)}
markerItems={metrics}
total={networkEvents.total}
/>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ describe('WaterfallChartWrapper', () => {

it('renders the correct sidebar items', () => {
const { getAllByTestId } = render(
<WaterfallChartWrapper data={extractItems(NETWORK_EVENTS.events)} total={1000} />
<WaterfallChartWrapper
data={extractItems(NETWORK_EVENTS.events)}
total={1000}
markerItems={[{ id: 'domContentLoaded', offset: 2352353 }]}
/>
);

const sideBarItems = getAllByTestId('middleTruncatedTextSROnly');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { useTrackMetric, METRIC_TYPE } from '../../../../../../../observability/
import { WaterfallFilter } from './waterfall_filter';
import { WaterfallFlyout } from './waterfall_flyout';
import { WaterfallSidebarItem } from './waterfall_sidebar_item';
import { MarkerItems } from '../../waterfall/context/waterfall_chart';

export const renderLegendItem: RenderItem<LegendItem> = (item) => {
return (
Expand All @@ -26,9 +27,10 @@ export const renderLegendItem: RenderItem<LegendItem> = (item) => {
interface Props {
total: number;
data: NetworkItems;
markerItems?: MarkerItems;
}

export const WaterfallChartWrapper: React.FC<Props> = ({ data, total }) => {
export const WaterfallChartWrapper: React.FC<Props> = ({ data, total, markerItems }) => {
const [query, setQuery] = useState<string>('');
const [activeFilters, setActiveFilters] = useState<string[]>([]);
const [onlyHighlighted, setOnlyHighlighted] = useState(false);
Expand Down Expand Up @@ -107,6 +109,7 @@ export const WaterfallChartWrapper: React.FC<Props> = ({ data, total }) => {

return (
<WaterfallProvider
markerItems={markerItems}
totalNetworkRequests={total}
fetchedNetworkRequests={networkData.length}
highlightedNetworkRequests={totalHighlightedRequests}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ export const WaterfallChartFixedAxisContainer = euiStyled.div`
height: ${FIXED_AXIS_HEIGHT}px;
z-index: ${(props) => props.theme.eui.euiZLevel4};
height: 100%;
&&& {
.echAnnotation__icon {
top: 8px;
}
}
`;

interface WaterfallChartSidebarContainer {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { WaterfallChartChartContainer, WaterfallChartTooltip } from './styles';
import { useWaterfallContext, WaterfallData } from '..';
import { WaterfallTooltipContent } from './waterfall_tooltip_content';
import { formatTooltipHeading } from '../../step_detail/waterfall/data_formatting';
import { WaterfallChartMarkers } from './waterfall_markers';

const getChartHeight = (data: WaterfallData): number => {
// We get the last item x(number of bars) and adds 1 to cater for 0 index
Expand Down Expand Up @@ -120,6 +121,7 @@ export const WaterfallBarChart = ({
styleAccessor={barStyleAccessor}
data={chartData}
/>
<WaterfallChartMarkers />
</Chart>
</WaterfallChartChartContainer>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
} from '@elastic/charts';
import { useChartTheme } from '../../../../../hooks/use_chart_theme';
import { WaterfallChartFixedAxisContainer } from './styles';
import { WaterfallChartMarkers } from './waterfall_markers';

interface Props {
tickFormat: TickFormatter;
Expand Down Expand Up @@ -59,6 +60,7 @@ export const WaterfallChartFixedAxis = ({ tickFormat, domain, barStyleAccessor }
styleAccessor={barStyleAccessor}
data={[{ x: 0, y0: 0, y1: 1 }]}
/>
<WaterfallChartMarkers />
</Chart>
</WaterfallChartFixedAxisContainer>
);
Expand Down
Loading