Browse Source

feat(perf): Replace HTTP module sample panel bar chart with timeseries (#68231)

Out with the old, in with the new. No more bar chart, instead there will
be a timeseries, and some controls to select what shows up in it. Also I
made some small tweaks.

- Clear mocks before running tests
- Update theme colours
- Show legend on response code chart
- Replace bar chart with time series
- Remove unused bar chart
George Gritsouk 11 months ago
parent
commit
23c00cc2e6

+ 2 - 0
static/app/views/performance/http/httpDomainSummaryPage.spec.tsx

@@ -46,6 +46,8 @@ describe('HTTPSummaryPage', function () {
   jest.mocked(useOrganization).mockReturnValue(organization);
 
   beforeEach(function () {
+    jest.clearAllMocks();
+
     domainTransactionsListRequestMock = MockApiClient.addMockResponse({
       url: `/organizations/${organization.slug}/events/`,
       method: 'GET',

+ 2 - 0
static/app/views/performance/http/httpLandingPage.spec.tsx

@@ -46,6 +46,8 @@ describe('HTTPLandingPage', function () {
   jest.mocked(useOrganization).mockReturnValue(organization);
 
   beforeEach(function () {
+    jest.clearAllMocks();
+
     spanListRequestMock = MockApiClient.addMockResponse({
       url: `/organizations/${organization.slug}/events/`,
       method: 'GET',

+ 24 - 8
static/app/views/performance/http/httpSamplesPanel.spec.tsx

@@ -114,15 +114,21 @@ describe('HTTPSamplesPanel', () => {
       });
 
       chartRequestMock = MockApiClient.addMockResponse({
-        url: `/organizations/${organization.slug}/events/`,
+        url: `/organizations/${organization.slug}/events-stats/`,
         method: 'GET',
-
         match: [
           MockApiClient.matchQuery({
-            referrer: 'api.starfish.http-module-samples-panel-response-bar-chart',
+            referrer: 'api.starfish.http-module-samples-panel-response-code-chart',
           }),
         ],
-        body: {},
+        body: {
+          'spm()': {
+            data: [
+              [1699907700, [{count: 7810.2}]],
+              [1699908000, [{count: 1216.8}]],
+            ],
+          },
+        },
       });
     });
 
@@ -158,20 +164,30 @@ describe('HTTPSamplesPanel', () => {
 
       expect(chartRequestMock).toHaveBeenNthCalledWith(
         1,
-        `/organizations/${organization.slug}/events/`,
+        `/organizations/${organization.slug}/events-stats/`,
         expect.objectContaining({
           method: 'GET',
           query: {
+            cursor: undefined,
             dataset: 'spansMetrics',
             environment: [],
-            field: ['span.status_code', 'count()'],
+            excludeOther: 0,
+            field: [],
+            interval: '30m',
+            orderby: undefined,
+            partial: 1,
             per_page: 50,
-            sort: 'span.status_code',
             project: [],
             query:
               'span.module:http span.domain:"\\*.sentry.dev" transaction:/api/0/users',
-            referrer: 'api.starfish.http-module-samples-panel-response-bar-chart',
+            referrer: 'api.starfish.http-module-samples-panel-response-code-chart',
             statsPeriod: '10d',
+            topEvents: undefined,
+            yAxis: [
+              'http_response_rate(3)',
+              'http_response_rate(4)',
+              'http_response_rate(5)',
+            ],
           },
         })
       );

+ 22 - 21
static/app/views/performance/http/httpSamplesPanel.tsx

@@ -7,7 +7,6 @@ import ProjectAvatar from 'sentry/components/avatar/projectAvatar';
 import {SegmentedControl} from 'sentry/components/segmentedControl';
 import {t} from 'sentry/locale';
 import {space} from 'sentry/styles/space';
-import type {Series} from 'sentry/types/echarts';
 import {DurationUnit, RateUnit} from 'sentry/utils/discover/fields';
 import {PageAlertProvider} from 'sentry/utils/performance/contexts/pageAlert';
 import {decodeScalar} from 'sentry/utils/queryString';
@@ -21,7 +20,7 @@ import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 import {AverageValueMarkLine} from 'sentry/views/performance/charts/averageValueMarkLine';
 import {DurationChart} from 'sentry/views/performance/http/durationChart';
 import decodePanel from 'sentry/views/performance/http/queryParameterDecoders/panel';
-import {ResponseCodeBarChart} from 'sentry/views/performance/http/responseCodeBarChart';
+import {ResponseRateChart} from 'sentry/views/performance/http/responseRateChart';
 import {SpanSamplesTable} from 'sentry/views/performance/http/spanSamplesTable';
 import {useSpanSamples} from 'sentry/views/performance/http/useSpanSamples';
 import {MetricReadout} from 'sentry/views/performance/metricReadout';
@@ -115,27 +114,16 @@ export function HTTPSamplesPanel() {
   });
 
   const {
-    data: responseCodeData,
     isFetching: isResponseCodeDataLoading,
-    error: responseCodeDataError,
-  } = useSpanMetrics({
+    data: responseCodeData,
+    error: responseCodeError,
+  } = useSpanMetricsSeries({
     search: MutableSearch.fromQueryObject(filters),
-    fields: ['span.status_code', 'count()'],
-    sorts: [{field: 'span.status_code', kind: 'asc'}],
+    yAxis: ['http_response_rate(3)', 'http_response_rate(4)', 'http_response_rate(5)'],
     enabled: isPanelOpen && query.panel === 'status',
-    referrer: 'api.starfish.http-module-samples-panel-response-bar-chart',
+    referrer: 'api.starfish.http-module-samples-panel-response-code-chart',
   });
 
-  const responseCodeBarChartSeries: Series = {
-    seriesName: 'span.status_code',
-    data: (responseCodeData ?? []).map(item => {
-      return {
-        name: item['span.status_code'] || t('N/A'),
-        value: item['count()'],
-      };
-    }),
-  };
-
   const durationAxisMax = computeAxisMax([durationData?.[`avg(span.self_time)`]]);
 
   const {
@@ -311,10 +299,23 @@ export function HTTPSamplesPanel() {
 
           {query.panel === 'status' && (
             <ModuleLayout.Full>
-              <ResponseCodeBarChart
-                series={responseCodeBarChartSeries}
+              <ResponseRateChart
+                series={[
+                  {
+                    ...responseCodeData[`http_response_rate(3)`],
+                    seriesName: t('3XX'),
+                  },
+                  {
+                    ...responseCodeData[`http_response_rate(4)`],
+                    seriesName: t('4XX'),
+                  },
+                  {
+                    ...responseCodeData[`http_response_rate(5)`],
+                    seriesName: t('5XX'),
+                  },
+                ]}
                 isLoading={isResponseCodeDataLoading}
-                error={responseCodeDataError}
+                error={responseCodeError}
               />
             </ModuleLayout.Full>
           )}

+ 0 - 35
static/app/views/performance/http/responseCodeBarChart.tsx

@@ -1,35 +0,0 @@
-import type {Series} from 'sentry/types/echarts';
-import {CHART_HEIGHT} from 'sentry/views/performance/database/settings';
-import {COUNT_COLOUR} from 'sentry/views/starfish/colours';
-import Chart, {ChartType} from 'sentry/views/starfish/components/chart';
-import ChartPanel from 'sentry/views/starfish/components/chartPanel';
-import {DataTitles} from 'sentry/views/starfish/views/spans/types';
-
-interface Props {
-  isLoading: boolean;
-  series: Series;
-  error?: Error | null;
-}
-
-export function ResponseCodeBarChart({series, isLoading, error}: Props) {
-  return (
-    <ChartPanel title={DataTitles.httpCodeBreakdown}>
-      <Chart
-        height={CHART_HEIGHT}
-        grid={{
-          left: '0',
-          right: '0',
-          top: '8px',
-          bottom: '0',
-        }}
-        aggregateOutputFormat="number"
-        data={[series]}
-        loading={isLoading}
-        error={error}
-        preserveIncompletePoints
-        chartColors={[COUNT_COLOUR]}
-        type={ChartType.BAR}
-      />
-    </ChartPanel>
-  );
-}

+ 1 - 0
static/app/views/performance/http/responseRateChart.tsx

@@ -20,6 +20,7 @@ export function ResponseRateChart({series, isLoading, error}: Props) {
   return (
     <ChartPanel title={DataTitles.unsuccessfulHTTPCodes}>
       <Chart
+        showLegend
         height={CHART_HEIGHT}
         grid={{
           left: '4px',

+ 4 - 3
static/app/views/starfish/colours.tsx

@@ -9,9 +9,10 @@ export const P95_COLOR = CHART_PALETTE[0][0];
 export const AVG_COLOR = CHART_PALETTE[0][0];
 export const ERRORS_COLOR = CHART_PALETTE[5][3];
 
-export const HTTP_RESPONSE_3XX_COLOR = CHART_PALETTE[2][0];
-export const HTTP_RESPONSE_4XX_COLOR = CHART_PALETTE[2][2];
-export const HTTP_RESPONSE_5XX_COLOR = CHART_PALETTE[2][1];
+// TODO: Synchronize with `theme.tsx` or `CHART_PALETTE`
+export const HTTP_RESPONSE_3XX_COLOR = '#F2B712';
+export const HTTP_RESPONSE_4XX_COLOR = '#F58C46';
+export const HTTP_RESPONSE_5XX_COLOR = '#E9626E';
 
 export const COLD_START_COLOR = '#3C74DD';
 export const WARM_START_COLOR = '#3A2D96';