Browse Source

feat(metrics): redesign extraction rule modal (#74709)

Ogi 7 months ago
parent
commit
9dc6d3a458

+ 6 - 8
static/app/views/metrics/layout.spec.tsx

@@ -52,9 +52,9 @@ describe('Metrics Layout', function () {
 
     render(<MetricsLayout />, {organization});
 
-    // Button: Add New Metric
+    // Button: Create metric
     expect(
-      await screen.findByRole('button', {name: 'Add New Metric'})
+      await screen.findByRole('button', {name: 'Create metric'})
     ).toBeInTheDocument();
 
     // Alert: No alert shall be rendered
@@ -85,10 +85,8 @@ describe('Metrics Layout', function () {
     // Button: Set Up Tracing
     expect(screen.getByRole('button', {name: 'Set Up Tracing'})).toBeInTheDocument();
 
-    // Not in the page: Add New Metric
-    expect(
-      screen.queryByRole('button', {name: 'Add New Metric'})
-    ).not.toBeInTheDocument();
+    // Not in the page: Create metric
+    expect(screen.queryByRole('button', {name: 'Create metric'})).not.toBeInTheDocument();
   });
 
   it('not using performance and have old custom metrics', async function () {
@@ -106,8 +104,8 @@ describe('Metrics Layout', function () {
       await screen.findByText(/Metrics using with the old API will stop being ingested/i)
     ).toBeInTheDocument();
 
-    // Button: Add New Metric
-    expect(screen.getByRole('button', {name: 'Add New Metric'})).toBeInTheDocument();
+    // Button: Create metric
+    expect(screen.getByRole('button', {name: 'Create metric'})).toBeInTheDocument();
 
     // Main View: Does not display the empty state.
     expect(screen.queryByText(/track and solve what matters/i)).not.toBeInTheDocument();

+ 3 - 3
static/app/views/metrics/pageHeaderActions.spec.tsx

@@ -27,7 +27,7 @@ describe('Metrics Page Header Actions', function () {
       expect(addCustomMetric).toHaveBeenCalled();
     });
 
-    it('display "add new metric" button', async function () {
+    it('display "Create metric" button', async function () {
       render(
         <PageHeaderActions showAddMetricButton addCustomMetric={() => jest.fn()} />,
         {
@@ -41,14 +41,14 @@ describe('Metrics Page Header Actions', function () {
       );
       renderGlobalModal();
 
-      const button = screen.getByRole('button', {name: 'Add New Metric'});
+      const button = screen.getByRole('button', {name: 'Create metric'});
 
       expect(button).toBeInTheDocument();
 
       await userEvent.click(button);
 
       expect(
-        await screen.findByRole('heading', {name: /Configure Metric/})
+        await screen.findByRole('heading', {name: 'Create Metric'})
       ).toBeInTheDocument();
     });
   });

+ 1 - 1
static/app/views/metrics/pageHeaderActions.tsx

@@ -150,7 +150,7 @@ export function PageHeaderActions({showAddMetricButton, addCustomMetric}: Props)
             onClick={() => openExtractionRuleCreateModal({})}
             size="sm"
           >
-            {t('Add New Metric')}
+            {t('Create metric')}
           </Button>
         ) : (
           <Button priority="primary" onClick={() => addCustomMetric()} size="sm">

+ 6 - 4
static/app/views/settings/projectMetrics/metricsExtractionRuleCreateModal.tsx

@@ -109,7 +109,7 @@ export function MetricsExtractionRuleCreateModal({
   return (
     <Fragment>
       <Header>
-        <h4>{t('Configure Metric')}</h4>
+        <h4>{t('Create Metric')}</h4>
       </Header>
       <CloseButton />
       <Body>
@@ -122,6 +122,7 @@ export function MetricsExtractionRuleCreateModal({
               options={projectOptions}
               value={projectId}
               onChange={({value}) => setProjectId(value)}
+              stacked={false}
             />
           </ProjectSelectionWrapper>
         ) : null}
@@ -207,15 +208,16 @@ function FormWrapper({
       onCancel={closeModal}
       onSubmit={handleSubmit}
       cardinality={cardinality}
+      submitDisabled={createExtractionRuleMutation.isLoading}
     />
   );
 }
 
 const ProjectSelectionWrapper = styled('div')`
   padding-bottom: ${space(2)};
-  padding-left: ${space(2)};
-  :not(:last-child) {
-    border-bottom: 1px solid ${p => p.theme.innerBorder};
+
+  & > label {
+    color: ${p => p.theme.gray300};
   }
 `;
 

+ 2 - 1
static/app/views/settings/projectMetrics/metricsExtractionRuleEditModal.tsx

@@ -98,7 +98,7 @@ export function MetricsExtractionRuleEditModal({
   return (
     <Fragment>
       <Header>
-        <h4>{metricExtractionRule.spanAttribute}</h4>
+        <h4>{t('Edit Metric')}</h4>
       </Header>
       <CloseButton />
       <Body>
@@ -110,6 +110,7 @@ export function MetricsExtractionRuleEditModal({
           onCancel={closeModal}
           onSubmit={handleSubmit}
           cardinality={cardinality}
+          submitDisabled={updateExtractionRuleMutation.isLoading}
           isEdit
           requireChanges
         />

+ 168 - 67
static/app/views/settings/projectMetrics/metricsExtractionRuleForm.tsx

@@ -1,6 +1,8 @@
 import {Fragment, useCallback, useId, useMemo, useState} from 'react';
 import styled from '@emotion/styled';
+import {Observer} from 'mobx-react';
 
+import Alert from 'sentry/components/alert';
 import {Button} from 'sentry/components/button';
 import SearchBar from 'sentry/components/events/searchBar';
 import SelectField from 'sentry/components/forms/fields/selectField';
@@ -9,7 +11,7 @@ import FormField from 'sentry/components/forms/formField';
 import type FormModel from 'sentry/components/forms/model';
 import ExternalLink from 'sentry/components/links/externalLink';
 import {Tooltip} from 'sentry/components/tooltip';
-import {IconAdd, IconClose, IconWarning} from 'sentry/icons';
+import {IconAdd, IconClose, IconQuestion, IconWarning} from 'sentry/icons';
 import {t, tct} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
 import type {SelectValue} from 'sentry/types/core';
@@ -194,7 +196,7 @@ export function MetricsExtractionRuleForm({
 }: Props) {
   const organization = useOrganization();
 
-  const [customAttributes, setCustomeAttributes] = useState<string[]>(() => {
+  const [customAttributes, setCustomAttributes] = useState<string[]>(() => {
     const {spanAttribute, tags} = props.initialData;
     return [...new Set(spanAttribute ? [...tags, spanAttribute] : tags)];
   });
@@ -265,7 +267,7 @@ export function MetricsExtractionRuleForm({
 
   const unitOptions = useMemo(() => {
     const options: SelectValue<string>[] = SUPPORTED_UNITS.map(unit => ({
-      label: unit,
+      label: unit + (unit === 'none' ? '' : 's'),
       value: unit,
     }));
     if (customUnit) {
@@ -308,89 +310,123 @@ export function MetricsExtractionRuleForm({
     <Form onSubmit={onSubmit && handleSubmit} {...props}>
       {({model}) => (
         <Fragment>
-          <SelectField
-            name="spanAttribute"
-            options={attributeOptions}
-            disabled={isEdit}
-            label={t('Measure')}
-            help={tct(
-              'Define the span attribute you want to track. Learn how to instrument custom attributes in [link:our docs].',
-              {
-                // TODO(telemetry-experience): add the correct link here once we have it!!!
-                link: (
-                  <ExternalLink href="https://docs.sentry.io/product/explore/metrics/" />
-                ),
+          <SpanAttributeUnitWrapper>
+            <SelectField
+              inline={false}
+              stacked
+              name="spanAttribute"
+              options={attributeOptions}
+              disabled={isEdit}
+              label={
+                <TooltipIconLabel
+                  label={t('Measure')}
+                  help={tct(
+                    'Define the span attribute you want to track. Learn how to instrument custom attributes in [link:our docs].',
+                    {
+                      // TODO(telemetry-experience): add the correct link here once we have it!!!
+                      link: (
+                        <ExternalLink href="https://docs.sentry.io/product/explore/metrics/" />
+                      ),
+                    }
+                  )}
+                />
               }
-            )}
-            placeholder={t('Select a span attribute')}
-            creatable
-            formatCreateLabel={value => `Custom: "${value}"`}
-            onCreateOption={value => {
-              setCustomeAttributes(curr => [...curr, value]);
-              model.setValue('spanAttribute', value);
-            }}
-            onChange={value => {
-              if (value in FIXED_UNITS_BY_ATTRIBUTE) {
-                model.setValue('unit', FIXED_UNITS_BY_ATTRIBUTE[value]);
-                setIsUnitDisabled(true);
-              } else {
-                setIsUnitDisabled(false);
+              placeholder={t('Select span attribute')}
+              creatable
+              formatCreateLabel={value => `Custom: "${value}"`}
+              onCreateOption={value => {
+                setCustomAttributes(curr => [...curr, value]);
+                model.setValue('spanAttribute', value);
+              }}
+              onChange={value => {
+                model.setValue('spanAttribute', value);
+                if (value in FIXED_UNITS_BY_ATTRIBUTE) {
+                  model.setValue('unit', FIXED_UNITS_BY_ATTRIBUTE[value]);
+                  setIsUnitDisabled(true);
+                } else {
+                  setIsUnitDisabled(false);
+                }
+              }}
+              required
+            />
+            <StyledFieldConnector>in</StyledFieldConnector>
+            <SelectField
+              inline={false}
+              stacked
+              allowEmpty
+              name="unit"
+              options={unitOptions}
+              disabled={isUnitDisabled}
+              label={
+                <ExternalLink href="https://docs.sentry.io/product/explore/metrics/">
+                  {t('Create Custom Attribute?')}
+                </ExternalLink>
               }
-            }}
-            required
-          />
-          <SelectField
-            name="unit"
-            options={unitOptions}
-            disabled={isUnitDisabled}
-            label={t('Unit')}
-            placeholder={t('Select a unit')}
-            creatable
-            formatCreateLabel={value => `Custom: "${value}"`}
-            onCreateOption={value => {
-              setCustomUnit(value);
-              model.setValue('unit', value);
-            }}
-            required
-          />
+              placeholder={t('Select unit')}
+              creatable
+              formatCreateLabel={value => `Custom: "${value}"`}
+              onCreateOption={value => {
+                setCustomUnit(value);
+                model.setValue('unit', value);
+              }}
+            />
+          </SpanAttributeUnitWrapper>
+
           <SelectField
+            inline={false}
+            stacked
             name="aggregates"
             required
             options={AGGREGATE_OPTIONS}
-            label={t('Aggregate')}
+            label={
+              <TooltipIconLabel
+                label={t('Aggregate')}
+                help={tct(
+                  'Select the aggregations you want to store. For more information, read [link:our docs]',
+                  {
+                    // TODO(telemetry-experience): add the correct link here once we have it!!!
+                    link: (
+                      <ExternalLink href="https://docs.sentry.io/product/explore/metrics/" />
+                    ),
+                  }
+                )}
+              />
+            }
             multiple
-            help={tct(
-              'Select the aggregations you want to store. For more information, read [link:our docs]',
-              {
-                // TODO(telemetry-experience): add the correct link here once we have it!!!
-                link: (
-                  <ExternalLink href="https://docs.sentry.io/product/explore/metrics/" />
-                ),
-              }
-            )}
           />
           <SelectField
+            inline={false}
+            stacked
             name="tags"
             options={tagOptions}
-            label={t('Group and filter by')}
             multiple
             placeholder={t('Select tags')}
-            help={t(
-              'Select the tags that can be used to group and filter the metric. Tag values have to be non-numeric.'
-            )}
+            label={
+              <TooltipIconLabel
+                label={t('Group and filter by')}
+                help={t(
+                  'Select the tags that can be used to group and filter the metric. Tag values have to be non-numeric.'
+                )}
+              />
+            }
             creatable
             formatCreateLabel={value => `Custom: "${value}"`}
             onCreateOption={value => {
-              setCustomeAttributes(curr => [...curr, value]);
+              setCustomAttributes(curr => [...curr, value]);
               const currentTags = model.getValue('tags') as string[];
               model.setValue('tags', [...currentTags, value]);
             }}
           />
           <FormField
-            label={t('Filters')}
-            help={t(
-              'Define filters to narrow down the metric to a specific set of spans.'
-            )}
+            stacked
+            label={
+              <TooltipIconLabel
+                label={t('Filters')}
+                help={t(
+                  'Define filters to narrow down the metric to a specific set of spans.'
+                )}
+              />
+            }
             name="conditions"
             inline={false}
             hasControlState={false}
@@ -457,7 +493,7 @@ export function MetricsExtractionRuleForm({
                                   handleChange(queryString, index);
                                 }
                               }}
-                              placeholder={t('Search for span attributes')}
+                              placeholder={t('Add span attributes')}
                               organization={organization}
                               supportedTags={supportedTags}
                               dataset={DiscoverDatasets.SPANS_INDEXED}
@@ -491,19 +527,84 @@ export function MetricsExtractionRuleForm({
               );
             }}
           </FormField>
+          <Observer>
+            {() =>
+              model.formChanged ? (
+                <Alert
+                  type="info"
+                  showIcon
+                  expand={
+                    <Fragment>
+                      <b>{t('Why that?')}</b>
+                      <p>
+                        {t(
+                          'Well, it’s because we’ll only collect data once you’ve created a metric and not before. Likewise, if you deleted an existing metric, then we’ll stop collecting data for that metric.'
+                        )}
+                      </p>
+                    </Fragment>
+                  }
+                >
+                  {t('Hey, we’ll need a moment to collect data that matches the above.')}
+                </Alert>
+              ) : null
+            }
+          </Observer>
         </Fragment>
       )}
     </Form>
   );
 }
 
+function TooltipIconLabel({label, help}) {
+  return (
+    <TooltipIconLabelWrapper>
+      {label}
+      <Tooltip title={help}>
+        <IconQuestion size="sm" color="gray200" />
+      </Tooltip>
+    </TooltipIconLabelWrapper>
+  );
+}
+
+const TooltipIconLabelWrapper = styled('span')`
+  display: inline-flex;
+  font-weight: bold;
+  color: ${p => p.theme.gray300};
+  gap: ${space(0.5)};
+
+  & > span {
+    margin-top: 1px;
+  }
+
+  & > span:hover {
+    cursor: pointer;
+  }
+`;
+
+const StyledFieldConnector = styled('div')`
+  color: ${p => p.theme.gray300};
+  padding-bottom: ${space(1)};
+`;
+
+const SpanAttributeUnitWrapper = styled('div')`
+  display: flex;
+  align-items: flex-end;
+
+  gap: ${space(1)};
+  padding-bottom: ${space(2)};
+
+  & > div:first-child {
+    flex: 1;
+    padding-bottom: 0;
+  }
+`;
+
 function SearchBarWithId(props: React.ComponentProps<typeof SearchBar>) {
   const id = useId();
   return <SearchBar id={id} {...props} />;
 }
 
 const ConditionsWrapper = styled('div')<{hasDelete: boolean}>`
-  padding: ${space(1)} 0;
   display: grid;
   align-items: center;
   gap: ${space(1)};

+ 1 - 3
static/app/views/settings/projectMetrics/metricsExtractionRulesTable.spec.tsx

@@ -116,9 +116,7 @@ describe('Metrics Extraction Rules Table', function () {
     const editButtons = await screen.findAllByLabelText('Edit metric');
     await userEvent.click(editButtons[1]);
 
-    expect(
-      await screen.findByRole('heading', {name: /span.duration/})
-    ).toBeInTheDocument();
+    expect(await screen.findByRole('heading', {name: 'Edit Metric'})).toBeInTheDocument();
   });
 
   it('shall display cardinality limit warning', async function () {