[BUGFIX] Prometheus calculate interval and minstep for query replacement (#475)

Signed-off-by: Seyed Mahmoud SHAHROKNI <[email protected]>

Author: shahrokni

prometheus/src/components/PromQLEditor.tsx

@@ -33,10 +33,21 @@ export type PromQLEditorProps = {
   completeConfig: CompleteConfiguration;
   datasource: PrometheusDatasourceSelector;
   isReadOnly?: boolean;
+  treeViewMetadata?: {
+    minStepMs: number;
+    intervalMs: number;
+  };
 } & Omit<ReactCodeMirrorProps, 'theme' | 'extensions'>;
 
-export function PromQLEditor({ completeConfig, datasource, isReadOnly, ...rest }: PromQLEditorProps): ReactElement {
+export function PromQLEditor({
+  completeConfig,
+  datasource,
+  isReadOnly,
+  treeViewMetadata,
+  ...rest
+}: PromQLEditorProps): ReactElement {
   const theme = useTheme();
+
   const isDarkMode = theme.palette.mode === 'dark';
   const [isTreeViewVisible, setTreeViewVisible] = useState(false);
   const readOnly = isReadOnly ?? false;
@@ -46,12 +57,9 @@ export function PromQLEditor({ completeConfig, datasource, isReadOnly, ...rest }
   }, [completeConfig]);
 
   let queryExpr = useReplaceVariablesInString(rest.value);
-  if (queryExpr) {
-    // TODO placeholder values for steps to be replaced with actual values
-    // Looks like providing proper values involves some refactoring: currently we'd need to rely on the timeseries query context,
-    // but these step values are actually computed independently / before the queries are getting fired, so it's useless to fire
-    // queries here, so maybe we should extract this part to independant hook(s), to be reused here?
-    queryExpr = replacePromBuiltinVariables(queryExpr, 12345, 12345);
+  if (queryExpr && treeViewMetadata) {
+    const { minStepMs, intervalMs } = treeViewMetadata;
+    queryExpr = replacePromBuiltinVariables(queryExpr, minStepMs, intervalMs);
   }
 
   const { data: parseQueryResponse, isLoading, error } = useParseQuery(queryExpr ?? '', datasource, isTreeViewVisible);

prometheus/src/plugins/prometheus-time-series-query/PrometheusTimeSeriesQueryEditor.tsx

@@ -15,16 +15,24 @@ import { produce } from 'immer';
 import {
   DatasourceSelect,
   DatasourceSelectProps,
+  replaceVariables,
+  useAllVariableValues,
   useDatasource,
   useDatasourceClient,
   useDatasourceSelectValueToSelector,
+  useSuggestedStepMs,
+  useTimeRange,
 } from '@perses-dev/plugin-system';
 import { useId } from '@perses-dev/components';
 import { FormControl, Stack, TextField } from '@mui/material';
-import { ReactElement } from 'react';
+import { ReactElement, useContext, useMemo } from 'react';
+import { PanelEditorContext } from '@perses-dev/dashboards';
 import {
   DEFAULT_PROM,
   DurationString,
+  getDurationStringSeconds,
+  getPrometheusTimeRange,
+  getRangeStep,
   isDefaultPromSelector,
   isPrometheusDatasourceSelector,
   PROM_DATASOURCE_KIND,
@@ -39,7 +47,6 @@ import {
   useFormatState,
   useMinStepState,
 } from './query-editor-model';
-
 /**
  * The options editor component for editing a PrometheusTimeSeriesQuery's spec.
  */
@@ -91,6 +98,37 @@ export function PrometheusTimeSeriesQueryEditor(props: PrometheusTimeSeriesQuery
     throw new Error('Got unexpected non-Prometheus datasource selector');
   };
 
+  const variableState = useAllVariableValues();
+  const { absoluteTimeRange } = useTimeRange();
+  const panelEditorContext = useContext(PanelEditorContext);
+  const suggestedStepMs = useSuggestedStepMs(panelEditorContext?.preview.previewPanelWidth);
+
+  const minStepMs = useMemo(() => {
+    /* Try catch is necessary, because when the minStep value is being typed, it will be valid when the duration unit is added. Example: 2m = 2 + m */
+    try {
+      const durationsSeconds = getDurationStringSeconds(
+        replaceVariables(minStepPlaceholder, variableState) as DurationString
+      );
+      return durationsSeconds !== undefined ? durationsSeconds * 1000 : undefined;
+    } catch {
+      return undefined;
+    }
+  }, [variableState, minStepPlaceholder]);
+
+  const intervalMs = useMemo(() => {
+    const minStepSeconds = (minStepMs ?? 0) / 1000;
+    return getRangeStep(getPrometheusTimeRange(absoluteTimeRange), minStepSeconds, undefined, suggestedStepMs) * 1000;
+  }, [absoluteTimeRange, minStepMs, suggestedStepMs]);
+
+  const treeViewMetadata = useMemo(() => {
+    return minStepMs && intervalMs
+      ? {
+          minStepMs,
+          intervalMs,
+        }
+      : undefined;
+  }, [minStepMs, intervalMs]);
+
   return (
     <Stack spacing={2}>
       <FormControl margin="dense" fullWidth={false}>
@@ -111,6 +149,7 @@ export function PrometheusTimeSeriesQueryEditor(props: PrometheusTimeSeriesQuery
         onChange={handleQueryChange}
         onBlur={handleQueryBlur}
         isReadOnly={isReadonly}
+        treeViewMetadata={treeViewMetadata}
       />
       <Stack direction="row" spacing={2}>
         <TextField

prometheus/src/plugins/prometheus-time-series-query/get-time-series-data.ts

@@ -73,6 +73,7 @@ export const getTimeSeriesData: TimeSeriesQueryPlugin<PrometheusTimeSeriesQueryS
       // TODO add a validation check to make sure the variable is a DurationString, to avoid the back & forth cast here
       replaceVariables(spec.minStep as string, context.variableState) as DurationString
     ) ?? datasourceScrapeInterval;
+
   const timeRange = getPrometheusTimeRange(context.timeRange);
   const step = getRangeStep(timeRange, minStep, undefined, context.suggestedStepMs); // TODO: resolution
 
@@ -99,6 +100,7 @@ export const getTimeSeriesData: TimeSeriesQueryPlugin<PrometheusTimeSeriesQueryS
   // Replace variable placeholders in PromQL query
   const intervalMs = step * 1000;
   const minStepMs = minStep * 1000;
+
   let query = replacePromBuiltinVariables(spec.query, minStepMs, intervalMs);
   query = replaceVariables(query, context.variableState);