fix(starfish): Render numbers in span table using field renderers (#51429)

- Eliminate unnecessary helper function
- Fix incorrect return types for event meta
- Use regular field renderer for p95
- Use regular field renderer for error count
- Add renderer for `time_spent_percentage()`
- Align time spent to the right
- Render percentile values based on field type
- Move out percent change cell component
- Align percent change to the right

static/app/utils/discover/fieldRenderers.tsx

@@ -46,7 +46,9 @@ import {
   SpanOperationBreakdownFilter,
   stringToFilter,
 } from 'sentry/views/performance/transactionSummary/filter';
-import {PercentChangeCell} from 'sentry/views/starfish/views/webServiceView/endpointList';
+import {PercentChangeCell} from 'sentry/views/starfish/components/tableCells/percentChangeCell';
+import {TimeSpentCell} from 'sentry/views/starfish/components/tableCells/timeSpentCell';
+import {SpanMetricsFields} from 'sentry/views/starfish/types';
 
 import {decodeScalar} from '../queryString';
 
@@ -103,6 +105,7 @@ type FieldFormatters = {
   duration: FieldFormatter;
   integer: FieldFormatter;
   number: FieldFormatter;
+  percent_change: FieldFormatter;
   percentage: FieldFormatter;
   size: FieldFormatter;
   string: FieldFormatter;
@@ -291,6 +294,22 @@ export const FIELD_FORMATTERS: FieldFormatters = {
       return <ArrayValue value={value} />;
     },
   },
+  percent_change: {
+    isSortable: true,
+    renderFunc: (fieldName, data) => {
+      const deltaValue = data[fieldName];
+
+      const sign = deltaValue >= 0 ? '+' : '-';
+      const delta = formatPercentage(Math.abs(deltaValue), 2);
+      const trendDirection = deltaValue < 0 ? 'good' : deltaValue > 0 ? 'bad' : 'neutral';
+
+      return (
+        <PercentChangeCell
+          trendDirection={trendDirection}
+        >{`${sign}${delta}`}</PercentChangeCell>
+      );
+    },
+  },
 };
 
 type SpecialFieldRenderFunc = (
@@ -674,9 +693,8 @@ type SpecialFunctionFieldRenderer = (
 ) => (data: EventData, baggage: RenderFunctionBaggage) => React.ReactNode;
 
 type SpecialFunctions = {
-  http_error_count_percent_change: SpecialFunctionFieldRenderer;
-  percentile_percent_change: SpecialFunctionFieldRenderer;
   sps_percent_change: SpecialFunctionFieldRenderer;
+  time_spent_percentage: SpecialFunctionFieldRenderer;
   user_misery: SpecialFunctionFieldRenderer;
 };
 
@@ -744,9 +762,6 @@ const SPECIAL_FUNCTIONS: SpecialFunctions = {
       </BarContainer>
     );
   },
-  // TODO: As soon as events endpoints `meta` give `*_percent_change` fields a
-  // type of `percentage_change` (as opposed to `percentage`, as it currently
-  // is) all this logic can move down into FIELD_RENDERERS and be consolidated
   sps_percent_change: fieldName => data => {
     const deltaValue = data[fieldName];
 
@@ -758,30 +773,12 @@ const SPECIAL_FUNCTIONS: SpecialFunctions = {
       <PercentChangeCell trendDirection="neutral">{`${sign}${delta}`}</PercentChangeCell>
     );
   },
-  percentile_percent_change: fieldName => data => {
-    const deltaValue = data[fieldName];
-
-    const sign = deltaValue >= 0 ? '+' : '-';
-    const delta = formatPercentage(Math.abs(deltaValue), 2);
-    const trendDirection = deltaValue < 0 ? 'good' : deltaValue > 0 ? 'bad' : 'neutral';
-
-    return (
-      <PercentChangeCell
-        trendDirection={trendDirection}
-      >{`${sign}${delta}`}</PercentChangeCell>
-    );
-  },
-  http_error_count_percent_change: fieldName => data => {
-    const deltaValue = data[fieldName];
-
-    const sign = deltaValue >= 0 ? '+' : '-';
-    const delta = formatPercentage(Math.abs(deltaValue), 2);
-    const trendDirection = deltaValue < 0 ? 'good' : deltaValue > 0 ? 'bad' : 'neutral';
-
+  time_spent_percentage: fieldName => data => {
     return (
-      <PercentChangeCell
-        trendDirection={trendDirection}
-      >{`${sign}${delta}`}</PercentChangeCell>
+      <TimeSpentCell
+        timeSpentPercentage={data[fieldName]}
+        totalSpanTime={data[`sum(${SpanMetricsFields.SPAN_SELF_TIME})`]}
+      />
     );
   },
 };

