Merge pull request #13491 from grafana/davkal/explore-perf

Explore: typeahead and render performance improvements

public/app/core/utils/explore.test.ts

@@ -7,6 +7,7 @@ const DEFAULT_EXPLORE_STATE: ExploreState = {
   datasourceLoading: null,
   datasourceMissing: false,
   datasourceName: '',
+  exploreDatasources: [],
   graphResult: null,
   history: [],
   latency: 0,

public/app/features/explore/Explore.tsx

@@ -2,7 +2,7 @@ import React from 'react';
 import { hot } from 'react-hot-loader';
 import Select from 'react-select';
 
-import { ExploreState, ExploreUrlState } from 'app/types/explore';
+import { ExploreState, ExploreUrlState, Query } from 'app/types/explore';
 import kbn from 'app/core/utils/kbn';
 import colors from 'app/core/utils/colors';
 import store from 'app/core/store';
@@ -61,37 +61,50 @@ interface ExploreProps {
 
 export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
   el: any;
+  /**
+   * Current query expressions of the rows including their modifications, used for running queries.
+   * Not kept in component state to prevent edit-render roundtrips.
+   */
+  queryExpressions: string[];
 
   constructor(props) {
     super(props);
-    // Split state overrides everything
     const splitState: ExploreState = props.splitState;
-    const { datasource, queries, range } = props.urlState;
-    this.state = {
-      datasource: null,
-      datasourceError: null,
-      datasourceLoading: null,
-      datasourceMissing: false,
-      datasourceName: datasource,
-      graphResult: null,
-      history: [],
-      latency: 0,
-      loading: false,
-      logsResult: null,
-      queries: ensureQueries(queries),
-      queryErrors: [],
-      queryHints: [],
-      range: range || { ...DEFAULT_RANGE },
-      requestOptions: null,
-      showingGraph: true,
-      showingLogs: true,
-      showingTable: true,
-      supportsGraph: null,
-      supportsLogs: null,
-      supportsTable: null,
-      tableResult: null,
-      ...splitState,
-    };
+    let initialQueries: Query[];
+    if (splitState) {
+      // Split state overrides everything
+      this.state = splitState;
+      initialQueries = splitState.queries;
+    } else {
+      const { datasource, queries, range } = props.urlState as ExploreUrlState;
+      initialQueries = ensureQueries(queries);
+      this.state = {
+        datasource: null,
+        datasourceError: null,
+        datasourceLoading: null,
+        datasourceMissing: false,
+        datasourceName: datasource,
+        exploreDatasources: [],
+        graphResult: null,
+        history: [],
+        latency: 0,
+        loading: false,
+        logsResult: null,
+        queries: initialQueries,
+        queryErrors: [],
+        queryHints: [],
+        range: range || { ...DEFAULT_RANGE },
+        requestOptions: null,
+        showingGraph: true,
+        showingLogs: true,
+        showingTable: true,
+        supportsGraph: null,
+        supportsLogs: null,
+        supportsTable: null,
+        tableResult: null,
+      };
+    }
+    this.queryExpressions = initialQueries.map(q => q.query);
   }
 
   async componentDidMount() {
@@ -101,8 +114,13 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
       throw new Error('No datasource service passed as props.');
     }
     const datasources = datasourceSrv.getExploreSources();
+    const exploreDatasources = datasources.map(ds => ({
+      value: ds.name,
+      label: ds.name,
+    }));
+
     if (datasources.length > 0) {
-      this.setState({ datasourceLoading: true });
+      this.setState({ datasourceLoading: true, exploreDatasources });
       // Priority: datasource in url, default datasource, first explore datasource
       let datasource;
       if (datasourceName) {
@@ -146,9 +164,10 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
     }
 
     // Keep queries but reset edit state
-    const nextQueries = this.state.queries.map(q => ({
+    const nextQueries = this.state.queries.map((q, i) => ({
       ...q,
-      edited: false,
+      key: generateQueryKey(i),
+      query: this.queryExpressions[i],
     }));
 
     this.setState(
@@ -177,6 +196,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
 
   onAddQueryRow = index => {
     const { queries } = this.state;
+    this.queryExpressions[index + 1] = '';
     const nextQueries = [
       ...queries.slice(0, index + 1),
       { query: '', key: generateQueryKey() },
@@ -203,29 +223,28 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
   };
 
   onChangeQuery = (value: string, index: number, override?: boolean) => {
-    const { queries } = this.state;
-    let { queryErrors, queryHints } = this.state;
-    const prevQuery = queries[index];
-    const edited = override ? false : prevQuery.query !== value;
-    const nextQuery = {
-      ...queries[index],
-      edited,
-      query: value,
-    };
-    const nextQueries = [...queries];
-    nextQueries[index] = nextQuery;
+    // Keep current value in local cache
+    this.queryExpressions[index] = value;
+
+    // Replace query row on override
     if (override) {
-      queryErrors = [];
-      queryHints = [];
+      const { queries } = this.state;
+      const nextQuery: Query = {
+        key: generateQueryKey(index),
+        query: value,
+      };
+      const nextQueries = [...queries];
+      nextQueries[index] = nextQuery;
+
+      this.setState(
+        {
+          queryErrors: [],
+          queryHints: [],
+          queries: nextQueries,
+        },
+        this.onSubmit
+      );
     }
-    this.setState(
-      {
-        queryErrors,
-        queryHints,
-        queries: nextQueries,
-      },
-      override ? () => this.onSubmit() : undefined
-    );
   };
 
   onChangeTime = nextRange => {
@@ -237,6 +256,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
   };
 
   onClickClear = () => {
+    this.queryExpressions = [''];
     this.setState(
       {
         graphResult: null,
@@ -269,9 +289,8 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
 
   onClickSplit = () => {
     const { onChangeSplit } = this.props;
-    const state = { ...this.state };
-    state.queries = state.queries.map(({ edited, ...rest }) => rest);
     if (onChangeSplit) {
+      const state = this.cloneState();
       onChangeSplit(true, state);
       this.saveState();
     }
@@ -291,23 +310,22 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
       let nextQueries;
       if (index === undefined) {
         // Modify all queries
-        nextQueries = queries.map(q => ({
-          ...q,
-          edited: false,
-          query: datasource.modifyQuery(q.query, action),
+        nextQueries = queries.map((q, i) => ({
+          key: generateQueryKey(i),
+          query: datasource.modifyQuery(this.queryExpressions[i], action),
         }));
       } else {
         // Modify query only at index
         nextQueries = [
           ...queries.slice(0, index),
           {
-            ...queries[index],
-            edited: false,
-            query: datasource.modifyQuery(queries[index].query, action),
+            key: generateQueryKey(index),
+            query: datasource.modifyQuery(this.queryExpressions[index], action),
           },
           ...queries.slice(index + 1),
         ];
       }
+      this.queryExpressions = nextQueries.map(q => q.query);
       this.setState({ queries: nextQueries }, () => this.onSubmit());
     }
   };
@@ -318,6 +336,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
       return;
     }
     const nextQueries = [...queries.slice(0, index), ...queries.slice(index + 1)];
+    this.queryExpressions = nextQueries.map(q => q.query);
     this.setState({ queries: nextQueries }, () => this.onSubmit());
   };
 
@@ -335,7 +354,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
     this.saveState();
   };
 
-  onQuerySuccess(datasourceId: string, queries: any[]): void {
+  onQuerySuccess(datasourceId: string, queries: string[]): void {
     // save queries to history
     let { history } = this.state;
     const { datasource } = this.state;
@@ -346,8 +365,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
     }
 
     const ts = Date.now();
-    queries.forEach(q => {
-      const { query } = q;
+    queries.forEach(query => {
       history = [{ query, ts }, ...history];
     });
 
@@ -362,16 +380,16 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
   }
 
   buildQueryOptions(targetOptions: { format: string; hinting?: boolean; instant?: boolean }) {
-    const { datasource, queries, range } = this.state;
+    const { datasource, range } = this.state;
     const resolution = this.el.offsetWidth;
     const absoluteRange = {
       from: parseDate(range.from, false),
       to: parseDate(range.to, true),
     };
     const { interval } = kbn.calculateInterval(absoluteRange, resolution, datasource.interval);
-    const targets = queries.map(q => ({
+    const targets = this.queryExpressions.map(q => ({
       ...targetOptions,
-      expr: q.query,
+      expr: q,
     }));
     return {
       interval,
@@ -381,7 +399,8 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
   }
 
   async runGraphQuery() {
-    const { datasource, queries } = this.state;
+    const { datasource } = this.state;
+    const queries = [...this.queryExpressions];
     if (!hasQuery(queries)) {
       return;
     }
@@ -403,7 +422,8 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
   }
 
   async runTableQuery() {
-    const { datasource, queries } = this.state;
+    const queries = [...this.queryExpressions];
+    const { datasource } = this.state;
     if (!hasQuery(queries)) {
       return;
     }
@@ -427,7 +447,8 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
   }
 
   async runLogsQuery() {
-    const { datasource, queries } = this.state;
+    const queries = [...this.queryExpressions];
+    const { datasource } = this.state;
     if (!hasQuery(queries)) {
       return;
     }
@@ -455,18 +476,27 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
     return datasource.metadataRequest(url);
   };
 
+  cloneState(): ExploreState {
+    // Copy state, but copy queries including modifications
+    return {
+      ...this.state,
+      queries: ensureQueries(this.queryExpressions.map(query => ({ query }))),
+    };
+  }
+
   saveState = () => {
     const { stateKey, onSaveState } = this.props;
-    onSaveState(stateKey, this.state);
+    onSaveState(stateKey, this.cloneState());
   };
 
   render() {
-    const { datasourceSrv, position, split } = this.props;
+    const { position, split } = this.props;
     const {
       datasource,
       datasourceError,
       datasourceLoading,
       datasourceMissing,
+      exploreDatasources,
       graphResult,
       history,
       latency,
@@ -491,10 +521,6 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
     const logsButtonActive = showingLogs ? 'active' : '';
     const tableButtonActive = showingBoth || showingTable ? 'active' : '';
     const exploreClass = split ? 'explore explore-split' : 'explore';
-    const datasources = datasourceSrv.getExploreSources().map(ds => ({
-      value: ds.name,
-      label: ds.name,
-    }));
     const selectedDatasource = datasource ? datasource.name : undefined;
 
     return (
@@ -508,19 +534,19 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
               </a>
             </div>
           ) : (
-            <div className="navbar-buttons explore-first-button">
-              <button className="btn navbar-button" onClick={this.onClickCloseSplit}>
-                Close Split
+              <div className="navbar-buttons explore-first-button">
+                <button className="btn navbar-button" onClick={this.onClickCloseSplit}>
+                  Close Split
               </button>
-            </div>
-          )}
+              </div>
+            )}
           {!datasourceMissing ? (
             <div className="navbar-buttons">
               <Select
                 clearable={false}
                 className="gf-form-input gf-form-input--form-dropdown datasource-picker"
                 onChange={this.onChangeDatasource}
-                options={datasources}
+                options={exploreDatasources}
                 isOpen={true}
                 placeholder="Loading datasources..."
                 value={selectedDatasource}

public/app/features/explore/PromQueryField.tsx

@@ -156,6 +156,7 @@ interface PromQueryFieldState {
   labelValues: { [index: string]: { [index: string]: string[] } }; // metric -> labelKey -> [labelValue,...]
   logLabelOptions: any[];
   metrics: string[];
+  metricsOptions: any[];
   metricsByPrefix: CascaderOption[];
 }
 
@@ -167,7 +168,7 @@ interface PromTypeaheadInput {
   value?: Value;
 }
 
-class PromQueryField extends React.Component<PromQueryFieldProps, PromQueryFieldState> {
+class PromQueryField extends React.PureComponent<PromQueryFieldProps, PromQueryFieldState> {
   plugins: any[];
 
   constructor(props: PromQueryFieldProps, context) {
@@ -189,6 +190,7 @@ class PromQueryField extends React.Component<PromQueryFieldProps, PromQueryField
       logLabelOptions: [],
       metrics: props.metrics || [],
       metricsByPrefix: props.metricsByPrefix || [],
+      metricsOptions: [],
     };
   }
 
@@ -258,10 +260,22 @@ class PromQueryField extends React.Component<PromQueryFieldProps, PromQueryField
   };
 
   onReceiveMetrics = () => {
-    if (!this.state.metrics) {
+    const { histogramMetrics, metrics, metricsByPrefix } = this.state;
+    if (!metrics) {
       return;
     }
+
+    // Update global prism config
     setPrismTokens(PRISM_SYNTAX, METRIC_MARK, this.state.metrics);
+
+    // Build metrics tree
+    const histogramOptions = histogramMetrics.map(hm => ({ label: hm, value: hm }));
+    const metricsOptions = [
+      { label: 'Histograms', value: HISTOGRAM_GROUP, children: histogramOptions },
+      ...metricsByPrefix,
+    ];
+
+    this.setState({ metricsOptions });
   };
 
   onTypeahead = (typeahead: TypeaheadInput): TypeaheadOutput => {
@@ -453,7 +467,7 @@ class PromQueryField extends React.Component<PromQueryFieldProps, PromQueryField
       const histogramSeries = this.state.labelValues[HISTOGRAM_SELECTOR];
       if (histogramSeries && histogramSeries['__name__']) {
         const histogramMetrics = histogramSeries['__name__'].slice().sort();
-        this.setState({ histogramMetrics });
+        this.setState({ histogramMetrics }, this.onReceiveMetrics);
       }
     });
   }
@@ -545,12 +559,7 @@ class PromQueryField extends React.Component<PromQueryFieldProps, PromQueryField
 
   render() {
     const { error, hint, supportsLogs } = this.props;
-    const { histogramMetrics, logLabelOptions, metricsByPrefix } = this.state;
-    const histogramOptions = histogramMetrics.map(hm => ({ label: hm, value: hm }));
-    const metricsOptions = [
-      { label: 'Histograms', value: HISTOGRAM_GROUP, children: histogramOptions },
-      ...metricsByPrefix,
-    ];
+    const { logLabelOptions, metricsOptions } = this.state;
 
     return (
       <div className="prom-query-field">
@@ -575,6 +584,7 @@ class PromQueryField extends React.Component<PromQueryFieldProps, PromQueryField
               onWillApplySuggestion={willApplySuggestion}
               onValueChanged={this.onChangeQuery}
               placeholder="Enter a PromQL query"
+              portalPrefix="prometheus"
             />
           </div>
           {error ? <div className="prom-query-field-info text-error">{error}</div> : null}

public/app/features/explore/QueryField.tsx

@@ -11,10 +11,17 @@ import NewlinePlugin from './slate-plugins/newline';
 import Typeahead from './Typeahead';
 import { makeFragment, makeValue } from './Value';
 
-export const TYPEAHEAD_DEBOUNCE = 300;
+export const TYPEAHEAD_DEBOUNCE = 100;
 
-function flattenSuggestions(s: any[]): any[] {
-  return s ? s.reduce((acc, g) => acc.concat(g.items), []) : [];
+function getSuggestionByIndex(suggestions: SuggestionGroup[], index: number): Suggestion {
+  // Flatten suggestion groups
+  const flattenedSuggestions = suggestions.reduce((acc, g) => acc.concat(g.items), []);
+  const correctedIndex = Math.max(index, 0) % flattenedSuggestions.length;
+  return flattenedSuggestions[correctedIndex];
+}
+
+function hasSuggestions(suggestions: SuggestionGroup[]): boolean {
+  return suggestions && suggestions.length > 0;
 }
 
 export interface Suggestion {
@@ -125,7 +132,7 @@ export interface TypeaheadOutput {
   suggestions: SuggestionGroup[];
 }
 
-class QueryField extends React.Component<TypeaheadFieldProps, TypeaheadFieldState> {
+class QueryField extends React.PureComponent<TypeaheadFieldProps, TypeaheadFieldState> {
   menuEl: HTMLElement | null;
   plugins: any[];
   resetTimer: any;
@@ -154,8 +161,14 @@ class QueryField extends React.Component<TypeaheadFieldProps, TypeaheadFieldStat
     clearTimeout(this.resetTimer);
   }
 
-  componentDidUpdate() {
-    this.updateMenu();
+  componentDidUpdate(prevProps, prevState) {
+    // Only update menu location when suggestion existence or text/selection changed
+    if (
+      this.state.value !== prevState.value ||
+      hasSuggestions(this.state.suggestions) !== hasSuggestions(prevState.suggestions)
+    ) {
+      this.updateMenu();
+    }
   }
 
   componentWillReceiveProps(nextProps) {
@@ -216,7 +229,7 @@ class QueryField extends React.Component<TypeaheadFieldProps, TypeaheadFieldStat
         wrapperNode,
       });
 
-      const filteredSuggestions = suggestions
+      let filteredSuggestions = suggestions
         .map(group => {
           if (group.items) {
             if (prefix) {
@@ -241,6 +254,11 @@ class QueryField extends React.Component<TypeaheadFieldProps, TypeaheadFieldStat
         })
         .filter(group => group.items && group.items.length > 0); // Filter out empty groups
 
+      // Keep same object for equality checking later
+      if (_.isEqual(filteredSuggestions, this.state.suggestions)) {
+        filteredSuggestions = this.state.suggestions;
+      }
+
       this.setState(
         {
           suggestions: filteredSuggestions,
@@ -326,12 +344,7 @@ class QueryField extends React.Component<TypeaheadFieldProps, TypeaheadFieldStat
             return undefined;
           }
 
-          // Get the currently selected suggestion
-          const flattenedSuggestions = flattenSuggestions(suggestions);
-          const selected = Math.abs(typeaheadIndex);
-          const selectedIndex = selected % flattenedSuggestions.length || 0;
-          const suggestion = flattenedSuggestions[selectedIndex];
-
+          const suggestion = getSuggestionByIndex(suggestions, typeaheadIndex);
           this.applyTypeahead(change, suggestion);
           return true;
         }
@@ -408,8 +421,7 @@ class QueryField extends React.Component<TypeaheadFieldProps, TypeaheadFieldStat
     }
 
     // No suggestions or blur, remove menu
-    const hasSuggesstions = suggestions && suggestions.length > 0;
-    if (!hasSuggesstions) {
+    if (!hasSuggestions(suggestions)) {
       menu.removeAttribute('style');
       return;
     }
@@ -436,18 +448,12 @@ class QueryField extends React.Component<TypeaheadFieldProps, TypeaheadFieldStat
 
   renderMenu = () => {
     const { portalPrefix } = this.props;
-    const { suggestions } = this.state;
-    const hasSuggesstions = suggestions && suggestions.length > 0;
-    if (!hasSuggesstions) {
+    const { suggestions, typeaheadIndex } = this.state;
+    if (!hasSuggestions(suggestions)) {
       return null;
     }
 
-    // Guard selectedIndex to be within the length of the suggestions
-    let selectedIndex = Math.max(this.state.typeaheadIndex, 0);
-    const flattenedSuggestions = flattenSuggestions(suggestions);
-    selectedIndex = selectedIndex % flattenedSuggestions.length || 0;
-    const selectedItem: Suggestion | null =
-      flattenedSuggestions.length > 0 ? flattenedSuggestions[selectedIndex] : null;
+    const selectedItem = getSuggestionByIndex(suggestions, typeaheadIndex);
 
     // Create typeahead in DOM root so we can later position it absolutely
     return (
@@ -482,7 +488,7 @@ class QueryField extends React.Component<TypeaheadFieldProps, TypeaheadFieldStat
   }
 }
 
-class Portal extends React.Component<{ index?: number; prefix: string }, {}> {
+class Portal extends React.PureComponent<{ index?: number; prefix: string }, {}> {
   node: HTMLElement;
 
   constructor(props) {

public/app/features/explore/QueryRows.tsx

@@ -44,14 +44,14 @@ class QueryRow extends PureComponent<any, {}> {
   };
 
   render() {
-    const { edited, history, query, queryError, queryHint, request, supportsLogs } = this.props;
+    const { history, query, queryError, queryHint, request, supportsLogs } = this.props;
     return (
       <div className="query-row">
         <div className="query-row-field">
           <QueryField
             error={queryError}
             hint={queryHint}
-            initialQuery={edited ? null : query}
+            initialQuery={query}
             history={history}
             portalPrefix="explore"
             onClickHintFix={this.onClickHintFix}
@@ -79,7 +79,7 @@ class QueryRow extends PureComponent<any, {}> {
 
 export default class QueryRows extends PureComponent<any, {}> {
   render() {
-    const { className = '', queries, queryErrors = [], queryHints = [], ...handlers } = this.props;
+    const { className = '', queries, queryErrors, queryHints, ...handlers } = this.props;
     return (
       <div className={className}>
         {queries.map((q, index) => (
@@ -89,7 +89,6 @@ export default class QueryRows extends PureComponent<any, {}> {
             query={q.query}
             queryError={queryErrors[index]}
             queryHint={queryHints[index]}
-            edited={q.edited}
             {...handlers}
           />
         ))}

public/app/features/explore/Typeahead.tsx

@@ -23,7 +23,9 @@ class TypeaheadItem extends React.PureComponent<TypeaheadItemProps, {}> {
 
   componentDidUpdate(prevProps) {
     if (this.props.isSelected && !prevProps.isSelected) {
-      scrollIntoView(this.el);
+      requestAnimationFrame(() => {
+        scrollIntoView(this.el);
+      });
     }
   }
 

public/app/features/explore/utils/query.ts

@@ -1,14 +1,16 @@
-export function generateQueryKey(index = 0) {
+import { Query } from 'app/types/explore';
+
+export function generateQueryKey(index = 0): string {
   return `Q-${Date.now()}-${Math.random()}-${index}`;
 }
 
-export function ensureQueries(queries?) {
+export function ensureQueries(queries?: Query[]): Query[] {
   if (queries && typeof queries === 'object' && queries.length > 0 && typeof queries[0].query === 'string') {
     return queries.map(({ query }, i) => ({ key: generateQueryKey(i), query }));
   }
   return [{ key: generateQueryKey(), query: '' }];
 }
 
-export function hasQuery(queries) {
-  return queries.some(q => q.query);
+export function hasQuery(queries: string[]): boolean {
+  return queries.some(q => Boolean(q));
 }

public/app/types/explore.ts

@@ -1,3 +1,8 @@
+interface ExploreDatasource {
+  value: string;
+  label: string;
+}
+
 export interface Range {
   from: string;
   to: string;
@@ -5,7 +10,6 @@ export interface Range {
 
 export interface Query {
   query: string;
-  edited?: boolean;
   key?: string;
 }
 
@@ -15,13 +19,25 @@ export interface ExploreState {
   datasourceLoading: boolean | null;
   datasourceMissing: boolean;
   datasourceName?: string;
+  exploreDatasources: ExploreDatasource[];
   graphResult: any;
   history: any[];
   latency: number;
   loading: any;
   logsResult: any;
+  /**
+   * Initial rows of queries to push down the tree.
+   * Modifications do not end up here, but in `this.queryExpressions`.
+   * The only way to reset a query is to change its `key`.
+   */
   queries: Query[];
+  /**
+   * Errors caused by the running the query row.
+   */
   queryErrors: any[];
+  /**
+   * Hints gathered for the query row.
+   */
   queryHints: any[];
   range: Range;
   requestOptions: any;