Get query hints per query transaction

fbed57ab · David Kaltschmidt · 2e02a8c8 · fbed57ab · fbed57ab · fbed57ab
Commit fbed57ab authored Oct 23, 2018 by David Kaltschmidt
7 changed files
--- a/public/app/core/utils/explore.test.ts
+++ b/public/app/core/utils/explore.test.ts
@@ -11,7 +11,6 @@ const DEFAULT_EXPLORE_STATE: ExploreState = {
  graphRange: DEFAULT_RANGE,
  history: [],
  queries: [],
-  queryHints: [],
  queryTransactions: [],
  range: DEFAULT_RANGE,
  showingGraph: true,

--- a/public/app/features/explore/Explore.tsx
+++ b/public/app/features/explore/Explore.tsx
@@ -25,11 +25,11 @@ import { ensureQueries, generateQueryKey, hasQuery } from './utils/query';
 const MAX_HISTORY_ITEMS = 100;
-function makeHints(hints) {
+function makeHints(transactions: QueryTransaction[]) {
  const hintsByIndex = [];
-  hints.forEach(hint => {
+  transactions.forEach(qt => {
-    if (hint) {
+    if (qt.hints && qt.hints.length > 0) {
-      hintsByIndex[hint.index] = hint;
+      hintsByIndex[qt.rowIndex] = qt.hints[0];
    }
  });
  return hintsByIndex;
@@ -113,7 +113,6 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
        graphRange: initialRange,
        history: [],
        queries: initialQueries,
-        queryHints: [],
        queryTransactions: [],
        range: initialRange,
        showingGraph: true,
@@ -246,7 +245,6 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
      datasource: null,
      datasourceError: null,
      datasourceLoading: true,
-      queryHints: [],
      queryTransactions: [],
    });
    const datasourceName = option.value;
@@ -274,7 +272,6 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
      this.setState(
        {
          queries: nextQueries,
-          queryHints: [],
          queryTransactions: nextQueryTransactions,
        },
        this.onSubmit
@@ -295,7 +292,6 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
    this.setState(
      {
        queries: ensureQueries(),
-        queryHints: [],
        queryTransactions: [],
      },
      this.saveState
@@ -458,7 +454,6 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
      const nextQueryTransactions = [...remainingTransactions, transaction];
      return {
-        queryHints: [],
        queryTransactions: nextQueryTransactions,
      };
    });
@@ -470,7 +465,6 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
    transactionId: string,
    result: any,
    latency: number,
-    hints: any[],
    queries: string[],
    datasourceId: string
  ) {
@@ -484,15 +478,23 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
      const { history, queryTransactions } = state;
      // Transaction might have been discarded
-      if (!queryTransactions.find(qt => qt.id === transactionId)) {
+      const transaction = queryTransactions.find(qt => qt.id === transactionId);
+      if (!transaction) {
        return null;
      }
+      // Get query hints
+      let hints;
+      if (datasource.getQueryHints) {
+        hints = datasource.getQueryHints(transaction.query, result);
+      }
      // Mark transactions as complete
      const nextQueryTransactions = queryTransactions.map(qt => {
        if (qt.id === transactionId) {
          return {
            ...qt,
+            hints,
            latency,
            result,
            done: true,
@@ -505,7 +507,6 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
      return {
        history: nextHistory,
-        queryHints: hints,
        queryTransactions: nextQueryTransactions,
      };
    });
@@ -562,8 +563,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
          const res = await datasource.query(transaction.options);
          const latency = Date.now() - now;
          const results = makeTimeSeriesList(res.data, transaction.options);
-          const queryHints = res.hints ? makeHints(res.hints) : [];
+          this.completeQueryTransaction(transaction.id, results, latency, queries, datasourceId);
-          this.completeQueryTransaction(transaction.id, results, latency, queryHints, queries, datasourceId);
          this.setState({ graphRange: transaction.options.range });
        } catch (response) {
          console.error(response);
@@ -590,7 +590,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
          const res = await datasource.query(transaction.options);
          const latency = Date.now() - now;
          const results = mergeTablesIntoModel(new TableModel(), ...res.data);
-          this.completeQueryTransaction(transaction.id, results, latency, [], queries, datasourceId);
+          this.completeQueryTransaction(transaction.id, results, latency, queries, datasourceId);
        } catch (response) {
          console.error(response);
          const queryError = response.data ? response.data.error : response;
@@ -616,7 +616,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
          const res = await datasource.query(transaction.options);
          const latency = Date.now() - now;
          const results = res.data;
-          this.completeQueryTransaction(transaction.id, results, latency, [], queries, datasourceId);
+          this.completeQueryTransaction(transaction.id, results, latency, queries, datasourceId);
        } catch (response) {
          console.error(response);
          const queryError = response.data ? response.data.error : response;
@@ -655,7 +655,6 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
      graphRange,
      history,
      queries,
-      queryHints,
      queryTransactions,
      range,
      showingGraph,
@@ -683,6 +682,7 @@ export class Explore extends React.PureComponent<ExploreProps, ExploreState> {
      queryTransactions.filter(qt => qt.resultType === 'Logs' && qt.done).map(qt => qt.result)
    );
    const loading = queryTransactions.some(qt => !qt.done);
+    const queryHints = makeHints(queryTransactions);
    return (
      <div className={exploreClass} ref={this.getRef}>

--- a/public/app/plugins/datasource/prometheus/datasource.ts
+++ b/public/app/plugins/datasource/prometheus/datasource.ts
@@ -176,7 +176,6 @@ export class PrometheusDatasource {
    return this.$q.all(allQueryPromise).then(responseList => {
      let result = [];
-      let hints = [];
      _.each(responseList, (response, index) => {
        if (response.status === 'error') {
@@ -196,19 +195,13 @@ export class PrometheusDatasource {
          end: queries[index].end,
          query: queries[index].expr,
          responseListLength: responseList.length,
-          responseIndex: index,
          refId: activeTargets[index].refId,
        };
        const series = this.resultTransformer.transform(response, transformerOptions);
        result = [...result, ...series];
-        if (queries[index].hinting) {
-          const queryHints = getQueryHints(series, this);
-          hints = [...hints, ...queryHints];
-        }
      });
-      return { data: result, hints };
+      return { data: result };
    });
  }
@@ -437,6 +430,10 @@ export class PrometheusDatasource {
    return state;
  }
+  getQueryHints(query: string, result: any[]) {
+    return getQueryHints(query, result, this);
+  }
  loadRules() {
    this.metadataRequest('/api/v1/rules')
      .then(res => res.data || res.json())

--- a/public/app/plugins/datasource/prometheus/query_hints.ts
+++ b/public/app/plugins/datasource/prometheus/query_hints.ts
 import _ from 'lodash';
-export function getQueryHints(series: any[], datasource?: any): any[] {
+export function getQueryHints(query: string, series?: any[], datasource?: any): any[] {
-  const hints = series.map((s, i) => {
+  const hints = [];
-    const query: string = s.query;
-    const index: number = s.responseIndex;
-    if (query === undefined || index === undefined) {
-      return null;
-    }
-    // ..._bucket metric needs a histogram_quantile()
+  // ..._bucket metric needs a histogram_quantile()
-    const histogramMetric = query.trim().match(/^\w+_bucket$/);
+  const histogramMetric = query.trim().match(/^\w+_bucket$/);
-    if (histogramMetric) {
+  if (histogramMetric) {
-      const label = 'Time series has buckets, you probably wanted a histogram.';
+    const label = 'Time series has buckets, you probably wanted a histogram.';
-      return {
+    hints.push({
-        index,
+      type: 'HISTOGRAM_QUANTILE',
-        label,
+      label,
-        fix: {
+      fix: {
-          label: 'Fix by adding histogram_quantile().',
+        label: 'Fix by adding histogram_quantile().',
-          action: {
+        action: {
-            type: 'ADD_HISTOGRAM_QUANTILE',
+          type: 'ADD_HISTOGRAM_QUANTILE',
-            query,
+          query,
-            index,
-          },
        },
-      };
+      },
-    }
+    });
+  }
-    // Check for monotony
+  // Check for monotony on series (table results are being ignored here)
-    const datapoints: number[][] = s.datapoints;
+  if (series && series.length > 0) {
-    if (query.indexOf('rate(') === -1 && datapoints.length > 1) {
+    series.forEach(s => {
-      let increasing = false;
+      const datapoints: number[][] = s.datapoints;
-      const nonNullData = datapoints.filter(dp => dp[0] !== null);
+      if (query.indexOf('rate(') === -1 && datapoints.length > 1) {
-      const monotonic = nonNullData.every((dp, index) => {
+        let increasing = false;
-        if (index === 0) {
+        const nonNullData = datapoints.filter(dp => dp[0] !== null);
-          return true;
+        const monotonic = nonNullData.every((dp, index) => {
-        }
+          if (index === 0) {
-        increasing = increasing || dp[0] > nonNullData[index - 1][0];
+            return true;
-        // monotonic?
+          }
-        return dp[0] >= nonNullData[index - 1][0];
+          increasing = increasing || dp[0] > nonNullData[index - 1][0];
-      });
+          // monotonic?
-      if (increasing && monotonic) {
+          return dp[0] >= nonNullData[index - 1][0];
-        const simpleMetric = query.trim().match(/^\w+$/);
+        });
-        let label = 'Time series is monotonously increasing.';
+        if (increasing && monotonic) {
-        let fix;
+          const simpleMetric = query.trim().match(/^\w+$/);
-        if (simpleMetric) {
+          let label = 'Time series is monotonously increasing.';
-          fix = {
+          let fix;
-            label: 'Fix by adding rate().',
+          if (simpleMetric) {
-            action: {
+            fix = {
-              type: 'ADD_RATE',
+              label: 'Fix by adding rate().',
-              query,
+              action: {
-              index,
+                type: 'ADD_RATE',
-            },
+                query,
-          };
+              },
-        } else {
+            };
-          label = `${label} Try applying a rate() function.`;
+          } else {
+            label = `${label} Try applying a rate() function.`;
+          }
+          hints.push({
+            type: 'APPLY_RATE',
+            label,
+            fix,
+          });
        }
-        return {
-          label,
-          index,
-          fix,
-        };
      }
-    }
+    });
+  }
-    // Check for recording rules expansion
+  // Check for recording rules expansion
-    if (datasource && datasource.ruleMappings) {
+  if (datasource && datasource.ruleMappings) {
-      const mapping = datasource.ruleMappings;
+    const mapping = datasource.ruleMappings;
-      const mappingForQuery = Object.keys(mapping).reduce((acc, ruleName) => {
+    const mappingForQuery = Object.keys(mapping).reduce((acc, ruleName) => {
-        if (query.search(ruleName) > -1) {
+      if (query.search(ruleName) > -1) {
-          return {
-            ...acc,
-            [ruleName]: mapping[ruleName],
-          };
-        }
-        return acc;
-      }, {});
-      if (_.size(mappingForQuery) > 0) {
-        const label = 'Query contains recording rules.';
        return {
-          label,
+          ...acc,
-          index,
+          [ruleName]: mapping[ruleName],
-          fix: {
-            label: 'Expand rules',
-            action: {
-              type: 'EXPAND_RULES',
-              query,
-              index,
-              mapping: mappingForQuery,
-            },
-          },
        };
      }
+      return acc;
+    }, {});
+    if (_.size(mappingForQuery) > 0) {
+      const label = 'Query contains recording rules.';
+      hints.push({
+        type: 'EXPAND_RULES',
+        label,
+        fix: {
+          label: 'Expand rules',
+          action: {
+            type: 'EXPAND_RULES',
+            query,
+            mapping: mappingForQuery,
+          },
+        },
+      });
    }
+  }
-    // No hint found
+  return hints.length > 0 ? hints : null;
-    return null;
-  });
-  return hints;
 }
--- a/public/app/plugins/datasource/prometheus/result_transformer.ts
+++ b/public/app/plugins/datasource/prometheus/result_transformer.ts
@@ -66,7 +66,6 @@ export class ResultTransformer {
    return {
      datapoints: dps,
      query: options.query,
-      responseIndex: options.responseIndex,
      target: metricLabel,
    };
  }

--- a/public/app/plugins/datasource/prometheus/specs/query_hints.test.ts
+++ b/public/app/plugins/datasource/prometheus/specs/query_hints.test.ts
@@ -2,34 +2,31 @@ import { getQueryHints } from '../query_hints';
 describe('getQueryHints()', () => {
  it('returns no hints for no series', () => {
-    expect(getQueryHints([])).toEqual([]);
+    expect(getQueryHints('', [])).toEqual(null);
  });
  it('returns no hints for empty series', () => {
-    expect(getQueryHints([{ datapoints: [], query: '' }])).toEqual([null]);
+    expect(getQueryHints('', [{ datapoints: [] }])).toEqual(null);
  });
  it('returns no hint for a monotonously decreasing series', () => {
-    const series = [{ datapoints: [[23, 1000], [22, 1001]], query: 'metric', responseIndex: 0 }];
+    const series = [{ datapoints: [[23, 1000], [22, 1001]] }];
-    const hints = getQueryHints(series);
+    const hints = getQueryHints('metric', series);
-    expect(hints).toEqual([null]);
+    expect(hints).toEqual(null);
  });
  it('returns no hint for a flat series', () => {
-    const series = [
+    const series = [{ datapoints: [[null, 1000], [23, 1001], [null, 1002], [23, 1003]] }];
-      { datapoints: [[null, 1000], [23, 1001], [null, 1002], [23, 1003]], query: 'metric', responseIndex: 0 },
+    const hints = getQueryHints('metric', series);
-    ];
+    expect(hints).toEqual(null);
-    const hints = getQueryHints(series);
-    expect(hints).toEqual([null]);
  });
  it('returns a rate hint for a monotonously increasing series', () => {
-    const series = [{ datapoints: [[23, 1000], [24, 1001]], query: 'metric', responseIndex: 0 }];
+    const series = [{ datapoints: [[23, 1000], [24, 1001]] }];
-    const hints = getQueryHints(series);
+    const hints = getQueryHints('metric', series);
    expect(hints.length).toBe(1);
    expect(hints[0]).toMatchObject({
      label: 'Time series is monotonously increasing.',
-      index: 0,
      fix: {
        action: {
          type: 'ADD_RATE',
@@ -40,26 +37,25 @@ describe('getQueryHints()', () => {
  });
  it('returns no rate hint for a monotonously increasing series that already has a rate', () => {
-    const series = [{ datapoints: [[23, 1000], [24, 1001]], query: 'rate(metric[1m])', responseIndex: 0 }];
+    const series = [{ datapoints: [[23, 1000], [24, 1001]] }];
-    const hints = getQueryHints(series);
+    const hints = getQueryHints('rate(metric[1m])', series);
-    expect(hints).toEqual([null]);
+    expect(hints).toEqual(null);
  });
  it('returns a rate hint w/o action for a complex monotonously increasing series', () => {
-    const series = [{ datapoints: [[23, 1000], [24, 1001]], query: 'sum(metric)', responseIndex: 0 }];
+    const series = [{ datapoints: [[23, 1000], [24, 1001]] }];
-    const hints = getQueryHints(series);
+    const hints = getQueryHints('sum(metric)', series);
    expect(hints.length).toBe(1);
    expect(hints[0].label).toContain('rate()');
    expect(hints[0].fix).toBeUndefined();
  });
  it('returns a rate hint for a monotonously increasing series with missing data', () => {
-    const series = [{ datapoints: [[23, 1000], [null, 1001], [24, 1002]], query: 'metric', responseIndex: 0 }];
+    const series = [{ datapoints: [[23, 1000], [null, 1001], [24, 1002]] }];
-    const hints = getQueryHints(series);
+    const hints = getQueryHints('metric', series);
    expect(hints.length).toBe(1);
    expect(hints[0]).toMatchObject({
      label: 'Time series is monotonously increasing.',
-      index: 0,
      fix: {
        action: {
          type: 'ADD_RATE',
@@ -70,12 +66,11 @@ describe('getQueryHints()', () => {
  });
  it('returns a histogram hint for a bucket series', () => {
-    const series = [{ datapoints: [[23, 1000]], query: 'metric_bucket', responseIndex: 0 }];
+    const series = [{ datapoints: [[23, 1000]] }];
-    const hints = getQueryHints(series);
+    const hints = getQueryHints('metric_bucket', series);
    expect(hints.length).toBe(1);
    expect(hints[0]).toMatchObject({
      label: 'Time series has buckets, you probably wanted a histogram.',
-      index: 0,
      fix: {
        action: {
          type: 'ADD_HISTOGRAM_QUANTILE',

--- a/public/app/types/explore.ts
+++ b/public/app/types/explore.ts
@@ -22,6 +22,7 @@ export interface QueryTransaction {
  id: string;
  done: boolean;
  error?: string;
+  hints?: any[];
  latency: number;
  options: any;
  query: string;
@@ -55,7 +56,6 @@ export interface ExploreState {
  /**
   * Hints gathered for the query row.
   */
-  queryHints: any[];
  queryTransactions: QueryTransaction[];
  range: Range;
  showingGraph: boolean;