Browse Source

Fully enable `reprocessing-v2` (frontend) and update logic showing "Reprocess Event" action (#68412)

This fully enables the feature by simply removing all the checks and
early returns for the feature flag.

The logic to show the "Reprocess Event" action was updated so that all
kinds of events that have debug files are eligible for reprocessing.
Arpad Borsos 9 months ago
parent
commit
4bdb82ef1b

+ 1 - 1
static/app/components/events/interfaces/debugMeta/debugImageDetails/index.tsx

@@ -257,7 +257,7 @@ export function DebugImageDetails({
   );
   const hasReprocessWarning =
     haveCandidatesUnappliedDebugFile &&
-    displayReprocessEventAction(organization.features, event) &&
+    displayReprocessEventAction(event) &&
     !!onReprocessEvent;
 
   if (isError) {

+ 0 - 2
static/app/components/modals/reprocessEventModal.spec.tsx

@@ -24,7 +24,6 @@ describe('ReprocessEventModal', function () {
       organization: {
         id: '4660',
         slug: 'org',
-        features: ['reprocessing-v2'],
       },
     });
 
@@ -73,7 +72,6 @@ describe('ReprocessEventModal', function () {
       organization: {
         id: '4660',
         slug: 'org',
-        features: ['reprocessing-v2'],
       },
     });
 

+ 9 - 16
static/app/utils/displayReprocessEventAction.spec.tsx

@@ -6,25 +6,18 @@ import {
 import {displayReprocessEventAction} from 'sentry/utils/displayReprocessEventAction';
 
 describe('DisplayReprocessEventAction', function () {
-  const orgFeatures = ['reprocessing-v2'];
-
-  it('returns false in case of no reprocessing-v2 feature', function () {
-    const event = EventStacktraceMessageFixture();
-    expect(displayReprocessEventAction([], event)).toBe(false);
-  });
-
   it('returns false in case of no event', function () {
-    expect(displayReprocessEventAction(orgFeatures)).toBe(false);
+    expect(displayReprocessEventAction()).toBe(false);
   });
 
   it('returns false if no exception entry is found', function () {
     const event = EventStacktraceMessageFixture();
-    expect(displayReprocessEventAction(orgFeatures, event)).toBe(false);
+    expect(displayReprocessEventAction(event)).toBe(false);
   });
 
   it('returns false if the event is not a mini-dump event or an Apple crash report event or a Native event', function () {
     const event = EventStacktraceExceptionFixture();
-    expect(displayReprocessEventAction(orgFeatures, event)).toBe(false);
+    expect(displayReprocessEventAction(event)).toBe(false);
   });
 
   describe('returns true', function () {
@@ -35,7 +28,7 @@ describe('DisplayReprocessEventAction', function () {
             platform: 'native',
           });
 
-          expect(displayReprocessEventAction(orgFeatures, event)).toBe(true);
+          expect(displayReprocessEventAction(event)).toBe(true);
         });
 
         it('cocoa', function () {
@@ -43,7 +36,7 @@ describe('DisplayReprocessEventAction', function () {
             platform: 'cocoa',
           });
 
-          expect(displayReprocessEventAction(orgFeatures, event)).toBe(true);
+          expect(displayReprocessEventAction(event)).toBe(true);
         });
       });
 
@@ -55,7 +48,7 @@ describe('DisplayReprocessEventAction', function () {
 
           event.entries[0].data.values[0].stacktrace.frames[0].platform = 'native';
 
-          expect(displayReprocessEventAction(orgFeatures, event)).toBe(true);
+          expect(displayReprocessEventAction(event)).toBe(true);
         });
 
         it('cocoa', function () {
@@ -65,7 +58,7 @@ describe('DisplayReprocessEventAction', function () {
 
           event.entries[0].data.values[0].stacktrace.frames[0].platform = 'cocoa';
 
-          expect(displayReprocessEventAction(orgFeatures, event)).toBe(true);
+          expect(displayReprocessEventAction(event)).toBe(true);
         });
       });
     });
@@ -82,7 +75,7 @@ describe('DisplayReprocessEventAction', function () {
         },
       };
 
-      expect(displayReprocessEventAction(orgFeatures, event)).toBe(true);
+      expect(displayReprocessEventAction(event)).toBe(true);
     });
 
     it('apple crash report event', function () {
@@ -97,7 +90,7 @@ describe('DisplayReprocessEventAction', function () {
         },
       };
 
-      expect(displayReprocessEventAction(orgFeatures, event)).toBe(true);
+      expect(displayReprocessEventAction(event)).toBe(true);
     });
   });
 });

