Skip to content

feat(logs): Without chart data when table is empty #103930

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 1 commit into from
Nov 25, 2025
Merged

feat(logs): Without chart data when table is empty #103930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 9 additions & 0 deletions static/app/views/explore/logs/confidenceFooter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ interface ConfidenceFooterProps {
hasUserQuery: boolean;
isLoading: boolean;
rawLogCounts: RawCounts;
disabled?: boolean;
}

export function ConfidenceFooter({
chartInfo,
hasUserQuery,
isLoading,
rawLogCounts,
disabled,
}: ConfidenceFooterProps) {
return (
<Container>
Expand All @@ -35,6 +37,7 @@ export function ConfidenceFooter({
isSampled={chartInfo.isSampled}
sampleCount={chartInfo.sampleCount}
topEvents={chartInfo.topEvents}
disabled={disabled}
/>
</Container>
);
Expand All @@ -46,6 +49,7 @@ interface ConfidenceMessageProps {
rawLogCounts: RawCounts;
confidence?: Confidence;
dataScanned?: 'full' | 'partial';
disabled?: boolean;
isSampled?: boolean | null;
sampleCount?: number;
topEvents?: number;
Expand All @@ -56,11 +60,16 @@ function ConfidenceMessage({
sampleCount,
dataScanned,
confidence: _confidence,
disabled,
topEvents,
hasUserQuery,
isLoading,
isSampled,
}: ConfidenceMessageProps) {
if (disabled) {
return <Placeholder width={0} />;
}

if (isLoading || !defined(sampleCount)) {
return <Placeholder width={180} />;
}
Expand Down
32 changes: 27 additions & 5 deletions static/app/views/explore/logs/logsGraph.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import {
import {Widget} from 'sentry/views/dashboards/widgets/widget/widget';
import {handleAddQueryToDashboard} from 'sentry/views/discover/utils';
import {ChartVisualization} from 'sentry/views/explore/components/chart/chartVisualization';
import type {ChartInfo} from 'sentry/views/explore/components/chart/types';
import {useLogsPageDataQueryResult} from 'sentry/views/explore/contexts/logs/logsPageData';
import {formatSort} from 'sentry/views/explore/contexts/pageParamsContext/sortBys';
import {
ChartIntervalUnspecifiedStrategy,
Expand Down Expand Up @@ -119,6 +121,8 @@ function Graph({
timeseriesResult,
visualize,
}: GraphProps) {
const {isEmpty: tableIsEmpty, isPending: tableIsPending} = useLogsPageDataQueryResult();

const aggregate = visualize.yAxis;
const userQuery = useQueryParamsQuery();
const topEventsLimit = useQueryParamsTopEventsLimit();
Expand All @@ -127,14 +131,23 @@ function Graph({
unspecifiedStrategy: ChartIntervalUnspecifiedStrategy.USE_SMALLEST,
});

const chartInfo = useMemo(() => {
const series = timeseriesResult.data[aggregate] ?? [];
const chartInfo: ChartInfo = useMemo(() => {
// If the table is empty or pending, we want to withhold the chart data.
// This is to avoid a state where there is data in the chart but not in
// the table which is very weird. By withholding the chart data, we create
// the illusion the 2 are being queries in sync.
const withholdData = tableIsEmpty || tableIsPending;
Copy link
Contributor

@cursor cursor bot Nov 24, 2025

Choose a reason for hiding this comment

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

Bug: Chart data not withheld during Continue Scanning

When the user clicks Continue Scanning with existing table data, the chart data isn't withheld as intended. The isPending property from useLogsPageDataQueryResult only includes isFetchingNextPage when data is empty. With existing data, isPending remains false during pagination, so withholdData becomes false, showing chart data while the table loads more results. This contradicts the PR's goal to withhold chart data during Continue Scanning operations. The code needs to also check isFetchingNextPage in the withholdData calculation.

Fix in Cursor Fix in Web


const series = withholdData ? [] : (timeseriesResult.data[aggregate] ?? []);
const isTopEvents = defined(topEventsLimit);
const samplingMeta = determineSeriesSampleCountAndIsSampled(series, isTopEvents);
return {
chartType: visualize.chartType,
series,
timeseriesResult,
timeseriesResult: {
...timeseriesResult,
isPending: timeseriesResult.isPending || tableIsPending,
} as ChartInfo['timeseriesResult'],
Copy link
Member Author

@Zylphrex Zylphrex Nov 24, 2025

Choose a reason for hiding this comment

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

Getting a weird type error here about not able to assign boolean to false here for some reason.

yAxis: aggregate,
confidence: combineConfidenceForSeries(series),
dataScanned: samplingMeta.dataScanned,
Expand All @@ -143,7 +156,14 @@ function Graph({
samplingMode: undefined,
topEvents: isTopEvents ? TOP_EVENTS_LIMIT : undefined,
};
}, [visualize.chartType, timeseriesResult, aggregate, topEventsLimit]);
}, [
visualize.chartType,
timeseriesResult,
aggregate,
topEventsLimit,
tableIsEmpty,
tableIsPending,
]);

const Title = (
<Widget.WidgetTitle title={prettifyAggregation(aggregate) ?? aggregate} />
Expand Down Expand Up @@ -204,9 +224,11 @@ function Graph({
visualize.visible && (
<ConfidenceFooter
chartInfo={chartInfo}
isLoading={timeseriesResult.isLoading}
// hold off on showing the chart while the table is loading
isLoading={timeseriesResult.isLoading || tableIsPending}
rawLogCounts={rawLogCounts}
hasUserQuery={!!userQuery}
disabled={tableIsPending ? false : tableIsEmpty}
/>
)
}
Expand Down
Loading