Review feedback

- fixed typo
- moved result calculation to explore utils
- use component keys instead of componentWillReceiveProps
- require logs to have and id, to as use as Logs component key
- render delay based on row count

public/app/core/logs_model.ts

@@ -58,6 +58,7 @@ export interface LogsMetaItem {
 }
 
 export interface LogsModel {
+  id: string; // Identify one logs result from another
   meta?: LogsMetaItem[];
   rows: LogRow[];
   series?: TimeSeries[];

public/app/core/utils/explore.ts

@@ -1,7 +1,10 @@
+import _ from 'lodash';
+
 import { renderUrl } from 'app/core/utils/url';
-import { ExploreState, ExploreUrlState, HistoryItem } from 'app/types/explore';
+import { ExploreState, ExploreUrlState, HistoryItem, QueryTransaction } from 'app/types/explore';
 import { DataQuery, RawTimeRange } from 'app/types/series';
 
+import TableModel, { mergeTablesIntoModel } from 'app/core/table_model';
 import kbn from 'app/core/utils/kbn';
 import colors from 'app/core/utils/colors';
 import TimeSeries from 'app/core/time_series2';
@@ -133,6 +136,35 @@ export function hasNonEmptyQuery(queries: DataQuery[]): boolean {
   return queries.some(query => Object.keys(query).length > 2);
 }
 
+export function calculcateResultsFromQueryTransactions(
+  queryTransactions: QueryTransaction[],
+  datasource: any,
+  graphInterval: number
+) {
+  const graphResult = _.flatten(
+    queryTransactions.filter(qt => qt.resultType === 'Graph' && qt.done && qt.result).map(qt => qt.result)
+  );
+  const tableResult = mergeTablesIntoModel(
+    new TableModel(),
+    ...queryTransactions.filter(qt => qt.resultType === 'Table' && qt.done && qt.result).map(qt => qt.result)
+  );
+  const logsResult =
+    datasource && datasource.mergeStreams
+      ? datasource.mergeStreams(
+          _.flatten(
+            queryTransactions.filter(qt => qt.resultType === 'Logs' && qt.done && qt.result).map(qt => qt.result)
+          ),
+          graphInterval
+        )
+      : undefined;
+
+  return {
+    graphResult,
+    tableResult,
+    logsResult,
+  };
+}
+
 export function getIntervals(
   range: RawTimeRange,
   datasource,

public/app/features/explore/Explore.tsx

@@ -16,6 +16,7 @@ import { RawTimeRange, DataQuery } from 'app/types/series';
 import store from 'app/core/store';
 import {
   DEFAULT_RANGE,
+  calculcateResultsFromQueryTransactions,
   ensureQueries,
   getIntervals,
   generateKey,
@@ -28,7 +29,7 @@ import ResetStyles from 'app/core/components/Picker/ResetStyles';
 import PickerOption from 'app/core/components/Picker/PickerOption';
 import IndicatorsContainer from 'app/core/components/Picker/IndicatorsContainer';
 import NoOptionsMessage from 'app/core/components/Picker/NoOptionsMessage';
-import TableModel, { mergeTablesIntoModel } from 'app/core/table_model';
+import TableModel from 'app/core/table_model';
 import { DatasourceSrv } from 'app/features/plugins/datasource_srv';
 
 import Panel from './Panel';
@@ -50,35 +51,6 @@ interface ExploreProps {
   urlState: ExploreUrlState;
 }
 
-function calulcateResultsFromQueryTransactions(
-  queryTransactions: QueryTransaction[],
-  datasource: any,
-  graphInterval: number
-) {
-  const graphResult = _.flatten(
-    queryTransactions.filter(qt => qt.resultType === 'Graph' && qt.done && qt.result).map(qt => qt.result)
-  );
-  const tableResult = mergeTablesIntoModel(
-    new TableModel(),
-    ...queryTransactions.filter(qt => qt.resultType === 'Table' && qt.done && qt.result).map(qt => qt.result)
-  );
-  const logsResult =
-    datasource && datasource.mergeStreams
-      ? datasource.mergeStreams(
-          _.flatten(
-            queryTransactions.filter(qt => qt.resultType === 'Logs' && qt.done && qt.result).map(qt => qt.result)
-          ),
-          graphInterval
-        )
-      : undefined;
-
-  return {
-    graphResult,
-    tableResult,
-    logsResult,
-  };
-}
-
 /**
  * Explore provides an area for quick query iteration for a given datasource.
  * Once a datasource is selected it populates the query section at the top.
@@ -144,6 +116,8 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
       const { datasource, queries, range } = props.urlState as ExploreUrlState;
       initialQueries = ensureQueries(queries);
       const initialRange = range || { ...DEFAULT_RANGE };
+      // Millies step for helper bar charts
+      const initialGraphInterval = 15 * 1000;
       this.state = {
         datasource: null,
         datasourceError: null,
@@ -151,7 +125,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
         datasourceMissing: false,
         datasourceName: datasource,
         exploreDatasources: [],
-        graphInterval: 15 * 1000,
+        graphInterval: initialGraphInterval,
         graphResult: [],
         initialQueries,
         history: [],
@@ -458,7 +432,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
 
         // Toggle off needs discarding of table queries
         const nextQueryTransactions = state.queryTransactions.filter(qt => qt.resultType !== 'Table');
-        const results = calulcateResultsFromQueryTransactions(
+        const results = calculcateResultsFromQueryTransactions(
           nextQueryTransactions,
           state.datasource,
           state.graphInterval
@@ -545,7 +519,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
 
         // Discard transactions related to row query
         const nextQueryTransactions = queryTransactions.filter(qt => qt.rowIndex !== index);
-        const results = calulcateResultsFromQueryTransactions(
+        const results = calculcateResultsFromQueryTransactions(
           nextQueryTransactions,
           state.datasource,
           state.graphInterval
@@ -660,7 +634,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
       // Append new transaction
       const nextQueryTransactions = [...remainingTransactions, transaction];
 
-      const results = calulcateResultsFromQueryTransactions(
+      const results = calculcateResultsFromQueryTransactions(
         nextQueryTransactions,
         state.datasource,
         state.graphInterval
@@ -718,7 +692,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
         return qt;
       });
 
-      const results = calulcateResultsFromQueryTransactions(
+      const results = calculcateResultsFromQueryTransactions(
         nextQueryTransactions,
         state.datasource,
         state.graphInterval
@@ -979,6 +953,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
                       <Panel label="Logs" loading={logsLoading} isOpen={showingLogs} onToggle={this.onClickLogsButton}>
                         <Logs
                           data={logsResult}
+                          key={logsResult.id}
                           loading={logsLoading}
                           position={position}
                           onChangeTime={this.onChangeTime}

public/app/features/explore/Logs.tsx

@@ -19,7 +19,7 @@ import { Switch } from 'app/core/components/Switch/Switch';
 
 import Graph from './Graph';
 
-const RENDER_LIMIT = 100;
+const PREVIEW_LIMIT = 100;
 
 const graphOptions = {
   series: {
@@ -170,20 +170,20 @@ export default class Logs extends PureComponent<LogsProps, LogsState> {
     showUtc: false,
   };
 
-  componentWillReceiveProps(nextProps) {
+  componentDidMount() {
-    // Reset to render minimal only
+    // Staged rendering
-    if (nextProps.data !== this.props.data) {
+    if (this.state.deferLogs) {
-      this.setState({ deferLogs: true, renderAll: false });
+      const { data } = this.props;
+      const rowCount = data && data.rows ? data.rows.length : 0;
+      // Render all right away if not too far over the limit
+      const renderAll = rowCount <= PREVIEW_LIMIT * 2;
+      this.deferLogsTimer = setTimeout(() => this.setState({ deferLogs: false, renderAll }), rowCount);
     }
   }
 
   componentDidUpdate(prevProps, prevState) {
     // Staged rendering
-    if (prevProps.data !== this.props.data && this.state.deferLogs) {
+    if (prevState.deferLogs && !this.state.deferLogs && !this.state.renderAll) {
-      clearTimeout(this.deferLogsTimer);
-      this.deferLogsTimer = setTimeout(() => this.setState({ deferLogs: false }), 1000);
-    } else if (prevState.deferLogs && !this.state.deferLogs) {
-      clearTimeout(this.renderAllTimer);
       this.renderAllTimer = setTimeout(() => this.setState({ renderAll: true }), 2000);
     }
   }
@@ -258,8 +258,8 @@ export default class Logs extends PureComponent<LogsProps, LogsState> {
     }
 
     // Staged rendering
-    const firstRows = dedupedData.rows.slice(0, RENDER_LIMIT);
+    const firstRows = dedupedData.rows.slice(0, PREVIEW_LIMIT);
-    const lastRows = dedupedData.rows.slice(RENDER_LIMIT);
+    const lastRows = dedupedData.rows.slice(PREVIEW_LIMIT);
 
     // Check for labels
     if (showLabels === null) {

public/app/plugins/datasource/logging/result_transformer.ts

@@ -140,6 +140,9 @@ export function processEntry(
 }
 
 export function mergeStreamsToLogs(streams: LogsStream[], limit = DEFAULT_LIMIT): LogsModel {
+  // Unique model identifier
+  const id = streams.map(stream => stream.labels).join();
+
   // Find unique labels for each stream
   streams = streams.map(stream => ({
     ...stream,
@@ -184,6 +187,7 @@ export function mergeStreamsToLogs(streams: LogsStream[], limit = DEFAULT_LIMIT)
   }
 
   return {
+    id,
     meta,
     rows: sortedRows,
   };