Browse Source

feat(insights): add release markings to fullscreen insight charts (#75012)

let's try https://github.com/getsentry/sentry/pull/74656 again

releases on span samples look like:

![image](https://github.com/user-attachments/assets/6a418e01-976d-495c-ac21-2c109372c76e)
Kevin Liu 7 months ago
parent
commit
b7ffc211dc

+ 5 - 2
static/app/components/modals/insightChartModal.tsx

@@ -5,7 +5,7 @@ import styled from '@emotion/styled';
 
 import type {ModalRenderProps} from 'sentry/actionCreators/modal';
 import {space} from 'sentry/styles/space';
-import {ChartHeightContext} from 'sentry/views/insights/common/components/chart';
+import {ChartRenderingContext} from 'sentry/views/insights/common/components/chart';
 
 export type InsightChartModalOptions = {
   children: React.ReactNode;
@@ -21,7 +21,9 @@ export default function InsightChartModal({Header, title, children}: Props) {
           <h3>{title}</h3>
         </Header>
 
-        <ChartHeightContext.Provider value={300}>{children}</ChartHeightContext.Provider>
+        <ChartRenderingContext.Provider value={{height: 300, isFullscreen: true}}>
+          {children}
+        </ChartRenderingContext.Provider>
       </Container>
     </Fragment>
   );
@@ -31,6 +33,7 @@ const Container = styled('div')<{height?: number | null}>`
   height: ${p => (p.height ? `${p.height}px` : 'auto')};
   position: relative;
   padding-bottom: ${space(3)};
+  z-index: 1000;
 `;
 
 export const modalCss = css`

+ 184 - 128
static/app/views/insights/common/components/chart.tsx

@@ -18,10 +18,11 @@ import type {AreaChartProps} from 'sentry/components/charts/areaChart';
 import {AreaChart} from 'sentry/components/charts/areaChart';
 import {BarChart} from 'sentry/components/charts/barChart';
 import BaseChart from 'sentry/components/charts/baseChart';
-import ChartZoom from 'sentry/components/charts/chartZoom';
+import ChartZoom, {type ZoomRenderProps} from 'sentry/components/charts/chartZoom';
 import type {FormatterOptions} from 'sentry/components/charts/components/tooltip';
 import {getFormatter} from 'sentry/components/charts/components/tooltip';
 import ErrorPanel from 'sentry/components/charts/errorPanel';
+import ReleaseSeries from 'sentry/components/charts/releaseSeries';
 import LineSeries from 'sentry/components/charts/series/lineSeries';
 import ScatterSeries from 'sentry/components/charts/series/scatterSeries';
 import TransitionChart from 'sentry/components/charts/transitionChart';
@@ -63,7 +64,12 @@ export enum ChartType {
   AREA = 2,
 }
 
-export const ChartHeightContext = createContext<number | undefined>(undefined);
+export interface ChartRenderingProps {
+  height: number;
+  isFullscreen: boolean;
+}
+
+export const ChartRenderingContext = createContext<ChartRenderingProps | null>(null);
 
 type Props = {
   data: Series[];
@@ -144,8 +150,12 @@ function Chart({
   const theme = useTheme();
   const pageFilters = usePageFilters();
   const {start, end, period, utc} = pageFilters.selection.datetime;
+  const {projects, environments} = pageFilters.selection;
+
+  const renderingContext = useContext(ChartRenderingContext);
 
-  const height = useContext(ChartHeightContext) ?? chartHeight;
+  const height = renderingContext?.height ?? chartHeight;
+  const isLegendVisible = renderingContext?.isFullscreen ?? showLegend;
 
   const defaultRef = useRef<ReactEchartsRef>(null);
   const chartRef = forwardedRef || defaultRef;
@@ -332,7 +342,7 @@ function Chart({
     grid,
     yAxes,
     utc,
-    legend: showLegend ? {top: 0, right: 10, formatter: legendFormatter} : undefined,
+    legend: isLegendVisible ? {top: 0, right: 10, formatter: legendFormatter} : undefined,
     isGroupedByDate: true,
     showTimeInTooltip: true,
     tooltip: {
@@ -355,6 +365,149 @@ function Chart({
     },
   } as Omit<AreaChartProps, 'series'>;
 
+  function getChartWithSeries(
+    zoomRenderProps: ZoomRenderProps,
+    releaseSeries?: Series[]
+  ) {
+    if (error) {
+      return (
+        <ErrorPanel height={`${height}px`} data-test-id="chart-error-panel">
+          <IconWarning color="gray300" size="lg" />
+        </ErrorPanel>
+      );
+    }
+
+    if (type === ChartType.LINE) {
+      return (
+        <BaseChart
+          {...zoomRenderProps}
+          ref={chartRef}
+          height={height}
+          previousPeriod={previousData}
+          additionalSeries={transformedThroughput}
+          xAxis={xAxis}
+          yAxes={areaChartProps.yAxes}
+          tooltip={areaChartProps.tooltip}
+          colors={colors}
+          grid={grid}
+          legend={
+            isLegendVisible ? {top: 0, right: 10, formatter: legendFormatter} : undefined
+          }
+          onClick={onClick}
+          onMouseOut={onMouseOut}
+          onMouseOver={onMouseOver}
+          onHighlight={onHighlight}
+          series={[
+            ...series.map(({seriesName, data: seriesData, ...options}) =>
+              LineSeries({
+                ...options,
+                name: seriesName,
+                data: seriesData?.map(({value, name}) => [name, value]),
+                animation: false,
+                animationThreshold: 1,
+                animationDuration: 0,
+              })
+            ),
+            ...(scatterPlot ?? []).map(({seriesName, data: seriesData, ...options}) =>
+              ScatterSeries({
+                ...options,
+                name: seriesName,
+                data: seriesData?.map(({value, name}) => [name, value]),
+                animation: false,
+              })
+            ),
+            ...incompleteSeries.map(({seriesName, data: seriesData, ...options}) =>
+              LineSeries({
+                ...options,
+                name: seriesName,
+                data: seriesData?.map(({value, name}) => [name, value]),
+                animation: false,
+                animationThreshold: 1,
+                animationDuration: 0,
+              })
+            ),
+            ...(releaseSeries ?? []).map(({seriesName, data: seriesData, ...options}) =>
+              LineSeries({
+                ...options,
+                name: seriesName,
+                data: seriesData?.map(({value, name}) => [name, value]),
+                animation: false,
+                animationThreshold: 1,
+                animationDuration: 0,
+              })
+            ),
+          ]}
+        />
+      );
+    }
+
+    if (type === ChartType.BAR) {
+      return (
+        <BarChart
+          height={height}
+          series={series}
+          xAxis={{
+            type: 'category',
+            axisTick: {show: true},
+            truncate: Infinity, // Show axis labels
+            axisLabel: {
+              interval: 0, // Show _all_ axis labels
+            },
+          }}
+          yAxis={{
+            minInterval: durationUnit ?? getDurationUnit(data),
+            splitNumber: definedAxisTicks,
+            max: dataMax,
+            axisLabel: {
+              color: theme.chartLabel,
+              formatter(value: number) {
+                return axisLabelFormatter(
+                  value,
+                  aggregateOutputFormat ?? aggregateOutputType(data[0].seriesName),
+                  undefined,
+                  durationUnit ?? getDurationUnit(data),
+                  rateUnit
+                );
+              },
+            },
+          }}
+          tooltip={{
+            valueFormatter: (value, seriesName) => {
+              return tooltipFormatter(
+                value,
+                aggregateOutputFormat ??
+                  aggregateOutputType(data?.length ? data[0].seriesName : seriesName)
+              );
+            },
+          }}
+          colors={colors}
+          grid={grid}
+          legend={
+            isLegendVisible ? {top: 0, right: 10, formatter: legendFormatter} : undefined
+          }
+          onClick={onClick}
+        />
+      );
+    }
+
+    return (
+      <AreaChart
+        forwardedRef={chartRef}
+        height={height}
+        {...zoomRenderProps}
+        series={[...series, ...incompleteSeries, ...(releaseSeries ?? [])]}
+        previousPeriod={previousData}
+        additionalSeries={transformedThroughput}
+        xAxis={xAxis}
+        stacked={stacked}
+        colors={colors}
+        onClick={onClick}
+        {...areaChartProps}
+        onLegendSelectChanged={onLegendSelectChanged}
+      />
+    );
+  }
+
   function getChart() {
     if (error) {
       return (
@@ -364,6 +517,13 @@ function Chart({
       );
     }
 
+    // add top-padding to the chart in full screen so that the legend
+    // and graph do not overlap
+    if (renderingContext?.isFullscreen) {
+      grid = {...grid, top: '20px'};
+    }
+
+    // overlay additional series data such as releases and issues on top of the original insights chart
     return (
       <ChartZoom
         router={router}
@@ -374,133 +534,29 @@ function Chart({
         utc={utc}
         onDataZoom={onDataZoom}
       >
-        {zoomRenderProps => {
-          if (type === ChartType.LINE) {
-            return (
-              <BaseChart
-                {...zoomRenderProps}
-                ref={chartRef}
-                height={height}
-                previousPeriod={previousData}
-                additionalSeries={transformedThroughput}
-                xAxis={xAxis}
-                yAxes={areaChartProps.yAxes}
-                tooltip={areaChartProps.tooltip}
-                colors={colors}
-                grid={grid}
-                legend={
-                  showLegend ? {top: 0, right: 10, formatter: legendFormatter} : undefined
-                }
-                onClick={onClick}
-                onMouseOut={onMouseOut}
-                onMouseOver={onMouseOver}
-                onHighlight={onHighlight}
-                series={[
-                  ...series.map(({seriesName, data: seriesData, ...options}) =>
-                    LineSeries({
-                      ...options,
-                      name: seriesName,
-                      data: seriesData?.map(({value, name}) => [name, value]),
-                      animation: false,
-                      animationThreshold: 1,
-                      animationDuration: 0,
-                    })
-                  ),
-                  ...(scatterPlot ?? []).map(
-                    ({seriesName, data: seriesData, ...options}) =>
-                      ScatterSeries({
-                        ...options,
-                        name: seriesName,
-                        data: seriesData?.map(({value, name}) => [name, value]),
-                        animation: false,
-                      })
-                  ),
-                  ...incompleteSeries.map(({seriesName, data: seriesData, ...options}) =>
-                    LineSeries({
-                      ...options,
-                      name: seriesName,
-                      data: seriesData?.map(({value, name}) => [name, value]),
-                      animation: false,
-                      animationThreshold: 1,
-                      animationDuration: 0,
-                    })
-                  ),
-                ]}
-              />
-            );
-          }
-
-          if (type === ChartType.BAR) {
-            return (
-              <BarChart
-                height={height}
-                series={series}
-                xAxis={{
-                  type: 'category',
-                  axisTick: {show: true},
-                  truncate: Infinity, // Show axis labels
-                  axisLabel: {
-                    interval: 0, // Show _all_ axis labels
-                  },
-                }}
-                yAxis={{
-                  minInterval: durationUnit ?? getDurationUnit(data),
-                  splitNumber: definedAxisTicks,
-                  max: dataMax,
-                  axisLabel: {
-                    color: theme.chartLabel,
-                    formatter(value: number) {
-                      return axisLabelFormatter(
-                        value,
-                        aggregateOutputFormat ?? aggregateOutputType(data[0].seriesName),
-                        undefined,
-                        durationUnit ?? getDurationUnit(data),
-                        rateUnit
-                      );
-                    },
-                  },
-                }}
-                tooltip={{
-                  valueFormatter: (value, seriesName) => {
-                    return tooltipFormatter(
-                      value,
-                      aggregateOutputFormat ??
-                        aggregateOutputType(
-                          data?.length ? data[0].seriesName : seriesName
-                        )
-                    );
-                  },
-                }}
-                colors={colors}
-                grid={grid}
-                legend={
-                  showLegend ? {top: 0, right: 10, formatter: legendFormatter} : undefined
-                }
-                onClick={onClick}
-              />
-            );
-          }
-
-          return (
-            <AreaChart
-              forwardedRef={chartRef}
-              height={height}
-              {...zoomRenderProps}
-              series={[...series, ...incompleteSeries]}
-              previousPeriod={previousData}
-              additionalSeries={transformedThroughput}
-              xAxis={xAxis}
-              stacked={stacked}
-              colors={colors}
-              onClick={onClick}
-              {...areaChartProps}
-              onLegendSelectChanged={onLegendSelectChanged}
-            />
-          );
-        }}
+        {zoomRenderProps =>
+          renderingContext?.isFullscreen ? (
+            <ReleaseSeries
+              start={start}
+              end={end}
+              queryExtra={undefined}
+              period={period}
+              utc={utc}
+              projects={projects}
+              environments={environments}
+            >
+              {({releaseSeries}) => {
+                return getChartWithSeries(zoomRenderProps, releaseSeries);
+              }}
+            </ReleaseSeries>
+          ) : (
+            getChartWithSeries(zoomRenderProps)
+          )
+        }
       </ChartZoom>
     );
   }
+
   return (
     <TransitionChart
       loading={loading}

+ 3 - 0
static/app/views/insights/http/components/charts/durationChart.tsx

@@ -29,6 +29,9 @@ export function DurationChart({
 }: 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 ?? [])];