Browse Source

feat(tracing-without-performance): Replay compatibility and ui improvements. (#54700)

This PR aims to make the new orphan error views compatible with
traceview in replayDetails and adds hidden messages for orphan error
search in traceviews, under the feature flag
`organizations:performance-tracing-without-performance`.

Testing with yarn dev-ui:
- Orphan errors traceview for testing search hidden messages:
[link](https://sentry.dev.getsentry.net:7999/performance/trace/2f870c047f0349888080e65a8fa69df5/?pageEnd=2023-08-15T03%3A23%3A45.496&pageStart=2023-08-14T03%3A23%3A45.496)
- Replay details trace should now load:[
link](https://sentry.dev.getsentry.net:7999/replays/3487099c4a6d41bb8b3b68bbaf4c8548/?query=&referrer=%2Freplays%2F&statsPeriod=14d&t_main=trace&yAxis=count%28%29)

Changes:
![Screenshot 2023-08-14 at 1 00 58
PM](https://github.com/getsentry/sentry/assets/60121741/f060d40b-a094-4782-b950-14ee29ffce07)
<img width="660" alt="Screenshot 2023-08-14 at 1 02 10 PM"
src="https://github.com/getsentry/sentry/assets/60121741/7494a331-e979-4857-ab46-f4c72df6bdcd">



Context:
We now want to show trace views for traces that contain orphan errors.
[JIRA-TICKET](https://getsentry.atlassian.net/browse/PERF-2052)

---------

Co-authored-by: Abdullah Khan <abdullahkhan@PG9Y57YDXQ.local>
Abdkhan14 1 year ago
parent
commit
7518d181cd

+ 5 - 14
static/app/views/performance/traceDetails/index.tsx

@@ -14,17 +14,16 @@ import {QueryError} from 'sentry/utils/discover/genericDiscoverQuery';
 import {TraceFullDetailedQuery} from 'sentry/utils/performance/quickTrace/traceFullQuery';
 import {TraceFullDetailedQuery} from 'sentry/utils/performance/quickTrace/traceFullQuery';
 import TraceMetaQuery from 'sentry/utils/performance/quickTrace/traceMetaQuery';
 import TraceMetaQuery from 'sentry/utils/performance/quickTrace/traceMetaQuery';
 import {
 import {
-  TraceError,
   TraceFullDetailed,
   TraceFullDetailed,
   TraceMeta,
   TraceMeta,
   TraceSplitResults,
   TraceSplitResults,
 } from 'sentry/utils/performance/quickTrace/types';
 } from 'sentry/utils/performance/quickTrace/types';
-import {isTraceSplitResult} from 'sentry/utils/performance/quickTrace/utils';
 import {decodeScalar} from 'sentry/utils/queryString';
 import {decodeScalar} from 'sentry/utils/queryString';
 import withApi from 'sentry/utils/withApi';
 import withApi from 'sentry/utils/withApi';
 import withOrganization from 'sentry/utils/withOrganization';
 import withOrganization from 'sentry/utils/withOrganization';
 
 
 import TraceDetailsContent from './content';
 import TraceDetailsContent from './content';
+import {getTraceSplitResults} from './utils';
 
 
 type Props = RouteComponentProps<{traceSlug: string}, {}> & {
 type Props = RouteComponentProps<{traceSlug: string}, {}> & {
   api: Client;
   api: Client;
@@ -87,18 +86,10 @@ class TraceSummary extends Component<Props> {
       meta: TraceMeta | null;
       meta: TraceMeta | null;
       traces: (TraceFullDetailed[] | TraceSplitResults<TraceFullDetailed>) | null;
       traces: (TraceFullDetailed[] | TraceSplitResults<TraceFullDetailed>) | null;
     }) => {
     }) => {
-      let transactions: TraceFullDetailed[] | undefined;
-      let orphanErrors: TraceError[] | undefined;
-      if (
-        traces &&
-        organization.features.includes('performance-tracing-without-performance') &&
-        isTraceSplitResult<TraceSplitResults<TraceFullDetailed>, TraceFullDetailed[]>(
-          traces
-        )
-      ) {
-        orphanErrors = traces.orphan_errors;
-        transactions = traces.transactions;
-      }
+      const {transactions, orphanErrors} = getTraceSplitResults<TraceFullDetailed>(
+        traces ?? [],
+        organization
+      );
 
 
       return (
       return (
         <TraceDetailsContent
         <TraceDetailsContent

+ 83 - 34
static/app/views/performance/traceDetails/traceView.tsx

@@ -10,6 +10,7 @@ import {
   boundsGenerator,
   boundsGenerator,
   getMeasurements,
   getMeasurements,
 } from 'sentry/components/events/interfaces/spans/utils';
 } from 'sentry/components/events/interfaces/spans/utils';
+import Panel from 'sentry/components/panels/panel';
 import {MessageRow} from 'sentry/components/performance/waterfall/messageRow';
 import {MessageRow} from 'sentry/components/performance/waterfall/messageRow';
 import {
 import {
   DividerSpacer,
   DividerSpacer,
@@ -29,7 +30,6 @@ import {
 } from 'sentry/utils/performance/quickTrace/types';
 } from 'sentry/utils/performance/quickTrace/types';
 import {
 import {
   TraceDetailBody,
   TraceDetailBody,
-  TracePanel,
   TraceViewContainer,
   TraceViewContainer,
   TraceViewHeaderContainer,
   TraceViewHeaderContainer,
 } from 'sentry/views/performance/traceDetails/styles';
 } from 'sentry/views/performance/traceDetails/styles';
@@ -64,24 +64,50 @@ type Props = Pick<RouteComponentProps<{}, {}>, 'location'> & {
 function TraceHiddenMessage({
 function TraceHiddenMessage({
   isVisible,
   isVisible,
   numberOfHiddenTransactionsAbove,
   numberOfHiddenTransactionsAbove,
+  numberOfHiddenErrorsAbove,
 }: {
 }: {
   isVisible: boolean;
   isVisible: boolean;
+  numberOfHiddenErrorsAbove: number;
   numberOfHiddenTransactionsAbove: number;
   numberOfHiddenTransactionsAbove: number;
 }) {
 }) {
-  if (!isVisible || numberOfHiddenTransactionsAbove < 1) {
+  if (
+    !isVisible ||
+    (numberOfHiddenTransactionsAbove < 1 && numberOfHiddenErrorsAbove < 1)
+  ) {
     return null;
     return null;
   }
   }
 
 
+  const numOfTransaction = <strong>{numberOfHiddenTransactionsAbove}</strong>;
+  const numOfErrors = <strong>{numberOfHiddenErrorsAbove}</strong>;
+
+  const hiddenTransactionsMessage =
+    numberOfHiddenTransactionsAbove < 1
+      ? ''
+      : numberOfHiddenTransactionsAbove === 1
+      ? tct('[numOfTransaction] hidden transaction', {
+          numOfTransaction,
+        })
+      : tct('[numOfTransaction] hidden transactions', {
+          numOfTransaction,
+        });
+
+  const hiddenErrorsMessage =
+    numberOfHiddenErrorsAbove < 1
+      ? ''
+      : numberOfHiddenErrorsAbove === 1
+      ? tct('[numOfErrors] hidden error', {
+          numOfErrors,
+        })
+      : tct('[numOfErrors] hidden errors', {
+          numOfErrors,
+        });
+
   return (
   return (
     <MessageRow>
     <MessageRow>
       <span key="trace-info-message">
       <span key="trace-info-message">
-        {numberOfHiddenTransactionsAbove === 1
-          ? tct('[numOfTransaction] hidden transaction', {
-              numOfTransaction: <strong>{numberOfHiddenTransactionsAbove}</strong>,
-            })
-          : tct('[numOfTransaction] hidden transactions', {
-              numOfTransaction: <strong>{numberOfHiddenTransactionsAbove}</strong>,
-            })}
+        {hiddenTransactionsMessage}
+        {hiddenErrorsMessage && hiddenTransactionsMessage && ', '}
+        {hiddenErrorsMessage}
       </span>
       </span>
     </MessageRow>
     </MessageRow>
   );
   );
@@ -193,6 +219,7 @@ export default function TraceView({
           <TraceHiddenMessage
           <TraceHiddenMessage
             isVisible={isVisible}
             isVisible={isVisible}
             numberOfHiddenTransactionsAbove={numberOfHiddenTransactionsAbove}
             numberOfHiddenTransactionsAbove={numberOfHiddenTransactionsAbove}
+            numberOfHiddenErrorsAbove={0}
           />
           />
           <TransactionGroup
           <TransactionGroup
             location={location}
             location={location}
@@ -283,33 +310,53 @@ export default function TraceView({
   );
   );
 
 
   // Build transaction groups for orphan errors
   // Build transaction groups for orphan errors
+  let numOfHiddenErrorsAbove = 0;
+  let totalNumOfHiddenErrors = 0;
   if (hasOrphanErrors) {
   if (hasOrphanErrors) {
     orphanErrors.forEach((error, index) => {
     orphanErrors.forEach((error, index) => {
       const isLastError = index === orphanErrors.length - 1;
       const isLastError = index === orphanErrors.length - 1;
+      const isVisible = isRowVisible(error, filteredEventIds);
+      const currentHiddenCount = numOfHiddenErrorsAbove;
+
+      if (!isVisible) {
+        numOfHiddenErrorsAbove += 1;
+        totalNumOfHiddenErrors += 1;
+      } else {
+        numOfHiddenErrorsAbove = 0;
+      }
+
       transactionGroups.push(
       transactionGroups.push(
-        <TransactionGroup
-          key={error.event_id}
-          location={location}
-          organization={organization}
-          traceInfo={traceInfo}
-          transaction={{
-            ...error,
-            generation: 1,
-          }}
-          generateBounds={generateBounds(traceInfo)}
-          measurements={
-            traces && traces.length > 0
-              ? getMeasurements(traces[0], generateBounds(traceInfo))
-              : undefined
-          }
-          continuingDepths={[]}
-          isOrphan
-          isLast={isLastError}
-          index={lastIndex + index + 1}
-          isVisible={isRowVisible(error, filteredEventIds)}
-          hasGuideAnchor
-          renderedChildren={[]}
-        />
+        <React.Fragment key={error.event_id}>
+          <TraceHiddenMessage
+            isVisible={isVisible}
+            numberOfHiddenTransactionsAbove={
+              index === 0 ? numberOfHiddenTransactionsAbove : 0
+            }
+            numberOfHiddenErrorsAbove={index > 0 ? currentHiddenCount : 0}
+          />
+          <TransactionGroup
+            location={location}
+            organization={organization}
+            traceInfo={traceInfo}
+            transaction={{
+              ...error,
+              generation: 1,
+            }}
+            generateBounds={generateBounds(traceInfo)}
+            measurements={
+              traces && traces.length > 0
+                ? getMeasurements(traces[0], generateBounds(traceInfo))
+                : undefined
+            }
+            continuingDepths={[]}
+            isOrphan
+            isLast={isLastError}
+            index={lastIndex + index + 1}
+            isVisible={isVisible}
+            hasGuideAnchor
+            renderedChildren={[]}
+          />
+        </React.Fragment>
       );
       );
     });
     });
   }
   }
@@ -399,6 +446,7 @@ export default function TraceView({
                   <TraceHiddenMessage
                   <TraceHiddenMessage
                     isVisible
                     isVisible
                     numberOfHiddenTransactionsAbove={numberOfHiddenTransactionsAbove}
                     numberOfHiddenTransactionsAbove={numberOfHiddenTransactionsAbove}
+                    numberOfHiddenErrorsAbove={totalNumOfHiddenErrors}
                   />
                   />
                   <LimitExceededMessage
                   <LimitExceededMessage
                     traceInfo={traceInfo}
                     traceInfo={traceInfo}
@@ -420,8 +468,9 @@ export default function TraceView({
   return traceView;
   return traceView;
 }
 }
 
 
-const StyledTracePanel = styled(TracePanel)`
-  overflow: visible;
+export const StyledTracePanel = styled(Panel)`
+  height: 100%;
+  overflow-x: visible;
 
 
   ${TraceViewContainer} {
   ${TraceViewContainer} {
     overflow-x: visible;
     overflow-x: visible;

+ 27 - 4
static/app/views/performance/traceDetails/utils.tsx

@@ -1,9 +1,14 @@
 import {LocationDescriptor, Query} from 'history';
 import {LocationDescriptor, Query} from 'history';
 
 
 import {PAGE_URL_PARAM} from 'sentry/constants/pageFilters';
 import {PAGE_URL_PARAM} from 'sentry/constants/pageFilters';
-import {OrganizationSummary} from 'sentry/types';
-import {TraceError, TraceFullDetailed} from 'sentry/utils/performance/quickTrace/types';
-import {reduceTrace} from 'sentry/utils/performance/quickTrace/utils';
+import {Organization, OrganizationSummary} from 'sentry/types';
+import {
+  TraceError,
+  TraceFull,
+  TraceFullDetailed,
+  TraceSplitResults,
+} from 'sentry/utils/performance/quickTrace/types';
+import {isTraceSplitResult, reduceTrace} from 'sentry/utils/performance/quickTrace/utils';
 
 
 import {TraceInfo} from './types';
 import {TraceInfo} from './types';
 
 
@@ -50,7 +55,7 @@ function transactionVisitor() {
 }
 }
 
 
 export function hasTraceData(
 export function hasTraceData(
-  traces: TraceFullDetailed[] | null,
+  traces: TraceFullDetailed[] | null | undefined,
   orphanErrors: TraceError[] | undefined
   orphanErrors: TraceError[] | undefined
 ): boolean {
 ): boolean {
   return Boolean(
   return Boolean(
@@ -58,6 +63,24 @@ export function hasTraceData(
   );
   );
 }
 }
 
 
+export function getTraceSplitResults<U extends TraceFullDetailed | TraceFull>(
+  trace: TraceSplitResults<U> | U[],
+  organization: Organization
+) {
+  let transactions: U[] | undefined;
+  let orphanErrors: TraceError[] | undefined;
+  if (
+    trace &&
+    organization.features.includes('performance-tracing-without-performance') &&
+    isTraceSplitResult<TraceSplitResults<U>, U[]>(trace)
+  ) {
+    orphanErrors = trace.orphan_errors;
+    transactions = trace.transactions;
+  }
+
+  return {transactions, orphanErrors};
+}
+
 export function getTraceInfo(
 export function getTraceInfo(
   traces: TraceFullDetailed[] = [],
   traces: TraceFullDetailed[] = [],
   orphanErrors: TraceError[] = []
   orphanErrors: TraceError[] = []

+ 32 - 13
static/app/views/replays/detail/trace/replayTransactionContext.tsx

@@ -15,13 +15,18 @@ import type {TableData} from 'sentry/utils/discover/discoverQuery';
 import EventView from 'sentry/utils/discover/eventView';
 import EventView from 'sentry/utils/discover/eventView';
 import {doDiscoverQuery} from 'sentry/utils/discover/genericDiscoverQuery';
 import {doDiscoverQuery} from 'sentry/utils/discover/genericDiscoverQuery';
 import parseLinkHeader, {ParsedHeader} from 'sentry/utils/parseLinkHeader';
 import parseLinkHeader, {ParsedHeader} from 'sentry/utils/parseLinkHeader';
-import {TraceFullDetailed} from 'sentry/utils/performance/quickTrace/types';
+import {
+  TraceError,
+  TraceFullDetailed,
+  TraceSplitResults,
+} from 'sentry/utils/performance/quickTrace/types';
 import {
 import {
   getTraceRequestPayload,
   getTraceRequestPayload,
   makeEventView,
   makeEventView,
 } from 'sentry/utils/performance/quickTrace/utils';
 } from 'sentry/utils/performance/quickTrace/utils';
 import useApi from 'sentry/utils/useApi';
 import useApi from 'sentry/utils/useApi';
 import useOrganization from 'sentry/utils/useOrganization';
 import useOrganization from 'sentry/utils/useOrganization';
+import {getTraceSplitResults} from 'sentry/views/performance/traceDetails/utils';
 import type {ReplayRecord} from 'sentry/views/replays/types';
 import type {ReplayRecord} from 'sentry/views/replays/types';
 
 
 type Options = {
 type Options = {
@@ -38,6 +43,7 @@ type InternalState = {
   indexError: undefined | Error;
   indexError: undefined | Error;
   isFetching: boolean;
   isFetching: boolean;
   traces: undefined | TraceFullDetailed[];
   traces: undefined | TraceFullDetailed[];
+  orphanErrors?: TraceError[];
 };
 };
 
 
 type ExternalState = {
 type ExternalState = {
@@ -45,6 +51,7 @@ type ExternalState = {
   errors: Error[];
   errors: Error[];
   isFetching: boolean;
   isFetching: boolean;
   traces: undefined | TraceFullDetailed[];
   traces: undefined | TraceFullDetailed[];
+  orphanErrors?: TraceError[];
 };
 };
 
 
 const INITIAL_STATE: InternalState = {
 const INITIAL_STATE: InternalState = {
@@ -56,6 +63,7 @@ const INITIAL_STATE: InternalState = {
   indexError: undefined,
   indexError: undefined,
   isFetching: false,
   isFetching: false,
   traces: undefined,
   traces: undefined,
+  orphanErrors: undefined,
 };
 };
 
 
 type TxnContextProps = {
 type TxnContextProps = {
@@ -111,19 +119,28 @@ function ReplayTransactionContext({children, replayRecord}: Options) {
   const fetchSingleTraceData = useCallback(
   const fetchSingleTraceData = useCallback(
     async traceId => {
     async traceId => {
       try {
       try {
-        const [trace, , _traceResp] = await doDiscoverQuery(
-          api,
-          `/organizations/${orgSlug}/events-trace/${traceId}/`,
-          singleTracePayload
+        const [trace, _traceResp] = await doDiscoverQuery<
+          TraceSplitResults<TraceFullDetailed> | TraceFullDetailed[]
+        >(api, `/organizations/${orgSlug}/events-trace/${traceId}/`, singleTracePayload);
+
+        const {transactions, orphanErrors} = getTraceSplitResults<TraceFullDetailed>(
+          trace,
+          organization
         );
         );
 
 
-        setState(prev => ({
-          ...prev,
-          traces: sortBy(
-            (prev.traces || []).concat(trace as TraceFullDetailed),
-            'start_timestamp'
-          ),
-        }));
+        setState(prev => {
+          return {
+            ...prev,
+            traces: sortBy(
+              (prev.traces || []).concat(transactions ?? (trace as TraceFullDetailed[])),
+              'start_timestamp'
+            ),
+            orphanErrors: sortBy(
+              (prev.orphanErrors || []).concat(orphanErrors ?? []),
+              'timestamp'
+            ),
+          };
+        });
       } catch (error) {
       } catch (error) {
         setState(prev => ({
         setState(prev => ({
           ...prev,
           ...prev,
@@ -131,7 +148,7 @@ function ReplayTransactionContext({children, replayRecord}: Options) {
         }));
         }));
       }
       }
     },
     },
-    [api, orgSlug, singleTracePayload]
+    [api, orgSlug, singleTracePayload, organization]
   );
   );
 
 
   const fetchTransactionData = useCallback(async () => {
   const fetchTransactionData = useCallback(async () => {
@@ -229,6 +246,7 @@ function internalToExternalState({
   indexComplete,
   indexComplete,
   indexError,
   indexError,
   traces,
   traces,
+  orphanErrors,
 }: InternalState): ExternalState {
 }: InternalState): ExternalState {
   const isComplete = indexComplete && detailsRequests === detailsResponses;
   const isComplete = indexComplete && detailsRequests === detailsResponses;
 
 
@@ -237,6 +255,7 @@ function internalToExternalState({
     errors: indexError ? [indexError] : [], // Ignoring detailsErrors for now
     errors: indexError ? [indexError] : [], // Ignoring detailsErrors for now
     isFetching: !isComplete,
     isFetching: !isComplete,
     traces,
     traces,
+    orphanErrors,
   };
   };
 }
 }
 
 

+ 22 - 7
static/app/views/replays/detail/trace/trace.tsx

@@ -6,12 +6,15 @@ import {IconSad} from 'sentry/icons';
 import {t} from 'sentry/locale';
 import {t} from 'sentry/locale';
 import {Organization} from 'sentry/types';
 import {Organization} from 'sentry/types';
 import EventView from 'sentry/utils/discover/eventView';
 import EventView from 'sentry/utils/discover/eventView';
-import {TraceFullDetailed} from 'sentry/utils/performance/quickTrace/types';
+import {TraceError, TraceFullDetailed} from 'sentry/utils/performance/quickTrace/types';
 import useRouteAnalyticsParams from 'sentry/utils/routeAnalytics/useRouteAnalyticsParams';
 import useRouteAnalyticsParams from 'sentry/utils/routeAnalytics/useRouteAnalyticsParams';
 import {useLocation} from 'sentry/utils/useLocation';
 import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 import useOrganization from 'sentry/utils/useOrganization';
 import useProjects from 'sentry/utils/useProjects';
 import useProjects from 'sentry/utils/useProjects';
-import TraceView from 'sentry/views/performance/traceDetails/traceView';
+import TraceView, {
+  StyledTracePanel,
+} from 'sentry/views/performance/traceDetails/traceView';
+import {hasTraceData} from 'sentry/views/performance/traceDetails/utils';
 import EmptyState from 'sentry/views/replays/detail/emptyState';
 import EmptyState from 'sentry/views/replays/detail/emptyState';
 import FluidHeight from 'sentry/views/replays/detail/layout/fluidHeight';
 import FluidHeight from 'sentry/views/replays/detail/layout/fluidHeight';
 import {
 import {
@@ -38,11 +41,13 @@ function TraceFound({
   performanceActive,
   performanceActive,
   eventView,
   eventView,
   traces,
   traces,
+  orphanErrors,
 }: {
 }: {
   eventView: EventView | null;
   eventView: EventView | null;
   organization: Organization;
   organization: Organization;
   performanceActive: boolean;
   performanceActive: boolean;
   traces: TraceFullDetailed[] | null;
   traces: TraceFullDetailed[] | null;
+  orphanErrors?: TraceError[];
 }) {
 }) {
   const location = useLocation();
   const location = useLocation();
 
 
@@ -50,7 +55,7 @@ function TraceFound({
   useRouteAnalyticsParams(performanceActive ? {trace_status: 'success'} : {});
   useRouteAnalyticsParams(performanceActive ? {trace_status: 'success'} : {});
 
 
   return (
   return (
-    <FluidHeight>
+    <OverflowScrollBorderedSection>
       <TraceView
       <TraceView
         meta={null}
         meta={null}
         traces={traces || []}
         traces={traces || []}
@@ -58,8 +63,9 @@ function TraceFound({
         organization={organization}
         organization={organization}
         traceEventView={eventView!}
         traceEventView={eventView!}
         traceSlug="Replay"
         traceSlug="Replay"
+        orphanErrors={orphanErrors}
       />
       />
-    </FluidHeight>
+    </OverflowScrollBorderedSection>
   );
   );
 }
 }
 
 
@@ -71,7 +77,7 @@ function Trace({replayRecord}: Props) {
   const organization = useOrganization();
   const organization = useOrganization();
   const {projects} = useProjects();
   const {projects} = useProjects();
   const {
   const {
-    state: {didInit, errors, isFetching, traces},
+    state: {didInit, errors, isFetching, traces, orphanErrors},
     eventView,
     eventView,
   } = useTransactionData();
   } = useTransactionData();
 
 
@@ -103,7 +109,7 @@ function Trace({replayRecord}: Props) {
   const performanceActive =
   const performanceActive =
     organization.features.includes('performance-view') && hasPerformance;
     organization.features.includes('performance-view') && hasPerformance;
 
 
-  if (!traces?.length) {
+  if (!hasTraceData(traces, orphanErrors)) {
     return <TracesNotFound performanceActive={performanceActive} />;
     return <TracesNotFound performanceActive={performanceActive} />;
   }
   }
 
 
@@ -112,7 +118,8 @@ function Trace({replayRecord}: Props) {
       performanceActive={performanceActive}
       performanceActive={performanceActive}
       organization={organization}
       organization={organization}
       eventView={eventView}
       eventView={eventView}
-      traces={traces}
+      traces={traces ?? []}
+      orphanErrors={orphanErrors}
     />
     />
   );
   );
 }
 }
@@ -129,4 +136,12 @@ const BorderedSection = styled(FluidHeight)`
   border-radius: ${p => p.theme.borderRadius};
   border-radius: ${p => p.theme.borderRadius};
 `;
 `;
 
 
+const OverflowScrollBorderedSection = styled(BorderedSection)`
+  overflow: scroll;
+
+  ${StyledTracePanel} {
+    border: none;
+  }
+`;
+
 export default Trace;
 export default Trace;