Browse Source

Explore: Uses RFC3339Nano string to retrieve LogRow contexts from Loki API (#17813)

* Refactor: Uses nanosecond string to retreive LogRow contexts

* Reafactor: Small changes to comments after PR comments
Hugo Häggmark 6 years ago
parent
commit
2379de53c4

+ 1 - 7
public/app/features/explore/LogRowContext.tsx

@@ -103,12 +103,6 @@ const LogRowContextGroupHeader: React.FunctionComponent<LogRowContextGroupHeader
   const theme = useContext(ThemeContext);
   const theme = useContext(ThemeContext);
   const { header } = getLogRowContextStyles(theme);
   const { header } = getLogRowContextStyles(theme);
 
 
-  // Filtering out the original row from the context.
-  // Loki requires a rowTimestamp+1ns for the following logs to be queried.
-  // We don't to ns-precision calculations in Loki log row context retrieval, hence the filtering here
-  // Also see: https://github.com/grafana/loki/issues/597
-  const logRowsToRender = rows.filter(contextRow => contextRow !== row.raw);
-
   return (
   return (
     <div className={header}>
     <div className={header}>
       <span
       <span
@@ -116,7 +110,7 @@ const LogRowContextGroupHeader: React.FunctionComponent<LogRowContextGroupHeader
           opacity: 0.6;
           opacity: 0.6;
         `}
         `}
       >
       >
-        Found {logRowsToRender.length} rows.
+        Found {rows.length} rows.
       </span>
       </span>
       {(rows.length >= 10 || (rows.length > 10 && rows.length % 10 !== 0)) && canLoadMoreRows && (
       {(rows.length >= 10 || (rows.length > 10 && rows.length % 10 !== 0)) && canLoadMoreRows && (
         <span
         <span

+ 61 - 30
public/app/features/explore/LogRowContextProvider.tsx

@@ -28,54 +28,85 @@ interface LogRowContextProviderProps {
   }) => JSX.Element;
   }) => JSX.Element;
 }
 }
 
 
+export const getRowContexts = async (
+  getRowContext: (row: LogRowModel, options?: any) => Promise<DataQueryResponse>,
+  row: LogRowModel,
+  limit: number
+) => {
+  const promises = [
+    getRowContext(row, {
+      limit,
+    }),
+    getRowContext(row, {
+      limit: limit + 1, // Lets add one more to the limit as we're filtering out one row see comment below
+      direction: 'FORWARD',
+    }),
+  ];
+
+  const results: Array<DataQueryResponse | DataQueryError> = await Promise.all(promises.map(p => p.catch(e => e)));
+
+  return {
+    data: results.map((result, index) => {
+      const dataResult: DataQueryResponse = result as DataQueryResponse;
+      if (!dataResult.data) {
+        return [];
+      }
+
+      // We need to filter out the row we're basing our search from because of how start/end params work in Loki API
+      // see https://github.com/grafana/loki/issues/597#issuecomment-506408980
+      // the alternative to create our own add 1 nanosecond method to the a timestamp string would be quite complex
+      return dataResult.data.map(series => {
+        const filteredRows = series.rows.filter((r: any) => r[0] !== row.timestamp);
+        return filteredRows.map((row: any) => row[1]);
+      });
+    }),
+    errors: results.map(result => {
+      const errorResult: DataQueryError = result as DataQueryError;
+      if (!errorResult.message) {
+        return null;
+      }
+
+      return errorResult.message;
+    }),
+  };
+};
+
 export const LogRowContextProvider: React.FunctionComponent<LogRowContextProviderProps> = ({
 export const LogRowContextProvider: React.FunctionComponent<LogRowContextProviderProps> = ({
   getRowContext,
   getRowContext,
   row,
   row,
   children,
   children,
 }) => {
 }) => {
+  // React Hook that creates a number state value called limit to component state and a setter function called setLimit
+  // The intial value for limit is 10
+  // Used for the number of rows to retrieve from backend from a specific point in time
   const [limit, setLimit] = useState(10);
   const [limit, setLimit] = useState(10);
+
+  // React Hook that creates an object state value called result to component state and a setter function called setResult
+  // The intial value for result is null
+  // Used for sorting the response from backend
   const [result, setResult] = useState<{
   const [result, setResult] = useState<{
     data: string[][];
     data: string[][];
     errors: string[];
     errors: string[];
   }>(null);
   }>(null);
+
+  // React Hook that creates an object state value called hasMoreContextRows to component state and a setter function called setHasMoreContextRows
+  // The intial value for hasMoreContextRows is {before: true, after: true}
+  // Used for indicating in UI if there are more rows to load in a given direction
   const [hasMoreContextRows, setHasMoreContextRows] = useState({
   const [hasMoreContextRows, setHasMoreContextRows] = useState({
     before: true,
     before: true,
     after: true,
     after: true,
   });
   });
 
 
+  // React Hook that resolves two promises every time the limit prop changes
+  // First promise fetches limit number of rows backwards in time from a specific point in time
+  // Second promise fetches limit number of rows forwards in time from a specific point in time
   const { value } = useAsync(async () => {
   const { value } = useAsync(async () => {
-    const promises = [
-      getRowContext(row, {
-        limit,
-      }),
-      getRowContext(row, {
-        limit,
-        direction: 'FORWARD',
-      }),
-    ];
-
-    const results: Array<DataQueryResponse | DataQueryError> = await Promise.all(promises.map(p => p.catch(e => e)));
-
-    return {
-      data: results.map(result => {
-        if ((result as DataQueryResponse).data) {
-          return (result as DataQueryResponse).data.map(series => {
-            return series.rows.map(row => row[1]);
-          });
-        } else {
-          return [];
-        }
-      }),
-      errors: results.map(result => {
-        if ((result as DataQueryError).message) {
-          return (result as DataQueryError).message;
-        } else {
-          return null;
-        }
-      }),
-    };
+    return await getRowContexts(getRowContext, row, limit); // Moved it to a separate function for debugging purposes
   }, [limit]);
   }, [limit]);
 
 
+  // React Hook that performs a side effect every time the value (from useAsync hook) prop changes
+  // The side effect changes the result state with the response from the useAsync hook
+  // The side effect changes the hasMoreContextRows state if there are more context rows before or after the current result
   useEffect(() => {
   useEffect(() => {
     if (value) {
     if (value) {
       setResult(currentResult => {
       setResult(currentResult => {

+ 2 - 2
public/app/plugins/datasource/loki/datasource.ts

@@ -310,13 +310,13 @@ export class LokiDatasource extends DataSourceApi<LokiQuery, LokiOptions> {
       return {
       return {
         ...commontTargetOptons,
         ...commontTargetOptons,
         start: timeEpochNs - contextTimeBuffer,
         start: timeEpochNs - contextTimeBuffer,
-        end: timeEpochNs,
+        end: row.timestamp,
         direction,
         direction,
       };
       };
     } else {
     } else {
       return {
       return {
         ...commontTargetOptons,
         ...commontTargetOptons,
-        start: timeEpochNs, // TODO: We should add 1ns here for the original row not no be included in the result
+        start: row.timestamp, // start param in Loki API is inclusive so we'll have to filter out the row that this request is based from
         end: timeEpochNs + contextTimeBuffer,
         end: timeEpochNs + contextTimeBuffer,
       };
       };
     }
     }