LogContext: Fix scroll position in upper context group (#56370)

* fix log row being falsely added to context

* do not add to many lines

* fix scroll position in context

* more flexible id comparison

* add original refid to context query

* added missing row comparison

* updated comment

Co-authored-by: Matias Chomicki <matyax@gmail.com>

Co-authored-by: Matias Chomicki <matyax@gmail.com>

public/app/features/logs/components/LogRowContext.tsx

@@ -1,11 +1,12 @@
 import { css, cx } from '@emotion/css';
-import React, { useRef, useState, useLayoutEffect, useEffect } from 'react';
+import React, { useEffect, useLayoutEffect, useRef, useState } from 'react';
+import usePrevious from 'react-use/lib/usePrevious';
 
-import { GrafanaTheme2, DataQueryError, LogRowModel, textUtil, LogsSortOrder } from '@grafana/data';
-import { useStyles2, Alert, ClickOutsideWrapper, CustomScrollbar, List, Button } from '@grafana/ui';
+import { DataQueryError, GrafanaTheme2, LogRowModel, LogsSortOrder, textUtil } from '@grafana/data';
+import { Alert, Button, ClickOutsideWrapper, CustomScrollbar, List, useStyles2 } from '@grafana/ui';
 
 import { LogMessageAnsi } from './LogMessageAnsi';
-import { LogRowContextRows, LogRowContextQueryErrors, HasMoreContextRows } from './LogRowContextProvider';
+import { HasMoreContextRows, LogRowContextQueryErrors, LogRowContextRows } from './LogRowContextProvider';
 
 export enum LogGroupPosition {
   Bottom = 'bottom',
@@ -155,15 +156,48 @@ export const LogRowContextGroup: React.FunctionComponent<LogRowContextGroupProps
 }) => {
   const { commonStyles, logs } = useStyles2(getLogRowContextStyles);
   const [scrollTop, setScrollTop] = useState(0);
-  const listContainerRef = useRef<HTMLDivElement>(null);
+  const [scrollHeight, setScrollHeight] = useState(0);
 
+  const listContainerRef = useRef<HTMLDivElement>(null);
+  const prevRows = usePrevious(rows);
+  const prevScrollTop = usePrevious(scrollTop);
+  const prevScrollHeight = usePrevious(scrollHeight);
+
+  /**
+   * This hook is responsible of keeping the right scroll position of the top
+   * context when rows are added. Since rows are added at the top of the DOM,
+   * the scroll position changes and we need to adjust the scrollTop.
+   */
   useLayoutEffect(() => {
-    // We want to scroll to bottom only when we receive first 10 log lines
-    const shouldScrollRows = rows.length > 0 && rows.length <= 10;
-    if (shouldScrollToBottom && shouldScrollRows && listContainerRef.current) {
-      setScrollTop(listContainerRef.current.offsetHeight);
+    if (!shouldScrollToBottom || !listContainerRef.current) {
+      return;
     }
-  }, [shouldScrollToBottom, rows]);
+
+    const previousRowsLength = prevRows?.length ?? 0;
+    const previousScrollHeight = prevScrollHeight ?? 0;
+    const previousScrollTop = prevScrollTop ?? 0;
+    const scrollElement = listContainerRef.current.parentElement;
+    let currentScrollHeight = 0;
+
+    if (scrollElement) {
+      currentScrollHeight = scrollElement.scrollHeight - scrollElement.clientHeight;
+      setScrollHeight(currentScrollHeight);
+    }
+
+    if (rows.length > previousRowsLength && currentScrollHeight > previousScrollHeight) {
+      setScrollTop(previousScrollTop + (currentScrollHeight - previousScrollHeight));
+    }
+  }, [shouldScrollToBottom, rows, prevRows, prevScrollTop, prevScrollHeight]);
+
+  /**
+   * Keeps track of the scroll position of the list container.
+   */
+  const updateScroll = () => {
+    const scrollElement = listContainerRef.current?.parentElement;
+    if (scrollElement) {
+      setScrollTop(listContainerRef.current?.parentElement.scrollTop);
+    }
+  };
 
   const headerProps = {
     row,
@@ -179,7 +213,7 @@ export const LogRowContextGroup: React.FunctionComponent<LogRowContextGroupProps
       {/* When displaying "after" context */}
       {shouldScrollToBottom && !error && <LogRowContextGroupHeader {...headerProps} />}
       <div className={logs}>
-        <CustomScrollbar autoHide scrollTop={scrollTop} autoHeightMin={'210px'}>
+        <CustomScrollbar autoHide onScroll={updateScroll} scrollTop={scrollTop} autoHeightMin={'210px'}>
           <div ref={listContainerRef}>
             {!error && (
               <List

public/app/features/logs/components/LogRowContextProvider.tsx

@@ -1,14 +1,14 @@
-import React, { useState, useEffect } from 'react';
+import React, { useEffect, useState } from 'react';
 import useAsync from 'react-use/lib/useAsync';
 
 import {
-  LogRowModel,
-  toDataFrame,
+  DataQueryError,
+  DataQueryResponse,
   Field,
   FieldCache,
+  LogRowModel,
   LogsSortOrder,
-  DataQueryResponse,
-  DataQueryError,
+  toDataFrame,
 } from '@grafana/data';
 
 export interface RowContextOptions {
@@ -93,7 +93,7 @@ export const getRowContexts = async (
           // ns which came before but they come in the response that search for logs after. This means right now
           // we will show those as if they came after. This is not strictly correct but seems better than losing them
           // and making this correct would mean quite a bit of complexity to shuffle things around and messing up
-          //counts.
+          // counts.
           if (idField.values.get(fieldIndex) === row.uid) {
             continue;
           }
@@ -109,9 +109,12 @@ export const getRowContexts = async (
         const lineField: Field<string> = dataFrame.fields.filter((field) => field.name === 'line')[0];
         const line = lineField.values.get(fieldIndex); // assuming that both fields have same length
 
+        // since we request limit+1 logs, we occasionally get one extra log in the response
+        if (data.length < limit) {
           data.push(line);
         }
       }
+    }
 
     return logsSortOrder === LogsSortOrder.Ascending ? data.reverse() : data;
   });

public/app/plugins/datasource/loki/datasource.ts

@@ -590,7 +590,7 @@ export class LokiDatasource
     const query: LokiQuery = {
       expr: `{${expr}}`,
       queryType: LokiQueryType.Range,
-      refId: '',
+      refId: row.dataFrame.refId ?? '',
       maxLines: limit,
       direction: queryDirection,
     };