Browse Source

ref(insights): Split up `DurationChart` component in HTTP insights (#82497)

This is a temporary clarifying measure. Every module has a
`components/durationChart.tsx` right now, and they all look largely the
same. The only exception is the one inside the `http` module. That one
is different because it can also plot samples! It's used by the HTTP
module and the Queues module.

While I work on consolidating the chart widgets, I'm going to rename
this component to `DurationChartWithSamples`, and add a `DurationChart`
that's a simple version. This will allow me to swap out all the charts
on the landing pages in Insights without affecting the sample panes.
George Gritsouk 2 months ago
parent
commit
fbb540cbef

+ 2 - 44
static/app/views/insights/http/components/charts/durationChart.tsx

@@ -1,6 +1,4 @@
-import type {ComponentProps} from 'react';
-
-import type {EChartHighlightHandler, Series} from 'sentry/types/echarts';
+import type {Series} from 'sentry/types/echarts';
 import {AVG_COLOR} from 'sentry/views/insights/colors';
 import Chart, {ChartType} from 'sentry/views/insights/common/components/chart';
 import ChartPanel from 'sentry/views/insights/common/components/chartPanel';
@@ -11,47 +9,9 @@ interface Props {
   isLoading: boolean;
   series: Series[];
   error?: Error | null;
-  onHighlight?: (highlights: Highlight[], event: Event) => void; // TODO: Correctly type this
-  scatterPlot?: ComponentProps<typeof Chart>['scatterPlot'];
 }
 
-interface Highlight {
-  dataPoint: Series['data'][number];
-  series: Series[];
-}
-
-export function DurationChart({
-  series,
-  scatterPlot,
-  isLoading,
-  error,
-  onHighlight,
-}: Props) {
-  // TODO: This is duplicated from `DurationChart` in `SampleList`. Resolve the duplication
-  const handleChartHighlight: EChartHighlightHandler = function (event) {
-    // ignore mouse hovering over the chart legend
-    if (!event.batch) {
-      return;
-    }
-
-    // TODO: Gross hack. Even though `scatterPlot` is a separate prop, it's just an array of `Series` that gets appended to the main series. To find the point that was hovered, we re-construct the correct series order. It would have been cleaner to just pass the scatter plot as its own, single series
-    const allSeries = [...series, ...(scatterPlot ?? [])];
-
-    const highlightedDataPoints = event.batch.map(batch => {
-      let {seriesIndex} = batch;
-      const {dataIndex} = batch;
-      // TODO: More hacks. The Chart component partitions the data series into a complete and incomplete series. Wrap the series index to work around overflowing index.
-      seriesIndex = seriesIndex % allSeries.length;
-
-      const highlightedSeries = allSeries?.[seriesIndex];
-      const highlightedDataPoint = highlightedSeries?.data?.[dataIndex];
-
-      return {series: highlightedSeries, dataPoint: highlightedDataPoint};
-    });
-
-    onHighlight?.(highlightedDataPoints, event);
-  };
-
+export function DurationChart({series, isLoading, error}: Props) {
   return (
     <ChartPanel title={getDurationChartTitle('http')}>
       <Chart
@@ -63,8 +23,6 @@ export function DurationChart({
           bottom: '0',
         }}
         data={series}
-        onHighlight={handleChartHighlight}
-        scatterPlot={scatterPlot}
         loading={isLoading}
         error={error}
         chartColors={[AVG_COLOR]}

+ 76 - 0
static/app/views/insights/http/components/charts/durationChartWithSamples.tsx

@@ -0,0 +1,76 @@
+import type {ComponentProps} from 'react';
+
+import type {EChartHighlightHandler, Series} from 'sentry/types/echarts';
+import {AVG_COLOR} from 'sentry/views/insights/colors';
+import Chart, {ChartType} from 'sentry/views/insights/common/components/chart';
+import ChartPanel from 'sentry/views/insights/common/components/chartPanel';
+import {getDurationChartTitle} from 'sentry/views/insights/common/views/spans/types';
+import {CHART_HEIGHT} from 'sentry/views/insights/http/settings';
+
+interface Props {
+  isLoading: boolean;
+  series: Series[];
+  error?: Error | null;
+  onHighlight?: (highlights: Highlight[], event: Event) => void; // TODO: Correctly type this
+  scatterPlot?: ComponentProps<typeof Chart>['scatterPlot'];
+}
+
+interface Highlight {
+  dataPoint: Series['data'][number];
+  series: Series[];
+}
+
+export function DurationChartWithSamples({
+  series,
+  scatterPlot,
+  isLoading,
+  error,
+  onHighlight,
+}: Props) {
+  // TODO: This is duplicated from `DurationChart` in `SampleList`. Resolve the duplication
+  const handleChartHighlight: EChartHighlightHandler = function (event) {
+    // ignore mouse hovering over the chart legend
+    if (!event.batch) {
+      return;
+    }
+
+    // TODO: Gross hack. Even though `scatterPlot` is a separate prop, it's just an array of `Series` that gets appended to the main series. To find the point that was hovered, we re-construct the correct series order. It would have been cleaner to just pass the scatter plot as its own, single series
+    const allSeries = [...series, ...(scatterPlot ?? [])];
+
+    const highlightedDataPoints = event.batch.map(batch => {
+      let {seriesIndex} = batch;
+      const {dataIndex} = batch;
+      // TODO: More hacks. The Chart component partitions the data series into a complete and incomplete series. Wrap the series index to work around overflowing index.
+      seriesIndex = seriesIndex % allSeries.length;
+
+      const highlightedSeries = allSeries?.[seriesIndex];
+      const highlightedDataPoint = highlightedSeries?.data?.[dataIndex];
+
+      return {series: highlightedSeries, dataPoint: highlightedDataPoint};
+    });
+
+    onHighlight?.(highlightedDataPoints, event);
+  };
+
+  return (
+    <ChartPanel title={getDurationChartTitle('http')}>
+      <Chart
+        height={CHART_HEIGHT}
+        grid={{
+          left: '0',
+          right: '0',
+          top: '8px',
+          bottom: '0',
+        }}
+        data={series}
+        onHighlight={handleChartHighlight}
+        scatterPlot={scatterPlot}
+        loading={isLoading}
+        error={error}
+        chartColors={[AVG_COLOR]}
+        type={ChartType.LINE}
+        aggregateOutputFormat="duration"
+      />
+    </ChartPanel>
+  );
+}

+ 2 - 2
static/app/views/insights/http/components/httpSamplesPanel.tsx

@@ -44,7 +44,7 @@ import {
   getThroughputTitle,
 } from 'sentry/views/insights/common/views/spans/types';
 import {useSampleScatterPlotSeries} from 'sentry/views/insights/common/views/spanSummaryPage/sampleList/durationChart/useSampleScatterPlotSeries';
-import {DurationChart} from 'sentry/views/insights/http/components/charts/durationChart';
+import {DurationChartWithSamples} from 'sentry/views/insights/http/components/charts/durationChartWithSamples';
 import {ResponseCodeCountChart} from 'sentry/views/insights/http/components/charts/responseCodeCountChart';
 import {SpanSamplesTable} from 'sentry/views/insights/http/components/tables/spanSamplesTable';
 import {HTTP_RESPONSE_STATUS_CODES} from 'sentry/views/insights/http/data/definitions';
@@ -430,7 +430,7 @@ export function HTTPSamplesPanel() {
           {query.panel === 'duration' && (
             <Fragment>
               <ModuleLayout.Full>
-                <DurationChart
+                <DurationChartWithSamples
                   series={[
                     {
                       ...durationData[`avg(span.self_time)`],

+ 2 - 2
static/app/views/insights/queues/components/messageSpanSamplesPanel.tsx

@@ -28,7 +28,7 @@ import {ReadoutRibbon} from 'sentry/views/insights/common/components/ribbon';
 import {useSpanMetricsSeries} from 'sentry/views/insights/common/queries/useDiscoverSeries';
 import {AverageValueMarkLine} from 'sentry/views/insights/common/utils/averageValueMarkLine';
 import {useSampleScatterPlotSeries} from 'sentry/views/insights/common/views/spanSummaryPage/sampleList/durationChart/useSampleScatterPlotSeries';
-import {DurationChart} from 'sentry/views/insights/http/components/charts/durationChart';
+import {DurationChartWithSamples} from 'sentry/views/insights/http/components/charts/durationChartWithSamples';
 import {useSpanSamples} from 'sentry/views/insights/http/queries/useSpanSamples';
 import {useDebouncedState} from 'sentry/views/insights/http/utils/useDebouncedState';
 import {useDomainViewFilters} from 'sentry/views/insights/pages/useFilters';
@@ -328,7 +328,7 @@ export function MessageSpanSamplesPanel() {
           </ModuleLayout.Full>
 
           <ModuleLayout.Full>
-            <DurationChart
+            <DurationChartWithSamples
               series={[
                 {
                   ...durationData[`avg(span.duration)`],