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

feat(starfish): API module consistency and continuity updaes #52347

Merged
merged 2 commits into from
Jul 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
12 changes: 11 additions & 1 deletion static/app/views/starfish/components/chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {IconWarning} from 'sentry/icons';
import {DateString} from 'sentry/types';
import {
EChartClickHandler,
EChartDataZoomHandler,
EChartEventHandler,
EChartHighlightHandler,
EChartMouseOutHandler,
Expand Down Expand Up @@ -88,6 +89,7 @@ type Props = {
isLineChart?: boolean;
log?: boolean;
onClick?: EChartClickHandler;
onDataZoom?: EChartDataZoomHandler;
onHighlight?: EChartHighlightHandler;
onLegendSelectChanged?: EChartEventHandler<{
name: string;
Expand Down Expand Up @@ -177,6 +179,7 @@ function Chart({
tooltipFormatterOptions = {},
errored,
onLegendSelectChanged,
onDataZoom,
}: Props) {
const router = useRouter();
const theme = useTheme();
Expand Down Expand Up @@ -383,7 +386,14 @@ function Chart({
: series;

return (
<ChartZoom router={router} period={statsPeriod} start={start} end={end} utc={utc}>
<ChartZoom
router={router}
period={statsPeriod}
start={start}
end={end}
utc={utc}
onDataZoom={onDataZoom}
>
{zoomRenderProps => {
if (isLineChart) {
return (
Expand Down
18 changes: 18 additions & 0 deletions static/app/views/starfish/components/tableCells/countCell.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import {NumberContainer} from 'sentry/utils/discover/styles';
import {formatAbbreviatedNumber} from 'sentry/utils/formatters';

type Props = {
count: number;
containerProps?: React.DetailedHTMLProps<
React.HTMLAttributes<HTMLDivElement>,
HTMLDivElement
>;
};

export default function DurationCell({count, containerProps}: Props) {
edwardgou-sentry marked this conversation as resolved.
Show resolved Hide resolved
return (
<NumberContainer {...containerProps}>
{formatAbbreviatedNumber(count)}
</NumberContainer>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ export const SORTABLE_FIELDS = new Set([
'sps_percent_change()',
'time_spent_percentage()',
'time_spent_percentage(local)',
'http_error_count()',
'http_error_count_percent_change()',
]);

export const renderHeadCell = ({column, location, sort}: Options) => {
Expand Down
1 change: 1 addition & 0 deletions static/app/views/starfish/queries/useSpanMetrics.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {useSpansQuery} from 'sentry/views/starfish/utils/useSpansQuery';

export type SpanMetrics = {
[metric: string]: number | string;
'http_error_count()': number;
'p95(span.self_time)': number;
'span.op': string;
'sps()': number;
Expand Down
33 changes: 31 additions & 2 deletions static/app/views/starfish/views/spanSummaryPage/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,13 @@ import {
} from 'sentry/utils/performance/contexts/pageError';
import useOrganization from 'sentry/utils/useOrganization';
import {normalizeUrl} from 'sentry/utils/withDomainRequired';
import {P95_COLOR, THROUGHPUT_COLOR} from 'sentry/views/starfish/colours';
import {ERRORS_COLOR, P95_COLOR, THROUGHPUT_COLOR} from 'sentry/views/starfish/colours';
import Chart, {useSynchronizeCharts} from 'sentry/views/starfish/components/chart';
import ChartPanel from 'sentry/views/starfish/components/chartPanel';
import StarfishDatePicker from 'sentry/views/starfish/components/datePicker';
import StarfishPageFilterContainer from 'sentry/views/starfish/components/pageFilterContainer';
import {SpanDescription} from 'sentry/views/starfish/components/spanDescription';
import CountCell from 'sentry/views/starfish/components/tableCells/countCell';
import DurationCell from 'sentry/views/starfish/components/tableCells/durationCell';
import ThroughputCell from 'sentry/views/starfish/components/tableCells/throughputCell';
import {TimeSpentCell} from 'sentry/views/starfish/components/tableCells/timeSpentCell';
Expand Down Expand Up @@ -79,6 +80,7 @@ function SpanSummaryPage({params, location}: Props) {
`sum(${SPAN_SELF_TIME})`,
`p95(${SPAN_SELF_TIME})`,
'time_spent_percentage()',
'http_error_count()',
],
'span-summary-page-metrics'
);
Expand All @@ -89,7 +91,7 @@ function SpanSummaryPage({params, location}: Props) {
useSpanMetricsSeries(
{group: groupId},
queryFilter,
[`p95(${SPAN_SELF_TIME})`, 'sps()'],
[`p95(${SPAN_SELF_TIME})`, 'sps()', 'http_error_count()'],
'span-summary-page-metrics'
);

Expand Down Expand Up @@ -163,6 +165,14 @@ function SpanSummaryPage({params, location}: Props) {
milliseconds={spanMetrics?.[`p95(${SPAN_SELF_TIME})`]}
/>
</Block>
{span?.['span.op']?.startsWith('http') && (
<Block
title={t('5XX Responses')}
description={t('5XX responses in this span')}
>
<CountCell count={spanMetrics?.[`http_error_count()`]} />
</Block>
)}
<Block
title={t('Time Spent')}
description={t(
Expand Down Expand Up @@ -227,6 +237,25 @@ function SpanSummaryPage({params, location}: Props) {
/>
</ChartPanel>
</Block>

{span?.['span.op']?.startsWith('http') && (
<Block>
<ChartPanel title={DataTitles.errorCount}>
<Chart
statsPeriod="24h"
height={140}
data={[spanMetricsSeriesData?.[`http_error_count()`]]}
start=""
end=""
loading={areSpanMetricsSeriesLoading}
utc={false}
chartColors={[ERRORS_COLOR]}
isLineChart
definedAxisTicks={4}
/>
</ChartPanel>
</Block>
)}
</BlockContainer>
)}

Expand Down
25 changes: 25 additions & 0 deletions static/app/views/starfish/views/spans/spanTimeCharts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import EventView from 'sentry/utils/discover/eventView';
import {DiscoverDatasets} from 'sentry/utils/discover/types';
import {useLocation} from 'sentry/utils/useLocation';
import usePageFilters from 'sentry/utils/usePageFilters';
import useRouter from 'sentry/utils/useRouter';
import {ERRORS_COLOR, P95_COLOR, THROUGHPUT_COLOR} from 'sentry/views/starfish/colours';
import Chart, {useSynchronizeCharts} from 'sentry/views/starfish/components/chart';
import ChartPanel from 'sentry/views/starfish/components/chartPanel';
Expand Down Expand Up @@ -140,6 +141,7 @@ function ThroughputChart({moduleName, filters}: ChartProps): JSX.Element {
tooltipFormatterOptions={{
valueFormatter: value => formatThroughput(value),
}}
onDataZoom={useSortPageByHandler('-sps()')}
/>
);
}
Expand Down Expand Up @@ -188,6 +190,7 @@ function DurationChart({moduleName, filters}: ChartProps): JSX.Element {
stacked
isLineChart
chartColors={[P95_COLOR]}
onDataZoom={useSortPageByHandler('-p95(span.self_time)')}
/>
);
}
Expand Down Expand Up @@ -225,6 +228,7 @@ function ErrorChart({moduleName, filters}: ChartProps): JSX.Element {
stacked
isLineChart
chartColors={[ERRORS_COLOR]}
onDataZoom={useSortPageByHandler('-http_error_count()')}
/>
);
}
Expand Down Expand Up @@ -286,6 +290,27 @@ const buildDiscoverQueryConditions = (
return result.join(' ');
};

function useSortPageByHandler(sort: string) {
const router = useRouter();
const location = useLocation();
return (_, chartRef) => {
// This is kind of jank but we need to check if the chart is hovered because
// onDataZoom is fired for all charts when one chart is zoomed.
const hoveredEchartElement = Array.from(document.querySelectorAll(':hover')).find(
element => {
return element.classList.contains('echarts-for-react');
}
);
const echartElement = document.querySelector(`[_echarts_instance_="${chartRef.id}"]`);
if (hoveredEchartElement === echartElement) {
router.replace({
pathname: location.pathname,
query: {...location.query, spansSort: sort},
});
}
};
}

const ChartsContainer = styled('div')`
display: flex;
flex-direction: row;
Expand Down
2 changes: 2 additions & 0 deletions static/app/views/starfish/views/spans/spansTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ export const SORTABLE_FIELDS = new Set([
'sps()',
'sps_percent_change()',
'time_spent_percentage()',
'http_error_count()',
'http_error_count_percent_change()',
]);

export default function SpansTable({
Expand Down