Browse Source

feat(metrics): Lower granularity limit for set metrics (#72494)

Disable interval options that are smaller than `1h` for set metrics.
Ensure dashboard widgets do not query with a more granular interval.

Closes https://github.com/getsentry/sentry/issues/70722
ArthurKnaus 9 months ago
parent
commit
502fab2203

+ 16 - 1
static/app/components/modals/metricWidgetViewerModal/visualization.tsx

@@ -11,7 +11,12 @@ import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import type {MetricsQueryApiResponse} from 'sentry/types/metrics';
 import {DEFAULT_SORT_STATE} from 'sentry/utils/metrics/constants';
-import type {FocusedMetricsSeries, SortState} from 'sentry/utils/metrics/types';
+import {parseMRI} from 'sentry/utils/metrics/mri';
+import {
+  type FocusedMetricsSeries,
+  MetricExpressionType,
+  type SortState,
+} from 'sentry/utils/metrics/types';
 import {
   type MetricsQueryApiQueryParams,
   useMetricsQuery,
@@ -128,8 +133,18 @@ export function MetricVisualization({
   interval,
 }: MetricVisualizationProps) {
   const {selection} = usePageFilters();
+  const hasSetMetric = useMemo(
+    () =>
+      expressions.some(
+        expression =>
+          expression.type === MetricExpressionType.QUERY &&
+          parseMRI(expression.mri)!.type === 's'
+      ),
+    [expressions]
+  );
   const {interval: validatedInterval} = useMetricsIntervalOptions({
     interval,
+    hasSetMetric,
     datetime: selection.datetime,
     onIntervalChange: EMPTY_FN,
   });

+ 22 - 1
static/app/views/dashboards/metrics/widgetCard.tsx

@@ -12,6 +12,8 @@ import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import type {PageFilters} from 'sentry/types/core';
 import type {Organization} from 'sentry/types/organization';
+import {parseMRI} from 'sentry/utils/metrics/mri';
+import {MetricExpressionType} from 'sentry/utils/metrics/types';
 import {useMetricsQuery} from 'sentry/utils/metrics/useMetricsQuery';
 import {MetricBigNumberContainer} from 'sentry/views/dashboards/metrics/bigNumber';
 import {MetricChartContainer} from 'sentry/views/dashboards/metrics/chart';
@@ -27,6 +29,7 @@ import {WidgetCardPanel, WidgetTitleRow} from 'sentry/views/dashboards/widgetCar
 import {DashboardsMEPContext} from 'sentry/views/dashboards/widgetCard/dashboardsMEPContext';
 import {Toolbar} from 'sentry/views/dashboards/widgetCard/toolbar';
 import WidgetCardContextMenu from 'sentry/views/dashboards/widgetCard/widgetCardContextMenu';
+import {useMetricsIntervalOptions} from 'sentry/views/metrics/utils/useMetricsIntervalParam';
 import {getWidgetTitle} from 'sentry/views/metrics/widget';
 
 type Props = {
@@ -46,6 +49,8 @@ type Props = {
   showContextMenu?: boolean;
 };
 
+const EMPTY_FN = () => {};
+
 export function MetricWidgetCard({
   organization,
   selection,
@@ -63,16 +68,32 @@ export function MetricWidgetCard({
     () => expressionsToApiQueries(getMetricExpressions(widget, dashboardFilters)),
     [widget, dashboardFilters]
   );
+  const hasSetMetric = useMemo(
+    () =>
+      getMetricExpressions(widget, dashboardFilters).some(
+        expression =>
+          expression.type === MetricExpressionType.QUERY &&
+          parseMRI(expression.mri)!.type === 's'
+      ),
+    [widget, dashboardFilters]
+  );
 
   const widgetMQL = useMemo(() => getWidgetTitle(metricQueries), [metricQueries]);
 
+  const {interval: validatedInterval} = useMetricsIntervalOptions({
+    interval: widget.interval,
+    hasSetMetric,
+    datetime: selection.datetime,
+    onIntervalChange: EMPTY_FN,
+  });
+
   const {
     data: timeseriesData,
     isLoading,
     isError,
     error,
   } = useMetricsQuery(metricQueries, selection, {
-    intervalLadder: widget.displayType === DisplayType.BAR ? 'bar' : 'dashboard',
+    interval: validatedInterval,
   });
 
   const vizualizationComponent = useMemo(() => {

+ 49 - 9
static/app/views/metrics/utils/useMetricsIntervalParam.tsx

@@ -15,9 +15,12 @@ import {t} from 'sentry/locale';
 import type {PageFilters} from 'sentry/types/core';
 import {parsePeriodToHours} from 'sentry/utils/duration/parsePeriodToHours';
 import {useUpdateQuery} from 'sentry/utils/metrics';
+import {parseMRI} from 'sentry/utils/metrics/mri';
+import {isMetricsQueryWidget} from 'sentry/utils/metrics/types';
 import {decodeScalar} from 'sentry/utils/queryString';
 import useLocationQuery from 'sentry/utils/url/useLocationQuery';
 import usePageFilters from 'sentry/utils/usePageFilters';
+import {useMetricsContext} from 'sentry/views/metrics/context';
 
 const ALL_INTERVAL_OPTIONS = [
   {value: '1m', label: t('1 minute')},
@@ -51,13 +54,21 @@ const maximumInterval = new GranularityLadder([
   [0, '1m'],
 ]);
 
-export function getIntervalOptionsForStatsPeriod(datetime: PageFilters['datetime']) {
+export function getIntervalOptionsForStatsPeriod(
+  datetime: PageFilters['datetime'],
+  maxInterval?: string
+) {
   const diffInMinutes = getDiffInMinutes(datetime);
+  const maxIntervalInHours = maxInterval && parsePeriodToHours(maxInterval);
 
   const minimumOption = minimumInterval.getInterval(diffInMinutes);
   const minimumOptionInHours = parsePeriodToHours(minimumOption);
 
-  const maximumOption = maximumInterval.getInterval(diffInMinutes);
+  const defaultMaximumOption = maximumInterval.getInterval(diffInMinutes);
+  const maximumOption =
+    maxIntervalInHours && maxIntervalInHours > parsePeriodToHours(defaultMaximumOption)
+      ? maxInterval
+      : defaultMaximumOption;
   const maximumOptionInHours = parsePeriodToHours(maximumOption);
 
   return ALL_INTERVAL_OPTIONS.filter(option => {
@@ -68,21 +79,31 @@ export function getIntervalOptionsForStatsPeriod(datetime: PageFilters['datetime
 
 export function validateInterval(
   interval: string,
-  options: {label: string; value: string}[]
+  options: {label: string; value: string; disabled?: boolean}[]
 ) {
   const isPeriod = !!parseStatsPeriod(interval);
-  const currentIntervalValues = options.map(option => option.value);
+  const enabledOptions = options.filter(option => !option.disabled);
+  const currentIntervalValues = enabledOptions.map(option => option.value);
   return isPeriod && currentIntervalValues.includes(interval)
     ? interval
     : // Take the 2nd most granular option if available
-      options[1]?.value ?? options[0].value;
+      enabledOptions[1]?.value ?? enabledOptions[0].value;
 }
 
 export function useMetricsIntervalParam() {
   const {datetime} = usePageFilters().selection;
   const {interval} = useLocationQuery({fields: {interval: decodeScalar}});
+  const {widgets} = useMetricsContext();
   const updateQuery = useUpdateQuery();
 
+  const hasSetMetric = useMemo(
+    () =>
+      widgets.some(
+        widget => isMetricsQueryWidget(widget) && parseMRI(widget.mri)!.type === 's'
+      ),
+    [widgets]
+  );
+
   const handleIntervalChange = useCallback(
     (newInterval: string) => {
       updateQuery({interval: newInterval}, {replace: true});
@@ -92,6 +113,7 @@ export function useMetricsIntervalParam() {
 
   const metricsIntervalOptions = useMetricsIntervalOptions({
     interval,
+    hasSetMetric,
     datetime,
     onIntervalChange: handleIntervalChange,
   });
@@ -107,19 +129,37 @@ export function useMetricsIntervalParam() {
 
 export interface MetricsIntervalParamProps {
   datetime: PageFilters['datetime'];
+  hasSetMetric: boolean;
   interval: string;
   onIntervalChange: (interval: string) => void;
 }
 
 export function useMetricsIntervalOptions({
   interval,
+  hasSetMetric,
   datetime,
   onIntervalChange,
 }: MetricsIntervalParamProps) {
-  const currentIntervalOptions = useMemo(
-    () => getIntervalOptionsForStatsPeriod(datetime),
-    [datetime]
-  );
+  const currentIntervalOptions = useMemo(() => {
+    const minInterval = hasSetMetric ? '1h' : undefined;
+    const options = getIntervalOptionsForStatsPeriod(datetime, minInterval);
+    if (!hasSetMetric) {
+      return options;
+    }
+
+    // Disable all intervals that are smaller than one hour for set metrics
+    return options.map(option => {
+      return parsePeriodToHours(option.value) < 1
+        ? {
+            ...option,
+            disabled: true,
+            tooltip: t(
+              'Intervals smaller than 1 hour are not available for set metrics.'
+            ),
+          }
+        : option;
+    });
+  }, [datetime, hasSetMetric]);
 
   const setInterval = useCallback(
     (newInterval: string) => {