Просмотр исходного кода

fix(perf): Improve time spent percentage column (#57615)

A few major improvements!

1. No more local time spent percentage. All percentages are global to
the app, for consistency. See more below
2. The percentage is only in the tooltip! It's distracting and not very
important, so we're going to tuck it away
3. Time Spent comes back to the sample panel, it was missing briefly
4. Better tooltip explanation, with link to documentation.
George Gritsouk 1 год назад
Родитель
Сommit
101fc0b81c

+ 1 - 0
static/app/utils/discover/fieldRenderers.tsx

@@ -768,6 +768,7 @@ const SPECIAL_FUNCTIONS: SpecialFunctions = {
       <TimeSpentCell
         percentage={data[fieldName]}
         total={data[`sum(${SpanMetricsField.SPAN_SELF_TIME})`]}
+        op={data[`span.op`]}
       />
     );
   },

+ 1 - 1
static/app/views/performance/database/databaseSpanSummaryPage.tsx

@@ -234,7 +234,7 @@ const CHART_HEIGHT = 160;
 
 const DEFAULT_SORT: Sort = {
   kind: 'desc',
-  field: 'time_spent_percentage(local)',
+  field: 'time_spent_percentage()',
 };
 
 const PaddedContainer = styled('div')`

+ 5 - 1
static/app/views/performance/landing/widgets/widgets/lineChartListWidget.tsx

@@ -468,7 +468,11 @@ export function LineChartListWidget(props: PerformanceWidgetProps) {
                       />
                     </StyledTextOverflow>
                     <RightAlignedCell>
-                      <TimeSpentCell percentage={timeSpentPercentage} total={totalTime} />
+                      <TimeSpentCell
+                        percentage={timeSpentPercentage}
+                        total={totalTime}
+                        op="db"
+                      />
                     </RightAlignedCell>
                     {!props.withStaticFilters && (
                       <ListClose

+ 0 - 4
static/app/views/starfish/components/chart.tsx

@@ -62,10 +62,6 @@ export const STARFISH_FIELDS: Record<string, {outputType: AggregationOutputType}
   [SpanMetricsField.SPAN_SELF_TIME]: {
     outputType: 'duration',
   },
-  // local is only used with `time_spent_percentage` function
-  local: {
-    outputType: 'duration',
-  },
 };
 
 type Props = {

+ 0 - 12
static/app/views/starfish/components/spanDescriptionLink.tsx

@@ -3,10 +3,8 @@ import * as qs from 'query-string';
 
 import {useLocation} from 'sentry/utils/useLocation';
 import {OverflowEllipsisTextContainer} from 'sentry/views/starfish/components/textAlign';
-import {SpanFunction} from 'sentry/views/starfish/types';
 import {extractRoute} from 'sentry/views/starfish/utils/extractRoute';
 import {useRoutingContext} from 'sentry/views/starfish/utils/routingContext';
-import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
 
 interface Props {
   description: React.ReactNode;
@@ -33,16 +31,6 @@ export function SpanDescriptionLink({
     endpointMethod,
   };
 
-  const sort: string | undefined = queryString[QueryParameterNames.SPANS_SORT];
-
-  // the spans page uses time_spent_percentage(local), so to persist the sort upon navigation we need to replace
-  if (sort?.includes(`${SpanFunction.TIME_SPENT_PERCENTAGE}()`)) {
-    queryString[QueryParameterNames.SPANS_SORT] = sort.replace(
-      `${SpanFunction.TIME_SPENT_PERCENTAGE}()`,
-      `${SpanFunction.TIME_SPENT_PERCENTAGE}(local)`
-    );
-  }
-
   return (
     <OverflowEllipsisTextContainer>
       {group ? (

+ 0 - 1
static/app/views/starfish/components/tableCells/renderHeadCell.tsx

@@ -36,7 +36,6 @@ export const SORTABLE_FIELDS = new Set([
   `${SPS}()`,
   `${SPM}()`,
   `${TIME_SPENT_PERCENTAGE}()`,
-  `${TIME_SPENT_PERCENTAGE}(local)`,
   `${HTTP_ERROR_COUNT}()`,
 ]);
 

+ 43 - 15
static/app/views/starfish/components/tableCells/timeSpentCell.tsx

@@ -1,41 +1,69 @@
-import styled from '@emotion/styled';
 import clamp from 'lodash/clamp';
 
+import ExternalLink from 'sentry/components/links/externalLink';
 import {Tooltip} from 'sentry/components/tooltip';
-import {tct} from 'sentry/locale';
+import {t, tct} from 'sentry/locale';
 import {defined} from 'sentry/utils';
+import {NumberContainer} from 'sentry/utils/discover/styles';
 import {formatPercentage, getDuration} from 'sentry/utils/formatters';
-import {TextAlignRight} from 'sentry/views/starfish/components/textAlign';
 
 interface Props {
+  containerProps?: React.DetailedHTMLProps<
+    React.HTMLAttributes<HTMLDivElement>,
+    HTMLDivElement
+  >;
+  op?: string;
   percentage?: number;
   total?: number;
 }
 
-export function TimeSpentCell({percentage, total}: Props) {
+export function TimeSpentCell({percentage, total, op, containerProps}: Props) {
   const formattedPercentage = formatPercentage(clamp(percentage ?? 0, 0, 1));
   const formattedTotal = getDuration((total ?? 0) / 1000, 2, true);
   const tooltip = tct(
-    'The application spent [percentage] of its total time on this span.',
+    'The application spent [percentage] of its total time on this [span]. Read more about Time Spent in our [documentation:documentation].',
     {
       percentage: formattedPercentage,
+      span: getSpanOperationDescription(op),
+      documentation: (
+        <ExternalLink href="https://docs.sentry.io/product/performance/queries/#what-is-time-spent" />
+      ),
     }
   );
 
   return (
-    <TextAlignRight>
+    <NumberContainer {...containerProps}>
       <Tooltip isHoverable title={tooltip} showUnderline>
         {defined(total) ? formattedTotal : '--'}
-        <Deemphasized>
-          {' ('}
-          {defined(percentage) ? formattedPercentage : '--%'}
-          {')'}
-        </Deemphasized>
       </Tooltip>
-    </TextAlignRight>
+    </NumberContainer>
   );
 }
 
-const Deemphasized = styled('span')`
-  color: ${p => p.theme.gray300};
-`;
+// TODO: This should use `getSpanOperationDescription` but it uppercases the
+// names. We should update `getSpanOperationDescription` to not uppercase the
+// descriptions needlessly, and use it here. Also, the names here are a little
+// shorter, which is friendlier
+function getSpanOperationDescription(spanOp?: string) {
+  if (spanOp?.startsWith('http')) {
+    return t('request');
+  }
+
+  if (spanOp?.startsWith('db')) {
+    return t('query');
+  }
+
+  if (spanOp?.startsWith('task')) {
+    return t('task');
+  }
+
+  if (spanOp?.startsWith('serialize')) {
+    return t('serializer');
+  }
+
+  if (spanOp?.startsWith('middleware')) {
+    return t('middleware');
+  }
+
+  return t('span');
+}

+ 1 - 8
static/app/views/starfish/queries/useSpanList.tsx

@@ -1,7 +1,6 @@
 import {Location} from 'history';
 import omit from 'lodash/omit';
 
-import {defined} from 'sentry/utils';
 import EventView from 'sentry/utils/discover/eventView';
 import type {Sort} from 'sentry/utils/discover/fields';
 import {DiscoverDatasets} from 'sentry/utils/discover/types';
@@ -24,7 +23,6 @@ export type SpanMetrics = {
   'spm()': number;
   'sum(span.self_time)': number;
   'time_spent_percentage()': number;
-  'time_spent_percentage(local)': number;
 };
 
 export const useSpanList = (
@@ -87,14 +85,9 @@ function getEventView(
     `sum(${SPAN_SELF_TIME})`,
     `avg(${SPAN_SELF_TIME})`,
     'http_error_count()',
+    'time_spent_percentage()',
   ];
 
-  if (defined(transaction)) {
-    fields.push('time_spent_percentage(local)');
-  } else {
-    fields.push('time_spent_percentage()');
-  }
-
   const eventView = EventView.fromNewQueryWithLocation(
     {
       name: '',

+ 3 - 3
static/app/views/starfish/queries/useSpanTransactionMetrics.tsx

@@ -16,7 +16,7 @@ export type SpanTransactionMetrics = {
   'http_error_count()': number;
   'spm()': number;
   'sum(span.self_time)': number;
-  'time_spent_percentage(local)': number;
+  'time_spent_percentage()': number;
   transaction: string;
   'transaction.method': string;
   'transaction.op': string;
@@ -68,11 +68,11 @@ function getEventView(location: Location, filters: MetricsFilters = {}, sorts?:
         'spm()',
         `sum(${SPAN_SELF_TIME})`,
         `avg(${SPAN_SELF_TIME})`,
-        'time_spent_percentage(local)',
+        'time_spent_percentage()',
         'transaction.op',
         'http_error_count()',
       ],
-      orderby: '-time_spent_percentage_local',
+      orderby: '-time_spent_percentage',
       dataset: DiscoverDatasets.SPANS_METRICS,
       version: 2,
     },

+ 0 - 2
static/app/views/starfish/types.tsx

@@ -59,8 +59,6 @@ export type MetricsResponse = {
   [Property in SpanStringFields as `${Property}`]: string;
 } & {
   [Property in SpanStringArrayFields as `${Property}`]: string[];
-} & {
-  'time_spent_percentage(local)': number;
 };
 
 export type MetricsFilters = {

Некоторые файлы не были показаны из-за большого количества измененных файлов