static/app/utils/fields/index.ts

@@ -132,6 +132,7 @@ export enum FieldValueType {
   NEVER = 'never',
   SIZE = 'size',
   RATE = 'rate',
+  PERCENT_CHANGE = 'percent_change',
 }
 
 export enum WebVital {

static/app/views/starfish/components/tableCells/percentChangeCell.tsx

@@ -0,0 +1,26 @@
+import React from 'react';
+import styled from '@emotion/styled';
+
+import {NumberContainer} from 'sentry/utils/discover/styles';
+
+type Props = {
+  children: React.ReactNode;
+  trendDirection: 'good' | 'bad' | 'neutral';
+};
+
+export function PercentChangeCell({trendDirection, children}: Props) {
+  return (
+    <NumberContainer>
+      <Colorized trendDirection={trendDirection}>{children}</Colorized>
+    </NumberContainer>
+  );
+}
+
+const Colorized = styled('div')<Props>`
+  color: ${p =>
+    p.trendDirection === 'good'
+      ? p.theme.successText
+      : p.trendDirection === 'bad'
+      ? p.theme.errorText
+      : p.theme.subText};
+`;

static/app/views/starfish/components/tableCells/timeSpentCell.tsx

@@ -1,5 +1,6 @@
 import {Tooltip} from 'sentry/components/tooltip';
 import {formatPercentage} from 'sentry/utils/formatters';
+import {TextAlignRight} from 'sentry/views/starfish/components/textAlign';
 import {getTooltip} from 'sentry/views/starfish/views/spans/types';
 
 export function TimeSpentCell({
@@ -12,10 +13,10 @@ export function TimeSpentCell({
   const toolTip = getTooltip('timeSpent', totalSpanTime);
   const percentage = timeSpentPercentage > 1 ? 1 : timeSpentPercentage;
   return (
-    <span>
+    <TextAlignRight>
       <Tooltip isHoverable title={toolTip}>
         {formatPercentage(percentage)}
       </Tooltip>
-    </span>
+    </TextAlignRight>
   );
 }

static/app/views/starfish/queries/useSpanList.tsx

@@ -7,7 +7,7 @@ import type {Sort} from 'sentry/utils/discover/fields';
 import {DiscoverDatasets} from 'sentry/utils/discover/types';
 import {useLocation} from 'sentry/utils/useLocation';
 import {ModuleName, SpanMetricsFields} from 'sentry/views/starfish/types';
-import {useSpansQuery} from 'sentry/views/starfish/utils/useSpansQuery';
+import {useWrappedDiscoverQuery} from 'sentry/views/starfish/utils/useSpansQuery';
 import {NULL_SPAN_CATEGORY} from 'sentry/views/starfish/views/webServiceView/spanGroupBreakdownContainer';
 
 const {SPAN_SELF_TIME} = SpanMetricsFields;
@@ -41,7 +41,7 @@ export const useSpanList = (
 
   const eventView = getEventView(moduleName, location, transaction, spanCategory, sorts);
 
-  const {isLoading, data, meta, pageLinks} = useSpansQuery<SpanMetrics[]>({
+  const {isLoading, data, meta, pageLinks} = useWrappedDiscoverQuery<SpanMetrics[]>({
     eventView,
     initialData: [],
     limit,

static/app/views/starfish/utils/useSpansQuery.tsx

@@ -1,7 +1,11 @@
 import moment from 'moment';
 
 import {useDiscoverQuery} from 'sentry/utils/discover/discoverQuery';
-import EventView, {encodeSort, MetaType} from 'sentry/utils/discover/eventView';
+import EventView, {
+  encodeSort,
+  EventsMetaType,
+  MetaType,
+} from 'sentry/utils/discover/eventView';
 import {
   DiscoverQueryProps,
   useGenericDiscoverQuery,
@@ -15,7 +19,7 @@ export const DATE_FORMAT = 'YYYY-MM-DDTHH:mm:ssZ';
 export type UseSpansQueryReturnType<T> = {
   data: T;
   isLoading: boolean;
-  meta?: MetaType;
+  meta?: MetaType | EventsMetaType;
   pageLinks?: string;
 };
 
@@ -34,11 +38,13 @@ export function useSpansQuery<T = any[]>({
   limit?: number;
   referrer?: string;
 }): UseSpansQueryReturnType<T> {
-  const queryFunction = getQueryFunction({
-    isTimeseriesQuery: (eventView?.yAxis?.length ?? 0) > 0,
-  });
+  const isTimeseriesQuery = (eventView?.yAxis?.length ?? 0) > 0;
+  const queryFunction = isTimeseriesQuery
+    ? useWrappedDiscoverTimeseriesQuery
+    : useWrappedDiscoverQuery;
+
   if (eventView) {
-    const response = queryFunction({
+    const response = queryFunction<T>({
       eventView,
       initialData,
       limit,
@@ -53,7 +59,7 @@ export function useSpansQuery<T = any[]>({
   throw new Error('eventView argument must be defined when Starfish useDiscover is true');
 }
 
-export function useWrappedDiscoverTimeseriesQuery({
+export function useWrappedDiscoverTimeseriesQuery<T>({
   eventView,
   enabled,
   initialData,
@@ -65,7 +71,11 @@ export function useWrappedDiscoverTimeseriesQuery({
   enabled?: boolean;
   initialData?: any;
   referrer?: string;
-}) {
+}): {
+  data: T;
+  isLoading: boolean;
+  meta?: MetaType; // TODO: This is probably not correct! Timeseries calls return `meta` along with each _series_, rather than as an overall part of the response
+} {
   const location = useLocation();
   const organization = useOrganization();
   const {isLoading, data} = useGenericDiscoverQuery<
@@ -106,7 +116,7 @@ export function useWrappedDiscoverTimeseriesQuery({
   };
 }
 
-export function useWrappedDiscoverQuery({
+export function useWrappedDiscoverQuery<T>({
   eventView,
   initialData,
   enabled,
@@ -120,7 +130,12 @@ export function useWrappedDiscoverQuery({
   initialData?: any;
   limit?: number;
   referrer?: string;
-}) {
+}): {
+  data: T;
+  isLoading: boolean;
+  meta?: EventsMetaType;
+  pageLinks?: string;
+} {
   const location = useLocation();
   const organization = useOrganization();
   const {isLoading, data, pageLinks} = useDiscoverQuery({
@@ -139,19 +154,11 @@ export function useWrappedDiscoverQuery({
   return {
     isLoading,
     data: isLoading && initialData ? initialData : data?.data,
-    meta: data?.meta ?? {},
+    meta: data?.meta as unknown as EventsMetaType, // TODO: useDiscoverQuery incorrectly states that it returns MetaType, but it does not!
     pageLinks,
   };
 }
 
-function getQueryFunction({isTimeseriesQuery}: {isTimeseriesQuery?: boolean}) {
-  if (isTimeseriesQuery) {
-    return useWrappedDiscoverTimeseriesQuery;
-  }
-
-  return useWrappedDiscoverQuery;
-}
-
 type Interval = {[key: string]: any; interval: string; group?: string};
 
 function processDiscoverTimeseriesResult(result, eventView: EventView) {

static/app/views/starfish/views/spans/spansTable.tsx

@@ -11,16 +11,13 @@ import SortLink from 'sentry/components/gridEditable/sortLink';
 import Link from 'sentry/components/links/link';
 import Pagination, {CursorHandler} from 'sentry/components/pagination';
 import {Organization} from 'sentry/types';
-import {MetaType} from 'sentry/utils/discover/eventView';
+import {EventsMetaType} from 'sentry/utils/discover/eventView';
 import {getFieldRenderer} from 'sentry/utils/discover/fieldRenderers';
 import type {Sort} from 'sentry/utils/discover/fields';
 import {decodeScalar} from 'sentry/utils/queryString';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
-import CountCell from 'sentry/views/starfish/components/tableCells/countCell';
-import DurationCell from 'sentry/views/starfish/components/tableCells/durationCell';
 import ThroughputCell from 'sentry/views/starfish/components/tableCells/throughputCell';
-import {TimeSpentCell} from 'sentry/views/starfish/components/tableCells/timeSpentCell';
 import {OverflowEllipsisTextContainer} from 'sentry/views/starfish/components/textAlign';
 import {useSpanList} from 'sentry/views/starfish/queries/useSpanList';
 import {ModuleName, SpanMetricsFields} from 'sentry/views/starfish/types';
@@ -146,7 +143,7 @@ function renderHeadCell(column: Column, sort: Sort, location: Location) {
 function renderBodyCell(
   column: Column,
   row: Row,
-  meta: MetaType | undefined,
+  meta: EventsMetaType | undefined,
   location: Location,
   organization: Organization,
   endpoint?: string,
@@ -170,32 +167,15 @@ function renderBodyCell(
     );
   }
 
-  if (column.key === 'time_spent_percentage()') {
-    return (
-      <TimeSpentCell
-        timeSpentPercentage={row['time_spent_percentage()']}
-        totalSpanTime={row[`sum(${SPAN_SELF_TIME})`]}
-      />
-    );
-  }
-
   if (column.key === 'sps()') {
     return <ThroughputCell throughputPerSecond={row['sps()']} />;
   }
 
-  if (column.key === 'p95(span.self_time)') {
-    return <DurationCell milliseconds={row['p95(span.self_time)']} />;
-  }
-
-  if (column.key === 'http_error_count()') {
-    return <CountCell count={row['http_error_count()']} />;
-  }
-
   if (!meta || !meta?.fields) {
     return row[column.key];
   }
 
-  const renderer = getFieldRenderer(column.key, meta, false);
+  const renderer = getFieldRenderer(column.key, meta.fields, false);
   const rendered = renderer(row, {location, organization});
 
   return rendered;

static/app/views/starfish/views/webServiceView/endpointList.tsx

@@ -27,6 +27,7 @@ import {getAggregateAlias} from 'sentry/utils/discover/fields';
 import {NumberContainer} from 'sentry/utils/discover/styles';
 import {formatPercentage} from 'sentry/utils/formatters';
 import {TableColumn} from 'sentry/views/discover/table/types';
+import {PercentChangeCell} from 'sentry/views/starfish/components/tableCells/percentChangeCell';
 import ThroughputCell from 'sentry/views/starfish/components/tableCells/throughputCell';
 import {TIME_SPENT_IN_SERVICE} from 'sentry/views/starfish/utils/generatePerformanceEventView';
 import {DataTitles} from 'sentry/views/starfish/views/spans/types';
@@ -336,18 +337,6 @@ function EndpointList({eventView, location, organization, setError}: Props) {
 
 export default EndpointList;
 
-export const PercentChangeCell = styled('div')<{
-  trendDirection: 'good' | 'bad' | 'neutral';
-}>`
-  color: ${p =>
-    p.trendDirection === 'good'
-      ? p.theme.successText
-      : p.trendDirection === 'bad'
-      ? p.theme.errorText
-      : p.theme.subText};
-  float: right;
-`;
-
 const StyledSearchBar = styled(BaseSearchBar)`
   margin-bottom: ${space(2)};
 `;

static/app/views/starfish/views/webServiceView/failureDetailPanel/failureDetailTable.tsx

@@ -15,7 +15,7 @@ import {getFieldRenderer} from 'sentry/utils/discover/fieldRenderers';
 import {NumberContainer} from 'sentry/utils/discover/styles';
 import {formatPercentage} from 'sentry/utils/formatters';
 import {TableColumn} from 'sentry/views/discover/table/types';
-import {PercentChangeCell} from 'sentry/views/starfish/views/webServiceView/endpointList';
+import {PercentChangeCell} from 'sentry/views/starfish/components/tableCells/percentChangeCell';
 import {FailureSpike} from 'sentry/views/starfish/views/webServiceView/types';
 
 type Props = {