Skip to content

Commit

Permalink
fix(perf): Add back condition for transaction_id (#68537)
Browse files Browse the repository at this point in the history
Accidentally replaced a conditional in
#68295

We were not checking for a `transaction_id` column, this PR puts it back
in place so that the table can handle the case of linking to both a
transaction ID and span ID
  • Loading branch information
0Calories authored Apr 9, 2024
1 parent ef75838 commit 6bdbae6
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,30 @@ export function SpanSamplesTable({
}

function renderBodyCell(column: GridColumnHeader, row: SpanTableRow): React.ReactNode {
if (column.key === 'transaction_id') {
return (
<Link
to={generateLinkToEventInTraceView({
eventSlug: generateEventSlug({
id: row['transaction.id'],
project: row.project,
}),
organization,
location,
eventView: EventView.fromLocation(location),
dataRow: {
id: row['transaction.id'],
trace: row.transaction?.trace,
timestamp: row.timestamp,
},
spanId: row.span_id,
})}
>
{row['transaction.id'].slice(0, 8)}
</Link>
);
}

if (column.key === 'span_id') {
return (
<Link
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,8 @@ export function ScreenLoadSampleContainer({
release={release}
columnOrder={[
{
key: 'transaction_id',
name: t('Event ID'),
key: 'span_id',
name: t('Span ID'),
width: COL_WIDTH_UNDEFINED,
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import {
waitForElementToBeRemoved,
} from 'sentry-test/reactTestingLibrary';

import {COL_WIDTH_UNDEFINED} from 'sentry/components/gridEditable';
import {t} from 'sentry/locale';
import type {PageFilters} from 'sentry/types';
import {SpanMetricsField} from 'sentry/views/starfish/types';

Expand Down Expand Up @@ -61,6 +63,65 @@ describe('SampleTable', function () {
await expectNever(() => container.getByText('No results found for your query'));
expect(container.queryByTestId('loading-indicator')).not.toBeInTheDocument();
});

it('should show span IDs by default', async () => {
const container = render(
<SampleTable
groupId="groupId123"
transactionMethod="GET"
transactionName="/endpoint"
/>
);

await waitFor(() =>
expect(container.queryByTestId('loading-indicator')).not.toBeInTheDocument()
);

expect(container.queryAllByTestId('grid-head-cell')[0]).toHaveTextContent(
'Span ID'
);
expect(container.queryAllByTestId('grid-body-cell')[0]).toHaveTextContent(
'span-id123'
);
});

it('should show transaction IDs instead of span IDs when in columnOrder', async () => {
const container = render(
<SampleTable
groupId="groupId123"
transactionMethod="GET"
transactionName="/endpoint"
columnOrder={[
{
key: 'transaction_id',
name: t('Event ID'),
width: COL_WIDTH_UNDEFINED,
},
{
key: 'profile_id',
name: t('Profile'),
width: COL_WIDTH_UNDEFINED,
},
{
key: 'avg_comparison',
name: t('Compared to Average'),
width: COL_WIDTH_UNDEFINED,
},
]}
/>
);

await waitFor(() =>
expect(container.queryByTestId('loading-indicator')).not.toBeInTheDocument()
);

expect(container.queryAllByTestId('grid-head-cell')[0]).toHaveTextContent(
'Event ID'
);
expect(container.queryAllByTestId('grid-body-cell')[0]).toHaveTextContent(
'transaction-id123'.slice(0, 8)
);
});
});

describe('When there is missing data', () => {
Expand Down

0 comments on commit 6bdbae6

Please sign in to comment.