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(cache): plot and highlight transaction samples #70521

Merged
merged 2 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,16 +1,31 @@
import type {EChartHighlightHandler} from 'sentry/types/echarts';
import {decodeScalar} from 'sentry/utils/queryString';
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
import useLocationQuery from 'sentry/utils/url/useLocationQuery';
import {Referrer} from 'sentry/views/performance/cache/referrers';
import {CHART_HEIGHT} from 'sentry/views/performance/cache/settings';
import type {DataRow} from 'sentry/views/performance/cache/tables/spanSamplesTable';
import {AVG_COLOR} from 'sentry/views/starfish/colors';
import Chart, {ChartType} from 'sentry/views/starfish/components/chart';
import ChartPanel from 'sentry/views/starfish/components/chartPanel';
import {useMetricsSeries} from 'sentry/views/starfish/queries/useDiscoverSeries';
import type {MetricsQueryFilters} from 'sentry/views/starfish/types';
import {DataTitles} from 'sentry/views/starfish/views/spans/types';
import {useSampleScatterPlotSeries} from 'sentry/views/starfish/views/spanSummaryPage/sampleList/durationChart/useSampleScatterPlotSeries';

export function TransactionDurationChart() {
type Props = {
averageTransactionDuration: number;
onHighlight: EChartHighlightHandler;
samples: DataRow[];
highlightedSpanId?: string;
};

export function TransactionDurationChart({
samples,
averageTransactionDuration,
onHighlight,
highlightedSpanId,
}: Props) {
const {transaction} = useLocationQuery({
fields: {
project: decodeScalar,
Expand All @@ -28,6 +43,33 @@ export function TransactionDurationChart() {
referrer: Referrer.SAMPLES_CACHE_TRANSACTION_DURATION,
});

const sampledSpanDataSeries = useSampleScatterPlotSeries(
samples,
averageTransactionDuration,
highlightedSpanId,
'transaction.duration'
);

// TODO: This is duplicated from `DurationChart` in `SampleList`. Resolve the duplication
const handleChartHighlight: EChartHighlightHandler = function (event) {
// TODO: Gross hack. Even though `scatterPlot` is a separate prop, it's just an array of `Series` that gets appended to the main series. To find the point that was hovered, we re-construct the correct series order. It would have been cleaner to just pass the scatter plot as its own, single series
const allSeries = [
data['avg(transaction.duration)'],
...(sampledSpanDataSeries ?? []),
];

const highlightedDataPoints = event.batch.map(batch => {
const {seriesIndex, dataIndex} = batch;

const highlightedSeries = allSeries?.[seriesIndex];
const highlightedDataPoint = highlightedSeries.data?.[dataIndex];

return {series: highlightedSeries, dataPoint: highlightedDataPoint};
});

onHighlight?.(highlightedDataPoints, event);
};

return (
<ChartPanel title={DataTitles.transactionDuration}>
<Chart
Expand All @@ -38,8 +80,10 @@ export function TransactionDurationChart() {
top: '8px',
bottom: '0',
}}
scatterPlot={sampledSpanDataSeries}
data={[data['avg(transaction.duration)']]}
loading={isLoading}
onHighlight={handleChartHighlight}
chartColors={[AVG_COLOR]}
type={ChartType.LINE}
/>
Expand Down
36 changes: 35 additions & 1 deletion static/app/views/performance/cache/samplePanel/samplePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {Referrer} from 'sentry/views/performance/cache/referrers';
import {TransactionDurationChart} from 'sentry/views/performance/cache/samplePanel/charts/transactionDurationChart';
import {BASE_FILTERS} from 'sentry/views/performance/cache/settings';
import {SpanSamplesTable} from 'sentry/views/performance/cache/tables/spanSamplesTable';
import {useDebouncedState} from 'sentry/views/performance/http/useDebouncedState';
import {MetricReadout} from 'sentry/views/performance/metricReadout';
import * as ModuleLayout from 'sentry/views/performance/moduleLayout';
import DetailPanel from 'sentry/views/starfish/components/detailPanel';
Expand All @@ -37,6 +38,7 @@ import {
SpanMetricsField,
type SpanMetricsQueryFilters,
} from 'sentry/views/starfish/types';
import {findSampleFromDataPoint} from 'sentry/views/starfish/utils/chart/findDataPoint';
import {DataTitles, getThroughputTitle} from 'sentry/views/starfish/views/spans/types';

// This is similar to http sample table, its difficult to use the generic span samples sidebar as we require a bunch of custom things.
Expand All @@ -51,6 +53,12 @@ export function CacheSamplePanel() {
},
});

const [highlightedSpanId, setHighlightedSpanId] = useDebouncedState<string | undefined>(
undefined,
[],
10
);

// `detailKey` controls whether the panel is open. If all required properties are ailable, concat them to make a key, otherwise set to `undefined` and hide the panel
const detailKey = query.transaction
? [query.transaction].filter(Boolean).join(':')
Expand Down Expand Up @@ -240,7 +248,30 @@ export function CacheSamplePanel() {

<Fragment>
<ModuleLayout.Full>
<TransactionDurationChart />
<TransactionDurationChart
samples={spansWithDuration}
averageTransactionDuration={
transactionDurationData?.[0]?.[
`avg(${MetricsFields.TRANSACTION_DURATION})`
]
}
highlightedSpanId={highlightedSpanId}
onHighlight={highlights => {
const firstHighlight = highlights[0];

if (!firstHighlight) {
setHighlightedSpanId(undefined);
return;
}

const sample = findSampleFromDataPoint<(typeof spansWithDuration)[0]>(
firstHighlight.dataPoint,
spansWithDuration,
'transaction.duration'
);
setHighlightedSpanId(sample?.span_id);
}}
/>
</ModuleLayout.Full>
</Fragment>
<Fragment>
Expand All @@ -255,6 +286,9 @@ export function CacheSamplePanel() {
units: {[SpanIndexedField.CACHE_ITEM_SIZE]: 'byte'},
}}
isLoading={isCacheSpanSamplesFetching || isFetchingTransactions}
highlightedSpanId={highlightedSpanId}
onSampleMouseOver={sample => setHighlightedSpanId(sample.span_id)}
onSampleMouseOut={() => setHighlightedSpanId(undefined)}
error={transactionError}
/>
</ModuleLayout.Full>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ type ColumnKeys =
| SpanIndexedField.CACHE_ITEM_SIZE
| 'transaction.duration';

type DataRow = Pick<IndexedResponse, DataRowKeys> & {'transaction.duration': number};
export type DataRow = Pick<IndexedResponse, DataRowKeys> & {
'transaction.duration': number;
};

type Column = GridColumnHeader<ColumnKeys>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export const useSpanSamples = <Fields extends IndexedProperty[]>(
| SpanIndexedField.TIMESTAMP
| SpanIndexedField.ID
| SpanIndexedField.PROFILE_ID
| SpanIndexedField.SPAN_SELF_TIME
>[]
// This type is a little awkward but it explicitly states that the response could be empty. This doesn't enable unchecked access errors, but it at least indicates that it's possible that there's no data
// eslint-disable-next-line @typescript-eslint/ban-types
Expand Down
16 changes: 9 additions & 7 deletions static/app/views/performance/http/httpSamplesPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ import {
SpanMetricsField,
type SpanMetricsQueryFilters,
} from 'sentry/views/starfish/types';
import {findSampleFromDataPoint} from 'sentry/views/starfish/utils/chart/findDataPoint';
import {DataTitles, getThroughputTitle} from 'sentry/views/starfish/views/spans/types';
import {useSampleScatterPlotSeries} from 'sentry/views/starfish/views/spanSummaryPage/sampleList/durationChart/useSampleScatterPlotSeries';

Expand Down Expand Up @@ -242,12 +243,6 @@ export function HTTPSamplesPanel() {
highlightedSpanId
);

const findSampleFromDataPoint = (dataPoint: {name: string | number; value: number}) => {
return durationSamplesData.find(
s => s.timestamp === dataPoint.name && s['span.self_time'] === dataPoint.value
);
};

const handleClose = () => {
router.replace({
pathname: router.location.pathname,
Expand Down Expand Up @@ -402,7 +397,14 @@ export function HTTPSamplesPanel() {
return;
}

const sample = findSampleFromDataPoint(firstHighlight.dataPoint);
const sample = findSampleFromDataPoint<
(typeof durationSamplesData)[0]
>(
firstHighlight.dataPoint,
durationSamplesData,
SpanIndexedField.SPAN_SELF_TIME
);

setHighlightedSpanId(sample?.span_id);
}}
isLoading={isDurationDataFetching}
Expand Down
9 changes: 9 additions & 0 deletions static/app/views/starfish/utils/chart/findDataPoint.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
export function findSampleFromDataPoint<T extends {timestamp: string}>(
dataPoint: {name: string | number; value: number},
data: T[],
matchKey: keyof T
) {
return data?.find(
s => s.timestamp === dataPoint.name && s[matchKey] === dataPoint.value
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,16 @@ import {getSampleChartSymbol} from 'sentry/views/starfish/views/spanSummaryPage/
export function useSampleScatterPlotSeries(
spans: Partial<IndexedResponse>[],
average?: number,
highlightedSpanId?: string
highlightedSpanId?: string,
key: string = 'span.self_time'
): Series[] {
const theme = useTheme();

return spans.map(span => {
let symbol, color;

if (span['span.self_time'] && defined(average)) {
({symbol, color} = getSampleChartSymbol(span['span.self_time'], average, theme));
if (span[key] && defined(average)) {
({symbol, color} = getSampleChartSymbol(span[key], average, theme));
} else {
symbol = 'circle';
color = AVG_COLOR;
Expand All @@ -30,7 +31,7 @@ export function useSampleScatterPlotSeries(
data: [
{
name: span?.timestamp ?? span.span_id ?? t('Span'),
value: span?.['span.self_time'] ?? 0,
value: span?.[key] ?? 0,
},
],
symbol,
Expand Down