Skip to content

Commit

Permalink
ref(perf): Move out the span transactions table (#69771)
Browse files Browse the repository at this point in the history
This one's gross! In short, we had a component called the
`SpanTransactionsTable`. It was a transactions table with some
conditional logic, to render different columns and fetch different data
depending on what module was using it.

The problem? The only module using it is the database module. So, I'm
dumping this "dynamic" table, and replacing it with a normal table, the
kind we have all over the place. This will make it easier to consolidate
the code, and remove a few rarely-used utils like `extractRoute`.

I'm also removing some cruft like the competing `endpoint` and
`transaction` parameters that we don't use anymore, lifting up the data
loading state, etc.

- Move transactions table into the module
- Lift up data fetching state
- Static column order
- Remove redundant wrapper
- Simplify routing
- Move out table cell renderer
- Export testable component
- Sorting
  • Loading branch information
gggritso committed May 8, 2024
1 parent 8021400 commit f49e7b2
Show file tree
Hide file tree
Showing 4 changed files with 232 additions and 271 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('DatabaseSpanSummaryPage', function () {
jest.mocked(useLocation).mockReturnValue({
pathname: '',
search: '',
query: {statsPeriod: '10d', transactionsCursor: '0:20:0'},
query: {statsPeriod: '10d', transactionsCursor: '0:25:0'},
hash: '',
state: undefined,
action: 'PUSH',
Expand Down Expand Up @@ -131,11 +131,9 @@ describe('DatabaseSpanSummaryPage', function () {
{...RouteComponentPropsFixture({})}
params={{
groupId: '1756baf8fd19c116',
endpoint: '',
endpointMethod: '',
transaction: '',
transactionMethod: '',
spansSort: '',
transactionsSort: '',
}}
/>
);
Expand Down Expand Up @@ -260,6 +258,7 @@ describe('DatabaseSpanSummaryPage', function () {
'http_error_count()',
],
per_page: 25,
cursor: '0:25:0',
project: [],
query: 'span.group:1756baf8fd19c116',
sort: '-time_spent_percentage()',
Expand Down Expand Up @@ -329,7 +328,7 @@ describe('DatabaseSpanSummaryPage', function () {
expect(screen.getByRole('cell', {name: 'GET /api/users'})).toBeInTheDocument();
expect(screen.getByRole('link', {name: 'GET /api/users'})).toHaveAttribute(
'href',
'/organizations/org-slug/starfish/spans/span/1756baf8fd19c116?statsPeriod=10d&transaction=%2Fapi%2Fusers&transactionMethod=GET&transactionsCursor=0%3A20%3A0'
'/organizations/org-slug/performance/database/spans/span/1756baf8fd19c116?statsPeriod=10d&transaction=%2Fapi%2Fusers&transactionMethod=GET&transactionsCursor=0%3A25%3A0'
);
expect(screen.getByRole('cell', {name: '17.9/s'})).toBeInTheDocument();
expect(screen.getByRole('cell', {name: '204.50ms'})).toBeInTheDocument();
Expand Down
64 changes: 44 additions & 20 deletions static/app/views/performance/database/databaseSpanSummaryPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,15 @@ import {EnvironmentPageFilter} from 'sentry/components/organizations/environment
import PageFilterBar from 'sentry/components/organizations/pageFilterBar';
import {t} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import {DurationUnit, RateUnit, type Sort} from 'sentry/utils/discover/fields';
import {DurationUnit, RateUnit} from 'sentry/utils/discover/fields';
import {decodeScalar, decodeSorts} from 'sentry/utils/queryString';
import {MutableSearch} from 'sentry/utils/tokenizeSearch';
import {useLocation} from 'sentry/utils/useLocation';
import useOrganization from 'sentry/utils/useOrganization';
import {normalizeUrl} from 'sentry/utils/withDomainRequired';
import {DurationChart} from 'sentry/views/performance/database/durationChart';
import {isAValidSort} from 'sentry/views/performance/database/queriesTable';
import {QueryTransactionsTable} from 'sentry/views/performance/database/queryTransactionsTable';
import {ThroughputChart} from 'sentry/views/performance/database/throughputChart';
import {useSelectedDurationAggregate} from 'sentry/views/performance/database/useSelectedDurationAggregate';
import {MetricReadout} from 'sentry/views/performance/metricReadout';
Expand All @@ -31,16 +34,12 @@ import type {SpanMetricsQueryFilters} from 'sentry/views/starfish/types';
import {SpanFunction, SpanMetricsField} from 'sentry/views/starfish/types';
import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
import {DataTitles, getThroughputTitle} from 'sentry/views/starfish/views/spans/types';
import {useModuleSort} from 'sentry/views/starfish/views/spans/useModuleSort';
import {SampleList} from 'sentry/views/starfish/views/spanSummaryPage/sampleList';
import {SpanTransactionsTable} from 'sentry/views/starfish/views/spanSummaryPage/spanTransactionsTable';

type Query = {
endpoint: string;
endpointMethod: string;
transaction: string;
transactionMethod: string;
[QueryParameterNames.SPANS_SORT]: string;
[QueryParameterNames.TRANSACTIONS_SORT]: string;
aggregate?: string;
};

Expand All @@ -53,21 +52,18 @@ export function DatabaseSpanSummaryPage({params}: Props) {
const [selectedAggregate] = useSelectedDurationAggregate();

const {groupId} = params;
const {transaction, transactionMethod, endpoint, endpointMethod} = location.query;
const {transaction, transactionMethod} = location.query;

const filters: SpanMetricsQueryFilters = {
'span.group': groupId,
};

if (endpoint) {
filters.transaction = endpoint;
}
const cursor = decodeScalar(location.query?.[QueryParameterNames.TRANSACTIONS_CURSOR]);

if (endpointMethod) {
filters['transaction.method'] = endpointMethod;
}
// TODO: Fetch sort information using `useLocationQuery`
const sortField = decodeScalar(location.query?.[QueryParameterNames.TRANSACTIONS_SORT]);

const sort = useModuleSort(QueryParameterNames.ENDPOINTS_SORT, DEFAULT_SORT);
const sort = decodeSorts(sortField).filter(isAValidSort).at(0) ?? DEFAULT_SORT;

const {data, isLoading: areSpanMetricsLoading} = useSpanMetrics({
search: MutableSearch.fromQueryObject(filters),
Expand All @@ -89,6 +85,29 @@ export function DatabaseSpanSummaryPage({params}: Props) {

const spanMetrics = data[0] ?? {};

const {
isLoading: isTransactionsListLoading,
data: transactionsList,
meta: transactionsListMeta,
error: transactionsListError,
pageLinks: transactionsListPageLinks,
} = useSpanMetrics({
search: MutableSearch.fromQueryObject(filters),
fields: [
'transaction',
'transaction.method',
'spm()',
`sum(${SpanMetricsField.SPAN_SELF_TIME})`,
`avg(${SpanMetricsField.SPAN_SELF_TIME})`,
'time_spent_percentage()',
`${SpanFunction.HTTP_ERROR_COUNT}()`,
],
sorts: [sort],
limit: TRANSACTIONS_TABLE_ROW_COUNT,
cursor,
referrer: 'api.starfish.span-transaction-metrics',
});

const span = {
...spanMetrics,
[SpanMetricsField.SPAN_GROUP]: groupId,
Expand Down Expand Up @@ -224,11 +243,14 @@ export function DatabaseSpanSummaryPage({params}: Props) {

{span && (
<ModuleLayout.Full>
<SpanTransactionsTable
<QueryTransactionsTable
span={span}
data={transactionsList}
error={transactionsListError}
isLoading={isTransactionsListLoading}
meta={transactionsListMeta}
pageLinks={transactionsListPageLinks}
sort={sort}
endpoint={endpoint}
endpointMethod={endpointMethod}
/>
</ModuleLayout.Full>
)}
Expand All @@ -245,11 +267,13 @@ export function DatabaseSpanSummaryPage({params}: Props) {
);
}

const DEFAULT_SORT: Sort = {
kind: 'desc',
field: 'time_spent_percentage()',
const DEFAULT_SORT = {
kind: 'desc' as const,
field: 'time_spent_percentage()' as const,
};

const TRANSACTIONS_TABLE_ROW_COUNT = 25;

const ChartContainer = styled('div')`
display: grid;
gap: 0;
Expand Down
184 changes: 184 additions & 0 deletions static/app/views/performance/database/queryTransactionsTable.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
import {Fragment} from 'react';
import {browserHistory} from 'react-router';
import type {Location} from 'history';
import * as qs from 'query-string';

import type {GridColumnHeader} from 'sentry/components/gridEditable';
import GridEditable, {COL_WIDTH_UNDEFINED} from 'sentry/components/gridEditable';
import Link from 'sentry/components/links/link';
import type {CursorHandler} from 'sentry/components/pagination';
import Pagination from 'sentry/components/pagination';
import {t} from 'sentry/locale';
import type {Organization} from 'sentry/types';
import type {EventsMetaType} from 'sentry/utils/discover/eventView';
import {getFieldRenderer} from 'sentry/utils/discover/fieldRenderers';
import type {Sort} from 'sentry/utils/discover/fields';
import {useLocation} from 'sentry/utils/useLocation';
import useOrganization from 'sentry/utils/useOrganization';
import {normalizeUrl} from 'sentry/utils/withDomainRequired';
import {renderHeadCell} from 'sentry/views/starfish/components/tableCells/renderHeadCell';
import {OverflowEllipsisTextContainer} from 'sentry/views/starfish/components/textAlign';
import type {SpanMetricsResponse} from 'sentry/views/starfish/types';
import {SpanMetricsField} from 'sentry/views/starfish/types';
import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
import {DataTitles, getThroughputTitle} from 'sentry/views/starfish/views/spans/types';

type Row = Pick<
SpanMetricsResponse,
| 'transaction'
| 'transaction.method'
| 'spm()'
| 'avg(span.self_time)'
| 'sum(span.self_time)'
| 'time_spent_percentage()'
>;

type Column = GridColumnHeader<
'transaction' | 'spm()' | 'avg(span.self_time)' | 'time_spent_percentage()'
>;

const COLUMN_ORDER: Column[] = [
{
key: 'transaction',
name: t('Found In'),
width: COL_WIDTH_UNDEFINED,
},
{
key: 'spm()',
name: getThroughputTitle('db'),
width: COL_WIDTH_UNDEFINED,
},
{
key: `avg(${SpanMetricsField.SPAN_SELF_TIME})`,
name: DataTitles.avg,
width: COL_WIDTH_UNDEFINED,
},
{
key: 'time_spent_percentage()',
name: DataTitles.timeSpent,
width: COL_WIDTH_UNDEFINED,
},
];

const SORTABLE_FIELDS = [
'avg(span.self_time)',
'spm()',
'time_spent_percentage()',
] as const;

type ValidSort = Sort & {
field: (typeof SORTABLE_FIELDS)[number];
};

export function isAValidSort(sort: Sort): sort is ValidSort {
return (SORTABLE_FIELDS as unknown as string[]).includes(sort.field);
}

interface Props {
data: Row[];
isLoading: boolean;
sort: ValidSort;
span: Pick<SpanMetricsResponse, SpanMetricsField.SPAN_GROUP | SpanMetricsField.SPAN_OP>;
error?: Error | null;
meta?: EventsMetaType;
pageLinks?: string;
}

export function QueryTransactionsTable({
data,
isLoading,
error,
meta,
pageLinks,
sort,
span,
}: Props) {
const location = useLocation();
const organization = useOrganization();

const handleCursor: CursorHandler = (newCursor, pathname, query) => {
browserHistory.push({
pathname,
query: {...query, [QueryParameterNames.TRANSACTIONS_CURSOR]: newCursor},
});
};

return (
<Fragment>
<GridEditable
aria-label={t('Transactions')}
isLoading={isLoading}
error={error}
data={data}
columnOrder={COLUMN_ORDER}
columnSortBy={[
{
key: sort.field,
order: sort.kind,
},
]}
grid={{
renderHeadCell: col =>
renderHeadCell({
column: col,
sort,
location,
sortParameterName: QueryParameterNames.TRANSACTIONS_SORT,
}),
renderBodyCell: (column, row) =>
renderBodyCell(column, row, meta, span, location, organization),
}}
location={location}
/>

<Pagination pageLinks={pageLinks} onCursor={handleCursor} />
</Fragment>
);
}

function renderBodyCell(
column: Column,
row: Row,
meta: EventsMetaType | undefined,
span: Pick<SpanMetricsResponse, SpanMetricsField.SPAN_GROUP | SpanMetricsField.SPAN_OP>,
location: Location,
organization: Organization
) {
if (column.key === 'transaction') {
const label =
row['transaction.method'] && !row.transaction.startsWith(row['transaction.method'])
? `${row['transaction.method']} ${row.transaction}`
: row.transaction;

const pathname = normalizeUrl(
`/organizations/${organization.slug}/performance/database/spans/span/${encodeURIComponent(span[SpanMetricsField.SPAN_GROUP])}`
);
const query: {[key: string]: string | undefined} = {
...location.query,
transaction: row.transaction,
transactionMethod: row['transaction.method'],
};

return (
<OverflowEllipsisTextContainer>
<Link to={`${pathname}?${qs.stringify(query)}`}>{label}</Link>
</OverflowEllipsisTextContainer>
);
}

if (!meta || !meta?.fields) {
return row[column.key];
}

const renderer = getFieldRenderer(column.key, meta.fields, false);
const rendered = renderer(
{...row, 'span.op': span['span.op']},
{
location,
organization,
unit: meta.units?.[column.key],
}
);

return rendered;
}

0 comments on commit f49e7b2

Please sign in to comment.