Browse Source

fix(starfish) Use metrics data instead of indexed spans on span summary page (#50736)

Indexed data might not be present! The span summary details page follows
span tables that are fetched using metrics, so this page should use
metrics, too. Sampled indexed spans should not be used this early in the
flow.
George Gritsouk 1 year ago
parent
commit
dbeaa3b5d2

+ 15 - 9
static/app/views/starfish/components/spanDescription.tsx

@@ -1,24 +1,30 @@
 import styled from '@emotion/styled';
 
 import {FormattedCode} from 'sentry/views/starfish/components/formattedCode';
-import type {IndexedSpan} from 'sentry/views/starfish/queries/types';
 import {highlightSql} from 'sentry/views/starfish/utils/highlightSql';
 
-export function SpanDescription({span}: {span: IndexedSpan}) {
-  if (span.op.startsWith('db')) {
-    return <DatabaseSpanDescription span={span} />;
+type SpanMeta = {
+  'span.action': string;
+  'span.description': string;
+  'span.domain': string;
+  'span.op': string;
+};
+
+export function SpanDescription({spanMeta}: {spanMeta: SpanMeta}) {
+  if (spanMeta['span.op'].startsWith('db')) {
+    return <DatabaseSpanDescription spanMeta={spanMeta} />;
   }
 
-  return <div>{span.description}</div>;
+  return <div>{spanMeta['span.description']}</div>;
 }
 
-function DatabaseSpanDescription({span}: {span: IndexedSpan}) {
+function DatabaseSpanDescription({spanMeta}: {spanMeta: SpanMeta}) {
   return (
     <CodeWrapper>
       <FormattedCode>
-        {highlightSql(span.description || '', {
-          action: span.action || '',
-          domain: span.domain || '',
+        {highlightSql(spanMeta['span.description'] || '', {
+          action: spanMeta['span.action'] || '',
+          domain: spanMeta['span.domain'] || '',
         })}
       </FormattedCode>
     </CodeWrapper>

+ 41 - 0
static/app/views/starfish/queries/useSpanMeta.tsx

@@ -0,0 +1,41 @@
+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 {useSpansQuery} from 'sentry/views/starfish/utils/useSpansQuery';
+
+export type SpanMeta = {
+  'span.action': string;
+  'span.description': string;
+  'span.domain': string;
+  'span.op': string;
+};
+
+export const useSpanMeta = (
+  group: string,
+  queryFilters: {transactionName?: string} = {},
+  referrer: string = 'span-metrics'
+) => {
+  const location = useLocation();
+
+  return useSpansQuery<SpanMeta[]>({
+    eventView: getEventView(group, location, queryFilters.transactionName),
+    initialData: [],
+    referrer,
+  });
+};
+
+function getEventView(groupId, location: Location, transaction?: string) {
+  return EventView.fromNewQueryWithLocation(
+    {
+      name: '',
+      query: `span.group:${groupId}${transaction ? ` transaction:${transaction}` : ''}`,
+      fields: ['span.op', 'span.description', 'span.action', 'span.domain', 'count()'], // TODO: Failing to pass a field like `count()` causes an error
+      dataset: DiscoverDatasets.SPANS_METRICS,
+      projects: [1],
+      version: 2,
+    },
+    location
+  );
+}

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

@@ -20,7 +20,7 @@ import {SpanDescription} from 'sentry/views/starfish/components/spanDescription'
 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 {useIndexedSpan} from 'sentry/views/starfish/queries/useIndexedSpan';
+import {useSpanMeta} from 'sentry/views/starfish/queries/useSpanMeta';
 import {useSpanMetrics} from 'sentry/views/starfish/queries/useSpanMetrics';
 import {useSpanMetricsSeries} from 'sentry/views/starfish/queries/useSpanMetricsSeries';
 import {DataTitles} from 'sentry/views/starfish/views/spans/types';
@@ -35,7 +35,19 @@ function SpanSummaryPage({params, location}: Props) {
   const {groupId} = params;
   const {transaction} = location.query;
 
-  const {data: span} = useIndexedSpan(groupId, 'span-summary-page');
+  const {data: spanMetas} = useSpanMeta(
+    groupId,
+    undefined,
+    'span-summary-page-span-meta'
+  );
+  // TODO: Span meta might in theory return more than one row! In that case, we
+  // need to indicate in the UI that more than one set of meta corresponds to
+  // the span group
+  const span = {
+    group: groupId,
+    ...spanMetas?.[0],
+  };
+
   const {data: spanMetrics} = useSpanMetrics(
     {group: groupId},
     undefined,
@@ -69,7 +81,7 @@ function SpanSummaryPage({params, location}: Props) {
                 <DatePageFilter alignDropdown="left" />
               </FilterOptionsContainer>
               <BlockContainer>
-                <Block title={t('Operation')}>{span?.op}</Block>
+                <Block title={t('Operation')}>{span?.['span.op']}</Block>
                 <Block
                   title={t('Throughput')}
                   description={t('Throughput of this span per second')}
@@ -94,13 +106,13 @@ function SpanSummaryPage({params, location}: Props) {
                 </Block>
               </BlockContainer>
 
-              {span?.description && (
+              {span?.['span.description'] && (
                 <BlockContainer>
                   <Block>
                     <Panel>
                       <PanelBody>
                         <DescriptionContainer>
-                          <SpanDescription span={span} />
+                          <SpanDescription spanMeta={spanMetas?.[0]} />
                         </DescriptionContainer>
                       </PanelBody>
                     </Panel>