Loki: Fix validation of step values to also allow e.g. ms values (#73270)

* use go duration validation

* add `isValidGrafanaDuration`

* use `isValidGrafanaDuration`

* improve jsdoc

packages/grafana-data/src/datetime/durationutil.test.ts

@@ -5,6 +5,7 @@ import {
   isValidDuration,
   isValidGoDuration,
   durationToMilliseconds,
+  isValidGrafanaDuration,
 } from './durationutil';
 
 describe('Duration util', () => {
@@ -68,6 +69,28 @@ describe('Duration util', () => {
     });
   });
 
+  describe('isValidGrafanaDuration', () => {
+    it('valid duration string returns true', () => {
+      const durationString = '7y 6M 5w 4d 3h 4m 1s 2ms 3us 5ns';
+      expect(isValidGrafanaDuration(durationString)).toEqual(true);
+    });
+
+    it('valid float number duration string returns true', () => {
+      const durationString = '7.1y 6.1M 5.1w 4.1d 3.1h 4.0m 0.1s 2.11ms 0.03us 5.3333ns';
+      expect(isValidGrafanaDuration(durationString)).toEqual(true);
+    });
+
+    it('invalid duration string returns false', () => {
+      const durationString = '3M 6v 5b 4m';
+      expect(isValidGrafanaDuration(durationString)).toEqual(false);
+    });
+
+    it('invalid float number duration string returns false', () => {
+      const durationString = '3.h -4.0m 0.s 2.ms -0.us 5.ns';
+      expect(isValidGrafanaDuration(durationString)).toEqual(false);
+    });
+  });
+
   describe('durationToMilliseconds', () => {
     it('converts a duration to milliseconds', () => {
       const duration = { hours: 1, minutes: 30, seconds: 45 };

packages/grafana-data/src/datetime/durationutil.ts

@@ -99,8 +99,11 @@ export function isValidDate(dateString: string): boolean {
 }
 
 /**
- * isValidDuration returns true if the given string can be parsed into a valid Duration object, false otherwise
+ * isValidDuration returns true if the given string can be parsed into a valid `date-fns` `Duration` object, false otherwise
  *
+ * Valid time units are "y", "Y", "years", "M", "months", "w", "W", "weeks", "d", "D", "days", "h", "H", "hours", "m", "minutes", "s", "S", "seconds"
+ *
+ * @see https://date-fns.org/v2.30.0/docs/Duration
  * @param durationString - string representation of a duration
  *
  * @public
@@ -127,7 +130,7 @@ export function isValidDuration(durationString: string): boolean {
  *
  * Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".
  *
- * Go docs: https://pkg.go.dev/time#ParseDuration
+ * @see https://pkg.go.dev/time#ParseDuration
  *
  * @param durationString - string representation of a duration
  *
@@ -135,6 +138,27 @@ export function isValidDuration(durationString: string): boolean {
  */
 export function isValidGoDuration(durationString: string): boolean {
   const timeUnits = ['h', 'm', 's', 'ms', 'us', 'µs', 'ns'];
+  return validateDurationByUnits(durationString, timeUnits);
+}
+
+/**
+ * isValidGrafanaDuration returns `true` if the given string can be parsed into a valid Duration object based on
+ * the Grafana SDK's gtime.parseDuration, `false` otherwise.
+ *
+ * Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h", "d", "w", "M", "y".
+ *
+ * @see https://pkg.go.dev/github.com/grafana/grafana-plugin-sdk-go/backend/gtime#ParseDuration
+ *
+ * @param durationString - string representation of a duration
+ *
+ * @internal
+ */
+export function isValidGrafanaDuration(durationString: string): boolean {
+  const timeUnits = ['y', 'M', 'w', 'd', 'h', 'm', 's', 'ms', 'us', 'µs', 'ns'];
+  return validateDurationByUnits(durationString, timeUnits);
+}
+
+function validateDurationByUnits(durationString: string, timeUnits: string[]): boolean {
   for (const value of durationString.trim().split(' ')) {
     const match = value.match(/([0-9]*[.]?[0-9]+)(.+)/);
     if (match === null || match.length !== 3) {

public/app/plugins/datasource/loki/querybuilder/components/LokiQueryBuilderOptions.test.tsx

@@ -140,6 +140,18 @@ describe('LokiQueryBuilderOptions', () => {
     await userEvent.click(screen.getByRole('button', { name: /Options/ }));
     expect(screen.queryByText(/Invalid step/)).not.toBeInTheDocument();
   });
+
+  it('does not shows error when valid millisecond value in step', async () => {
+    setup({ expr: 'rate({foo="bar"}[5m]', step: '1ms' });
+    await userEvent.click(screen.getByRole('button', { name: /Options/ }));
+    expect(screen.queryByText(/Invalid step/)).not.toBeInTheDocument();
+  });
+
+  it('does not shows error when valid day value in step', async () => {
+    setup({ expr: 'rate({foo="bar"}[5m]', step: '1d' });
+    await userEvent.click(screen.getByRole('button', { name: /Options/ }));
+    expect(screen.queryByText(/Invalid step/)).not.toBeInTheDocument();
+  });
 });
 
 function setup(queryOverrides: Partial<LokiQuery> = {}) {

public/app/plugins/datasource/loki/querybuilder/components/LokiQueryBuilderOptions.tsx

@@ -1,7 +1,7 @@
 import { trim } from 'lodash';
 import React, { useMemo, useState } from 'react';
 
-import { CoreApp, isValidDuration, SelectableValue } from '@grafana/data';
+import { CoreApp, isValidDuration, isValidGrafanaDuration, SelectableValue } from '@grafana/data';
 import { EditorField, EditorRow } from '@grafana/experimental';
 import { config, reportInteraction } from '@grafana/runtime';
 import { Alert, AutoSizeInput, RadioButtonGroup, Select } from '@grafana/ui';
@@ -71,7 +71,7 @@ export const LokiQueryBuilderOptions = React.memo<Props>(
     const isLogQuery = isLogsQuery(query.expr);
 
     const isValidStep = useMemo(() => {
-      if (!query.step || isValidDuration(query.step) || !isNaN(Number(query.step))) {
+      if (!query.step || isValidGrafanaDuration(query.step) || !isNaN(Number(query.step))) {
         return true;
       }
       return false;