Просмотр исходного кода

fix(performance): change errors to failure rate on performance change explorer (#54454)

Changed `errors` to `failure rate` so that we have a more accurate
comparison from before and after the breakpoint. Comparing number of
errors before and after was not always accurate if the breakpoint wasn't
in the middle. This way a rate is being compared to another rate.
<img width="693" alt="image"
src="https://github.com/getsentry/sentry/assets/72356613/acacca02-c877-4c6d-98dc-1c55451f48f1">
nikkikapadia 1 год назад
Родитель
Сommit
d051ef8c38

+ 3 - 0
static/app/views/performance/trends/changeExplorer.spec.tsx

@@ -304,6 +304,7 @@ describe('Performance > Trends > Performance Change Explorer', function () {
             'p50()': 47.34580982348902,
             'tps()': 3.7226926286168966,
             'count()': 345,
+            'failure_rate()': 0.23498234,
             'examples()': ['dkwj4w8sdjk', 'asdi389a8'],
           },
         ],
@@ -313,6 +314,7 @@ describe('Performance > Trends > Performance Change Explorer', function () {
             '950()': 'duration',
             'tps()': 'number',
             'count()': 'number',
+            'failure_rate()': 'number',
             'examples()': 'Array',
           },
           units: {
@@ -320,6 +322,7 @@ describe('Performance > Trends > Performance Change Explorer', function () {
             'p50()': 'millisecond',
             'tps()': null,
             'count()': null,
+            'failure_rate()': null,
             'examples()': null,
           },
           isMetricsData: true,

+ 28 - 57
static/app/views/performance/trends/changeExplorerUtils/metricsTable.tsx

@@ -41,7 +41,7 @@ type MetricsTableProps = {
   trendView: TrendView;
 };
 
-const fieldsNeeded: AggregationKeyWithAlias[] = ['tps', 'p50', 'p95'];
+const fieldsNeeded: AggregationKeyWithAlias[] = ['tps', 'p50', 'p95', 'failure_rate'];
 
 type MetricColumnKey = 'metric' | 'before' | 'after' | 'change';
 
@@ -135,35 +135,6 @@ export function MetricsTable(props: MetricsTableProps) {
     )
   );
 
-  const {data: beforeBreakpointErrors, isLoading: isLoadingBeforeErrors} =
-    useDiscoverQuery(
-      getQueryParams(
-        startTime,
-        breakpointTime,
-        ['count'],
-        'error',
-        DiscoverDatasets.DISCOVER,
-        organization,
-        trendView,
-        transaction.transaction,
-        location
-      )
-    );
-
-  const {data: afterBreakpointErrors, isLoading: isLoadingAfterErrors} = useDiscoverQuery(
-    getQueryParams(
-      breakpointTime,
-      endTime,
-      ['count'],
-      'error',
-      DiscoverDatasets.DISCOVER,
-      organization,
-      trendView,
-      transaction.transaction,
-      location
-    )
-  );
-
   const throughput: TableDataRow = getEventsRowData(
     'tps()',
     'Throughput',
@@ -198,21 +169,21 @@ export function MetricsTable(props: MetricsTableProps) {
       )
     : p95;
 
-  const errors: TableDataRow = getEventsRowData(
-    'count()',
-    'Errors',
-    '',
+  const failureRate: TableDataRow = getEventsRowData(
+    'failure_rate()',
+    'Failure Rate',
+    '%',
     0,
     true,
-    beforeBreakpointErrors,
-    afterBreakpointErrors
+    beforeBreakpoint,
+    afterBreakpoint
   );
 
   const columnOrder = MetricColumnOrder.map(column => COLUMNS[column]);
 
   return (
     <GridEditable
-      data={[throughput, p50Events, p95Events, errors]}
+      data={[throughput, p50Events, p95Events, failureRate]}
       columnOrder={columnOrder}
       columnSortBy={[]}
       grid={{
@@ -220,13 +191,7 @@ export function MetricsTable(props: MetricsTableProps) {
         renderBodyCell,
       }}
       location={location}
-      isLoading={
-        isLoadingBefore ||
-        isLoadingAfter ||
-        isLoading ||
-        isLoadingBeforeErrors ||
-        isLoadingAfterErrors
-      }
+      isLoading={isLoadingBefore || isLoadingAfter || isLoading}
     />
   );
 }
@@ -236,26 +201,32 @@ function getEventsRowData(
   rowTitle: string,
   suffix: string,
   nullValue: string | number,
-  wholeNumbers: boolean,
+  percentage: boolean,
   beforeData?: TableData,
   afterData?: TableData
 ): TableDataRow {
-  if (beforeData?.data[0][field] && afterData?.data[0][field]) {
+  if (
+    beforeData?.data[0][field] !== undefined &&
+    afterData?.data[0][field] !== undefined
+  ) {
     return {
       metric: rowTitle,
-      before: !wholeNumbers
+      before: !percentage
         ? toFormattedNumber(beforeData.data[0][field].toString(), 1) + ' ' + suffix
-        : beforeData.data[0][field],
-      after: !wholeNumbers
+        : formatPercentage(beforeData.data[0][field] as number, 1),
+      after: !percentage
         ? toFormattedNumber(afterData.data[0][field].toString(), 1) + ' ' + suffix
-        : afterData.data[0][field],
-      change: formatPercentage(
-        relativeChange(
-          beforeData.data[0][field] as number,
-          afterData.data[0][field] as number
-        ),
-        1
-      ),
+        : formatPercentage(afterData.data[0][field] as number, 1),
+      change:
+        beforeData.data[0][field] && afterData.data[0][field]
+          ? formatPercentage(
+              relativeChange(
+                beforeData.data[0][field] as number,
+                afterData.data[0][field] as number
+              ),
+              1
+            )
+          : '-',
     };
   }
   return {

+ 7 - 1
static/app/views/performance/trends/index.spec.tsx

@@ -261,6 +261,8 @@ describe('Performance > Trends', function () {
             'p95()': 1010.9232499999998,
             'p50()': 47.34580982348902,
             'tps()': 3.7226926286168966,
+            'count()': 34872349,
+            'failure_rate()': 0.43428379,
             'examples()': ['djk3w308er', '3298a9ui3h'],
           },
         ],
@@ -269,12 +271,16 @@ describe('Performance > Trends', function () {
             'p95()': 'duration',
             '950()': 'duration',
             'tps()': 'number',
+            'count()': 'number',
+            'failure_rate()': 'number',
             'examples()': 'Array',
           },
           units: {
             'p95()': 'millisecond',
             'p50()': 'millisecond',
             'tps()': null,
+            'count()': null,
+            'failure_rate()': null,
             'examples()': null,
           },
           isMetricsData: true,
@@ -378,7 +384,7 @@ describe('Performance > Trends', function () {
       expect(screen.getByText('Throughput')).toBeInTheDocument();
       expect(screen.getByText('P95')).toBeInTheDocument();
       expect(screen.getByText('P50')).toBeInTheDocument();
-      expect(screen.getByText('Errors')).toBeInTheDocument();
+      expect(screen.getByText('Failure Rate')).toBeInTheDocument();
     });
   });