Browse Source

feat(starfish): Remove `has:span.category` filter (#50839)

[This](https://github.com/getsentry/sentry/pull/50839/files#diff-52f9db9081f5321466d2ad0b6d9eb14cca8eafad52bfdafed5d3f7ced96addd6L50-L53)
is the main change

Also use the `useEventsStatsQuery` hook to get events stats.
Basically the `useWrappedDiscoverTimeseriesQuery` hook
that ed added, but without the post processing
Handle the null span category case in span view.

---------

Co-authored-by: George Gritsouk <989898+gggritso@users.noreply.github.com>
Shruthi 1 year ago
parent
commit
75a596134e

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

@@ -1,4 +1,5 @@
 import {Location} from 'history';
+import omit from 'lodash/omit';
 import moment, {Moment} from 'moment';
 
 import {defined} from 'sentry/utils';
@@ -10,6 +11,7 @@ import {ModuleName} from 'sentry/views/starfish/types';
 import {getDateFilters} from 'sentry/views/starfish/utils/dates';
 import {getDateQueryFilter} from 'sentry/views/starfish/utils/getDateQueryFilter';
 import {useSpansQuery} from 'sentry/views/starfish/utils/useSpansQuery';
+import {NULL_SPAN_CATEGORY} from 'sentry/views/starfish/views/webServiceView/spanGroupBreakdownContainer';
 
 const SPAN_FILTER_KEYS = ['span.op', 'span.domain', 'span.action'];
 const SPAN_FILTER_KEY_TO_LOCAL_FIELD = {
@@ -154,7 +156,7 @@ function getEventView(
       projects: [1],
       version: 2,
     },
-    location
+    omit(location, 'span.category')
   );
 }
 
@@ -177,7 +179,11 @@ function buildEventViewQuery(
   }
 
   if (defined(spanCategory)) {
-    result.push(`span.category:${spanCategory}`);
+    if (spanCategory === NULL_SPAN_CATEGORY) {
+      result.push(`!has:span.category`);
+    } else if (spanCategory !== 'Other') {
+      result.push(`span.category:${spanCategory}`);
+    }
   }
 
   if (transaction) {

+ 50 - 0
static/app/views/starfish/utils/useEventsStatsQuery.tsx

@@ -0,0 +1,50 @@
+import {EventsStats, MultiSeriesEventsStats} from 'sentry/types';
+import EventView, {encodeSort} from 'sentry/utils/discover/eventView';
+import {
+  DiscoverQueryProps,
+  useGenericDiscoverQuery,
+} from 'sentry/utils/discover/genericDiscoverQuery';
+import {useLocation} from 'sentry/utils/useLocation';
+import useOrganization from 'sentry/utils/useOrganization';
+
+export function useEventsStatsQuery({
+  eventView,
+  enabled,
+  initialData,
+  referrer,
+}: {
+  eventView: EventView;
+  enabled?: boolean;
+  initialData?: any;
+  referrer?: string;
+}) {
+  const location = useLocation();
+  const organization = useOrganization();
+  const {isLoading, data} = useGenericDiscoverQuery<
+    EventsStats | MultiSeriesEventsStats,
+    DiscoverQueryProps
+  >({
+    route: 'events-stats',
+    eventView,
+    location,
+    orgSlug: organization.slug,
+    getRequestPayload: () => ({
+      ...eventView.getEventsAPIPayload(location),
+      yAxis: eventView.yAxis,
+      topEvents: eventView.topEvents,
+      excludeOther: 0,
+      partial: 1,
+      orderby: eventView.sorts?.[0] ? encodeSort(eventView.sorts?.[0]) : undefined,
+      interval: eventView.interval,
+    }),
+    options: {
+      enabled,
+      refetchOnWindowFocus: false,
+    },
+    referrer,
+  });
+  return {
+    isLoading,
+    data: isLoading && initialData ? initialData : data,
+  };
+}

+ 25 - 4
static/app/views/starfish/views/spans/selectors/actionSelector.tsx

@@ -10,20 +10,26 @@ import {useLocation} from 'sentry/utils/useLocation';
 import usePageFilters from 'sentry/utils/usePageFilters';
 import {ModuleName} from 'sentry/views/starfish/types';
 import {useSpansQuery} from 'sentry/views/starfish/utils/useSpansQuery';
+import {NULL_SPAN_CATEGORY} from 'sentry/views/starfish/views/webServiceView/spanGroupBreakdownContainer';
 
 type Props = {
   moduleName?: ModuleName;
+  spanCategory?: string;
   value?: string;
 };
 
-export function ActionSelector({value = '', moduleName = ModuleName.ALL}: Props) {
+export function ActionSelector({
+  value = '',
+  moduleName = ModuleName.ALL,
+  spanCategory,
+}: Props) {
   // TODO: This only returns the top 25 actions. It should either load them all, or paginate, or allow searching
   //
   const {selection} = usePageFilters();
 
   const location = useLocation();
   const query = getQuery(moduleName);
-  const eventView = getEventView(moduleName, selection);
+  const eventView = getEventView(moduleName, selection, spanCategory);
 
   const useHTTPActions = moduleName === ModuleName.HTTP;
 
@@ -91,12 +97,27 @@ function getQuery(moduleName?: string) {
   `;
 }
 
-function getEventView(moduleName: ModuleName, pageFilters: PageFilters) {
+function getEventView(
+  moduleName: ModuleName,
+  pageFilters: PageFilters,
+  spanCategory?: string
+) {
+  const queryConditions: string[] = [];
+  if (moduleName) {
+    queryConditions.push('!span.action:""');
+  }
+  if (spanCategory) {
+    if (spanCategory === NULL_SPAN_CATEGORY) {
+      queryConditions.push(`!has:span.category`);
+    } else if (spanCategory !== 'Other') {
+      queryConditions.push(`span.category:${spanCategory}`);
+    }
+  }
   return EventView.fromSavedQuery({
     name: '',
     fields: ['span.action', 'count()'],
     orderby: '-count',
-    query: moduleName ? `!span.action:"" span.module:${moduleName}` : '!span.action:""',
+    query: queryConditions.join(' '),
     dataset: DiscoverDatasets.SPANS_METRICS,
     start: pageFilters.datetime.start ?? undefined,
     end: pageFilters.datetime.end ?? undefined,

+ 25 - 4
static/app/views/starfish/views/spans/selectors/domainSelector.tsx

@@ -10,20 +10,26 @@ import {useLocation} from 'sentry/utils/useLocation';
 import usePageFilters from 'sentry/utils/usePageFilters';
 import {ModuleName} from 'sentry/views/starfish/types';
 import {useSpansQuery} from 'sentry/views/starfish/utils/useSpansQuery';
+import {NULL_SPAN_CATEGORY} from 'sentry/views/starfish/views/webServiceView/spanGroupBreakdownContainer';
 
 type Props = {
   moduleName?: ModuleName;
+  spanCategory?: string;
   value?: string;
 };
 
-export function DomainSelector({value = '', moduleName = ModuleName.ALL}: Props) {
+export function DomainSelector({
+  value = '',
+  moduleName = ModuleName.ALL,
+  spanCategory,
+}: Props) {
   // TODO: This only returns the top 25 domains. It should either load them all, or paginate, or allow searching
   //
   const {selection} = usePageFilters();
 
   const location = useLocation();
   const query = getQuery(moduleName);
-  const eventView = getEventView(moduleName, selection);
+  const eventView = getEventView(moduleName, selection, spanCategory);
 
   const {data: domains} = useSpansQuery<[{'span.domain': string}]>({
     eventView,
@@ -79,12 +85,27 @@ function getQuery(moduleName?: string) {
   `;
 }
 
-function getEventView(moduleName: string, pageFilters: PageFilters) {
+function getEventView(
+  moduleName: string,
+  pageFilters: PageFilters,
+  spanCategory?: string
+) {
+  const queryConditions: string[] = [`!span.domain:""`];
+  if (moduleName) {
+    queryConditions.push(`span.module:${moduleName}`);
+  }
+  if (spanCategory) {
+    if (spanCategory === NULL_SPAN_CATEGORY) {
+      queryConditions.push(`!has:span.category`);
+    } else if (spanCategory !== 'Other') {
+      queryConditions.push(`span.category:${spanCategory}`);
+    }
+  }
   return EventView.fromSavedQuery({
     name: '',
     fields: ['span.domain', 'count()'],
     orderby: '-count',
-    query: moduleName ? `!span.domain:"" span.module:${moduleName}` : '!span.domain:""',
+    query: queryConditions.join(' '),
     dataset: DiscoverDatasets.SPANS_METRICS,
     start: pageFilters.datetime.start ?? undefined,
     end: pageFilters.datetime.end ?? undefined,

+ 25 - 4
static/app/views/starfish/views/spans/selectors/spanOperationSelector.tsx

@@ -9,20 +9,26 @@ import {useLocation} from 'sentry/utils/useLocation';
 import usePageFilters from 'sentry/utils/usePageFilters';
 import {ModuleName} from 'sentry/views/starfish/types';
 import {useSpansQuery} from 'sentry/views/starfish/utils/useSpansQuery';
+import {NULL_SPAN_CATEGORY} from 'sentry/views/starfish/views/webServiceView/spanGroupBreakdownContainer';
 
 type Props = {
   value: string;
   moduleName?: ModuleName;
+  spanCategory?: string;
 };
 
-export function SpanOperationSelector({value = '', moduleName = ModuleName.ALL}: Props) {
+export function SpanOperationSelector({
+  value = '',
+  moduleName = ModuleName.ALL,
+  spanCategory,
+}: Props) {
   // TODO: This only returns the top 25 operations. It should either load them all, or paginate, or allow searching
   //
   const {selection} = usePageFilters();
 
   const location = useLocation();
   const query = getQuery(moduleName);
-  const eventView = getEventView(moduleName, selection);
+  const eventView = getEventView(moduleName, selection, spanCategory);
 
   const {data: operations} = useSpansQuery<[{'span.op': string}]>({
     eventView,
@@ -68,12 +74,27 @@ function getQuery(moduleName: ModuleName) {
   `;
 }
 
-function getEventView(moduleName: ModuleName, pageFilters: PageFilters) {
+function getEventView(
+  moduleName: ModuleName,
+  pageFilters: PageFilters,
+  spanCategory?: string
+) {
+  const queryConditions: string[] = [];
+  if (moduleName) {
+    queryConditions.push(`span.module:${moduleName}`);
+  }
+  if (spanCategory) {
+    if (spanCategory === NULL_SPAN_CATEGORY) {
+      queryConditions.push(`!has:span.category`);
+    } else if (spanCategory !== 'Other') {
+      queryConditions.push(`span.category:${spanCategory}`);
+    }
+  }
   return EventView.fromSavedQuery({
     name: '',
     fields: ['span.op', 'count()'],
     orderby: '-count',
-    query: moduleName ? `span.module:${moduleName}` : '',
+    query: queryConditions.join(' '),
     start: pageFilters.datetime.start ?? undefined,
     end: pageFilters.datetime.end ?? undefined,
     range: pageFilters.datetime.period ?? undefined,

+ 23 - 5
static/app/views/starfish/views/spans/spanTimeCharts.tsx

@@ -24,10 +24,12 @@ import {useSpansQuery} from 'sentry/views/starfish/utils/useSpansQuery';
 import {zeroFillSeries} from 'sentry/views/starfish/utils/zeroFillSeries';
 import {useErrorRateQuery as useErrorCountQuery} from 'sentry/views/starfish/views/spans/queries';
 import {DataTitles} from 'sentry/views/starfish/views/spans/types';
+import {NULL_SPAN_CATEGORY} from 'sentry/views/starfish/views/webServiceView/spanGroupBreakdownContainer';
 
 type Props = {
   appliedFilters: AppliedFilters;
   moduleName: ModuleName;
+  spanCategory?: string;
 };
 
 type AppliedFilters = {
@@ -42,11 +44,17 @@ type ChartProps = {
   moduleName: ModuleName;
 };
 
-export function SpanTimeCharts({moduleName, appliedFilters}: Props) {
+export function SpanTimeCharts({moduleName, appliedFilters, spanCategory}: Props) {
   const {selection} = usePageFilters();
   const location = useLocation();
 
-  const eventView = getEventView(moduleName, location, selection, appliedFilters);
+  const eventView = getEventView(
+    moduleName,
+    location,
+    selection,
+    appliedFilters,
+    spanCategory
+  );
 
   const {isLoading} = useSpansQuery({
     eventView,
@@ -299,9 +307,10 @@ const getEventView = (
   moduleName: ModuleName,
   location: Location,
   pageFilters: PageFilters,
-  appliedFilters: AppliedFilters
+  appliedFilters: AppliedFilters,
+  spanCategory?: string
 ) => {
-  const query = buildDiscoverQueryConditions(moduleName, appliedFilters);
+  const query = buildDiscoverQueryConditions(moduleName, appliedFilters, spanCategory);
 
   return EventView.fromNewQueryWithLocation(
     {
@@ -320,7 +329,8 @@ const getEventView = (
 
 const buildDiscoverQueryConditions = (
   moduleName: ModuleName,
-  appliedFilters: AppliedFilters
+  appliedFilters: AppliedFilters,
+  spanCategory?: string
 ) => {
   const result = Object.keys(appliedFilters)
     .filter(key => SPAN_FILTER_KEYS.includes(key))
@@ -333,6 +343,14 @@ const buildDiscoverQueryConditions = (
     result.push(`span.module:${moduleName}`);
   }
 
+  if (spanCategory) {
+    if (spanCategory === NULL_SPAN_CATEGORY) {
+      result.push(`!has:span.category`);
+    } else if (spanCategory !== 'Other') {
+      result.push(`span.category:${spanCategory}`);
+    }
+  }
+
   return result.join(' ');
 };
 

+ 4 - 0
static/app/views/starfish/views/spans/spansView.tsx

@@ -45,16 +45,19 @@ export default function SpansView(props: Props) {
         <SpanOperationSelector
           moduleName={props.moduleName}
           value={appliedFilters['span.op'] || ''}
+          spanCategory={props.spanCategory}
         />
 
         <DomainSelector
           moduleName={props.moduleName}
           value={appliedFilters['span.domain'] || ''}
+          spanCategory={props.spanCategory}
         />
 
         <ActionSelector
           moduleName={props.moduleName}
           value={appliedFilters['span.action'] || ''}
+          spanCategory={props.spanCategory}
         />
       </FilterOptionsContainer>
 
@@ -62,6 +65,7 @@ export default function SpansView(props: Props) {
         <SpanTimeCharts
           moduleName={props.moduleName || ModuleName.ALL}
           appliedFilters={appliedFilters}
+          spanCategory={props.spanCategory}
         />
       </PaddedContainer>
 

+ 60 - 24
static/app/views/starfish/views/webServiceView/spanGroupBreakdownContainer.tsx

@@ -6,17 +6,19 @@ import {Panel} from 'sentry/components/panels';
 import Placeholder from 'sentry/components/placeholder';
 import {space} from 'sentry/styles/space';
 import {PageFilters} from 'sentry/types';
-import {Series} from 'sentry/types/echarts';
+import {Series, SeriesDataUnit} from 'sentry/types/echarts';
+import {defined} from 'sentry/utils';
 import {useDiscoverQuery} from 'sentry/utils/discover/discoverQuery';
 import EventView from 'sentry/utils/discover/eventView';
 import {DiscoverDatasets} from 'sentry/utils/discover/types';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 import usePageFilters from 'sentry/utils/usePageFilters';
-import {useWrappedDiscoverTimeseriesQuery} from 'sentry/views/starfish/utils/useSpansQuery';
+import {useEventsStatsQuery} from 'sentry/views/starfish/utils/useEventsStatsQuery';
 import {SpanGroupBreakdown} from 'sentry/views/starfish/views/webServiceView/spanGroupBreakdown';
 
 export const OTHER_SPAN_GROUP_MODULE = 'Other';
+export const NULL_SPAN_CATEGORY = '<null>';
 
 type Props = {
   transaction?: string;
@@ -45,21 +47,21 @@ export function SpanGroupBreakdownContainer({transaction, transactionMethod}: Pr
   const theme = useTheme();
 
   const {data: segments, isLoading: isSegmentsLoading} = useDiscoverQuery({
-    eventView: getEventView(
+    eventView: getCumulativeTimeEventView(
       selection,
-      // TODO: Fix has:span.category in the backend
-      `transaction.op:http.server has:span.category ${
-        transaction ? `transaction:${transaction}` : ''
-      } ${transactionMethod ? `http.method:${transactionMethod}` : ''}`,
+      `transaction.op:http.server ${transaction ? `transaction:${transaction}` : ''} ${
+        transactionMethod ? `http.method:${transactionMethod}` : ''
+      }`,
       ['span.category']
     ),
     orgSlug: organization.slug,
+    referrer: 'starfish-web-service.span-category-breakdown',
     location,
     limit: 4,
   });
 
-  const {data: cumulativeTime} = useDiscoverQuery({
-    eventView: getEventView(
+  const {data: cumulativeTime, isLoading: isCumulativeDataLoading} = useDiscoverQuery({
+    eventView: getCumulativeTimeEventView(
       selection,
       `transaction.op:http.server ${transaction ? `transaction:${transaction}` : ''} ${
         transactionMethod ? `http.method:${transactionMethod}` : ''
@@ -67,22 +69,32 @@ export function SpanGroupBreakdownContainer({transaction, transactionMethod}: Pr
       []
     ),
     orgSlug: organization.slug,
+    referrer: 'starfish-web-service.total-time',
     location,
   });
 
-  const {isLoading: isTopDataLoading, data: topData} = useWrappedDiscoverTimeseriesQuery({
+  const {isLoading: isTopDataLoading, data: topData} = useEventsStatsQuery({
     eventView: getEventView(
       selection,
-      `transaction.op:http.server has:span.category ${
-        transaction ? `transaction:${transaction}` : ''
-      } ${transactionMethod ? `http.method:${transactionMethod}` : ''}`,
+      `transaction.op:http.server ${transaction ? `transaction:${transaction}` : ''} ${
+        transactionMethod ? `http.method:${transactionMethod}` : ''
+      }`,
       ['span.category'],
       true
     ),
+    enabled: true,
+    referrer: 'starfish-web-service.span-category-breakdown-timeseries',
     initialData: [],
   });
 
-  if (!segments?.data || !cumulativeTime?.data || topData.length === 0) {
+  if (
+    isSegmentsLoading ||
+    isCumulativeDataLoading ||
+    isTopDataLoading ||
+    !defined(segments) ||
+    !defined(cumulativeTime) ||
+    !defined(topData)
+  ) {
     return <Placeholder height="285px" />;
   }
 
@@ -101,10 +113,11 @@ export function SpanGroupBreakdownContainer({transaction, transactionMethod}: Pr
 
   for (let index = 0; index < segments.data.length; index++) {
     const element = segments.data[index];
+    const category = element['span.category'] as string;
     transformedData.push({
       cumulativeTime: parseInt(element['sum(span.duration)'] as string, 10),
       group: {
-        'span.category': element['span.category'] as string,
+        'span.category': category === '' ? NULL_SPAN_CATEGORY : category,
       },
     });
   }
@@ -121,21 +134,24 @@ export function SpanGroupBreakdownContainer({transaction, transactionMethod}: Pr
   const seriesByDomain: {[category: string]: Series} = {};
   const colorPalette = theme.charts.getColorPalette(transformedData.length - 2);
 
-  if (!isTopDataLoading && topData.length > 0 && transformedData.length > 0) {
+  if (!isTopDataLoading && transformedData.length > 0) {
     transformedData.forEach((segment, index) => {
-      const label = segment.group['span.category'];
+      const category = segment.group['span.category'] as string;
+      const label = category === '' ? NULL_SPAN_CATEGORY : category;
       seriesByDomain[label] = {
-        seriesName: `${label}`,
+        seriesName: label,
         data: [],
         color: colorPalette[index],
       };
     });
 
-    topData.forEach(value => {
-      seriesByDomain[value.group].data.push({
-        value: value['p95(span.duration)'],
-        name: value.interval,
-      });
+    Object.keys(topData).forEach(key => {
+      const seriesData = topData?.[key];
+      const label = key === '' ? NULL_SPAN_CATEGORY : key;
+      seriesByDomain[label].data =
+        seriesData?.data.map(datum => {
+          return {name: datum[0], value: datum[1][0].count} as SeriesDataUnit;
+        }) ?? [];
     });
   }
 
@@ -174,7 +190,7 @@ const getEventView = (
   return EventView.fromSavedQuery({
     name: '',
     fields: ['sum(span.duration)', 'p95(span.duration)', ...groups],
-    yAxis: getTimeseries ? ['sum(span.duration)', 'p95(span.duration)'] : [],
+    yAxis: getTimeseries ? ['p95(span.duration)'] : [],
     query,
     dataset: DiscoverDatasets.SPANS_METRICS,
     start: pageFilters.datetime.start ?? undefined,
@@ -187,3 +203,23 @@ const getEventView = (
     interval: getTimeseries ? getInterval(pageFilters.datetime, 'low') : undefined,
   });
 };
+
+const getCumulativeTimeEventView = (
+  pageFilters: PageFilters,
+  query: string,
+  groups: string[]
+) => {
+  return EventView.fromSavedQuery({
+    name: '',
+    fields: ['sum(span.duration)', ...groups],
+    query,
+    dataset: DiscoverDatasets.SPANS_METRICS,
+    start: pageFilters.datetime.start ?? undefined,
+    end: pageFilters.datetime.end ?? undefined,
+    range: pageFilters.datetime.period ?? undefined,
+    orderby: '-sum_span_duration',
+    projects: [1],
+    version: 2,
+    topEvents: groups.length > 0 ? '4' : undefined,
+  });
+};