Skip to content

feat(tracemetrics): Improve samples panel rendering #101448

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Oct 14, 2025
Merged

feat(tracemetrics): Improve samples panel rendering #101448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b786a89
feat(tracemetrics): Improve samples panel rendering
narsaynorath Oct 14, 2025
8b6ee8c
Make sure error result is passed
narsaynorath Oct 14, 2025
5329bc5
fix extraction
narsaynorath Oct 14, 2025
855dad3
Add location
narsaynorath Oct 14, 2025
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
48 changes: 46 additions & 2 deletions static/app/views/explore/metrics/hooks/useTraceTelemetry.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ import {useMemo} from 'react';

import {MutableSearch} from 'sentry/components/searchSyntax/mutableSearch';
import type {NewQuery} from 'sentry/types/organization';
import {useDiscoverQuery, type TableDataRow} from 'sentry/utils/discover/discoverQuery';
import EventView from 'sentry/utils/discover/eventView';
import {DiscoverDatasets} from 'sentry/utils/discover/types';
import {useLocation} from 'sentry/utils/useLocation';
import useOrganization from 'sentry/utils/useOrganization';
import usePageFilters from 'sentry/utils/usePageFilters';
import {useSpansQuery} from 'sentry/views/insights/common/queries/useSpansQuery';

Expand All @@ -13,6 +16,7 @@ interface UseTraceTelemetryOptions {
}

interface TraceTelemetryData {
errorsCount: number;
logsCount: number;
spansCount: number;
trace: string;
Expand All @@ -27,7 +31,35 @@ export function useTraceTelemetry({
enabled,
traceIds,
}: UseTraceTelemetryOptions): TraceTelemetryResult {
const organization = useOrganization();
const {selection} = usePageFilters();
const location = useLocation();

// Query for error count
const errorsEventView = useMemo(() => {
const traceFilter = new MutableSearch('').addFilterValueList('trace', traceIds);
const discoverQuery: NewQuery = {
id: undefined,
name: 'Error Count',
fields: ['trace', 'count()'],
orderby: '-count',
query: traceFilter.formatString(),
version: 2,
dataset: DiscoverDatasets.ERRORS,
};
return EventView.fromNewQueryWithPageFilters(discoverQuery, selection);
}, [traceIds, selection]);

const errorsResult = useDiscoverQuery({
eventView: errorsEventView,
limit: traceIds.length,
referrer: 'api.explore.trace-errors-count',
orgSlug: organization.slug,
location,
Copy link
Contributor

@seer-by-sentry seer-by-sentry bot Oct 14, 2025

Choose a reason for hiding this comment

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

Potential bug: The useDiscoverQuery hook is called with an undefined location variable, which will cause a runtime ReferenceError.

  • Description: The useDiscoverQuery hook is called on line 56 with a location variable. However, this variable is not defined within the scope of the useTraceTelemetry hook. It is not passed as a parameter, imported, or declared locally. Since useDiscoverQuery requires location as a mandatory prop, this will result in a ReferenceError: location is not defined when the component renders and the query is executed, causing a runtime crash.

  • Suggested fix: The useTraceTelemetry hook needs to accept location as a parameter. Update its options interface, add it to the function signature, and pass it down to useDiscoverQuery. The calling component, samplesTab.tsx, should then pass its location object to the hook.
    severity: 0.85, confidence: 1.0

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Member Author

@narsaynorath narsaynorath Oct 14, 2025

Choose a reason for hiding this comment

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

Weird, this wasn't caught by my linter earlier.

options: {
enabled: enabled && errorsEventView !== null,
},
});

// Query for spans count
const spansEventView = useMemo(() => {
Expand Down Expand Up @@ -90,6 +122,7 @@ export function useTraceTelemetry({
trace: traceId,
spansCount: 0,
logsCount: 0,
errorsCount: 0,
});
});

Expand All @@ -115,11 +148,22 @@ export function useTraceTelemetry({
});
}

// Populate errors count
if (errorsResult.data) {
errorsResult.data.data.forEach((row: TableDataRow) => {
const traceId = row.trace as string;
const count = row['count()'] as number;
if (dataMap.has(traceId)) {
dataMap.get(traceId)!.errorsCount = count;
}
});
}

return dataMap;
}, [traceIds, spansResult.data, logsResult.data]);
}, [traceIds, spansResult.data, logsResult.data, errorsResult.data]);
Copy link
Contributor

@cursor cursor bot Oct 14, 2025

