Browse Source

feat(starfish): Split out span view table delta columns (#51393)

The "Change" columns are now separate, have their own headings, and are
sortable. I ended up doing this the "right" way, which is to hook up
response meta, and the field renderer pipeline. Unfortunately, our
TypeScript types for the responses are _wrong_, so there's still some
work to do, but this is a good first step! From now on we can do a lot
more of this kind of reusable rendering.
George Gritsouk 1 year ago
parent
commit
48d032f69c

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

@@ -46,6 +46,7 @@ import {
   SpanOperationBreakdownFilter,
   stringToFilter,
 } from 'sentry/views/performance/transactionSummary/filter';
+import {PercentChangeCell} from 'sentry/views/starfish/views/webServiceView/endpointList';
 
 import {decodeScalar} from '../queryString';
 
@@ -673,6 +674,9 @@ type SpecialFunctionFieldRenderer = (
 ) => (data: EventData, baggage: RenderFunctionBaggage) => React.ReactNode;
 
 type SpecialFunctions = {
+  http_error_count_percent_change: SpecialFunctionFieldRenderer;
+  percentile_percent_change: SpecialFunctionFieldRenderer;
+  sps_percent_change: SpecialFunctionFieldRenderer;
   user_misery: SpecialFunctionFieldRenderer;
 };
 
@@ -740,6 +744,46 @@ 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];
+
+    const sign = deltaValue >= 0 ? '+' : '-';
+    const delta = formatPercentage(Math.abs(deltaValue), 2);
+
+    return (
+      // N.B. For throughput, the change is neither good nor bad regardless of value! Throughput is just throughput
+      <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';
+
+    return (
+      <PercentChangeCell
+        trendDirection={trendDirection}
+      >{`${sign}${delta}`}</PercentChangeCell>
+    );
+  },
 };
 
 /**

+ 2 - 2
static/app/views/starfish/queries/useSpanList.tsx

@@ -41,7 +41,7 @@ export const useSpanList = (
 
   const eventView = getEventView(moduleName, location, transaction, spanCategory, sorts);
 
-  const {isLoading, data, pageLinks} = useSpansQuery<SpanMetrics[]>({
+  const {isLoading, data, meta, pageLinks} = useSpansQuery<SpanMetrics[]>({
     eventView,
     initialData: [],
     limit,
@@ -49,7 +49,7 @@ export const useSpanList = (
     cursor,
   });
 
-  return {isLoading, data, pageLinks};
+  return {isLoading, data, meta, pageLinks};
 };
 
 function getEventView(

+ 17 - 2
static/app/views/starfish/utils/useSpansQuery.tsx

@@ -1,7 +1,7 @@
 import moment from 'moment';
 
 import {useDiscoverQuery} from 'sentry/utils/discover/discoverQuery';
-import EventView, {encodeSort} from 'sentry/utils/discover/eventView';
+import EventView, {encodeSort, MetaType} from 'sentry/utils/discover/eventView';
 import {
   DiscoverQueryProps,
   useGenericDiscoverQuery,
@@ -15,6 +15,7 @@ export const DATE_FORMAT = 'YYYY-MM-DDTHH:mm:ssZ';
 export type UseSpansQueryReturnType<T> = {
   data: T;
   isLoading: boolean;
+  meta?: MetaType;
   pageLinks?: string;
 };
 
@@ -37,8 +38,18 @@ export function useSpansQuery<T = any[]>({
     isTimeseriesQuery: (eventView?.yAxis?.length ?? 0) > 0,
   });
   if (eventView) {
-    return queryFunction({eventView, initialData, limit, enabled, referrer, cursor});
+    const response = queryFunction({
+      eventView,
+      initialData,
+      limit,
+      enabled,
+      referrer,
+      cursor,
+    });
+
+    return response;
   }
+
   throw new Error('eventView argument must be defined when Starfish useDiscover is true');
 }
 
@@ -60,6 +71,7 @@ export function useWrappedDiscoverTimeseriesQuery({
   const {isLoading, data} = useGenericDiscoverQuery<
     {
       data: any[];
+      meta: MetaType;
     },
     DiscoverQueryProps
   >({
@@ -83,12 +95,14 @@ export function useWrappedDiscoverTimeseriesQuery({
     },
     referrer,
   });
+
   return {
     isLoading,
     data:
       isLoading && initialData
         ? initialData
         : processDiscoverTimeseriesResult(data, eventView),
+    meta: data?.meta,
   };
 }
 
@@ -121,6 +135,7 @@ export function useWrappedDiscoverQuery({
       refetchOnWindowFocus: false,
     },
   });
+
   return {
     isLoading,
     data: isLoading && initialData ? initialData : data?.data,

+ 37 - 23
static/app/views/starfish/views/spans/spansTable.tsx

@@ -10,9 +10,13 @@ import GridEditable, {
 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 {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';
@@ -74,10 +78,11 @@ export default function SpansTable({
   limit = 25,
 }: Props) {
   const location = useLocation();
+  const organization = useOrganization();
 
   const spansCursor = decodeScalar(location.query?.[QueryParameterNames.CURSOR]);
 
-  const {isLoading, data, pageLinks} = useSpanList(
+  const {isLoading, data, meta, pageLinks} = useSpanList(
     moduleName ?? ModuleName.ALL,
     undefined,
     spanCategory,
@@ -109,7 +114,7 @@ export default function SpansTable({
         grid={{
           renderHeadCell: column => renderHeadCell(column, sort, location),
           renderBodyCell: (column, row) =>
-            renderBodyCell(column, row, location, endpoint, method),
+            renderBodyCell(column, row, meta, location, organization, endpoint, method),
         }}
         location={location}
       />
@@ -141,7 +146,9 @@ function renderHeadCell(column: Column, sort: Sort, location: Location) {
 function renderBodyCell(
   column: Column,
   row: Row,
+  meta: MetaType | undefined,
   location: Location,
+  organization: Organization,
   endpoint?: string,
   method?: string
 ): React.ReactNode {
@@ -173,33 +180,25 @@ function renderBodyCell(
   }
 
   if (column.key === 'sps()') {
-    return (
-      <ThroughputCell
-        throughputPerSecond={row['sps()']}
-        delta={row['sps_percent_change()']}
-      />
-    );
+    return <ThroughputCell throughputPerSecond={row['sps()']} />;
   }
 
   if (column.key === 'p95(span.self_time)') {
-    return (
-      <DurationCell
-        milliseconds={row['p95(span.self_time)']}
-        delta={row['percentile_percent_change(span.self_time, 0.95)']}
-      />
-    );
+    return <DurationCell milliseconds={row['p95(span.self_time)']} />;
   }
 
   if (column.key === 'http_error_count()') {
-    return (
-      <CountCell
-        count={row['http_error_count()']}
-        delta={row['http_error_count_percent_change()']}
-      />
-    );
+    return <CountCell count={row['http_error_count()']} />;
   }
 
-  return row[column.key];
+  if (!meta || !meta?.fields) {
+    return row[column.key];
+  }
+
+  const renderer = getFieldRenderer(column.key, meta, false);
+  const rendered = renderer(row, {location, organization});
+
+  return rendered;
 }
 
 function getDomainHeader(moduleName: ModuleName) {
@@ -249,12 +248,22 @@ function getColumns(moduleName: ModuleName): Column[] {
     {
       key: 'sps()',
       name: 'Throughput',
-      width: 175,
+      width: COL_WIDTH_UNDEFINED,
+    },
+    {
+      key: 'sps_percent_change()',
+      name: DataTitles.change,
+      width: COL_WIDTH_UNDEFINED,
     },
     {
       key: `p95(${SPAN_SELF_TIME})`,
       name: DataTitles.p95,
-      width: 175,
+      width: COL_WIDTH_UNDEFINED,
+    },
+    {
+      key: `percentile_percent_change(${SPAN_SELF_TIME}, 0.95)`,
+      name: DataTitles.change,
+      width: COL_WIDTH_UNDEFINED,
     },
     ...(moduleName === ModuleName.HTTP
       ? [
@@ -263,6 +272,11 @@ function getColumns(moduleName: ModuleName): Column[] {
             name: DataTitles.errorCount,
             width: COL_WIDTH_UNDEFINED,
           } as Column,
+          {
+            key: 'http_error_count_percent_change()',
+            name: DataTitles.change,
+            width: COL_WIDTH_UNDEFINED,
+          } as Column,
         ]
       : []),
     {

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

@@ -5,6 +5,7 @@ import {t} from 'sentry/locale';
 import DurationCell from 'sentry/views/starfish/components/tableCells/durationCell';
 
 export type DataKey =
+  | 'change'
   | 'timeSpent'
   | 'p50p95'
   | 'p50'
@@ -14,6 +15,7 @@ export type DataKey =
   | 'errorCount';
 
 export const DataTitles: Record<DataKey, string> = {
+  change: t('Change'),
   timeSpent: t('Time Spent'),
   p50p95: t('Duration (P50, P95)'),
   p50: t('Duration (P50)'),