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

feat(performance): added frontend option to display transaction summary page with interpolation and without zerofill (#27536)

Updated Performance Transaction Summary frontend to utilize withoutZerofill on main graph when feature flagged with performance-chart-interpolation
edwardgou-sentry 3 лет назад
Родитель
Сommit
f1d6e20b09

+ 3 - 0
static/app/actionCreators/events.tsx

@@ -30,6 +30,7 @@ type Options = {
   topEvents?: number;
   topEvents?: number;
   orderby?: string;
   orderby?: string;
   partial: boolean;
   partial: boolean;
+  withoutZerofill?: boolean;
 };
 };
 
 
 /**
 /**
@@ -65,6 +66,7 @@ export const doEventsRequest = (
     topEvents,
     topEvents,
     orderby,
     orderby,
     partial,
     partial,
+    withoutZerofill,
   }: Options
   }: Options
 ): Promise<EventsStats | MultiSeriesEventsStats> => {
 ): Promise<EventsStats | MultiSeriesEventsStats> => {
   const shouldDoublePeriod = canIncludePreviousPeriod(includePrevious, period);
   const shouldDoublePeriod = canIncludePreviousPeriod(includePrevious, period);
@@ -80,6 +82,7 @@ export const doEventsRequest = (
       topEvents,
       topEvents,
       orderby,
       orderby,
       partial: partial ? '1' : undefined,
       partial: partial ? '1' : undefined,
+      withoutZerofill: withoutZerofill ? '1' : undefined,
     }).filter(([, value]) => typeof value !== 'undefined')
     }).filter(([, value]) => typeof value !== 'undefined')
   );
   );
 
 

+ 14 - 0
static/app/components/charts/eventsRequest.tsx

@@ -26,6 +26,7 @@ export type TimeSeriesData = {
   originalPreviousTimeseriesData?: EventsStatsData | null;
   originalPreviousTimeseriesData?: EventsStatsData | null;
   previousTimeseriesData?: Series | null;
   previousTimeseriesData?: Series | null;
   timeAggregatedData?: Series | {};
   timeAggregatedData?: Series | {};
+  timeframe?: {start: string; end: string};
 };
 };
 
 
 type LoadingStatus = {
 type LoadingStatus = {
@@ -158,6 +159,10 @@ type EventsRequestPartialProps = {
    * Hide error toast (used for pages which also query eventsV2)
    * Hide error toast (used for pages which also query eventsV2)
    */
    */
   hideError?: boolean;
   hideError?: boolean;
+  /**
+   * Whether or not to zerofill results
+   */
+  withoutZerofill?: boolean;
 };
 };
 
 
 type TimeAggregationProps =
 type TimeAggregationProps =
@@ -395,9 +400,17 @@ class EventsRequest extends React.PureComponent<EventsRequestProps, EventsReques
       // Convert the timeseries data into a multi-series result set.
       // Convert the timeseries data into a multi-series result set.
       // As the server will have replied with a map like:
       // As the server will have replied with a map like:
       // {[titleString: string]: EventsStats}
       // {[titleString: string]: EventsStats}