Choose a reason for hiding this comment

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

Bug: Incorrect Dependency in useMemo Hook

The telemetryData useMemo hook's dependency array includes errorsResult.data, but the error count data is accessed via errorsResult.data.data. This means the memoized value won't update when the actual error data changes, leading to stale error counts and an inconsistent data access pattern compared to other queries.

Additional Locations (1)

Fix in Cursor Fix in Web


return {
data: telemetryData,
isLoading: spansResult.isPending || logsResult.isPending,
isLoading: spansResult.isPending || logsResult.isPending || errorsResult.isPending,
};
}
195 changes: 113 additions & 82 deletions static/app/views/explore/metrics/metricInfoTabs/samplesTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {useMemo, useRef} from 'react';
import {useTheme} from '@emotion/react';
import styled from '@emotion/styled';

import {Flex} from '@sentry/scraps/layout';
Expand All @@ -10,6 +11,7 @@ import LoadingIndicator from 'sentry/components/loadingIndicator';
import LoadingMask from 'sentry/components/loadingMask';
import type {Alignments} from 'sentry/components/tables/gridEditable/sortLink';
import {GridBodyCell, GridHeadCell} from 'sentry/components/tables/gridEditable/styles';
import {IconFire, IconSpan, IconTerminal} from 'sentry/icons';
import {IconArrow} from 'sentry/icons/iconArrow';
import {IconWarning} from 'sentry/icons/iconWarning';
import {t} from 'sentry/locale';
Expand All @@ -26,6 +28,10 @@ import {
TableStatus,
useTableStyles,
} from 'sentry/views/explore/components/table';
import {LogAttributesRendererMap} from 'sentry/views/explore/logs/fieldRenderers';
import {getLogColors} from 'sentry/views/explore/logs/styles';
import {OurLogKnownFieldKey} from 'sentry/views/explore/logs/types';
import {SeverityLevel} from 'sentry/views/explore/logs/utils';
import {useMetricSamplesTable} from 'sentry/views/explore/metrics/hooks/useMetricSamplesTable';
import {useTraceTelemetry} from 'sentry/views/explore/metrics/hooks/useTraceTelemetry';
import {Table} from 'sentry/views/explore/multiQueryMode/components/miniTable';
Expand Down Expand Up @@ -53,7 +59,7 @@ export function SamplesTab({metricName}: SamplesTabProps) {
enabled: Boolean(metricName),
limit: RESULT_LIMIT,
metricName,
fields: ['timestamp', 'trace', 'value'],
fields: ['timestamp', 'value', 'trace'],
});

