Skip to content

Commit e033a48

Browse files
authored
fix(ourlogs): Add drawer QP and fix scroll (#103979)
### Summary This fixes draw scrollTo for log exceptions to not infinitely scroll, and fixes both metrics and logs to have a query param to have it open for linking.
1 parent 0c187ec commit e033a48

File tree

6 files changed

+139
-25
lines changed

6 files changed

+139
-25
lines changed

static/app/components/events/metrics/metricsSection.tsx

Lines changed: 38 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {useCallback, useRef} from 'react';
1+
import {useCallback, useEffect, useRef} from 'react';
22

33
import {Flex} from '@sentry/scraps/layout/flex';
44

@@ -12,8 +12,11 @@ import type {Event} from 'sentry/types/event';
1212
import type {Group} from 'sentry/types/group';
1313
import type {Project} from 'sentry/types/project';
1414
import {trackAnalytics} from 'sentry/utils/analytics';
15+
import {useLocation} from 'sentry/utils/useLocation';
16+
import {useNavigate} from 'sentry/utils/useNavigate';
1517
import useOrganization from 'sentry/utils/useOrganization';
1618
import {TraceItemAttributeProvider} from 'sentry/views/explore/contexts/traceItemAttributeContext';
19+
import {METRICS_DRAWER_QUERY_PARAM} from 'sentry/views/explore/metrics/constants';
1720
import {MetricsSamplesTable} from 'sentry/views/explore/metrics/metricInfoTabs/metricsSamplesTable';
1821
import {canUseMetricsUI} from 'sentry/views/explore/metrics/metricsFlags';
1922
import {TraceItemDataset} from 'sentry/views/explore/types';
@@ -68,6 +71,8 @@ function MetricsSectionContent({
6871
traceId: string;
6972
}) {
7073
const organization = useOrganization();
74+
const navigate = useNavigate();
75+
const location = useLocation();
7176
const {openDrawer} = useDrawer();
7277
const viewAllButtonRef = useRef<HTMLButtonElement>(null);
7378
const {result, error} = useMetricsIssueSection({traceId});
@@ -81,6 +86,24 @@ function MetricsSectionContent({
8186
trackAnalytics('metrics.issue_details.drawer_opened', {
8287
organization,
8388
});
89+
90+
navigate(
91+
{
92+
...location,
93+
query: {
94+
...location.query,
95+
[METRICS_DRAWER_QUERY_PARAM]: 'true',
96+
},
97+
},
98+
{replace: true}
99+
);
100+
},
101+
[navigate, location, organization]
102+
);
103+
104+
useEffect(() => {
105+
const shouldOpenDrawer = location.query[METRICS_DRAWER_QUERY_PARAM] === 'true';
106+
if (shouldOpenDrawer) {
84107
openDrawer(
85108
() => (
86109
<TraceViewMetricsProviderWrapper traceSlug={traceId}>
@@ -99,14 +122,24 @@ function MetricsSectionContent({
99122
const viewAllButton = viewAllButtonRef.current;
100123
return !viewAllButton?.contains(element);
101124
},
125+
onClose: () => {
126+
navigate(
127+
{
128+
...location,
129+
query: {
130+
...location.query,
131+
[METRICS_DRAWER_QUERY_PARAM]: undefined,
132+
},
133+
},
134+
{replace: true}
135+
);
136+
},
102137
}
103138
);
104-
},
105-
[group, event, project, openDrawer, organization, traceId]
106-
);
139+
}
140+
}, [location.query, traceId, group, event, project, openDrawer, navigate, location]);
107141

108142
if (!result.data || result.data.length === 0 || error) {
109-
// Don't show the metrics section if there are no metrics
110143
return null;
111144
}
112145

static/app/components/events/ourlogs/ourlogsDrawer.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ interface LogIssueDrawerProps {
4343
event: Event;
4444
group: Group;
4545
project: Project;
46+
additionalData?: {
47+
event?: Event;
48+
scrollToDisabled?: boolean;
49+
};
4650
embeddedOptions?: {
4751
openWithExpandedIds?: string[];
4852
};
@@ -53,6 +57,7 @@ export function OurlogsDrawer({
5357
project,
5458
group,
5559
embeddedOptions,
60+
additionalData: propAdditionalData,
5661
}: LogIssueDrawerProps) {
5762
const organization = useOrganization();
5863
const setLogsQuery = useSetQueryParamsQuery();
@@ -81,8 +86,9 @@ export function OurlogsDrawer({
8186
const additionalData = useMemo(
8287
() => ({
8388
event,
89+
scrollToDisabled: propAdditionalData?.scrollToDisabled,
8490
}),
85-
[event]
91+
[event, propAdditionalData?.scrollToDisabled]
8692
);
8793

8894
const exploreUrl = useMemo(() => {

static/app/components/events/ourlogs/ourlogsSection.tsx

Lines changed: 53 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {useCallback, useRef} from 'react';
1+
import {useCallback, useEffect, useRef} from 'react';
22
import styled from '@emotion/styled';
33

44
import {Button} from 'sentry/components/core/button';
@@ -11,13 +11,16 @@ import type {Group} from 'sentry/types/group';
1111
import type {Project} from 'sentry/types/project';
1212
import {trackAnalytics} from 'sentry/utils/analytics';
1313
import {LogsAnalyticsPageSource} from 'sentry/utils/analytics/logsAnalyticsEvent';
14+
import {useLocation} from 'sentry/utils/useLocation';
15+
import {useNavigate} from 'sentry/utils/useNavigate';
1416
import useOrganization from 'sentry/utils/useOrganization';
1517
import {TableBody} from 'sentry/views/explore/components/table';
1618
import {
1719
LogsPageDataProvider,
1820
useLogsPageDataQueryResult,
1921
} from 'sentry/views/explore/contexts/logs/logsPageData';
2022
import {TraceItemAttributeProvider} from 'sentry/views/explore/contexts/traceItemAttributeContext';
23+
import {LOGS_DRAWER_QUERY_PARAM} from 'sentry/views/explore/logs/constants';
2124
import {LogsQueryParamsProvider} from 'sentry/views/explore/logs/logsQueryParamsProvider';
2225
import {LogRowContent} from 'sentry/views/explore/logs/tables/logsTableRow';
2326
import {useQueryParamsSearch} from 'sentry/views/explore/queryParams/context';
@@ -58,6 +61,8 @@ function OurlogsSectionContent({
5861
project: Project;
5962
}) {
6063
const organization = useOrganization();
64+
const navigate = useNavigate();
65+
const location = useLocation();
6166
const feature = organization.features.includes('ourlogs-enabled');
6267
const tableData = useLogsPageDataQueryResult();
6368
const logsSearch = useQueryParamsSearch();
@@ -73,6 +78,34 @@ function OurlogsSectionContent({
7378
trackAnalytics('logs.issue_details.drawer_opened', {
7479
organization,
7580
});
81+
82+
navigate(
83+
{
84+
...location,
85+
query: {
86+
...location.query,
87+
[LOGS_DRAWER_QUERY_PARAM]: 'true',
88+
...(expandedLogId && {expandedLogId}),
89+
},
90+
},
91+
{replace: true}
92+
);
93+
},
94+
[navigate, location, organization]
95+
);
96+
97+
const onEmbeddedRowClick = useCallback(
98+
(logItemId: string, clickEvent: React.MouseEvent) => {
99+
onOpenLogsDrawer(clickEvent, logItemId);
100+
},
101+
[onOpenLogsDrawer]
102+
);
103+
104+
useEffect(() => {
105+
const shouldOpenDrawer = location.query[LOGS_DRAWER_QUERY_PARAM] === 'true';
106+
if (shouldOpenDrawer && traceId) {
107+
const expandedLogId = location.query.expandedLogId as string | undefined;
108+
76109
openDrawer(
77110
() => (
78111
<LogsQueryParamsProvider
@@ -89,6 +122,10 @@ function OurlogsSectionContent({
89122
embeddedOptions={
90123
expandedLogId ? {openWithExpandedIds: [expandedLogId]} : undefined
91124
}
125+
additionalData={{
126+
event,
127+
scrollToDisabled: !!expandedLogId,
128+
}}
92129
/>
93130
</TraceItemAttributeProvider>
94131
</LogsPageDataProvider>
@@ -97,23 +134,27 @@ function OurlogsSectionContent({
97134
{
98135
ariaLabel: 'logs drawer',
99136
drawerKey: 'logs-issue-drawer',
100-
101137
shouldCloseOnInteractOutside: element => {
102138
const viewAllButton = viewAllButtonRef.current;
103139
return !viewAllButton?.contains(element);
104140
},
141+
onClose: () => {
142+
navigate(
143+
{
144+
...location,
145+
query: {
146+
...location.query,
147+
[LOGS_DRAWER_QUERY_PARAM]: undefined,
148+
expandedLogId: undefined,
149+
},
150+
},
151+
{replace: true}
152+
);
153+
},
105154
}
106155
);
107-
},
108-
[group, event, project, openDrawer, organization, traceId]
109-
);
110-
111-
const onEmbeddedRowClick = useCallback(
112-
(logItemId: string, clickEvent: React.MouseEvent) => {
113-
onOpenLogsDrawer(clickEvent, logItemId);
114-
},
115-
[onOpenLogsDrawer]
116-
);
156+
}
157+
}, [location.query, traceId, group, event, project, openDrawer, navigate, location]);
117158
if (!feature) {
118159
return null;
119160
}

static/app/views/explore/logs/constants.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,12 @@ export const LOGS_INSTRUCTIONS_URL =
7979

8080
export const LOGS_FILTER_KEY_SECTIONS: FilterKeySection[] = [LOGS_FILTERS];
8181

82+
/**
83+
* Query parameter key for controlling the logs drawer state.
84+
* When this parameter is set to 'true', the logs drawer should open automatically.
85+
*/
86+
export const LOGS_DRAWER_QUERY_PARAM = 'logsDrawer';
87+
8288
export const VIRTUAL_STREAMED_INTERVAL_MS = 250;
8389
export const MINIMUM_INFINITE_SCROLL_FETCH_COOLDOWN_MS = 1000;
8490

static/app/views/explore/logs/tables/logsInfiniteTable.tsx

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type {CSSProperties} from 'react';
1+
import type {CSSProperties, RefObject} from 'react';
22
import {Fragment, useCallback, useEffect, useMemo, useRef, useState} from 'react';
33
import {useTheme} from '@emotion/react';
44
import styled from '@emotion/styled';
@@ -74,6 +74,7 @@ import {EmptyStateText} from 'sentry/views/explore/tables/tracesTable/styles';
7474
type LogsTableProps = {
7575
additionalData?: {
7676
event?: Event;
77+
scrollToDisabled?: boolean;
7778
};
7879
allowPagination?: boolean;
7980
embedded?: boolean;
@@ -132,6 +133,7 @@ export function LogsInfiniteTable({
132133
} = useLogsPageDataQueryResult();
133134

134135
const baseData = localOnlyItemFilters?.filteredItems ?? originalData;
136+
const baseDataLength = useBox(baseData.length);
135137

136138
const pseudoRowIndex = useMemo(() => {
137139
if (
@@ -149,7 +151,7 @@ export function LogsInfiniteTable({
149151
row =>
150152
isRegularLogResponseItem(row) && getLogRowTimestampMillis(row) < eventTimestamp
151153
);
152-
return index === -1 ? baseData.length : index; // If the event is older than all the data, add it to the end.
154+
return index === -1 ? -2 : index; // If the event is older than all the data, add it to the end with a sentinel value of -2. This causes the useEffect to not continously add it.
153155
}, [additionalData, baseData, isPending, isError]);
154156

155157
const data: LogTableRowItem[] = useMemo(() => {
@@ -165,16 +167,18 @@ export function LogsInfiniteTable({
165167
}
166168

167169
const newData: LogTableRowItem[] = [...baseData];
170+
const newSelectedIndex =
171+
pseudoRowIndex === -2 ? baseDataLength.current : pseudoRowIndex;
168172
newData.splice(
169-
pseudoRowIndex,
173+
newSelectedIndex,
170174
0,
171175
createPseudoLogResponseItem(
172176
additionalData.event,
173177
additionalData.event.projectID || ''
174178
)
175179
);
176180
return newData;
177-
}, [baseData, additionalData, isPending, isError, pseudoRowIndex]);
181+
}, [baseData, additionalData, isPending, isError, pseudoRowIndex, baseDataLength]);
178182

179183
// Calculate quantized start and end times for replay links
180184
const {logStart, logEnd} = useMemo(() => {
@@ -280,15 +284,27 @@ export function LogsInfiniteTable({
280284
);
281285

282286
useEffect(() => {
283-
if (pseudoRowIndex !== -1 && scrollContainer?.current) {
287+
if (
288+
pseudoRowIndex !== -1 &&
289+
scrollContainer?.current &&
290+
!additionalData?.scrollToDisabled
291+
) {
284292
setTimeout(() => {
285-
containerVirtualizer.scrollToIndex(pseudoRowIndex, {
293+
const scrollToIndex =
294+
pseudoRowIndex === -2 ? baseDataLength.current : pseudoRowIndex;
295+
containerVirtualizer.scrollToIndex(scrollToIndex, {
286296
behavior: 'smooth',
287297
align: 'center',
288298
});
289299
}, 100);
290300
}
291-
}, [pseudoRowIndex, containerVirtualizer, scrollContainer]);
301+
}, [
302+
pseudoRowIndex,
303+
containerVirtualizer,
304+
scrollContainer,
305+
baseDataLength,
306+
additionalData?.scrollToDisabled,
307+
]);
292308

293309
const hasReplay = !!embeddedOptions?.replay;
294310

@@ -773,3 +789,9 @@ function BackToTopButton({
773789
</Button>
774790
);
775791
}
792+
793+
function useBox<T>(value: T): RefObject<T> {
794+
const box = useRef(value);
795+
box.current = value;
796+
return box;
797+
}

static/app/views/explore/metrics/constants.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,3 +179,9 @@ export const DEFAULT_YAXIS_BY_TYPE: Record<string, string> = {
179179
distribution: 'p75',
180180
gauge: 'avg',
181181
};
182+
183+
/**
184+
* Query parameter key for controlling the metrics drawer state.
185+
* When this parameter is set to 'true', the metrics drawer should open automatically.
186+
*/
187+
export const METRICS_DRAWER_QUERY_PARAM = 'metricsDrawer';

0 commit comments

Comments
 (0)