+      let timeframe: {start: number; end: number} | undefined = undefined;
       const results: MultiSeriesResults = Object.keys(timeseriesData)
       const results: MultiSeriesResults = Object.keys(timeseriesData)
         .map((seriesName: string): [number, Series] => {
         .map((seriesName: string): [number, Series] => {
           const seriesData: EventsStats = timeseriesData[seriesName];
           const seriesData: EventsStats = timeseriesData[seriesName];
+          // Use the first timeframe we find from the series since all series have the same timeframe anyways
+          if (seriesData.start && seriesData.end && !timeframe) {
+            timeframe = {
+              start: seriesData.start * 1000,
+              end: seriesData.end * 1000,
+            };
+          }
           const transformed = this.transformTimeseriesData(
           const transformed = this.transformTimeseriesData(
             seriesData.data,
             seriesData.data,
             seriesName
             seriesName
@@ -412,6 +425,7 @@ class EventsRequest extends React.PureComponent<EventsRequestProps, EventsReques
         reloading,
         reloading,
         errored,
         errored,
         results,
         results,
+        timeframe,
         // sometimes we want to reference props that were given to EventsRequest
         // sometimes we want to reference props that were given to EventsRequest
         ...props,
         ...props,
       });
       });

+ 5 - 1
static/app/types/index.tsx

@@ -454,10 +454,14 @@ export type EventsStats = {
   data: EventsStatsData;
   data: EventsStatsData;
   totals?: {count: number};
   totals?: {count: number};
   order?: number;
   order?: number;
+  start?: number;
+  end?: number;
 };
 };
 
 
 // API response format for multiple series
 // API response format for multiple series
-export type MultiSeriesEventsStats = {[seriesName: string]: EventsStats};
+export type MultiSeriesEventsStats = {
+  [seriesName: string]: EventsStats;
+};
 
 
 /**
 /**
  * Avatars are a more primitive version of User.
  * Avatars are a more primitive version of User.

+ 12 - 1
static/app/views/performance/transactionSummary/charts.tsx

@@ -79,6 +79,7 @@ type Props = {
   eventView: EventView;
   eventView: EventView;
   totalValues: number | null;
   totalValues: number | null;
   currentFilter: SpanOperationBreakdownFilter;
   currentFilter: SpanOperationBreakdownFilter;
+  withoutZerofill: boolean;
 };
 };
 
 
 class TransactionSummaryCharts extends Component<Props> {
 class TransactionSummaryCharts extends Component<Props> {
@@ -110,7 +111,14 @@ class TransactionSummaryCharts extends Component<Props> {
   };
   };
 
 
   render() {
   render() {
-    const {totalValues, eventView, organization, location, currentFilter} = this.props;
+    const {
+      totalValues,
+      eventView,
+      organization,
+      location,
+      currentFilter,
+      withoutZerofill,
+    } = this.props;
 
 
     const TREND_PARAMETERS_OPTIONS: SelectValue<string>[] = getTrendsParameters({
     const TREND_PARAMETERS_OPTIONS: SelectValue<string>[] = getTrendsParameters({
       canSeeSpanOpTrends: organization.features.includes('performance-ops-breakdown'),
       canSeeSpanOpTrends: organization.features.includes('performance-ops-breakdown'),
@@ -176,6 +184,7 @@ class TransactionSummaryCharts extends Component<Props> {
               end={eventView.end}
               end={eventView.end}
               statsPeriod={eventView.statsPeriod}
               statsPeriod={eventView.statsPeriod}
               currentFilter={currentFilter}
               currentFilter={currentFilter}
+              withoutZerofill={withoutZerofill}
             />
             />
           )}
           )}
           {display === DisplayModes.DURATION_PERCENTILE && (
           {display === DisplayModes.DURATION_PERCENTILE && (
@@ -202,6 +211,7 @@ class TransactionSummaryCharts extends Component<Props> {
               start={eventView.start}
               start={eventView.start}
               end={eventView.end}
               end={eventView.end}
               statsPeriod={eventView.statsPeriod}
               statsPeriod={eventView.statsPeriod}
+              withoutZerofill={withoutZerofill}
             />
             />
           )}
           )}
           {display === DisplayModes.VITALS && (
           {display === DisplayModes.VITALS && (
@@ -214,6 +224,7 @@ class TransactionSummaryCharts extends Component<Props> {
               start={eventView.start}
               start={eventView.start}
               end={eventView.end}
               end={eventView.end}
               statsPeriod={eventView.statsPeriod}
               statsPeriod={eventView.statsPeriod}
+              withoutZerofill={withoutZerofill}
             />
             />
           )}
           )}
         </ChartContainer>
         </ChartContainer>

+ 4 - 0
static/app/views/performance/transactionSummary/content.tsx

@@ -220,6 +220,9 @@ class SummaryContent extends React.Component<Props, State> {
     const hasPerformanceEventsPage = organization.features.includes(
     const hasPerformanceEventsPage = organization.features.includes(
       'performance-events-page'
       'performance-events-page'
     );
     );
+    const hasPerformanceChartInterpolation = organization.features.includes(
+      'performance-chart-interpolation'
+    );
 
 
     const {incompatibleAlertNotice} = this.state;
     const {incompatibleAlertNotice} = this.state;
     const query = decodeScalar(location.query.query, '');
     const query = decodeScalar(location.query.query, '');
@@ -354,6 +357,7 @@ class SummaryContent extends React.Component<Props, State> {
               eventView={eventView}
               eventView={eventView}
               totalValues={totalCount}
               totalValues={totalCount}
               currentFilter={spanOperationBreakdownFilter}
               currentFilter={spanOperationBreakdownFilter}
+              withoutZerofill={hasPerformanceChartInterpolation}
             />
             />
             <TransactionsList
             <TransactionsList
               location={location}
               location={location}

+ 34 - 24
static/app/views/performance/transactionSummary/durationChart.tsx

@@ -48,6 +48,7 @@ type Props = ReactRouter.WithRouterProps &
     organization: OrganizationSummary;
     organization: OrganizationSummary;
     queryExtra: Query;
     queryExtra: Query;
     currentFilter: SpanOperationBreakdownFilter;
     currentFilter: SpanOperationBreakdownFilter;
+    withoutZerofill: boolean;
   };
   };
 
 
 function generateYAxisValues() {
 function generateYAxisValues() {
@@ -87,6 +88,7 @@ class DurationChart extends Component<Props> {
       router,
       router,
       queryExtra,
       queryExtra,
       currentFilter,
       currentFilter,
+      withoutZerofill,
     } = this.props;
     } = this.props;
 
 
     const start = this.props.start ? getUtcToLocalDateObject(this.props.start) : null;
     const start = this.props.start ? getUtcToLocalDateObject(this.props.start) : null;
@@ -105,29 +107,6 @@ class DurationChart extends Component<Props> {
       period: statsPeriod,
       period: statsPeriod,
     };
     };
 
 
-    const chartOptions = {
-      grid: {
-        left: '10px',
-        right: '10px',
-        top: '40px',
-        bottom: '0px',
-      },
-      seriesOptions: {
-        showSymbol: false,
-      },
-      tooltip: {
-        trigger: 'axis' as const,
-        valueFormatter: tooltipFormatter,
-      },
-      yAxis: {
-        axisLabel: {
-          color: theme.chartLabel,
-          // p50() coerces the axis to be time based
-          formatter: (value: number) => axisLabelFormatter(value, 'p50()'),
-        },
-      },
-    };
-
     const headerTitle =
     const headerTitle =
       currentFilter === SpanOperationBreakdownFilter.None
       currentFilter === SpanOperationBreakdownFilter.None
         ? t('Duration Breakdown')
         ? t('Duration Breakdown')
@@ -169,8 +148,9 @@ class DurationChart extends Component<Props> {
               includePrevious={false}
               includePrevious={false}
               yAxis={generateYAxisValues()}
               yAxis={generateYAxisValues()}
               partial
               partial
+              withoutZerofill={withoutZerofill}
             >
             >
-              {({results, errored, loading, reloading}) => {
+              {({results, errored, loading, reloading, timeframe}) => {
                 if (errored) {
                 if (errored) {
                   return (
                   return (
                     <ErrorPanel>
                     <ErrorPanel>
@@ -178,6 +158,36 @@ class DurationChart extends Component<Props> {
                     </ErrorPanel>
                     </ErrorPanel>
                   );
                   );
                 }
                 }
+
+                const chartOptions = {
+                  grid: {
+                    left: '10px',
+                    right: '10px',
+                    top: '40px',
+                    bottom: '0px',
+                  },
+                  seriesOptions: {
+                    showSymbol: false,
+                  },
+                  tooltip: {
+                    trigger: 'axis' as const,
+                    valueFormatter: tooltipFormatter,
+                  },
+                  xAxis: timeframe
+                    ? {
+                        min: timeframe.start,
+                        max: timeframe.end,
+                      }
+                    : undefined,
+                  yAxis: {
+                    axisLabel: {
+                      color: theme.chartLabel,
+                      // p50() coerces the axis to be time based
+                      formatter: (value: number) => axisLabelFormatter(value, 'p50()'),
+                    },
+                  },
+                };
+
                 const colors =
                 const colors =
                   (results && theme.charts.getColorPalette(results.length - 2)) || [];
                   (results && theme.charts.getColorPalette(results.length - 2)) || [];
 
 

+ 34 - 25
static/app/views/performance/transactionSummary/trendChart.tsx

@@ -48,6 +48,7 @@ type Props = ReactRouter.WithRouterProps &
     organization: OrganizationSummary;
     organization: OrganizationSummary;
     queryExtra: Query;
     queryExtra: Query;
     trendDisplay: string;
     trendDisplay: string;
+    withoutZerofill: boolean;
   };
   };
 
 
 class TrendChart extends Component<Props> {
 class TrendChart extends Component<Props> {
@@ -79,6 +80,7 @@ class TrendChart extends Component<Props> {
       router,
       router,
       trendDisplay,
       trendDisplay,
       queryExtra,
       queryExtra,
+      withoutZerofill,
     } = this.props;
     } = this.props;
 
 
     const start = this.props.start ? getUtcToLocalDateObject(this.props.start) : null;
     const start = this.props.start ? getUtcToLocalDateObject(this.props.start) : null;
@@ -97,30 +99,6 @@ class TrendChart extends Component<Props> {
       period: statsPeriod,
       period: statsPeriod,
     };
     };
 
 
-    const chartOptions = {
-      grid: {
-        left: '10px',
-        right: '10px',
-        top: '40px',
-        bottom: '0px',
-      },
-      seriesOptions: {
-        showSymbol: false,
-      },
-      tooltip: {
-        trigger: 'axis' as const,
-        valueFormatter: value => tooltipFormatter(value, 'p50()'),
-      },
-      yAxis: {
-        min: 0,
-        axisLabel: {
-          color: theme.chartLabel,
-          // p50() coerces the axis to be time based
-          formatter: (value: number) => axisLabelFormatter(value, 'p50()'),
-        },
-      },
-    };
-
     return (
     return (
       <Fragment>
       <Fragment>
         <HeaderTitleLegend>
         <HeaderTitleLegend>
@@ -154,8 +132,9 @@ class TrendChart extends Component<Props> {
               yAxis={trendDisplay}
               yAxis={trendDisplay}
               currentSeriesName={trendDisplay}
               currentSeriesName={trendDisplay}
               partial
               partial
+              withoutZerofill={withoutZerofill}
             >
             >
-              {({errored, loading, reloading, timeseriesData}) => {
+              {({errored, loading, reloading, timeseriesData, timeframe}) => {
                 if (errored) {
                 if (errored) {
                   return (
                   return (
                     <ErrorPanel>
                     <ErrorPanel>
@@ -164,6 +143,36 @@ class TrendChart extends Component<Props> {
                   );
                   );
                 }
                 }
 
 
+                const chartOptions = {
+                  grid: {
+                    left: '10px',
+                    right: '10px',
+                    top: '40px',
+                    bottom: '0px',
+                  },
+                  seriesOptions: {
+                    showSymbol: false,
+                  },
+                  tooltip: {
+                    trigger: 'axis' as const,
+                    valueFormatter: value => tooltipFormatter(value, 'p50()'),
+                  },
+                  xAxis: timeframe
+                    ? {
+                        min: timeframe.start,
+                        max: timeframe.end,
+                      }
+                    : undefined,
+                  yAxis: {
+                    min: 0,
+                    axisLabel: {
+                      color: theme.chartLabel,
+                      // p50() coerces the axis to be time based
+                      formatter: (value: number) => axisLabelFormatter(value, 'p50()'),
+                    },
+                  },
+                };
+
                 const series = timeseriesData
                 const series = timeseriesData
                   ? timeseriesData
                   ? timeseriesData
                       .map(values => {
                       .map(values => {

+ 35 - 25
static/app/views/performance/transactionSummary/vitalsChart.tsx

@@ -47,6 +47,7 @@ type Props = ReactRouter.WithRouterProps &
     location: Location;
     location: Location;
     organization: OrganizationSummary;
     organization: OrganizationSummary;
     queryExtra: object;
     queryExtra: object;
+    withoutZerofill: boolean;
   };
   };
 
 
 const YAXIS_VALUES = [
 const YAXIS_VALUES = [
@@ -84,6 +85,7 @@ class VitalsChart extends Component<Props> {
       statsPeriod,
       statsPeriod,
       router,
       router,
       queryExtra,
       queryExtra,
+      withoutZerofill,
     } = this.props;
     } = this.props;
 
 
     const start = this.props.start ? getUtcToLocalDateObject(this.props.start) : null;
     const start = this.props.start ? getUtcToLocalDateObject(this.props.start) : null;
@@ -112,30 +114,6 @@ class VitalsChart extends Component<Props> {
       period: statsPeriod,
       period: statsPeriod,
     };
     };
 
 
-    const chartOptions = {
-      grid: {
-        left: '10px',
-        right: '10px',
-        top: '40px',
-        bottom: '0px',
-      },
-      seriesOptions: {
-        showSymbol: false,
-      },
-      tooltip: {
-        trigger: 'axis' as const,
-        valueFormatter: tooltipFormatter,
-      },
-      yAxis: {
-        axisLabel: {
-          color: theme.chartLabel,
-          // p75(measurements.fcp) coerces the axis to be time based
-          formatter: (value: number) =>
-            axisLabelFormatter(value, 'p75(measurements.fcp)'),
-        },
-      },
-    };
-
     return (
     return (
       <Fragment>
       <Fragment>
         <HeaderTitleLegend>
         <HeaderTitleLegend>
@@ -170,8 +148,9 @@ class VitalsChart extends Component<Props> {
               includePrevious={false}
               includePrevious={false}
               yAxis={YAXIS_VALUES}
               yAxis={YAXIS_VALUES}
               partial
               partial
+              withoutZerofill={withoutZerofill}
             >
             >
-              {({results, errored, loading, reloading}) => {
+              {({results, errored, loading, reloading, timeframe}) => {
                 if (errored) {
                 if (errored) {
                   return (
                   return (
                     <ErrorPanel>
                     <ErrorPanel>
@@ -179,6 +158,37 @@ class VitalsChart extends Component<Props> {
                     </ErrorPanel>
                     </ErrorPanel>
                   );
                   );
                 }
                 }
+
+                const chartOptions = {
+                  grid: {
+                    left: '10px',
+                    right: '10px',
+                    top: '40px',
+                    bottom: '0px',
+                  },
+                  seriesOptions: {
+                    showSymbol: false,
+                  },
+                  tooltip: {
+                    trigger: 'axis' as const,
+                    valueFormatter: tooltipFormatter,
+                  },
+                  xAxis: timeframe
+                    ? {
+                        min: timeframe.start,
+                        max: timeframe.end,
+                      }
+                    : undefined,
+                  yAxis: {
+                    axisLabel: {
+                      color: theme.chartLabel,
+                      // p75(measurements.fcp) coerces the axis to be time based
+                      formatter: (value: number) =>
+                        axisLabelFormatter(value, 'p75(measurements.fcp)'),
+                    },
+                  },
+                };
+
                 const colors =
                 const colors =
                   (results && theme.charts.getColorPalette(results.length - 2)) || [];
                   (results && theme.charts.getColorPalette(results.length - 2)) || [];
 
 

+ 31 - 0
tests/js/spec/components/charts/eventsRequest.spec.jsx

@@ -517,4 +517,35 @@ describe('EventsRequest', function () {
       );
       );
     });
     });
   });
   });
+
+  describe('timeframe', function () {
+    beforeEach(function () {
+      doEventsRequest.mockClear();
+    });
+
+    it('passes query timeframe start and end to the child if supplied by timeseriesData', async function () {
+      doEventsRequest.mockImplementation(() =>
+        Promise.resolve({
+          p95: {
+            data: [[new Date(), [COUNT_OBJ]]],
+            start: 1627402280,
+            end: 1627402398,
+          },
+        })
+      );
+      wrapper = mountWithTheme(<EventsRequest {...DEFAULTS}>{mock}</EventsRequest>);
+
+      await tick();
+      wrapper.update();
+
+      expect(mock).toHaveBeenLastCalledWith(
+        expect.objectContaining({
+          timeframe: {
+            start: 1627402280000,
+            end: 1627402398000,
+          },
+        })
+      );
+    });
+  });
 });
 });

+ 44 - 0
tests/js/spec/views/performance/transactionSummary/content.spec.tsx

@@ -188,4 +188,48 @@ describe('Transaction Summary Content', function () {
     expect(transactionListProps.generatePerformanceTransactionEventsView).toBeDefined();
     expect(transactionListProps.generatePerformanceTransactionEventsView).toBeDefined();
     expect(transactionListProps.handleOpenAllEventsClick).toBeDefined();
     expect(transactionListProps.handleOpenAllEventsClick).toBeDefined();
   });
   });
+
+  it('Renders TransactionSummaryCharts withoutZerofill when feature flagged', async function () {
+    // @ts-expect-error
+    const projects = [TestStubs.Project()];
+    const {
+      organization,
+      location,
+      eventView,
+      spanOperationBreakdownFilter,
+      transactionName,
+    } = initialize(projects, {}, [
+      'performance-events-page',
+      'performance-chart-interpolation',
+    ]);
+    // @ts-expect-error
+    const routerContext = TestStubs.routerContext([{organization}]);
+
+    const wrapper = mountWithTheme(
+      <SummaryContent
+        location={location}
+        organization={organization}
+        eventView={eventView}
+        transactionName={transactionName}
+        isLoading={false}
+        totalValues={null}
+        spanOperationBreakdownFilter={spanOperationBreakdownFilter}
+        error={null}
+        onChangeFilter={() => {}}
+      />,
+      routerContext
+    );
+
+    // @ts-expect-error
+    await tick();
+    wrapper.update();
+
+    expect(wrapper.find('TransactionSummaryCharts')).toHaveLength(1);
+
+    const transactionSummaryChartsProps = wrapper
+      .find('TransactionSummaryCharts')
+      .first()
+      .props();
+    expect(transactionSummaryChartsProps.withoutZerofill).toEqual(true);
+  });
 });
 });