// Extract trace IDs from the result
Expand All @@ -74,13 +80,8 @@ export function SamplesTab({metricName}: SamplesTabProps) {
const sorts = useQueryParamsSortBys();
const setSorts = useSetQueryParamsSortBys();

// Add telemetry columns to the fields list
const displayFields = useMemo(() => {
return [...fields, 'logs', 'spans'];
}, [fields]);

const tableRef = useRef<HTMLTableElement>(null);
const {initialTableStyles} = useTableStyles(displayFields, tableRef, {
const {initialTableStyles} = useTableStyles(fields, tableRef, {
minimumColumnWidth: 50,
});

Expand All @@ -90,8 +91,103 @@ export function SamplesTab({metricName}: SamplesTabProps) {
trace: t('Trace'),
value: t('Value'),
timestamp: t('Timestamp'),
logs: t('Logs'),
spans: t('Spans'),
};

const theme = useTheme();

const renderTraceCell = (row: any, traceId: string, telemetry: any) => {
const timestamp = row.timestamp as number;
const target = getTraceDetailsUrl({
organization,
traceSlug: traceId,
dateSelection: {
start: selection.datetime.start,
end: selection.datetime.end,
statsPeriod: selection.datetime.period,
},
timestamp: timestamp / 1000,
location,
source: TraceViewSources.TRACES,
});

return (
<Flex gap="xs" display="inline-flex">
<Link to={target} style={{minWidth: '66px'}}>
{getShortEventId(traceId)}
</Link>
<Flex gap="xs" style={{color: theme.red300}}>
<IconFire />
{telemetry?.errorsCount ?? 0}
</Flex>
<Flex gap="xs" style={{color: theme.purple400}}>
<IconTerminal />
{telemetry?.logsCount ?? 0}
</Flex>
<Flex gap="xs" style={{color: theme.gray300}}>
<IconSpan color="gray300" />
{telemetry?.spansCount ?? 0}
</Flex>
</Flex>
);
};

const renderTimestampCell = (field: string, row: any, originalFieldIndex: number) => {
const customRenderer = LogAttributesRendererMap[OurLogKnownFieldKey.TIMESTAMP];

if (!customRenderer) {
return (
<FieldRenderer
column={columns[originalFieldIndex]}
data={row}
unit={meta?.units?.[field]}
meta={meta}
/>
);
}

return customRenderer({
item: {
fieldKey: field,
value: row[field],
},
extra: {
attributes: row,
attributeTypes: meta.fields ?? {},
highlightTerms: [],
logColors: getLogColors(SeverityLevel.INFO, theme),
location,
organization,
theme,
},
});
};

const renderDefaultCell = (field: string, row: any, originalFieldIndex: number) => {
return (
<FieldRenderer
column={columns[originalFieldIndex]}
data={row}
unit={meta?.units?.[field]}
meta={meta}
/>
);
};

const renderFieldCell = (field: string, row: any, traceId: string, telemetry: any) => {
const originalFieldIndex = fields.indexOf(field);

if (originalFieldIndex === -1) {
return null;
}

switch (field) {
case 'trace':
return renderTraceCell(row, traceId, telemetry);
case 'timestamp':
return renderTimestampCell(field, row, originalFieldIndex);
default:
return renderDefaultCell(field, row, originalFieldIndex);
}
};

return (
Expand All @@ -100,33 +196,21 @@ export function SamplesTab({metricName}: SamplesTabProps) {
<Table ref={tableRef} style={initialTableStyles} scrollable height={TABLE_HEIGHT}>
<TableHead>
<TableRow>
{displayFields.map((field, i) => {
{fields.map((field, i) => {
const label = fieldLabels[field] ?? field;
const fieldType = meta.fields?.[field];
const align = fieldAlignment(field, fieldType);

// Don't allow sorting on telemetry fields
const isTelemetryField = field === 'logs' || field === 'spans';
const direction = isTelemetryField
? undefined
: sorts.find(s => s.field === field)?.kind;
const direction = sorts.find(s => s.field === field)?.kind;

function updateSort() {
if (isTelemetryField) {
return;
}
const kind = direction === 'desc' ? 'asc' : 'desc';
setSorts([{field, kind}]);
}

return (
<TableHeadCell align={align} key={i} isFirst={i === 0}>
<TableHeadCellContent
align="center"
gap="sm"
onClick={updateSort}
style={isTelemetryField ? {cursor: 'default'} : undefined}
>
<TableHeadCellContent align="center" gap="sm" onClick={updateSort}>
<Tooltip showOnlyOnOverflow title={label}>
{label}
</Tooltip>
Expand Down Expand Up @@ -160,64 +244,11 @@ export function SamplesTab({metricName}: SamplesTabProps) {

return (
<TableRow key={i}>
{displayFields.map((field, j) => {
if (field === 'trace') {
const timestamp = row.timestamp as number;
const target = getTraceDetailsUrl({
organization,
traceSlug: traceId,
dateSelection: {
start: selection.datetime.start,
end: selection.datetime.end,
statsPeriod: selection.datetime.period,
},
timestamp: timestamp / 1000,
location,
source: TraceViewSources.TRACES,
});

return (
<StyledTableBodyCell key={j}>
<Link to={target} style={{minWidth: '66px'}}>
{getShortEventId(traceId)}
</Link>
</StyledTableBodyCell>
);
}

if (field === 'logs') {
return (
<StyledTableBodyCell key={j} align="right">
{telemetry?.logsCount ?? 0}
</StyledTableBodyCell>
);
}

if (field === 'spans') {
return (
<StyledTableBodyCell key={j} align="right">
{telemetry?.spansCount ?? 0}
</StyledTableBodyCell>
);
}

// Find the index in original fields array
const originalFieldIndex = fields.indexOf(field);
if (originalFieldIndex === -1) {
return null;
}

return (
<StyledTableBodyCell key={j}>
<FieldRenderer
column={columns[originalFieldIndex]}
data={row}
unit={meta?.units?.[field]}
meta={meta}
/>
</StyledTableBodyCell>
);
})}
{fields.map((field, j) => (
<StyledTableBodyCell key={j}>
{renderFieldCell(field, row, traceId, telemetry)}
</StyledTableBodyCell>
))}
</TableRow>
);
})
Expand Down
Loading