Skip to content

Commit

Permalink
ref(feedback): move trace section to below replay and add section tit…
Browse files Browse the repository at this point in the history
…le/icon (#77668)

Ref #68481. Also refactors
TraceDataSection to its own file.

After:
![Screenshot 2024-09-17 at 3 58
31 PM](https://github.com/user-attachments/assets/1c0cbb21-8f35-46c2-82d6-ed3c485997ae)

Before:
![Screenshot 2024-09-17 at 3 59
24 PM](https://github.com/user-attachments/assets/669e8ffd-3cce-4fb7-8f2e-2482c2e93565)
  • Loading branch information
aliu39 authored and harshithadurai committed Sep 19, 2024
1 parent e340aaf commit 057bf8f
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 64 deletions.
73 changes: 9 additions & 64 deletions static/app/components/feedback/feedbackItem/feedbackItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import Section from 'sentry/components/feedback/feedbackItem/feedbackItemSection
import FeedbackReplay from 'sentry/components/feedback/feedbackItem/feedbackReplay';
import MessageSection from 'sentry/components/feedback/feedbackItem/messageSection';
import TagsSection from 'sentry/components/feedback/feedbackItem/tagsSection';
import TraceDataSection from 'sentry/components/feedback/feedbackItem/traceDataSection';
import PanelItem from 'sentry/components/panels/panelItem';
import QuestionTooltip from 'sentry/components/questionTooltip';
import TextCopyInput from 'sentry/components/textCopyInput';
Expand All @@ -19,11 +20,8 @@ import {t} from 'sentry/locale';
import {space} from 'sentry/styles/space';
import type {Event} from 'sentry/types/event';
import type {Group} from 'sentry/types/group';
import {trackAnalytics} from 'sentry/utils/analytics';
import type {FeedbackIssue} from 'sentry/utils/feedback/types';
import useOrganization from 'sentry/utils/useOrganization';
import {TraceDataSection as IssueDetailsTraceDataSection} from 'sentry/views/issueDetails/traceDataSection';
import {useTraceTimelineEvents} from 'sentry/views/issueDetails/traceTimeline/useTraceTimelineEvents';

interface Props {
eventData: Event | undefined;
Expand Down Expand Up @@ -62,14 +60,6 @@ export default function FeedbackItem({feedbackItem, eventData, tags}: Props) {
<MessageSection eventData={eventData} feedbackItem={feedbackItem} />
</Section>

{eventData ? (
<TraceDataSection
eventData={eventData}
crashReportId={crashReportId}
hasProject={!!feedbackItem.project}
/>
) : null}

{!crashReportId || (crashReportId && url) ? (
<Section icon={<IconLink size="xs" />} title={t('URL')}>
<TextCopyInput
Expand Down Expand Up @@ -106,6 +96,14 @@ export default function FeedbackItem({feedbackItem, eventData, tags}: Props) {
organization={organization}
/>

{eventData ? (
<TraceDataSection
eventData={eventData}
crashReportId={crashReportId}
hasProject={!!feedbackItem.project}
/>
) : null}

<Section icon={<IconTag size="xs" />} title={t('Tags')}>
<TagsSection tags={tags} />
</Section>
Expand Down Expand Up @@ -133,59 +131,6 @@ export default function FeedbackItem({feedbackItem, eventData, tags}: Props) {
);
}

function TraceDataSection({
eventData,
crashReportId,
hasProject,
}: {
crashReportId: string | undefined;
eventData: Event;
hasProject: boolean;
}) {
// If there's a linked error from a crash report and only one other issue, showing both could be redundant.
// TODO: we could add a jest test .spec for this ^
const organization = useOrganization();
const {oneOtherIssueEvent, traceEvents, isLoading, isError} = useTraceTimelineEvents({
event: eventData,
});
const show =
!isLoading &&
!isError &&
traceEvents.length > 1 && // traceEvents include the current event.
(!hasProject || !crashReportId || oneOtherIssueEvent?.id === crashReportId);

useEffect(() => {
if (isError) {
trackAnalytics('feedback.trace-section.error', {organization});
} else if (!isLoading) {
if (traceEvents.length > 1) {
trackAnalytics('feedback.trace-section.loaded', {
numEvents: traceEvents.length - 1,
organization,
});
}
if (hasProject && !!crashReportId && oneOtherIssueEvent?.id === crashReportId) {
trackAnalytics('feedback.trace-section.crash-report-dup', {organization});
}
}
}, [
crashReportId,
hasProject,
isError,
isLoading,
oneOtherIssueEvent?.id,
organization,
traceEvents.length,
]);

// Note a timeline will only be shown for >1 same-trace issues.
return show && organization.features.includes('user-feedback-trace-section') ? (
<Section>
<IssueDetailsTraceDataSection event={eventData} />
</Section>
) : null;
}

// 0 padding-bottom because <ActivitySection> has space(2) built-in.
const OverflowPanelItem = styled(PanelItem)`
overflow: auto;
Expand Down
67 changes: 67 additions & 0 deletions static/app/components/feedback/feedbackItem/traceDataSection.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import {useEffect} from 'react';

import Section from 'sentry/components/feedback/feedbackItem/feedbackItemSection';
import {IconSpan} from 'sentry/icons';
import {t} from 'sentry/locale';
import type {Event} from 'sentry/types/event';
import {trackAnalytics} from 'sentry/utils/analytics';
import useOrganization from 'sentry/utils/useOrganization';
import {TraceDataSection as IssuesTraceDataSection} from 'sentry/views/issueDetails/traceDataSection';
import {useTraceTimelineEvents} from 'sentry/views/issueDetails/traceTimeline/useTraceTimelineEvents';

/**
* Doesn't require a Section wrapper. Rendered conditionally if
* 1. there are 2 or more same-trace issues (timeline).
* 2. there is 1 same-trace issue, different from crashReportId (issue link).
*/
export default function TraceDataSection({
eventData,
crashReportId,
hasProject,
}: {
crashReportId: string | undefined;
eventData: Event;
hasProject: boolean;
}) {
// If there's a linked error from a crash report and only one other issue, showing both could be redundant.
// TODO: we could add a jest test .spec for this ^
const organization = useOrganization();
const {oneOtherIssueEvent, traceEvents, isLoading, isError} = useTraceTimelineEvents({
event: eventData,
});
const show =
!isLoading &&
!isError &&
traceEvents.length > 1 && // traceEvents include the current event.
(!hasProject || !crashReportId || oneOtherIssueEvent?.id === crashReportId);

useEffect(() => {
if (isError) {
trackAnalytics('feedback.trace-section.error', {organization});
} else if (!isLoading) {
if (traceEvents.length > 1) {
trackAnalytics('feedback.trace-section.loaded', {
numEvents: traceEvents.length - 1,
organization,
});
}
if (hasProject && !!crashReportId && oneOtherIssueEvent?.id === crashReportId) {
trackAnalytics('feedback.trace-section.crash-report-dup', {organization});
}
}
}, [
crashReportId,
hasProject,
isError,
isLoading,
oneOtherIssueEvent?.id,
organization,
traceEvents.length,
]);

return show && organization.features.includes('user-feedback-trace-section') ? (
<Section icon={<IconSpan size="xs" />} title={t('Data From The Same Trace')}>
<IssuesTraceDataSection event={eventData} />
</Section>
) : null;
}

0 comments on commit 057bf8f

Please sign in to comment.