Browse Source

ref(starfish): More sophisticated API typing (#55506)

tl;dr this PR introduces a new set of types that makes it possible to
have much more sophisticated typing for API responses and improves DX.

- It is illegal to try to fetch a field that doesn't exist.
- The server response is _typed_. Only the fields that were requested are
available on the returned object! You can't get `data['sps()']` if you
didn't request that field.
- The server response fields have correct types (this was already the
case, but it's cleaner now).
George Gritsouk 1 year ago
parent
commit
c1aa51a04f

+ 2 - 2
static/app/views/starfish/components/spanDescription.tsx

@@ -1,12 +1,12 @@
 import styled from '@emotion/styled';
 
 import {CodeSnippet} from 'sentry/components/codeSnippet';
-import {SpanMetricsField, SpanMetricsFieldTypes} from 'sentry/views/starfish/types';
+import {MetricsResponse, SpanMetricsField} from 'sentry/views/starfish/types';
 import {SQLishFormatter} from 'sentry/views/starfish/utils/sqlish/SQLishFormatter';
 
 type Props = {
   span: Pick<
-    SpanMetricsFieldTypes,
+    MetricsResponse,
     SpanMetricsField.SPAN_OP | SpanMetricsField.SPAN_DESCRIPTION
   >;
 };

+ 15 - 14
static/app/views/starfish/queries/useSpanMetrics.tsx

@@ -3,29 +3,24 @@ import {Location} from 'history';
 import EventView from 'sentry/utils/discover/eventView';
 import {DiscoverDatasets} from 'sentry/utils/discover/types';
 import {useLocation} from 'sentry/utils/useLocation';
-import {SpanMetricsField} from 'sentry/views/starfish/types';
+import {
+  MetricsProperty,
+  MetricsResponse,
+  SpanMetricsField,
+} from 'sentry/views/starfish/types';
 import {useSpansQuery} from 'sentry/views/starfish/utils/useSpansQuery';
 
 const {SPAN_GROUP} = SpanMetricsField;
 
-export type SpanMetrics = {
-  [metric: string]: number | string;
-  'http_error_count()': number;
-  'p95(span.self_time)': number;
-  'span.op': string;
-  'spm()': number;
-  'time_spent_percentage()': number;
-};
-
 export type SpanSummaryQueryFilters = {
   'transaction.method'?: string;
   transactionName?: string;
 };
 
-export const useSpanMetrics = (
+export const useSpanMetrics = <T extends MetricsProperty[]>(
   group: string,
   queryFilters: SpanSummaryQueryFilters,
-  fields: string[] = [],
+  fields: T,
   referrer: string = 'span-metrics'
 ) => {
   const location = useLocation();
@@ -37,14 +32,20 @@ export const useSpanMetrics = (
     Boolean(group) && Object.values(queryFilters).every(value => Boolean(value));
 
   // TODO: Add referrer
-  const result = useSpansQuery<SpanMetrics[]>({
+  const result = useSpansQuery({
     eventView,
     initialData: [],
     enabled,
     referrer,
   });
 
-  return {...result, data: result?.data?.[0] ?? {}, isEnabled: enabled};
+  const data = (result?.data?.[0] ?? {}) as Pick<MetricsResponse, T[number]>;
+
+  return {
+    ...result,
+    data,
+    isEnabled: enabled,
+  };
 };
 
 function getEventView(

+ 30 - 16
static/app/views/starfish/types.tsx

@@ -28,17 +28,38 @@ export enum SpanMetricsField {
   PROJECT_ID = 'project.id',
 }
 
-export type SpanMetricsFieldTypes = {
-  [SpanMetricsField.SPAN_OP]: string;
-  [SpanMetricsField.SPAN_DESCRIPTION]: string;
-  [SpanMetricsField.SPAN_MODULE]: string;
-  [SpanMetricsField.SPAN_ACTION]: string;
-  [SpanMetricsField.SPAN_DOMAIN]: string;
-  [SpanMetricsField.SPAN_GROUP]: string;
-  [SpanMetricsField.SPAN_SELF_TIME]: number;
-  [SpanMetricsField.SPAN_DURATION]: number;
+export type SpanNumberFields = 'span.self_time' | 'span.duration';
+
+export type SpanStringFields =
+  | 'span.op'
+  | 'span.description'
+  | 'span.module'
+  | 'span.action'
+  | 'span.domain'
+  | 'span.group'
+  | 'project.id';
+
+export type SpanFunctions =
+  | 'sps'
+  | 'spm'
+  | 'count'
+  | 'time_spent_percentage'
+  | 'http_error_count';
+
+export type MetricsResponse = {
+  [Property in SpanNumberFields as `avg(${Property})`]: number;
+} & {
+  [Property in SpanNumberFields as `sum(${Property})`]: number;
+} & {
+  [Property in SpanFunctions as `${Property}()`]: number;
+} & {
+  [Property in SpanStringFields as `${Property}`]: string;
+} & {
+  'time_spent_percentage(local)': number;
 };
 
+export type MetricsProperty = keyof MetricsResponse;
+
 export enum SpanIndexedField {
   SPAN_SELF_TIME = 'span.self_time',
   SPAN_GROUP = 'span.group', // Span group computed from the normalized description. Matches the group in the metrics data set
@@ -78,7 +99,6 @@ export type Op = SpanIndexedFieldTypes[SpanIndexedField.SPAN_OP];
 export enum SpanFunction {
   SPS = 'sps',
   SPM = 'spm',
-  SPS_PERCENENT_CHANGE = 'sps_percent_change',
   TIME_SPENT_PERCENTAGE = 'time_spent_percentage',
   HTTP_ERROR_COUNT = 'http_error_count',
 }
@@ -116,10 +136,4 @@ export const STARFISH_AGGREGATION_FIELDS: Record<
     kind: FieldKind.FUNCTION,
     valueType: FieldValueType.NUMBER,
   },
-  [SpanFunction.SPS_PERCENENT_CHANGE]: {
-    desc: t('Spans per second percentage change'),
-    defaultOutputType: 'percentage',
-    kind: FieldKind.FUNCTION,
-    valueType: FieldValueType.NUMBER,
-  },
 };

+ 5 - 14
static/app/views/starfish/views/spanSummaryPage/index.tsx

@@ -19,7 +19,6 @@ import {
   SpanSummaryQueryFilters,
   useSpanMetrics,
 } from 'sentry/views/starfish/queries/useSpanMetrics';
-import {SpanFunction, SpanMetricsField} from 'sentry/views/starfish/types';
 import {extractRoute} from 'sentry/views/starfish/utils/extractRoute';
 import {ROUTE_NAMES} from 'sentry/views/starfish/utils/routeNames';
 import {QueryParameterNames} from 'sentry/views/starfish/views/queryParameters';
@@ -64,24 +63,16 @@ function SpanSummaryPage({params, location}: Props) {
   const {data: spanMetrics, isLoading: isSpanMetricsLoading} = useSpanMetrics(
     groupId,
     queryFilter,
-    [
-      SpanMetricsField.SPAN_OP,
-      SpanMetricsField.SPAN_GROUP,
-      SpanMetricsField.PROJECT_ID,
-      `${SpanFunction.SPS}()`,
-    ],
+    ['span.op', 'span.group', 'project.id', 'sps()'],
     'api.starfish.span-summary-page-metrics'
   );
 
   const span = {
-    [SpanMetricsField.SPAN_OP]: spanMetrics[SpanMetricsField.SPAN_OP],
-    [SpanMetricsField.SPAN_GROUP]: groupId,
+    ['span.op']: spanMetrics['span.op'],
+    ['span.group']: groupId,
   };
 
-  const title = [
-    getSpanOperationDescription(span[SpanMetricsField.SPAN_OP]),
-    t('Summary'),
-  ].join(' ');
+  const title = [getSpanOperationDescription(span['span.op']), t('Summary')].join(' ');
 
   const crumbs: Crumb[] = [];
   crumbs.push({
@@ -137,7 +128,7 @@ function SpanSummaryPage({params, location}: Props) {
                 )}
 
                 <SampleList
-                  groupId={span[SpanMetricsField.SPAN_GROUP]}
+                  groupId={span['span.group']}
                   transactionName={transaction}
                   transactionMethod={transactionMethod}
                 />

+ 2 - 5
static/app/views/starfish/views/spanSummaryPage/spanTransactionsTable.tsx

@@ -29,10 +29,10 @@ import {
   useSpanTransactionMetrics,
 } from 'sentry/views/starfish/queries/useSpanTransactionMetrics';
 import {
+  MetricsResponse,
   SpanIndexedField,
   SpanIndexedFieldTypes,
   SpanMetricsField,
-  SpanMetricsFieldTypes,
 } from 'sentry/views/starfish/types';
 import {extractRoute} from 'sentry/views/starfish/utils/extractRoute';
 import {useRoutingContext} from 'sentry/views/starfish/utils/routingContext';
@@ -50,10 +50,7 @@ type Row = {
 
 type Props = {
   sort: ValidSort;
-  span: Pick<
-    SpanMetricsFieldTypes,
-    SpanMetricsField.SPAN_GROUP | SpanMetricsField.SPAN_OP
-  >;
+  span: Pick<MetricsResponse, SpanMetricsField.SPAN_GROUP | SpanMetricsField.SPAN_OP>;
   endpoint?: string;
   endpointMethod?: string;
 };