Browse Source

feat(alerts): on demand alerts UI (#55838)

Ogi 1 year ago
parent
commit
f28faf3ef2

+ 0 - 1
src/sentry/assistant/guides.py

@@ -28,7 +28,6 @@ GUIDES = {
     "explain_archive_button_issue_details": 29,
     "explain_archive_tab_issue_stream": 30,
     "explain_new_default_event_issue_detail": 31,
-    "explain_empty_on_demand_alert": 32,
 }
 
 # demo mode has different guides

+ 94 - 0
static/app/components/alerts/onDemandMetricAlert.tsx

@@ -0,0 +1,94 @@
+import React from 'react';
+import styled from '@emotion/styled';
+
+import Alert from 'sentry/components/alert';
+import {Button} from 'sentry/components/button';
+import {Tooltip} from 'sentry/components/tooltip';
+import {IconClose, IconWarning} from 'sentry/icons';
+import {t} from 'sentry/locale';
+import {Color} from 'sentry/utils/theme';
+import useDismissAlert from 'sentry/utils/useDismissAlert';
+
+const EXTRAPOLATED_AREA_STRIPE_IMG =
+  'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAoAAABkAQMAAACFAjPUAAAAAXNSR0IB2cksfwAAAAlwSFlzAAALEwAACxMBAJqcGAAAAAZQTFRFpKy5SVlzL3npZAAAAA9JREFUeJxjsD/AMIqIQwBIyGOd43jaDwAAAABJRU5ErkJggg==';
+
+export const extrapolatedAreaStyle = {
+  color: {
+    repeat: 'repeat',
+    image: EXTRAPOLATED_AREA_STRIPE_IMG,
+    rotation: 0.785,
+    scaleX: 0.5,
+  },
+  opacity: 1.0,
+};
+
+export function OnDemandWarningIcon({
+  msg,
+  color = 'gray300',
+}: {
+  msg: React.ReactNode;
+  color?: Color;
+}) {
+  return (
+    <Tooltip skipWrapper title={msg}>
+      <HoverableIconWarning color={color} />
+    </Tooltip>
+  );
+}
+
+const LOCAL_STORAGE_KEY = 'on-demand-empty-alert-dismissed';
+
+export function OnDemandMetricAlert({
+  message,
+  dismissable = false,
+}: {
+  message: React.ReactNode;
+  dismissable?: boolean;
+}) {
+  const {dismiss, isDismissed} = useDismissAlert({key: LOCAL_STORAGE_KEY});
+
+  if (isDismissed) {
+    return null;
+  }
+
+  return (
+    <InfoAlert showIcon>
+      {message}
+      {dismissable && (
+        <DismissButton
+          priority="link"
+          size="sm"
+          icon={<IconClose size="xs" />}
+          aria-label={t('Close Alert')}
+          onClick={dismiss}
+        />
+      )}
+    </InfoAlert>
+  );
+}
+
+const InfoAlert = styled(Alert)`
+  display: flex;
+  align-items: flex-start;
+
+  & > span {
+    display: flex;
+    flex-grow: 1;
+    justify-content: space-between;
+
+    line-height: 1.5em;
+  }
+`;
+
+const DismissButton = styled(Button)`
+  pointer-events: all;
+  &:hover {
+    opacity: 0.5;
+  }
+`;
+
+const HoverableIconWarning = styled(IconWarning)`
+  &:hover {
+    cursor: pointer;
+  }
+`;

+ 0 - 13
static/app/components/assistant/getGuidesContent.tsx

@@ -486,18 +486,5 @@ function getDemoModeGuides(): GuidesContent {
         },
       ],
     },
-    {
-      guide: 'explain_empty_on_demand_alert',
-      requiredTargets: ['empty_on_demand_chart'],
-      steps: [
-        {
-          title: t('Why is the chart empty?'),
-          target: 'empty_on_demand_chart',
-          description: t(
-            `This alert uses specific filters that we don't routinely collect metrics from. This means we don't have any historical data to show right now, but we'll capture all metrics that match the filters from this point on.`
-          ),
-        },
-      ],
-    },
   ];
 }

