Merge pull request #111 from quickwit-oss/fmassot/fix-time-range-get-tag-keys-values

Fix time range for getTagKeys and getTagValues function.

Author: ddelemeny

package-lock.json

@@ -21,8 +21,8 @@
   "license": "AGPL-3.0",
   "devDependencies": {
     "@babel/core": "^7.21.4",
-    "@grafana/e2e": "10.0.3",
-    "@grafana/e2e-selectors": "10.0.3",
+    "@grafana/e2e": "10.1.9",
+    "@grafana/e2e-selectors": "10.1.9",
     "@grafana/eslint-config": "^6.0.0",
     "@grafana/tsconfig": "^1.2.0-rc1",
     "@storybook/addon-essentials": "^7.6.17",
@@ -74,10 +74,10 @@
   },
   "dependencies": {
     "@emotion/css": "^11.11",
-    "@grafana/data": "10.0.3",
-    "@grafana/experimental": "^1.7.6",
-    "@grafana/runtime": "10.0.3",
-    "@grafana/ui": "10.0.3",
+    "@grafana/data": "10.1.9",
+    "@grafana/experimental": "^1.7.10",
+    "@grafana/runtime": "10.1.9",
+    "@grafana/ui": "10.1.9",
     "@reduxjs/toolkit": "^1.9.5",
     "@uiw/react-codemirror": "^4.21.21",
     "lucene": "^2.1.1",

package.json

@@ -11,6 +11,7 @@ import {
   dateTime,
   LogRowModel,
   rangeUtil,
+  TimeRange,
 } from '@grafana/data';
 
 import { ElasticsearchQuery, Logs, LogsSortDirection} from '../types';
@@ -26,20 +27,16 @@ export enum LogRowContextQueryDirection {
     Forward = 'FORWARD',
 }
 
-function createContextTimeRange(rowTimeEpochMs: number, direction: string) {
+export function createContextTimeRange(rowTimeEpochMs: number, direction?: LogRowContextQueryDirection): TimeRange {
   const offset = 7;
-  // For log context, we want to request data from 7 subsequent/previous indices
-  if (direction === LogRowContextQueryDirection.Forward) {
-    return {
-      from: dateTime(rowTimeEpochMs).utc(),
-      to: dateTime(rowTimeEpochMs).add(offset, 'hours').utc(),
-    };
-  } else {
-    return {
-      from: dateTime(rowTimeEpochMs).subtract(offset, 'hours').utc(),
-      to: dateTime(rowTimeEpochMs).utc(),
-    };
+  const timeFrom = dateTime(rowTimeEpochMs)
+  const timeTo = dateTime(rowTimeEpochMs)
+
+  const timeRange = {
+    from: (direction === LogRowContextQueryDirection.Forward) ? timeFrom.utc() : timeFrom.subtract(offset, 'hours').utc(),
+    to: (direction === LogRowContextQueryDirection.Backward) ? timeTo.utc() : timeTo.add(offset, 'hours').utc(),
   }
+  return { ...timeRange, raw:timeRange }
 }
 
 export class LogContextProvider {
@@ -76,12 +73,7 @@ export class LogContextProvider {
       query: this.contextQuery == null ? origQuery?.query : this.contextQuery,
     };
 
-    const timeRange = createContextTimeRange(row.timeEpochMs, direction);
-    const range = {
-      from: timeRange.from,
-      to: timeRange.to,
-      raw: timeRange,
-    };
+    const range = createContextTimeRange(row.timeEpochMs, direction);
 
     const interval = rangeUtil.calculateInterval(range, 1);
 

src/LogContext/LogContextProvider.ts

@@ -11,6 +11,7 @@ import { DatasourceContext } from "@/components/QueryEditor/ElasticsearchQueryCo
 import { BaseQuickwitDataSource } from "@/datasource/base";
 import { useDatasourceFields } from "@/datasource/utils";
 import { Field, FieldContingency, Filter } from "../types";
+import { createContextTimeRange } from "LogContext/LogContextProvider";
 
 // TODO : define sensible defaults here
 // const excludedFields = [
@@ -51,7 +52,9 @@ export function LogContextUI(props: LogContextUIProps ){
   const {query, parsedQuery, setQuery, setParsedQuery} = builder;
   const [canRunQuery, setCanRunQuery] = useState<boolean>(false);
   const {row, origQuery, updateQuery, runContextQuery } = props;
-  const {fields, getSuggestions} = useDatasourceFields(props.datasource);
+
+  const fieldsSuggestionTimeRange = useMemo(()=>createContextTimeRange(row.timeEpochMs), [row])
+  const {fields, getSuggestions} = useDatasourceFields(props.datasource, fieldsSuggestionTimeRange);
 
   useEffect(()=>{
     setQuery(origQuery?.query || '')

src/LogContext/components/LogContextUI.tsx

@@ -13,7 +13,7 @@ import { useDispatch } from '@/hooks/useStatelessReducer';
 import { ElasticsearchQuery } from '@/types';
 
 import { BucketAggregationsEditor } from './BucketAggregationsEditor';
-import { ElasticsearchProvider, useDatasource } from './ElasticsearchQueryContext';
+import { ElasticsearchProvider, useDatasource, useRange } from './ElasticsearchQueryContext';
 import { MetricAggregationsEditor } from './MetricAggregationsEditor';
 import { metricAggregationConfig } from './MetricAggregationsEditor/utils';
 import { changeQuery } from './state';
@@ -67,7 +67,8 @@ type ElasticSearchQueryFieldProps = {
 export const ElasticSearchQueryField = ({ value, onChange, onSubmit }: ElasticSearchQueryFieldProps) => {
   const styles = useStyles2(getStyles);
   const datasource = useDatasource()
-  const { getSuggestions } = useDatasourceFields(datasource);
+  const range = useRange();
+  const { getSuggestions } = useDatasourceFields(datasource, range);
 
   return (
     <div className={styles.queryItem}>

src/components/QueryEditor/index.tsx

@@ -46,7 +46,7 @@ type FieldCapsSpec = {
   aggregatable?: boolean,
   searchable?: boolean,
   type?: string[],
-  _range?: TimeRange
+  range?: TimeRange
 }
 
 export class BaseQuickwitDataSource
@@ -163,7 +163,8 @@ export class BaseQuickwitDataSource
     )
   }
 
-  getFields(spec: FieldCapsSpec={}, range = getDefaultTimeRange()): Observable<MetricFindValue[]> {
+  getFields(spec: FieldCapsSpec={}): Observable<MetricFindValue[]> {
+    const range = spec.range || getDefaultTimeRange();
     return from(this.getResource('_elastic/' + this.index + '/_field_caps', {
       start_timestamp: Math.floor(range.from.valueOf()/SECOND),
       end_timestamp: Math.ceil(range.to.valueOf()/SECOND),
@@ -209,17 +210,16 @@ export class BaseQuickwitDataSource
   /**
    * Get tag keys for adhoc filters
    */
-  getTagKeys(spec?: FieldCapsSpec) {
-    const fields = this.getFields(spec)
+  getTagKeys(options: any) {
+    const fields = this.getFields({aggregatable:true, range: options.timeRange})
     return lastValueFrom(fields, {defaultValue:[]});
   }
 
   /**
    * Get tag values for adhoc filters
    */
   getTagValues(options: any) {
-    const range = getDefaultTimeRange();
-    const terms = this.getTerms({ field: options.key }, range)
+    const terms = this.getTerms({ field: options.key }, options.timeRange)
     return lastValueFrom(terms, {defaultValue:[]});
   }
 
@@ -292,7 +292,7 @@ export class BaseQuickwitDataSource
     if (query) {
       if (parsedQuery.find === 'fields') {
         parsedQuery.type = this.interpolateLuceneQuery(parsedQuery.type);
-        return lastValueFrom(this.getFields({aggregatable:true, type:parsedQuery.type, _range:range}), {defaultValue:[]});
+        return lastValueFrom(this.getFields({aggregatable:true, type:parsedQuery.type, range:range}), {defaultValue:[]});
       }
       if (parsedQuery.find === 'terms') {
         parsedQuery.field = this.interpolateLuceneQuery(parsedQuery.field);

src/datasource/base.ts

@@ -1,6 +1,6 @@
 import { BaseQuickwitDataSource } from "./base";
 import { useState, useEffect, useCallback } from "react";
-import{ MetricFindValue } from '@grafana/data';
+import{ MetricFindValue, TimeRange } from '@grafana/data';
 
 /**
  * Provide suggestions based on datasource fields
@@ -15,22 +15,22 @@ export type Suggestion = {
   }>
 }
 
-export function useDatasourceFields(datasource: BaseQuickwitDataSource) {
+export function useDatasourceFields(datasource: BaseQuickwitDataSource, range: TimeRange) {
   const [fields, setFields] = useState<MetricFindValue[]>([]);
 
   useEffect(() => {
     if (datasource.getTagKeys) {
-      datasource.getTagKeys({ searchable: true }).then(setFields);
+      datasource.getTagKeys({ searchable: true, timeRange: range}).then(setFields);
     }
-  }, [datasource, setFields]);
+  }, [datasource, range, setFields]);
 
   const getSuggestions = useCallback(async (word: string): Promise<Suggestion> => {
     let suggestions: Suggestion = { from: 0, options: [] };
 
     const wordIsField = word.match(/([^:\s]+):"?([^"\s]*)"?/);
     if (wordIsField?.length) {
       const [_match, fieldName, _fieldValue] = wordIsField;
-      const candidateValues = await datasource.getTagValues({ key: fieldName });
+      const candidateValues = await datasource.getTagValues({ key: fieldName, timeRange: range });
       suggestions.from = fieldName.length + 1; // Replace only the value part
       suggestions.options = candidateValues.map(v => ({
         type: 'text',
@@ -47,7 +47,7 @@ export function useDatasourceFields(datasource: BaseQuickwitDataSource) {
     }
     return suggestions;
 
-  }, [datasource, fields]);
+  }, [datasource, fields, range]);
 
   return {fields, getSuggestions}
 }

src/datasource/utils.ts

@@ -50,39 +50,39 @@ describe('useFields hook', () => {
       { wrapper, initialProps: 'cardinality' }
     );
     result.current();
-    expect(getFields).toHaveBeenLastCalledWith({aggregatable:true, type:[], _range:timeRange});
+    expect(getFields).toHaveBeenLastCalledWith({aggregatable:true, type:[], range:timeRange});
 
     // All other metric aggregations only work on numbers
     rerender('avg');
     result.current();
-    expect(getFields).toHaveBeenLastCalledWith({aggregatable:true, type:['number'], _range:timeRange});
+    expect(getFields).toHaveBeenLastCalledWith({aggregatable:true, type:['number'], range:timeRange});
 
     //
     // BUCKET AGGREGATIONS
     //
     // Date Histrogram only works on dates
     rerender('date_histogram');
     result.current();
-    expect(getFields).toHaveBeenLastCalledWith({aggregatable:true, type:['date'], _range:timeRange});
+    expect(getFields).toHaveBeenLastCalledWith({aggregatable:true, type:['date'], range:timeRange});
 
     // Histrogram only works on numbers
     rerender('histogram');
     result.current();
-    expect(getFields).toHaveBeenLastCalledWith({aggregatable:true, type:['number'], _range:timeRange});
+    expect(getFields).toHaveBeenLastCalledWith({aggregatable:true, type:['number'], range:timeRange});
 
     // Geohash Grid only works on geo_point data
     rerender('geohash_grid');
     result.current();
-    expect(getFields).toHaveBeenLastCalledWith({aggregatable:true, type:['geo_point'], _range:timeRange});
+    expect(getFields).toHaveBeenLastCalledWith({aggregatable:true, type:['geo_point'], range:timeRange});
 
     // All other bucket aggregation work on any kind of data
     rerender('terms');
     result.current();
-    expect(getFields).toHaveBeenLastCalledWith({aggregatable:true, type:[], _range:timeRange});
+    expect(getFields).toHaveBeenLastCalledWith({aggregatable:true, type:[], range:timeRange});
 
     // top_metrics work on only on numeric data in 7.7
     rerender('top_metrics');
     result.current();
-    expect(getFields).toHaveBeenLastCalledWith({aggregatable:true, type:['number'], _range:timeRange});
+    expect(getFields).toHaveBeenLastCalledWith({aggregatable:true, type:['number'], range:timeRange});
   });
 });

src/hooks/useFields.test.tsx

@@ -62,7 +62,7 @@ export const useFields = (type: AggregationType | string[]) => {
   return async (q?: string) => {
     // _mapping doesn't support filtering, we avoid sending a request everytime q changes
     if (!rawFields) {
-      rawFields = await lastValueFrom(datasource.getFields({aggregatable:true, type:filter, _range:range}));
+      rawFields = await lastValueFrom(datasource.getFields({aggregatable:true, type:filter, range:range}));
     }
 
     return rawFields.filter(({ text }) => q === undefined || text.includes(q)).map(toSelectableValue);