Browse Source

feat(perf): Improve chart error handling in new modules (#65364)

A few improvements to module charts!

1. A small refactor. Changes our chart component to accept an `error`
object instead of booleans like `errored` or `isError`. This is a little
simpler, since it accepts all error-like objects, but also is more
powerful since we can, if we want, use the error info to render some
error text.
2. A small improvement. Adds error states to the charts in the Database
and HTTP modules
George Gritsouk 1 year ago
parent
commit
a60b989439

+ 3 - 1
static/app/utils/discover/genericDiscoverQuery.tsx

@@ -22,10 +22,12 @@ export interface DiscoverQueryExtras {
 interface _DiscoverQueryExtras {
   queryExtras?: DiscoverQueryExtras;
 }
-export class QueryError {
+export class QueryError extends Error {
   message: string;
   private originalError: any; // For debugging in case parseError picks a value that doesn't make sense.
   constructor(errorMessage: string, originalError?: any) {
+    super(errorMessage);
+
     this.message = errorMessage;
     this.originalError = originalError;
   }

+ 16 - 8
static/app/views/performance/database/databaseLandingPage.tsx

@@ -97,15 +97,21 @@ export function DatabaseLandingPage() {
     referrer: 'api.starfish.use-span-list',
   });
 
-  const {isLoading: isThroughputDataLoading, data: throughputData} = useSpanMetricsSeries(
-    {
-      filters: chartFilters,
-      yAxis: ['spm()'],
-      referrer: 'api.starfish.span-landing-page-metrics-chart',
-    }
-  );
+  const {
+    isLoading: isThroughputDataLoading,
+    data: throughputData,
+    error: throughputError,
+  } = useSpanMetricsSeries({
+    filters: chartFilters,
+    yAxis: ['spm()'],
+    referrer: 'api.starfish.span-landing-page-metrics-chart',
+  });
 
-  const {isLoading: isDurationDataLoading, data: durationData} = useSpanMetricsSeries({
+  const {
+    isLoading: isDurationDataLoading,
+    data: durationData,
+    error: durationError,
+  } = useSpanMetricsSeries({
     filters: chartFilters,
     yAxis: [`${selectedAggregate}(${SpanMetricsField.SPAN_SELF_TIME})`],
     referrer: 'api.starfish.span-landing-page-metrics-chart',
@@ -173,6 +179,7 @@ export function DatabaseLandingPage() {
                   <ThroughputChart
                     series={throughputData['spm()']}
                     isLoading={isThroughputDataLoading}
+                    error={throughputError}
                   />
                 </ModuleLayout.Half>
 
@@ -180,6 +187,7 @@ export function DatabaseLandingPage() {
                   <DurationChart
                     series={durationData[`${selectedAggregate}(span.self_time)`]}
                     isLoading={isDurationDataLoading}
+                    error={durationError}
                   />
                 </ModuleLayout.Half>
 

+ 16 - 8
static/app/views/performance/database/databaseSpanSummaryPage.tsx

@@ -93,15 +93,21 @@ function SpanSummaryPage({params}: Props) {
     [SpanMetricsField.SPAN_GROUP]: string;
   };
 
-  const {isLoading: isThroughputDataLoading, data: throughputData} = useSpanMetricsSeries(
-    {
-      filters,
-      yAxis: ['spm()'],
-      referrer: 'api.starfish.span-summary-page-metrics-chart',
-    }
-  );
+  const {
+    isLoading: isThroughputDataLoading,
+    data: throughputData,
+    error: throughputError,
+  } = useSpanMetricsSeries({
+    filters,
+    yAxis: ['spm()'],
+    referrer: 'api.starfish.span-summary-page-metrics-chart',
+  });
 
-  const {isLoading: isDurationDataLoading, data: durationData} = useSpanMetricsSeries({
+  const {
+    isLoading: isDurationDataLoading,
+    data: durationData,
+    error: durationError,
+  } = useSpanMetricsSeries({
     filters,
     yAxis: [`${selectedAggregate}(${SpanMetricsField.SPAN_SELF_TIME})`],
     referrer: 'api.starfish.span-summary-page-metrics-chart',
@@ -168,6 +174,7 @@ function SpanSummaryPage({params}: Props) {
             <ThroughputChart
               series={throughputData['spm()']}
               isLoading={isThroughputDataLoading}
+              error={throughputError}
             />
 
             <DurationChart
@@ -175,6 +182,7 @@ function SpanSummaryPage({params}: Props) {
                 durationData[`${selectedAggregate}(${SpanMetricsField.SPAN_SELF_TIME})`]
               }
               isLoading={isDurationDataLoading}
+              error={durationError}
             />
           </ChartContainer>
 

+ 3 - 1
static/app/views/performance/database/durationChart.tsx

@@ -8,9 +8,10 @@ import ChartPanel from 'sentry/views/starfish/components/chartPanel';
 interface Props {
   isLoading: boolean;
   series: Series;
+  error?: Error | null;
 }
 
-export function DurationChart({series, isLoading}: Props) {
+export function DurationChart({series, isLoading, error}: Props) {
   return (
     <ChartPanel title={<DurationAggregateSelector />}>
       <Chart
@@ -23,6 +24,7 @@ export function DurationChart({series, isLoading}: Props) {
         }}
         data={[series]}
         loading={isLoading}
+        error={error}
         chartColors={[AVG_COLOR]}
         isLineChart
       />

+ 1 - 0
static/app/views/performance/database/throughputChart.tsx

@@ -10,6 +10,7 @@ import {getThroughputChartTitle} from 'sentry/views/starfish/views/spans/types';
 interface Props {
   isLoading: boolean;
   series: Series;
+  error?: Error | null;
 }
 
 export function ThroughputChart({series, isLoading}: Props) {

+ 16 - 8
static/app/views/performance/http/httpLandingPage.tsx

@@ -48,15 +48,21 @@ export function HTTPLandingPage() {
 
   const cursor = decodeScalar(location.query?.[QueryParameterNames.DOMAINS_CURSOR]);
 
-  const {isLoading: isThroughputDataLoading, data: throughputData} = useSpanMetricsSeries(
-    {
-      filters: chartFilters,
-      yAxis: ['spm()'],
-      referrer: 'api.starfish.http-module-landing-throughput-chart',
-    }
-  );
+  const {
+    isLoading: isThroughputDataLoading,
+    data: throughputData,
+    error: throughputError,
+  } = useSpanMetricsSeries({
+    filters: chartFilters,
+    yAxis: ['spm()'],
+    referrer: 'api.starfish.http-module-landing-throughput-chart',
+  });
 
-  const {isLoading: isDurationDataLoading, data: durationData} = useSpanMetricsSeries({
+  const {
+    isLoading: isDurationDataLoading,
+    data: durationData,
+    error: durationError,
+  } = useSpanMetricsSeries({
     filters: chartFilters,
     yAxis: [`avg(span.self_time)`],
     referrer: 'api.starfish.http-module-landing-duration-chart',
@@ -124,6 +130,7 @@ export function HTTPLandingPage() {
                   <ThroughputChart
                     series={throughputData['spm()']}
                     isLoading={isThroughputDataLoading}
+                    error={throughputError}
                   />
                 </ModuleLayout.Half>
 
@@ -131,6 +138,7 @@ export function HTTPLandingPage() {
                   <DurationChart
                     series={durationData[`avg(span.self_time)`]}
                     isLoading={isDurationDataLoading}
+                    error={durationError}
                   />
                 </ModuleLayout.Half>
 

+ 3 - 1
static/app/views/performance/http/throughputChart.tsx

@@ -10,9 +10,10 @@ import {getThroughputChartTitle} from 'sentry/views/starfish/views/spans/types';
 interface Props {
   isLoading: boolean;
   series: Series;
+  error?: Error | null;
 }
 
-export function ThroughputChart({series, isLoading}: Props) {
+export function ThroughputChart({series, isLoading, error}: Props) {
   return (
     <ChartPanel title={getThroughputChartTitle('http')}>
       <Chart
@@ -25,6 +26,7 @@ export function ThroughputChart({series, isLoading}: Props) {
         }}
         data={[series]}
         loading={isLoading}
+        error={error}
         chartColors={[THROUGHPUT_COLOR]}
         isLineChart
         aggregateOutputFormat="rate"

+ 1 - 1
static/app/views/performance/landing/widgets/widgets/slowScreens.tsx

@@ -306,7 +306,7 @@ function SlowScreensByTTID(props: PerformanceWidgetProps) {
             valueFormatter: value =>
               tooltipFormatterUsingAggregateOutputType(value, OUTPUT_TYPE[YAxis.TTID]),
           }}
-          errored={provided.widgetData.chart.isErrored}
+          error={provided.widgetData.chart.error}
           disableXAxis
           showLegend={false}
         />

+ 13 - 0
static/app/views/starfish/components/chart.spec.tsx

@@ -0,0 +1,13 @@
+import {render, screen} from 'sentry-test/reactTestingLibrary';
+
+import Chart from 'sentry/views/starfish/components/chart';
+
+describe('Chart', function () {
+  test('it shows an error panel if an error prop is supplied', function () {
+    const parsingError = new Error('Could not parse chart data');
+
+    render(<Chart error={parsingError} data={[]} loading={false} />);
+
+    expect(screen.getByTestId('chart-error-panel')).toBeInTheDocument();
+  });
+});

+ 4 - 4
static/app/views/starfish/components/chart.tsx

@@ -81,7 +81,7 @@ type Props = {
   definedAxisTicks?: number;
   disableXAxis?: boolean;
   durationUnit?: number;
-  errored?: boolean;
+  error?: Error | null;
   forwardedRef?: RefObject<ReactEchartsRef>;
   grid?: AreaChartProps['grid'];
   height?: number;
@@ -180,7 +180,7 @@ function Chart({
   forwardedRef,
   chartGroup,
   tooltipFormatterOptions = {},
-  errored,
+  error,
   onLegendSelectChanged,
   onDataZoom,
   legendFormatter,
@@ -366,9 +366,9 @@ function Chart({
       };
 
   function getChart() {
-    if (errored) {
+    if (error) {
       return (
-        <ErrorPanel>
+        <ErrorPanel height={`${height}px`} data-test-id="chart-error-panel">
           <IconWarning color="gray300" size="lg" />
         </ErrorPanel>
       );

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