Browse Source

fix(dashboards): span metric widget alias (#75229)

Ogi 7 months ago
parent
commit
747cdb4d3f

+ 10 - 1
static/app/components/metrics/queryFieldGroup.tsx

@@ -71,11 +71,20 @@ function CompactSelect<Value extends SelectKey>({
   );
 }
 
-function DeleteButton({title, onClick}: {onClick: () => void; title: string}) {
+function DeleteButton({
+  title,
+  onClick,
+  disabled,
+}: {
+  onClick: () => void;
+  title: string;
+  disabled?: boolean;
+}) {
   return (
     <Tooltip title={title} delay={SLOW_TOOLTIP_DELAY}>
       <StyledButton
         icon={<IconDelete size="xs" />}
+        disabled={disabled}
         aria-label={title}
         onClick={onClick}
       />

+ 36 - 7
static/app/components/modals/metricWidgetViewerModal.tsx

@@ -31,9 +31,11 @@ import type {
 } from 'sentry/views/dashboards/metrics/types';
 import {
   expressionsToWidget,
+  formatAlias,
   getMetricEquations,
   getMetricQueries,
   getMetricWidgetTitle,
+  getVirtualAlias,
   useGenerateExpressionId,
 } from 'sentry/views/dashboards/metrics/utils';
 import {DisplayType} from 'sentry/views/dashboards/types';
@@ -56,7 +58,8 @@ function MetricWidgetViewerModal({
   dashboardFilters,
 }: Props) {
   const {selection} = usePageFilters();
-  const {resolveVirtualMRI, getVirtualMRIQuery, isLoading} = useVirtualMetricsContext();
+  const {resolveVirtualMRI, getVirtualMRIQuery, getExtractionRule, isLoading} =
+    useVirtualMetricsContext();
   const [userHasModified, setUserHasModified] = useState(false);
   const [displayType, setDisplayType] = useState(widget.displayType);
   const [metricQueries, setMetricQueries] = useState<DashboardMetricsQuery[]>(() =>
@@ -72,10 +75,15 @@ function MetricWidgetViewerModal({
     [metricEquations]
   );
 
-  const expressions = useMemo(
-    () => [...metricQueries, ...filteredEquations],
-    [metricQueries, filteredEquations]
-  );
+  const expressions = useMemo(() => {
+    const formattedAliasQueries = metricQueries.map(query => {
+      if (query.alias) {
+        return {...query, alias: formatAlias(query.alias)};
+      }
+      return query;
+    });
+    return [...formattedAliasQueries, ...filteredEquations];
+  }, [metricQueries, filteredEquations]);
 
   const generateQueryId = useGenerateExpressionId(metricQueries);
   const generateEquationId = useGenerateExpressionId(metricEquations);
@@ -100,12 +108,33 @@ function MetricWidgetViewerModal({
     (data: Partial<DashboardMetricsQuery>, index: number) => {
       setMetricQueries(curr => {
         const updated = [...curr];
-        updated[index] = {...updated[index], ...data} as DashboardMetricsQuery;
+        const currentQuery = updated[index];
+        const updatedQuery = {...updated[index], ...data} as DashboardMetricsQuery;
+        const currentSpanAttribute = getExtractionRule(currentQuery.mri)?.spanAttribute;
+        const spanAttribute = getExtractionRule(updatedQuery.mri)?.spanAttribute;
+
+        if (spanAttribute) {
+          if (!updatedQuery.alias) {
+            updatedQuery.alias = getVirtualAlias(updatedQuery.aggregation, spanAttribute);
+          } else if (currentQuery.alias && currentSpanAttribute !== spanAttribute) {
+            if (
+              currentQuery.alias.trim() ===
+              getVirtualAlias(currentQuery.aggregation, currentSpanAttribute)
+            ) {
+              updatedQuery.alias = getVirtualAlias(
+                updatedQuery.aggregation,
+                spanAttribute
+              );
+            }
+          }
+        }
+
+        updated[index] = updatedQuery;
         return updated;
       });
       setUserHasModified(true);
     },
-    [setMetricQueries]
+    [setMetricQueries, getExtractionRule]
   );
 
   const handleEquationChange = useCallback(

+ 26 - 7
static/app/components/modals/metricWidgetViewerModal/queries.tsx

@@ -1,4 +1,4 @@
-import {Fragment, memo, useCallback, useMemo, useState} from 'react';
+import {Fragment, memo, useCallback, useEffect, useMemo, useState} from 'react';
 import styled from '@emotion/styled';
 import debounce from 'lodash/debounce';
 
@@ -38,7 +38,12 @@ import type {
   DashboardMetricsExpression,
   DashboardMetricsQuery,
 } from 'sentry/views/dashboards/metrics/types';
-import {getMetricQueryName} from 'sentry/views/dashboards/metrics/utils';
+import {
+  formatAlias,
+  getMetricQueryName,
+  isVirtualAlias,
+  isVirtualExpression,
+} from 'sentry/views/dashboards/metrics/utils';
 import {DisplayType} from 'sentry/views/dashboards/types';
 import {getCreateAlert} from 'sentry/views/metrics/metricQueryContextMenu';
 
@@ -200,7 +205,7 @@ export const Queries = memo(function Queries({
 });
 
 /**
- * Wrapper for  the QueryBuilder to memoize the onChange handler
+ * Wrapper for the QueryBuilder to memoize the onChange handler
  */
 function WrappedQueryBuilder({
   index,
@@ -448,18 +453,27 @@ function ExpressionAliasForm({
   onChange: (alias: string | undefined) => void;
   hasContextMenu?: boolean;
 }) {
+  const organization = useOrganization();
+
   return (
     <ExpressionAliasWrapper hasOwnRow={hasContextMenu}>
-      {hasMetricsNewInputs(useOrganization()) ? (
+      {hasMetricsNewInputs(organization) ? (
         <QueryFieldGroup>
           <QueryFieldGroup.Label>As</QueryFieldGroup.Label>
           <QueryFieldGroup.DebouncedInput
             type="text"
-            value={expression.alias}
-            onChange={e => onChange(e.target.value)}
+            value={formatAlias(expression.alias)}
+            onChange={e => {
+              if (isVirtualAlias(expression.alias)) {
+                onChange(`v|${e.target.value}`);
+              } else {
+                onChange(e.target.value);
+              }
+            }}
             placeholder={t('Add alias')}
           />
           <QueryFieldGroup.DeleteButton
+            disabled={isVirtualExpression(expression)}
             title={t('Clear Alias')}
             onClick={() => onChange(undefined)}
           />
@@ -469,12 +483,13 @@ function ExpressionAliasForm({
           <StyledLabel>as</StyledLabel>
           <StyledDebouncedInput
             type="text"
-            value={expression.alias}
+            value={formatAlias(expression.alias)}
             onChange={e => onChange(e.target.value)}
             placeholder={t('Add alias')}
           />
           <Tooltip title={t('Clear alias')} delay={SLOW_TOOLTIP_DELAY}>
             <StyledButton
+              disabled={isVirtualExpression(expression)}
               icon={<IconDelete size="xs" />}
               aria-label={t('Clear Alias')}
               onClick={() => onChange(undefined)}
@@ -496,6 +511,10 @@ export function DebouncedInput({
     inputProps.value
   );
 
+  useEffect(() => {
+    setValue(inputProps.value);
+  }, [inputProps.value]);
+
   const handleChange = useMemo(
     () =>
       debounce((e: React.ChangeEvent<HTMLInputElement>) => {

+ 26 - 20
static/app/components/modals/widgetBuilder/addToDashboardModal.tsx

@@ -1,4 +1,4 @@
-import {useEffect, useState} from 'react';
+import {useEffect, useMemo, useState} from 'react';
 import type {InjectedRouter} from 'react-router';
 import {css} from '@emotion/react';
 import styled from '@emotion/styled';
@@ -208,6 +208,30 @@ function AddToDashboardModal({
 
   const canSubmit = selectedDashboardId !== null;
 
+  const options = useMemo(() => {
+    if (dashboards === null) {
+      return null;
+    }
+
+    return [
+      allowCreateNewDashboard && {
+        label: t('+ Create New Dashboard'),
+        value: 'new',
+      },
+      ...dashboards.map(({title, id, widgetDisplay}) => ({
+        label: title,
+        value: id,
+        disabled: widgetDisplay.length >= MAX_WIDGETS,
+        tooltip:
+          widgetDisplay.length >= MAX_WIDGETS &&
+          tct('Max widgets ([maxWidgets]) per dashboard reached.', {
+            maxWidgets: MAX_WIDGETS,
+          }),
+        tooltipOptions: {position: 'right'},
+      })),
+    ].filter(Boolean) as SelectValue<string>[];
+  }, [allowCreateNewDashboard, dashboards]);
+
   return (
     <OrganizationContext.Provider value={organization}>
       <Header closeButton>
@@ -221,25 +245,7 @@ function AddToDashboardModal({
             name="dashboard"
             placeholder={t('Select Dashboard')}
             value={selectedDashboardId}
-            options={
-              dashboards && [
-                allowCreateNewDashboard && {
-                  label: t('+ Create New Dashboard'),
-                  value: 'new',
-                },
-                ...dashboards.map(({title, id, widgetDisplay}) => ({
-                  label: title,
-                  value: id,
-                  disabled: widgetDisplay.length >= MAX_WIDGETS,
-                  tooltip:
-                    widgetDisplay.length >= MAX_WIDGETS &&
-                    tct('Max widgets ([maxWidgets]) per dashboard reached.', {
-                      maxWidgets: MAX_WIDGETS,
-                    }),
-                  tooltipOptions: {position: 'right'},
-                })),
-              ]
-            }
+            options={options}
             onChange={(option: SelectValue<string>) => {
               if (option.disabled) {
                 return;

+ 1 - 0
static/app/views/dashboards/metrics/types.tsx

@@ -24,6 +24,7 @@ export interface DashboardMetricsEquation {
   isHidden: boolean;
   type: MetricExpressionType.EQUATION;
   alias?: string;
+  isQueryOnly?: boolean;
 }
 
 export type DashboardMetricsExpression = DashboardMetricsQuery | DashboardMetricsEquation;

+ 32 - 1
static/app/views/dashboards/metrics/utils.tsx

@@ -3,7 +3,11 @@ import {useMemo} from 'react';
 import {getEquationSymbol} from 'sentry/components/metrics/equationSymbol';
 import {getQuerySymbol} from 'sentry/components/metrics/querySymbol';
 import type {MetricAggregation, MRI} from 'sentry/types/metrics';
-import {getDefaultAggregation, unescapeMetricsFormula} from 'sentry/utils/metrics';
+import {
+  getDefaultAggregation,
+  isVirtualMetric,
+  unescapeMetricsFormula,
+} from 'sentry/utils/metrics';
 import {NO_QUERY_ID} from 'sentry/utils/metrics/constants';
 import {
   formatMRIField,
@@ -310,3 +314,30 @@ export function defaultMetricWidget(): Widget {
     DisplayType.LINE
   );
 }
+
+export const isVirtualExpression = (expression: DashboardMetricsExpression) => {
+  if ('mri' in expression) {
+    return isVirtualMetric(expression);
+  }
+  return false;
+};
+
+export const isVirtualAlias = (alias?: string) => {
+  return alias?.startsWith('v|');
+};
+
+export const formatAlias = (alias?: string) => {
+  if (!alias) {
+    return alias;
+  }
+
+  if (!isVirtualAlias(alias)) {
+    return alias;
+  }
+
+  return alias.replace('v|', '');
+};
+
+export const getVirtualAlias = (aggregation, spanAttribute) => {
+  return `v|${aggregation}(${spanAttribute})`;
+};

+ 53 - 14
static/app/views/dashboards/metrics/widgetCard.tsx

@@ -1,4 +1,4 @@
-import {useMemo} from 'react';
+import {Fragment, useMemo} from 'react';
 import type {InjectedRouter} from 'react-router';
 import styled from '@emotion/styled';
 import {ErrorBoundary} from '@sentry/react';
@@ -6,23 +6,28 @@ import type {Location} from 'history';
 
 import ErrorPanel from 'sentry/components/charts/errorPanel';
 import {HeaderTitle} from 'sentry/components/charts/styles';
+import {EquationFormatter} from 'sentry/components/metrics/equationInput/syntax/formatter';
 import TextOverflow from 'sentry/components/textOverflow';
 import {IconWarning} from 'sentry/icons';
 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 {getFormattedMQL, unescapeMetricsFormula} from 'sentry/utils/metrics';
 import {hasMetricsNewInputs} from 'sentry/utils/metrics/features';
-import {parseMRI} from 'sentry/utils/metrics/mri';
+import {formatMRIField, MRIToField, parseMRI} from 'sentry/utils/metrics/mri';
 import {MetricExpressionType} from 'sentry/utils/metrics/types';
 import {useMetricsQuery} from 'sentry/utils/metrics/useMetricsQuery';
 import {useVirtualMetricsContext} from 'sentry/utils/metrics/virtualMetricsContext';
 import {MetricBigNumberContainer} from 'sentry/views/dashboards/metrics/bigNumber';
 import {MetricChartContainer} from 'sentry/views/dashboards/metrics/chart';
 import {MetricTableContainer} from 'sentry/views/dashboards/metrics/table';
+import type {DashboardMetricsExpression} from 'sentry/views/dashboards/metrics/types';
 import {
   expressionsToApiQueries,
+  formatAlias,
   getMetricExpressions,
+  isMetricsEquation,
   toMetricDisplayType,
 } from 'sentry/views/dashboards/metrics/utils';
 import type {DashboardFilters, Widget} from 'sentry/views/dashboards/types';
@@ -32,7 +37,6 @@ import {DashboardsMEPContext} from 'sentry/views/dashboards/widgetCard/dashboard
 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 = {
   isEditingDashboard: boolean;
@@ -53,6 +57,30 @@ type Props = {
 
 const EMPTY_FN = () => {};
 
+export function getWidgetTitle(expressions: DashboardMetricsExpression[]) {
+  const filteredExpressions = expressions.filter(query => !query.isQueryOnly);
+
+  if (filteredExpressions.length === 1) {
+    const firstQuery = filteredExpressions[0];
+    if (isMetricsEquation(firstQuery)) {
+      return (
+        <Fragment>
+          <EquationFormatter equation={unescapeMetricsFormula(firstQuery.formula)} />
+        </Fragment>
+      );
+    }
+    return formatAlias(firstQuery.alias) ?? getFormattedMQL(firstQuery);
+  }
+
+  return filteredExpressions
+    .map(q =>
+      isMetricsEquation(q)
+        ? formatAlias(q.alias) ?? unescapeMetricsFormula(q.formula)
+        : formatAlias(q.alias) ?? formatMRIField(MRIToField(q.mri, q.aggregation))
+    )
+    .join(', ');
+}
+
 export function MetricWidgetCard({
   organization,
   selection,
@@ -70,29 +98,40 @@ export function MetricWidgetCard({
   const {getVirtualMRIQuery, isLoading: isLoadingVirtualMetrics} =
     useVirtualMetricsContext();
 
-  const metricQueries = useMemo(
-    () =>
-      expressionsToApiQueries(
-        getMetricExpressions(widget, dashboardFilters, getVirtualMRIQuery),
-        metricsNewInputs
-      ),
-    [widget, dashboardFilters, getVirtualMRIQuery, metricsNewInputs]
+  const metricExpressions = getMetricExpressions(
+    widget,
+    dashboardFilters,
+    getVirtualMRIQuery
   );
+
   const hasSetMetric = useMemo(
     () =>
-      getMetricExpressions(widget, dashboardFilters, getVirtualMRIQuery).some(
+      metricExpressions.some(
         expression =>
           expression.type === MetricExpressionType.QUERY &&
           parseMRI(expression.mri)!.type === 's'
       ),
-    [widget, dashboardFilters, getVirtualMRIQuery]
+    [metricExpressions]
   );
 
   const widgetMQL = useMemo(
-    () => (isLoadingVirtualMetrics ? '' : getWidgetTitle(metricQueries)),
-    [isLoadingVirtualMetrics, metricQueries]
+    () => (isLoadingVirtualMetrics ? '' : getWidgetTitle(metricExpressions)),
+    [isLoadingVirtualMetrics, metricExpressions]
   );
 
+  const metricQueries = useMemo(() => {
+    const formattedAliasQueries = expressionsToApiQueries(
+      metricExpressions,
+      metricsNewInputs
+    ).map(query => {
+      if (query.alias) {
+        return {...query, alias: formatAlias(query.alias)};
+      }
+      return query;
+    });
+    return [...formattedAliasQueries];
+  }, [metricExpressions, metricsNewInputs]);
+
   const {interval: validatedInterval} = useMetricsIntervalOptions({
     // TODO: Figure out why this can be undefined
     interval: widget.interval ?? '',