fix(ourlogs): Open log line in drawer (#98134)

k-fish · lzhao-sentry · commit 6634032a5d3b · 2025-08-25T12:58:52.000-04:00
### Summary This auto-expands the clicked log line when clicking from the issues section. Also in this PR is a style fix for the refetch spinners since embedded views don't have headers. <img width="786" height="175" alt="Screenshot 2025-08-22 at 1 28 18 PM" src="https://github.com/user-attachments/assets/a66ff7d1-11c4-415e-8ec0-54e55ae810cb" /> LOGS-224
diff --git a/static/app/components/events/ourlogs/ourlogsDrawer.tsx b/static/app/components/events/ourlogs/ourlogsDrawer.tsx
@@ -37,9 +37,17 @@ interface LogIssueDrawerProps {
   event: Event;
   group: Group;
   project: Project;
+  embeddedOptions?: {
+    openWithExpandedIds?: string[];
+  };
 }
 
-export function OurlogsDrawer({event, project, group}: LogIssueDrawerProps) {
+export function OurlogsDrawer({
+  event,
+  project,
+  group,
+  embeddedOptions,
+}: LogIssueDrawerProps) {
   const setLogsSearch = useSetLogsSearch();
   const logsSearch = useLogsSearch();
   const hasInfiniteFeature = useOrganization().features.includes(
@@ -90,7 +98,11 @@ export function OurlogsDrawer({event, project, group}: LogIssueDrawerProps) {
         <EventDrawerBody ref={containerRef}>
           <LogsTableContainer>
             {hasInfiniteFeature ? (
-              <LogsInfiniteTable embedded scrollContainer={containerRef} />
+              <LogsInfiniteTable
+                embedded
+                scrollContainer={containerRef}
+                embeddedOptions={embeddedOptions}
+              />
             ) : (
               <LogsTable allowPagination embedded />
             )}
diff --git a/static/app/components/events/ourlogs/ourlogsSection.spec.tsx b/static/app/components/events/ourlogs/ourlogsSection.spec.tsx
@@ -1,16 +1,60 @@
 import {EventFixture} from 'sentry-fixture/event';
 import {GroupFixture} from 'sentry-fixture/group';
+import {LogFixture} from 'sentry-fixture/log';
 import {OrganizationFixture} from 'sentry-fixture/organization';
 import {ProjectFixture} from 'sentry-fixture/project';
 
-import {render, screen, userEvent, within} from 'sentry-test/reactTestingLibrary';
+import {
+  render,
+  screen,
+  userEvent,
+  waitFor,
+  within,
+} from 'sentry-test/reactTestingLibrary';
 
 import {OurlogsSection} from 'sentry/components/events/ourlogs/ourlogsSection';
+import PageFiltersStore from 'sentry/stores/pageFiltersStore';
+import ProjectsStore from 'sentry/stores/projectsStore';
 import {OurLogKnownFieldKey} from 'sentry/views/explore/logs/types';
 
 const TRACE_ID = '00000000000000000000000000000000';
 
-const organization = OrganizationFixture({features: ['ourlogs-enabled']});
+jest.mock('@tanstack/react-virtual', () => {
+  return {
+    useWindowVirtualizer: jest.fn().mockReturnValue({
+      getVirtualItems: jest.fn().mockReturnValue([
+        {key: '1', index: 0, start: 0, end: 50, lane: 0},
+        {key: '2', index: 1, start: 50, end: 100, lane: 0},
+        {key: '3', index: 2, start: 100, end: 150, lane: 0},
+      ]),
+      getTotalSize: jest.fn().mockReturnValue(150),
+      options: {
+        scrollMargin: 0,
+      },
+      scrollDirection: 'forward',
+      scrollOffset: 0,
+      isScrolling: false,
+    }),
+    useVirtualizer: jest.fn().mockReturnValue({
+      getVirtualItems: jest.fn().mockReturnValue([
+        {key: '1', index: 0, start: 0, end: 50, lane: 0},
+        {key: '2', index: 1, start: 50, end: 100, lane: 0},
+        {key: '3', index: 2, start: 100, end: 150, lane: 0},
+      ]),
+      getTotalSize: jest.fn().mockReturnValue(150),
+      options: {
+        scrollMargin: 0,
+      },
+      scrollDirection: 'forward',
+      scrollOffset: 0,
+      isScrolling: false,
+    }),
+  };
+});
+
+const organization = OrganizationFixture({
+  features: ['ourlogs-enabled', 'ourlogs-infinite-scroll'],
+});
 const project = ProjectFixture();
 const group = GroupFixture();
 const event = EventFixture({
@@ -65,13 +109,69 @@ const event = EventFixture({
 });
 
 describe('OurlogsSection', () => {
+  let logId: string;
+  let mockRequest: jest.Mock;
   beforeEach(() => {
-    // the search query combobox is firing updates and causing console.errors
-    jest.spyOn(console, 'error').mockImplementation(() => {});
+    logId = '11111111111111111111111111111111';
+
+    ProjectsStore.loadInitialData([project]);
+
+    PageFiltersStore.init();
+    PageFiltersStore.onInitializeUrlState(
+      {
+        projects: [parseInt(project.id, 10)],
+        environments: [],
+        datetime: {
+          period: '14d',
+          start: null,
+          end: null,
+          utc: null,
+        },
+      },
+      new Set()
+    );
+
+    MockApiClient.addMockResponse({
+      url: `/projects/`,
+      body: [project],
+    });
+
+    MockApiClient.addMockResponse({
+      url: `/projects/${organization.slug}/${project.slug}/`,
+      body: project,
+    });
+
+    mockRequest = MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/trace-logs/`,
+      body: {
+        data: [
+          LogFixture({
+            [OurLogKnownFieldKey.ID]: logId,
+            [OurLogKnownFieldKey.PROJECT_ID]: project.id,
+            [OurLogKnownFieldKey.ORGANIZATION_ID]: Number(organization.id),
+            [OurLogKnownFieldKey.TRACE_ID]: TRACE_ID,
+            [OurLogKnownFieldKey.SEVERITY_NUMBER]: 0,
+            [OurLogKnownFieldKey.MESSAGE]: 'i am a log',
+          }),
+        ],
+        meta: {},
+      },
+    });
+
+    MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/trace-items/attributes/`,
+      method: 'GET',
+      body: {},
+    });
+
+    MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/recent-searches/',
+      body: [],
+    });
   });
 
   it('renders empty', () => {
-    const mockRequest = MockApiClient.addMockResponse({
+    const mockRequestEmpty = MockApiClient.addMockResponse({
       url: `/organizations/${organization.slug}/trace-logs/`,
       body: {
         data: [],
@@ -81,44 +181,40 @@ describe('OurlogsSection', () => {
     render(<OurlogsSection event={event} project={project} group={group} />, {
       organization,
     });
-    expect(mockRequest).toHaveBeenCalledTimes(1);
+    expect(mockRequestEmpty).toHaveBeenCalledTimes(1);
     expect(screen.queryByText(/Logs/)).not.toBeInTheDocument();
   });
 
   it('renders logs', async () => {
-    const now = new Date();
-    const mockRequest = MockApiClient.addMockResponse({
-      url: `/organizations/${organization.slug}/trace-logs/`,
+    const mockRowDetailsRequest = MockApiClient.addMockResponse({
+      url: `/projects/${organization.slug}/${project.slug}/trace-items/${logId}/`,
+      method: 'GET',
       body: {
-        data: [
-          {
-            'sentry.item_id': '11111111111111111111111111111111',
-            'project.id': 1,
-            trace: TRACE_ID,
-            severity_number: 0,
-            severity: 'info',
-            timestamp: now.toISOString(),
-            [OurLogKnownFieldKey.TIMESTAMP_PRECISE]: now.getTime() * 1e6,
-            message: 'i am a log',
-          },
+        itemId: logId,
+        timestamp: '2025-04-03T15:50:10+00:00',
+        attributes: [
+          {name: 'severity', type: 'str', value: 'error'},
+          {name: 'special_field', type: 'str', value: 'special value'},
         ],
-        meta: {},
       },
     });
+
     render(<OurlogsSection event={event} project={project} group={group} />, {
       organization,
+      initialRouterConfig: {
+        location: {
+          pathname: `/organizations/${organization.slug}/issues/${group.id}/`,
+          query: {
+            project: project.id,
+          },
+        },
+      },
     });
     expect(mockRequest).toHaveBeenCalledTimes(1);
 
-    // without waiting a few ticks, the test fails just before the
-    // promise corresponding to the request resolves
-    // by adding some ticks, it forces the test to wait a little longer
-    // until the promise is resolved
-    for (let i = 0; i < 10; i++) {
-      await tick();
-    }
-
-    expect(screen.getByText(/i am a log/)).toBeInTheDocument();
+    await waitFor(() => {
+      expect(screen.getByText(/i am a log/)).toBeInTheDocument();
+    });
 
     expect(
       screen.queryByRole('complementary', {name: 'logs drawer'})
@@ -129,6 +225,15 @@ describe('OurlogsSection', () => {
     const aside = screen.getByRole('complementary', {name: 'logs drawer'});
     expect(aside).toBeInTheDocument();
 
-    expect(within(aside).getByText(/i am a log/)).toBeInTheDocument();
+    await waitFor(() => {
+      expect(mockRowDetailsRequest).toHaveBeenCalledTimes(1);
+    });
+
+    await waitFor(() => {
+      expect(within(aside).getByText(/special value/)).toBeInTheDocument();
+    });
+
+    expect(within(aside).getByTestId('tree-key-severity')).toBeInTheDocument();
+    expect(within(aside).getByTestId('tree-key-severity')).toHaveTextContent('severity');
   });
 });
diff --git a/static/app/components/events/ourlogs/ourlogsSection.tsx b/static/app/components/events/ourlogs/ourlogsSection.tsx
@@ -72,7 +72,7 @@ function OurlogsSectionContent({
 
   const limitToTraceId = event.contexts?.trace?.trace_id;
   const onOpenLogsDrawer = useCallback(
-    (e: React.MouseEvent<HTMLDivElement | HTMLButtonElement>) => {
+    (e: React.MouseEvent, expandedLogId?: string) => {
       e.stopPropagation();
       trackAnalytics('logs.issue_details.drawer_opened', {
         organization,
@@ -87,7 +87,14 @@ function OurlogsSectionContent({
             >
               <LogsPageDataProvider>
                 <TraceItemAttributeProvider traceItemType={TraceItemDataset.LOGS} enabled>
-                  <OurlogsDrawer group={group} event={event} project={project} />
+                  <OurlogsDrawer
+                    group={group}
+                    event={event}
+                    project={project}
+                    embeddedOptions={
+                      expandedLogId ? {openWithExpandedIds: [expandedLogId]} : undefined
+                    }
+                  />
                 </TraceItemAttributeProvider>
               </LogsPageDataProvider>
             </LogsPageParamsProvider>
@@ -106,6 +113,13 @@ function OurlogsSectionContent({
     },
     [group, event, project, openDrawer, organization, limitToTraceId]
   );
+
+  const onEmbeddedRowClick = useCallback(
+    (logItemId: string, clickEvent: React.MouseEvent) => {
+      onOpenLogsDrawer(clickEvent, logItemId);
+    },
+    [onOpenLogsDrawer]
+  );
   if (!feature) {
     return null;
   }
@@ -125,7 +139,7 @@ function OurlogsSectionContent({
       title={t('Logs')}
       data-test-id="logs-data-section"
     >
-      <SmallTableContentWrapper onClick={onOpenLogsDrawer}>
+      <SmallTableContentWrapper>
         <SmallTable>
           <TableBody>
             {abbreviatedTableData?.map((row, index) => (
@@ -137,6 +151,7 @@ function OurlogsSectionContent({
                 sharedHoverTimeoutRef={sharedHoverTimeoutRef}
                 key={index}
                 blockRowExpanding
+                onEmbeddedRowClick={onEmbeddedRowClick}
               />
             ))}
           </TableBody>
diff --git a/static/app/views/explore/logs/styles.tsx b/static/app/views/explore/logs/styles.tsx
@@ -452,8 +452,8 @@ export const FloatingBackToTopContainer = styled('div')<{
 
 export const HoveringRowLoadingRendererContainer = styled('div')<{
   headerHeight: number;
+  height: number;
   position: 'top' | 'bottom';
-  rowHeight: number;
 }>`
   position: absolute;
   left: 0;
@@ -469,6 +469,6 @@ export const HoveringRowLoadingRendererContainer = styled('div')<{
   );
   align-items: center;
   justify-content: center;
-  height: ${p => p.rowHeight * 3}px;
+  height: ${p => p.height}px;
   ${p => (p.position === 'top' ? 'top: 0px;' : 'bottom: 0px;')}
 `;
diff --git a/static/app/views/explore/logs/tables/logsInfiniteTable.tsx b/static/app/views/explore/logs/tables/logsInfiniteTable.tsx
diff --git a/static/app/views/explore/logs/tables/logsTableRow.tsx b/static/app/views/explore/logs/tables/logsTableRow.tsx