+ 107 - 79
static/app/utils/displayReprocessEventAction.tsx

@@ -8,106 +8,134 @@ import type {
 } from 'sentry/types';
 import {EntryType} from 'sentry/types/event';
 
-const NATIVE_PLATFORMS: PlatformKey[] = ['cocoa', 'native'];
+/** All platforms that always use Debug Files. */
+const DEBUG_FILE_PLATFORMS: Set<PlatformKey> = new Set([
+  'objc',
+  'cocoa',
+  'swift',
+  'native',
+  'c',
+]);
+/** Other platforms that may use Debug Files. */
+const MAYBE_DEBUG_FILE_PLATFORMS: Set<PlatformKey> = new Set(['csharp', 'java']);
+
+/**
+ * Returns whether to display the "Reprocess Event" action.
+ *
+ * That is the case when we have a "reprocessable" event, which is an event that needs
+ * Debug Files for proper processing, as those Debug Files could have been uploaded *after*
+ * the Event was ingested.
+ */
+export function displayReprocessEventAction(event?: Event): boolean {
+  if (!event) {
+    return false;
+  }
 
-// Finds all frames in a given data blob and returns it's platforms
-function getPlatforms(exceptionValue: ExceptionValue | StacktraceType | null) {
-  const frames = exceptionValue?.frames ?? [];
-  const stacktraceFrames = (exceptionValue as ExceptionValue)?.stacktrace?.frames ?? [];
+  const eventPlatforms = getEventPlatform(event);
+  // Check Events from platforms that always use Debug Files as a fast-path
+  if (hasIntersection(eventPlatforms, DEBUG_FILE_PLATFORMS)) {
+    return true;
+  }
 
-  if (!frames.length && !stacktraceFrames.length) {
-    return [];
+  const hasDebugImages = (event?.entries ?? []).some(
+    entry => entry.type === EntryType.DEBUGMETA && entry.data.images.length > 0
+  );
+
+  // Otherwise, check alternative platforms if they actually have Debug Files
+  if (hasIntersection(eventPlatforms, MAYBE_DEBUG_FILE_PLATFORMS) && hasDebugImages) {
+    return true;
   }
 
-  return [...frames, ...stacktraceFrames]
-    .map(frame => frame.platform)
-    .filter(platform => !!platform);
-}
+  // Finally, fall back to checking the `platform` of each frame
+  const exceptionEntry = event.entries.find(
+    entry => entry.type === EntryType.EXCEPTION
+  ) as EntryException | undefined;
+
+  if (!exceptionEntry) {
+    return false;
+  }
 
-function getStackTracePlatforms(event: Event, exceptionEntry: EntryException) {
-  // Fetch platforms in stack traces of an exception entry
-  const exceptionEntryPlatforms = (exceptionEntry.data.values ?? []).flatMap(
-    getPlatforms
+  return hasIntersection(
+    getStackTracePlatforms(event, exceptionEntry),
+    DEBUG_FILE_PLATFORMS
   );
+}
+
+/**
+ * Returns whether the two Sets have intersecting elements.
+ */
+function hasIntersection<T>(set1: Set<T>, set2: Set<T>): boolean {
+  for (const v of set1) {
+    if (set2.has(v)) {
+      return true;
+    }
+  }
+  return false;
+}
+
+/**
+ * Returns the event platform as a Set.
+ */
+function getEventPlatform(event: Event): Set<PlatformKey> {
+  const platforms = new Set<PlatformKey>();
+  addPlatforms(platforms, [event]);
+  return platforms;
+}
+
+/**
+ * Returns a Set of all platforms found in the `event` and `exceptionEntry`.
+ */
+function getStackTracePlatforms(
+  event: Event,
+  exceptionEntry: EntryException
+): Set<PlatformKey> {
+  const platforms = new Set<PlatformKey>();
+
+  // Add platforms in stack traces of an exception entry
+  (exceptionEntry.data.values ?? []).forEach(exc => addFramePlatforms(platforms, exc));
 
-  // Fetch platforms in an exception entry
+  // Add platforms in a stack trace entry
   const stackTraceEntry = (event.entries.find(
     entry => entry.type === EntryType.STACKTRACE
   )?.data ?? {}) as StacktraceType;
 
-  // Fetch platforms in an exception entry
-  const stackTraceEntryPlatforms = Object.keys(stackTraceEntry).flatMap(key =>
-    getPlatforms(stackTraceEntry[key])
+  Object.keys(stackTraceEntry).forEach(key =>
+    addFramePlatforms(platforms, stackTraceEntry[key])
   );
 
-  // Fetch platforms in an thread entry
+  // Add platforms in a thread entry
   const threadEntry = (event.entries.find(entry => entry.type === EntryType.THREADS)?.data
     .values ?? []) as Array<Thread>;
 
-  // Fetch platforms in a thread entry
-  const threadEntryPlatforms = threadEntry.flatMap(({stacktrace}) =>
-    getPlatforms(stacktrace)
-  );
+  threadEntry.forEach(({stacktrace}) => addFramePlatforms(platforms, stacktrace));
 
-  return new Set([
-    ...exceptionEntryPlatforms,
-    ...stackTraceEntryPlatforms,
-    ...threadEntryPlatforms,
-  ]);
+  return platforms;
 }
 
-// Checks whether an event indicates that it is a native event.
-function isNativeEvent(event: Event, exceptionEntry: EntryException) {
-  const {platform} = event;
-
-  if (platform && NATIVE_PLATFORMS.includes(platform)) {
-    return true;
-  }
-
-  const stackTracePlatforms = getStackTracePlatforms(event, exceptionEntry);
-
-  return NATIVE_PLATFORMS.some(nativePlatform => stackTracePlatforms.has(nativePlatform));
-}
-
-//  Checks whether an event indicates that it has an associated minidump.
-function isMinidumpEvent(exceptionEntry: EntryException) {
-  const {data} = exceptionEntry;
-  return (data.values ?? []).some(value => value.mechanism?.type === 'minidump');
-}
+/**
+ * Adds all the platforms in the frames of `exceptionValue` to the `platforms` Set.
+ */
+function addFramePlatforms(
+  platforms: Set<PlatformKey>,
+  exceptionValue: ExceptionValue | StacktraceType | null
+) {
+  const frames = exceptionValue?.frames ?? [];
+  const stacktraceFrames = (exceptionValue as ExceptionValue)?.stacktrace?.frames ?? [];
 
-// Checks whether an event indicates that it has an apple crash report.
-function isAppleCrashReportEvent(exceptionEntry: EntryException) {
-  const {data} = exceptionEntry;
-  return (data.values ?? []).some(value => value.mechanism?.type === 'applecrashreport');
+  addPlatforms(platforms, frames);
+  addPlatforms(platforms, stacktraceFrames);
 }
 
-export function displayReprocessEventAction(orgFeatures: Array<string>, event?: Event) {
-  if (!event || !orgFeatures.includes('reprocessing-v2')) {
-    return false;
-  }
-
-  const {entries} = event;
-  const exceptionEntry = entries.find(entry => entry.type === EntryType.EXCEPTION) as
-    | EntryException
-    | undefined;
-
-  if (!exceptionEntry) {
-    return false;
+/**
+ * Adds all the `platform` properties found in `iter` to the `platforms` Set.
+ */
+function addPlatforms(
+  platforms: Set<PlatformKey>,
+  iter: Array<{platform?: PlatformKey | null}>
+) {
+  for (const o of iter) {
+    if (o.platform) {
+      platforms.add(o.platform);
+    }
   }
-
-  // We want to show the reprocessing button if the issue in question is native or contains native frames.
-  // The logic is taken from the symbolication pipeline in Python, where it is used to determine whether reprocessing
-  // payloads should be stored:
-  // https://github.com/getsentry/sentry/blob/cb7baef414890336881d67b7a8433ee47198c701/src/sentry/lang/native/processing.py#L425-L426
-  // It is still not ideal as one can always merge native and non-native events together into one issue,
-  // but it's the best approximation we have.
-  if (
-    !isMinidumpEvent(exceptionEntry) &&
-    !isAppleCrashReportEvent(exceptionEntry) &&
-    !isNativeEvent(event, exceptionEntry)
-  ) {
-    return false;
-  }
-
-  return true;
 }

+ 1 - 2
static/app/views/issueDetails/actions/index.spec.tsx

@@ -37,7 +37,6 @@ const group = GroupFixture({
 const organization = OrganizationFixture({
   id: '4660',
   slug: 'org',
-  features: ['reprocessing-v2'],
 });
 
 describe('GroupActions', function () {
@@ -132,7 +131,7 @@ describe('GroupActions', function () {
   });
 
   describe('reprocessing', function () {
-    it('renders ReprocessAction component if org has feature flag reprocessing-v2 and native exception event', async function () {
+    it('renders ReprocessAction component if org has native exception event', async function () {
       const event = EventStacktraceExceptionFixture({
         platform: 'native',
       });

+ 1 - 1
static/app/views/issueDetails/actions/index.tsx

@@ -412,7 +412,7 @@ export function Actions(props: Props) {
           {
             key: 'reprocess',
             label: t('Reprocess events'),
-            hidden: !displayReprocessEventAction(organization.features, event),
+            hidden: !displayReprocessEventAction(event),
             onAction: onReprocessEvent,
           },
           {

+ 30 - 41
static/app/views/issueDetails/groupDetails.tsx

@@ -161,24 +161,21 @@ function getReprocessingNewRoute({
   const {routes, params, location} = router;
   const {groupId} = params;
   const {currentTab, baseUrl} = getCurrentRouteInfo({group, event, organization, router});
-  const hasReprocessingV2Feature = organization.features?.includes('reprocessing-v2');
 
   const {id: nextGroupId} = group;
 
   const reprocessingStatus = getGroupReprocessingStatus(group);
 
   if (groupId !== nextGroupId) {
-    if (hasReprocessingV2Feature) {
-      // Redirects to the Activities tab
-      if (
-        reprocessingStatus === ReprocessingStatus.REPROCESSED_AND_HASNT_EVENT &&
-        currentTab !== Tab.ACTIVITY
-      ) {
-        return {
-          pathname: `${baseUrl}${Tab.ACTIVITY}/`,
-          query: {...params, groupId: nextGroupId},
-        };
-      }
+    // Redirects to the Activities tab
+    if (
+      reprocessingStatus === ReprocessingStatus.REPROCESSED_AND_HASNT_EVENT &&
+      currentTab !== Tab.ACTIVITY
+    ) {
+      return {
+        pathname: `${baseUrl}${Tab.ACTIVITY}/`,
+        query: {...params, groupId: nextGroupId},
+      };
     }
 
     return recreateRoute('', {
@@ -188,48 +185,40 @@ function getReprocessingNewRoute({
     });
   }
 
-  if (hasReprocessingV2Feature) {
-    if (
-      reprocessingStatus === ReprocessingStatus.REPROCESSING &&
-      currentTab !== Tab.DETAILS
-    ) {
-      return {
-        pathname: baseUrl,
-        query: params,
-      };
-    }
+  if (
+    reprocessingStatus === ReprocessingStatus.REPROCESSING &&
+    currentTab !== Tab.DETAILS
+  ) {
+    return {
+      pathname: baseUrl,
+      query: params,
+    };
+  }
 
-    if (
-      reprocessingStatus === ReprocessingStatus.REPROCESSED_AND_HASNT_EVENT &&
-      currentTab !== Tab.ACTIVITY &&
-      currentTab !== Tab.USER_FEEDBACK
-    ) {
-      return {
-        pathname: `${baseUrl}${Tab.ACTIVITY}/`,
-        query: params,
-      };
-    }
+  if (
+    reprocessingStatus === ReprocessingStatus.REPROCESSED_AND_HASNT_EVENT &&
+    currentTab !== Tab.ACTIVITY &&
+    currentTab !== Tab.USER_FEEDBACK
+  ) {
+    return {
+      pathname: `${baseUrl}${Tab.ACTIVITY}/`,
+      query: params,
+    };
   }
+
   return undefined;
 }
 
 function useRefetchGroupForReprocessing({
   refetchGroup,
 }: Pick<FetchGroupDetailsState, 'refetchGroup'>) {
-  const organization = useOrganization();
-  const hasReprocessingV2Feature = organization.features?.includes('reprocessing-v2');
-
   useEffect(() => {
-    let refetchInterval: number;
-
-    if (hasReprocessingV2Feature) {
-      refetchInterval = window.setInterval(refetchGroup, 30000);
-    }
+    const refetchInterval = window.setInterval(refetchGroup, 30000);
 
     return () => {
       window.clearInterval(refetchInterval);
     };
-  }, [hasReprocessingV2Feature, refetchGroup]);
+  }, [refetchGroup]);
 }
 
 function useEventApiQuery({

+ 1 - 4
static/app/views/issueDetails/groupEventDetails/groupEventDetails.tsx

@@ -70,8 +70,6 @@ function GroupEventDetails(props: GroupEventDetailsProps) {
     eventError,
     params,
   } = props;
-  // Reprocessing
-  const hasReprocessingV2Feature = organization.features?.includes('reprocessing-v2');
   const projectId = project.id;
   const environments = useEnvironmentsFromUrl();
   const prevEnvironment = usePrevious(environments);
@@ -174,8 +172,7 @@ function GroupEventDetails(props: GroupEventDetailsProps) {
         isLoading={loadingEvent}
       >
         <StyledLayoutBody data-test-id="group-event-details">
-          {hasReprocessingV2Feature &&
-          groupReprocessingStatus === ReprocessingStatus.REPROCESSING ? (
+          {groupReprocessingStatus === ReprocessingStatus.REPROCESSING ? (
             <ReprocessingProgress
               totalEvents={
                 (getGroupMostRecentActivity(group.activity) as GroupActivityReprocess)

+ 1 - 7
static/app/views/issueDetails/header.tsx

@@ -174,12 +174,6 @@ function GroupHeader({
   const location = useLocation();
 
   const disabledTabs = useMemo(() => {
-    const hasReprocessingV2Feature = organization.features.includes('reprocessing-v2');
-
-    if (!hasReprocessingV2Feature) {
-      return [];
-    }
-
     if (groupReprocessingStatus === ReprocessingStatus.REPROCESSING) {
       return [
         Tab.ACTIVITY,
@@ -205,7 +199,7 @@ function GroupHeader({
     }
 
     return [];
-  }, [organization, groupReprocessingStatus]);
+  }, [groupReprocessingStatus]);
 
   const eventRoute = useMemo(() => {
     const searchTermWithoutQuery = omit(location.query, 'query');

+ 1 - 7
static/app/views/issueList/overview.tsx

@@ -960,13 +960,7 @@ class IssueListOverview extends Component<Props, State> {
   };
 
   displayReprocessingTab() {
-    const {organization} = this.props;
-    const {queryCounts} = this.state;
-
-    return (
-      organization.features.includes('reprocessing-v2') &&
-      !!queryCounts?.[Query.REPROCESSING]?.count
-    );
+    return !!this.state.queryCounts?.[Query.REPROCESSING]?.count;
   }
 
   displayReprocessingLayout(showReprocessingTab: boolean, query: string) {

Some files were not shown because too many files changed in this diff