+ 5 - 0
static/app/components/charts/utils.tsx

@@ -4,6 +4,7 @@ import moment from 'moment';
 
 import {DEFAULT_STATS_PERIOD} from 'sentry/constants';
 import {EventsStats, MultiSeriesEventsStats, PageFilters} from 'sentry/types';
+import {Series} from 'sentry/types/echarts';
 import {defined, escape} from 'sentry/utils';
 import {getFormattedDate, parsePeriodToHours} from 'sentry/utils/dates';
 import type {TableDataWithTitle} from 'sentry/utils/discover/discoverQuery';
@@ -413,3 +414,7 @@ export function useEchartsAriaLabels(
     label: {description: [title, ...seriesDescriptions].join('. ')},
   };
 }
+
+export function isEmptySeries(series: Series) {
+  return series.data.every(dataPoint => dataPoint.value === 0);
+}

+ 4 - 29
static/app/utils/onDemandMetrics/index.tsx

@@ -1,5 +1,4 @@
 import React from 'react';
-import styled from '@emotion/styled';
 
 import {
   ParseResult,
@@ -7,8 +6,6 @@ import {
   Token,
   TokenResult,
 } from 'sentry/components/searchSyntax/parser';
-import {Tooltip} from 'sentry/components/tooltip';
-import {IconWarning} from 'sentry/icons';
 import {Organization} from 'sentry/types';
 import {FieldKey, getFieldDefinition} from 'sentry/utils/fields';
 import {
@@ -97,18 +94,10 @@ function getTokenKeyValuePair(
   return null;
 }
 
-const EXTRAPOLATED_AREA_STRIPE_IMG =
-  'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAoAAABkAQMAAACFAjPUAAAAAXNSR0IB2cksfwAAAAlwSFlzAAALEwAACxMBAJqcGAAAAAZQTFRFpKy5SVlzL3npZAAAAA9JREFUeJxjsD/AMIqIQwBIyGOd43jaDwAAAABJRU5ErkJggg==';
-
-export const extrapolatedAreaStyle = {
-  color: {
-    repeat: 'repeat',
-    image: EXTRAPOLATED_AREA_STRIPE_IMG,
-    rotation: 0.785,
-    scaleX: 0.5,
-  },
-  opacity: 1.0,
-};
+export function getOnDemandKeys(query: string): string[] {
+  const searchFilterKeys = getSearchFilterKeys(query);
+  return searchFilterKeys.filter(isOnDemandSearchKey);
+}
 
 export function hasOnDemandMetricAlertFeature(organization: Organization) {
   return organization.features.includes('on-demand-metrics-extraction');
@@ -120,17 +109,3 @@ export function hasOnDemandMetricWidgetFeature(organization: Organization) {
     organization.features.includes('on-demand-metrics-extraction-experimental')
   );
 }
-
-export function OnDemandWarningIcon({msg}: {msg: React.ReactNode}) {
-  return (
-    <Tooltip title={msg}>
-      <HoverableIconWarning color="gray300" />
-    </Tooltip>
-  );
-}
-
-const HoverableIconWarning = styled(IconWarning)`
-  &:hover {
-    cursor: pointer;
-  }
-`;

+ 3 - 1
static/app/views/alerts/rules/metric/details/body.tsx

@@ -139,6 +139,8 @@ export default function MetricDetailsBody({
   const isSnoozed = rule.snooze;
   const ruleActionCategory = getAlertRuleActionCategory(rule);
 
+  const isOnDemandAlert = isOnDemandMetricAlert(dataset, query);
+
   return (
     <Fragment>
       {selectedIncident?.alertRule.status === AlertRuleStatus.SNAPSHOT && (
@@ -189,7 +191,7 @@ export default function MetricDetailsBody({
             interval={getPeriodInterval()}
             query={isCrashFreeAlert(dataset) ? query : queryWithTypeFilter}
             filter={getFilter()}
-            isOnDemandMetricAlert={isOnDemandMetricAlert(dataset, query)}
+            isOnDemandAlert={isOnDemandAlert}
           />
           <DetailWrapper>
             <ActivityWrapper>

+ 64 - 65
static/app/views/alerts/rules/metric/details/metricChart.tsx

@@ -9,7 +9,7 @@ import momentTimezone from 'moment-timezone';
 
 import {Client} from 'sentry/api';
 import Feature from 'sentry/components/acl/feature';
-import GuideAnchor from 'sentry/components/assistant/guideAnchor';
+import {OnDemandMetricAlert} from 'sentry/components/alerts/onDemandMetricAlert';
 import {Button} from 'sentry/components/button';
 import {AreaChart, AreaChartSeries} from 'sentry/components/charts/areaChart';
 import ChartZoom from 'sentry/components/charts/chartZoom';
@@ -25,6 +25,7 @@ import {
   SectionHeading,
   SectionValue,
 } from 'sentry/components/charts/styles';
+import {isEmptySeries} from 'sentry/components/charts/utils';
 import CircleIndicator from 'sentry/components/circleIndicator';
 import {
   parseStatsPeriod,
@@ -87,7 +88,7 @@ type Props = WithRouterProps & {
   rule: MetricRule;
   timePeriod: TimePeriodType;
   incidents?: Incident[];
-  isOnDemandMetricAlert?: boolean;
+  isOnDemandAlert?: boolean;
   selectedIncident?: Incident | null;
 };
 
@@ -197,8 +198,7 @@ class MetricChart extends PureComponent<Props, State> {
     totalDuration: number,
     criticalDuration: number,
     warningDuration: number,
-    waitingForDataDuration: number,
-    isOnDemandMetricAlert?: boolean
+    waitingForDataDuration: number
   ) {
     const {rule, organization, project, timePeriod, query} = this.props;
 
@@ -226,59 +226,40 @@ class MetricChart extends PureComponent<Props, State> {
         1
       );
 
-    const isOnDemandAlertWihtoutData =
-      isOnDemandMetricAlert && waitingForDataDuration === totalDuration;
-
     return (
       <StyledChartControls>
         <StyledInlineContainer>
-          {isOnDemandAlertWihtoutData ? (
-            <Fragment>
-              <GuideAnchor
-                disabled={false}
-                target="empty_on_demand_chart"
-                position="right"
-              >
-                <OnDemandNoDataWrapper>
-                  {t(
-                    'This alert lacks historical data due to filters for which we don’t routinely extract metrics.'
+          <Fragment>
+            <SectionHeading>{t('Summary')}</SectionHeading>
+            <StyledSectionValue>
+              <ValueItem>
+                <IconCheckmark color="successText" isCircled />
+                {resolvedPercent ? resolvedPercent.toFixed(2) : 0}%
+              </ValueItem>
+              <ValueItem>
+                <IconWarning color="warningText" />
+                {warningPercent ? warningPercent.toFixed(2) : 0}%
+              </ValueItem>
+              <ValueItem>
+                <IconFire color="errorText" />
+                {criticalPercent ? criticalPercent.toFixed(2) : 0}%
+              </ValueItem>
+              {waitingForDataPercent > 0 && (
+                <StyledTooltip
+                  underlineColor="gray200"
+                  showUnderline
+                  title={t(
+                    'The time spent waiting for metrics matching the filters used.'
                   )}
-                </OnDemandNoDataWrapper>
-              </GuideAnchor>
-            </Fragment>
-          ) : (
-            <Fragment>
-              <SectionHeading>{t('Summary')}</SectionHeading>
-              <StyledSectionValue>
-                <ValueItem>
-                  <IconCheckmark color="successText" isCircled />
-                  {resolvedPercent ? resolvedPercent.toFixed(2) : 0}%
-                </ValueItem>
-                <ValueItem>
-                  <IconWarning color="warningText" />
-                  {warningPercent ? warningPercent.toFixed(2) : 0}%
-                </ValueItem>
-                <ValueItem>
-                  <IconFire color="errorText" />
-                  {criticalPercent ? criticalPercent.toFixed(2) : 0}%
-                </ValueItem>
-                {waitingForDataPercent > 0 && (
-                  <StyledTooltip
-                    underlineColor="gray200"
-                    showUnderline
-                    title={t(
-                      'The time spent waiting for metrics matching the filters used.'
-                    )}
-                  >
-                    <ValueItem>
-                      <IconClock />
-                      {waitingForDataPercent.toFixed(2)}%
-                    </ValueItem>
-                  </StyledTooltip>
-                )}
-              </StyledSectionValue>
-            </Fragment>
-          )}
+                >
+                  <ValueItem>
+                    <IconClock />
+                    {waitingForDataPercent.toFixed(2)}%
+                  </ValueItem>
+                </StyledTooltip>
+              )}
+            </StyledSectionValue>
+          </Fragment>
         </StyledInlineContainer>
         {!isSessionAggregate(rule.aggregate) && (
           <Feature features={['discover-basic']}>
@@ -334,7 +315,7 @@ class MetricChart extends PureComponent<Props, State> {
       rule,
       incidents,
       selectedIncident,
-      isOnDemandMetricAlert: this.props.isOnDemandMetricAlert,
+      isOnDemandMetricAlert: this.props.isOnDemandAlert,
       handleIncidentClick,
     });
 
@@ -509,13 +490,27 @@ class MetricChart extends PureComponent<Props, State> {
           totalDuration,
           criticalDuration,
           warningDuration,
-          waitingForDataDuration,
-          this.props.isOnDemandMetricAlert
+          waitingForDataDuration
         )}
       </ChartPanel>
     );
   }
 
+  renderEmptyOnDemandAlert(timeseriesData: Series[] = [], loading?: boolean) {
+    if (loading || !this.props.isOnDemandAlert || !isEmptySeries(timeseriesData[0])) {
+      return null;
+    }
+
+    return (
+      <OnDemandMetricAlert
+        dismissable
+        message={t(
+          'This alert lacks historical data due to filters for which we don’t routinely extract metrics.'
+        )}
+      />
+    );
+  }
+
   renderEmpty(placeholderText = '') {
     return (
       <ChartPanel>
@@ -536,7 +531,7 @@ class MetricChart extends PureComponent<Props, State> {
       interval,
       query,
       location,
-      isOnDemandMetricAlert,
+      isOnDemandAlert,
     } = this.props;
     const {aggregate, timeWindow, environment, dataset} = rule;
 
@@ -611,11 +606,19 @@ class MetricChart extends PureComponent<Props, State> {
         partial={false}
         queryExtras={queryExtras}
         referrer="api.alerts.alert-rule-chart"
-        useOnDemandMetrics={isOnDemandMetricAlert}
+        useOnDemandMetrics={isOnDemandAlert}
       >
-        {({loading, timeseriesData, comparisonTimeseriesData}) =>
-          this.renderChart(loading, timeseriesData, undefined, comparisonTimeseriesData)
-        }
+        {({loading, timeseriesData, comparisonTimeseriesData}) => (
+          <Fragment>
+            {this.renderEmptyOnDemandAlert(timeseriesData, loading)}
+            {this.renderChart(
+              loading,
+              timeseriesData,
+              undefined,
+              comparisonTimeseriesData
+            )}
+          </Fragment>
+        )}
       </EventsRequest>
     );
   }
@@ -668,10 +671,6 @@ const StyledSectionValue = styled(SectionValue)`
   margin: 0 0 0 ${space(1.5)};
 `;
 
-const OnDemandNoDataWrapper = styled(SectionValue)`
-  margin: 0;
-`;
-
 const ValueItem = styled('div')`
   display: grid;
   grid-template-columns: repeat(2, auto);

+ 2 - 5
static/app/views/alerts/rules/metric/details/sidebar.tsx

@@ -3,6 +3,7 @@ import styled from '@emotion/styled';
 import capitalize from 'lodash/capitalize';
 
 import AlertBadge from 'sentry/components/alertBadge';
+import {OnDemandWarningIcon} from 'sentry/components/alerts/onDemandMetricAlert';
 import ActorAvatar from 'sentry/components/avatar/actorAvatar';
 import {SectionHeading} from 'sentry/components/charts/styles';
 import DateTime from 'sentry/components/dateTime';
@@ -14,11 +15,7 @@ import {t, tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import type {Actor} from 'sentry/types';
 import getDynamicText from 'sentry/utils/getDynamicText';
-import {
-  getSearchFilters,
-  isOnDemandSearchKey,
-  OnDemandWarningIcon,
-} from 'sentry/utils/onDemandMetrics/index';
+import {getSearchFilters, isOnDemandSearchKey} from 'sentry/utils/onDemandMetrics/index';
 import {COMPARISON_DELTA_OPTIONS} from 'sentry/views/alerts/rules/metric/constants';
 import {
   Action,

+ 33 - 43
static/app/views/alerts/rules/metric/ruleConditionsForm.tsx

@@ -7,7 +7,10 @@ import pick from 'lodash/pick';
 
 import {addErrorMessage} from 'sentry/actionCreators/indicator';
 import {Client} from 'sentry/api';
-import Alert from 'sentry/components/alert';
+import {
+  OnDemandMetricAlert,
+  OnDemandWarningIcon,
+} from 'sentry/components/alerts/onDemandMetricAlert';
 import SearchBar from 'sentry/components/events/searchBar';
 import SelectControl from 'sentry/components/forms/controls/selectControl';
 import SelectField from 'sentry/components/forms/fields/selectField';
@@ -18,13 +21,12 @@ import Panel from 'sentry/components/panels/panel';
 import PanelBody from 'sentry/components/panels/panelBody';
 import {InvalidReason} from 'sentry/components/searchSyntax/parser';
 import {SearchInvalidTag} from 'sentry/components/smartSearchBar/searchInvalidTag';
-import {IconInfo} from 'sentry/icons';
 import {t, tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import {Environment, Organization, Project, SelectValue} from 'sentry/types';
 import {getDisplayName} from 'sentry/utils/environment';
 import {
-  createOnDemandFilterWarning,
+  getOnDemandKeys,
   hasOnDemandMetricAlertFeature,
   isOnDemandQueryString,
 } from 'sentry/utils/onDemandMetrics';
@@ -388,28 +390,18 @@ class RuleConditionsForm extends PureComponent<Props, State> {
         []),
     ];
 
-    const getOnDemandFilterWarning = createOnDemandFilterWarning(
-      tct(
-        'We don’t routinely collect metrics from this property. However, we’ll do so [strong:once this alert has been saved.]',
-        {
-          strong: <strong />,
-        }
-      )
-    );
-
     return (
       <Fragment>
         <ChartPanel>
-          {isExtrapolatedChartData && (
-            <OnDemandMetricInfoAlert>
-              <OnDemandMetricInfoIcon size="sm" />
-              {t(
-                'The chart data is an estimate based on the stored transactions that match the filters specified.'
-              )}
-            </OnDemandMetricInfoAlert>
-          )}
           <StyledPanelBody>{this.props.thresholdChart}</StyledPanelBody>
         </ChartPanel>
+        {isExtrapolatedChartData && (
+          <OnDemandMetricAlert
+            message={t(
+              'The chart data above is an estimate based on the stored transactions that match the filters specified.'
+            )}
+          />
+        )}
         {this.renderInterval()}
         <StyledListItem>{t('Filter events')}</StyledListItem>
         <FormRow noMargin columns={1 + (allowChangeEventTypes ? 1 : 0) + 1}>
@@ -448,7 +440,7 @@ class RuleConditionsForm extends PureComponent<Props, State> {
             }}
             flexibleControlStateSize
           >
-            {({onChange, onBlur, onKeyDown, initialData}) => {
+            {({onChange, onBlur, onKeyDown, initialData, value}) => {
               return (
                 <SearchContainer>
                   <StyledSearchBar
@@ -476,11 +468,6 @@ class RuleConditionsForm extends PureComponent<Props, State> {
                         />
                       );
                     }}
-                    getFilterWarning={
-                      hasOnDemandMetricAlertFeature(organization)
-                        ? getOnDemandFilterWarning
-                        : undefined
-                    }
                     searchSource="alert_builder"
                     defaultQuery={initialData?.query ?? ''}
                     {...getSupportedAndOmittedTags(dataset, organization)}
@@ -523,6 +510,24 @@ class RuleConditionsForm extends PureComponent<Props, State> {
                     }}
                     hasRecentSearches={dataset !== Dataset.SESSIONS}
                   />
+                  {isExtrapolatedChartData && (
+                    <OnDemandWarningIcon
+                      color="gray500"
+                      msg={tct(
+                        `We don’t routinely collect metrics from [fields]. However, we’ll do so [strong:once this alert has been saved.]`,
+                        {
+                          fields: (
+                            <strong>
+                              {getOnDemandKeys(value)
+                                .map(key => `"${key}"`)
+                                .join(', ')}
+                            </strong>
+                          ),
+                          strong: <strong />,
+                        }
+                      )}
+                    />
+                  )}
                 </SearchContainer>
               );
             }}
@@ -553,6 +558,8 @@ const StyledPanelBody = styled(PanelBody)`
 
 const SearchContainer = styled('div')`
   display: flex;
+  align-items: center;
+  gap: ${space(1)};
 `;
 
 const StyledSearchBar = styled(SearchBar)`
@@ -567,23 +574,6 @@ const StyledSearchBar = styled(SearchBar)`
   `}
 `;
 
-const OnDemandMetricInfoAlert = styled(Alert)`
-  border-radius: ${space(0.5)} ${space(0.5)} 0 0;
-  border: none;
-  border-bottom: 1px solid ${p => p.theme.blue400};
-  margin-bottom: 0;
-
-  & > span {
-    display: flex;
-    align-items: center;
-  }
-`;
-
-const OnDemandMetricInfoIcon = styled(IconInfo)`
-  color: ${p => p.theme.blue400};
-  margin-right: ${space(1.5)};
-`;
-
 const StyledListItem = styled(ListItem)`
   margin-bottom: ${space(0.5)};
   font-size: ${p => p.theme.fontSizeExtraLarge};

+ 1 - 1
static/app/views/alerts/rules/metric/triggers/chart/thresholdsChart.tsx

@@ -4,6 +4,7 @@ import type {TooltipComponentFormatterCallbackParams} from 'echarts';
 import debounce from 'lodash/debounce';
 import flatten from 'lodash/flatten';
 
+import {extrapolatedAreaStyle} from 'sentry/components/alerts/onDemandMetricAlert';
 import {AreaChart} from 'sentry/components/charts/areaChart';
 import Graphic from 'sentry/components/charts/components/graphic';
 import {defaultFormatAxisLabel} from 'sentry/components/charts/components/tooltip';
@@ -14,7 +15,6 @@ import {CHART_PALETTE} from 'sentry/constants/chartPalette';
 import {space} from 'sentry/styles/space';
 import {PageFilters} from 'sentry/types';
 import {ReactEchartsRef, Series} from 'sentry/types/echarts';
-import {extrapolatedAreaStyle} from 'sentry/utils/onDemandMetrics';
 import theme from 'sentry/utils/theme';
 import {
   ALERT_CHART_MIN_MAX_BUFFER,

Some files were not shown because too many files changed in this diff