fix(discover): Correct display when unfurling top events charts (#29537)

Some top events charts (like equations, some aggregations)
should unfurl line charts to mimic discover.

src/sentry/charts/types.py

@@ -14,5 +14,6 @@ class ChartType(Enum):
     SLACK_DISCOVER_TOTAL_PERIOD = "slack:discover.totalPeriod"
     SLACK_DISCOVER_TOTAL_DAILY = "slack:discover.totalDaily"
     SLACK_DISCOVER_TOP5_PERIOD = "slack:discover.top5Period"
+    SLACK_DISCOVER_TOP5_PERIOD_LINE = "slack:discover.top5PeriodLine"
     SLACK_DISCOVER_TOP5_DAILY = "slack:discover.top5Daily"
     SLACK_DISCOVER_PREVIOUS_PERIOD = "slack:discover.previousPeriod"

src/sentry/integrations/slack/unfurl/discover.py

@@ -25,10 +25,29 @@ display_modes: Mapping[str, ChartType] = {
     "default": ChartType.SLACK_DISCOVER_TOTAL_PERIOD,
     "daily": ChartType.SLACK_DISCOVER_TOTAL_DAILY,
     "top5": ChartType.SLACK_DISCOVER_TOP5_PERIOD,
+    "top5line": ChartType.SLACK_DISCOVER_TOP5_PERIOD_LINE,
     "dailytop5": ChartType.SLACK_DISCOVER_TOP5_DAILY,
     "previous": ChartType.SLACK_DISCOVER_PREVIOUS_PERIOD,
 }
 
+# All `multiPlotType: line` fields in /static/app/utils/discover/fields.tsx
+line_plot_fields = {
+    "count_unique",
+    "failure_count",
+    "min",
+    "max",
+    "p50",
+    "p75",
+    "p95",
+    "p99",
+    "p100",
+    "percentile",
+    "avg",
+    "apdex",
+    "user_misery",
+    "failure_rate",
+}
+
 TOP_N = 5
 MAX_PERIOD_DAYS_INCLUDE_PREVIOUS = 45
 DEFAULT_PERIOD = "14d"
@@ -48,6 +67,13 @@ def get_double_period(period: str) -> str:
     return f"{value * 2}{unit}"
 
 
+def get_top5_display_mode(field: str) -> str:
+    if is_equation(field):
+        return "top5line"
+
+    return "top5line" if field.split("(")[0] in line_plot_fields else "top5"
+
+
 def is_aggregate(field: str) -> bool:
     field_match = re.match(AGGREGATE_PATTERN, field)
     if field_match:
@@ -141,6 +167,10 @@ def unfurl_discover(
                 )
             else:
                 params.setlist("topEvents", [f"{TOP_N}"])
+
+            y_axis = params.getlist("yAxis")[0]
+            display_mode = get_top5_display_mode(y_axis)
+
         else:
             # topEvents param persists in the URL in some cases, we want to discard
             # it if it's not a top n display type.

src/sentry/web/frontend/debug/debug_chart_renderer.py

@@ -281,6 +281,8 @@ class DebugChartRendererView(View):
         charts.append(generate_chart(ChartType.SLACK_DISCOVER_TOTAL_DAILY, discover_empty))
         charts.append(generate_chart(ChartType.SLACK_DISCOVER_TOP5_PERIOD, discover_top5))
         charts.append(generate_chart(ChartType.SLACK_DISCOVER_TOP5_PERIOD, discover_empty))
+        charts.append(generate_chart(ChartType.SLACK_DISCOVER_TOP5_PERIOD_LINE, discover_top5))
+        charts.append(generate_chart(ChartType.SLACK_DISCOVER_TOP5_PERIOD_LINE, discover_empty))
         charts.append(generate_chart(ChartType.SLACK_DISCOVER_TOP5_DAILY, discover_top5))
         charts.append(generate_chart(ChartType.SLACK_DISCOVER_TOP5_DAILY, discover_empty))
         charts.append(

static/app/chartcuterie/discover.tsx

@@ -200,6 +200,62 @@ discoverCharts.push({
   ...slackChartSize,
 });
 
+discoverCharts.push({
+  key: ChartType.SLACK_DISCOVER_TOP5_PERIOD_LINE,
+  getOption: (
+    data: {stats: Record<string, EventsStats>} | {seriesName?: string; stats: EventsStats}
+  ) => {
+    if (isArray(data.stats.data)) {
+      const color = theme.charts.getColorPalette(data.stats.data.length - 2);
+
+      const lineSeries = LineSeries({
+        data: data.stats.data.map(([timestamp, countsForTimestamp]) => [
+          timestamp * 1000,
+          countsForTimestamp.reduce((acc, {count}) => acc + count, 0),
+        ]),
+        lineStyle: {color: color?.[0], opacity: 1},
+        itemStyle: {color: color?.[0]},
+      });
+
+      return {
+        ...slackChartDefaults,
+        useUTC: true,
+        color,
+        series: [lineSeries],
+      };
+    }
+
+    const stats = Object.values(data.stats);
+    const hasOther = Object.keys(data.stats).includes('Other');
+    const color = theme.charts.getColorPalette(stats.length - 2 - (hasOther ? 1 : 0));
+    if (hasOther) {
+      color.push(theme.chartOther);
+    }
+
+    const series = stats
+      .sort((a, b) => (a.order ?? 0) - (b.order ?? 0))
+      .map((topSeries, i) =>
+        LineSeries({
+          data: topSeries.data.map(([timestamp, countsForTimestamp]) => [
+            timestamp * 1000,
+            countsForTimestamp.reduce((acc, {count}) => acc + count, 0),
+          ]),
+          lineStyle: {color: color?.[i], opacity: 1},
+          itemStyle: {color: color?.[i]},
+        })
+      );
+
+    return {
+      ...slackChartDefaults,
+      xAxis: discoverxAxis,
+      useUTC: true,
+      color,
+      series,
+    };
+  },
+  ...slackChartSize,
+});
+
 discoverCharts.push({
   key: ChartType.SLACK_DISCOVER_TOP5_DAILY,
   getOption: (

static/app/chartcuterie/types.tsx

@@ -11,6 +11,7 @@ export enum ChartType {
   SLACK_DISCOVER_TOTAL_PERIOD = 'slack:discover.totalPeriod',
   SLACK_DISCOVER_TOTAL_DAILY = 'slack:discover.totalDaily',
   SLACK_DISCOVER_TOP5_PERIOD = 'slack:discover.top5Period',
+  SLACK_DISCOVER_TOP5_PERIOD_LINE = 'slack:discover.top5PeriodLine',
   SLACK_DISCOVER_TOP5_DAILY = 'slack:discover.top5Daily',
   SLACK_DISCOVER_PREVIOUS_PERIOD = 'slack:discover.previousPeriod',
 }

static/app/utils/discover/fields.tsx

@@ -402,7 +402,7 @@ export const AGGREGATIONS = {
     ],
     outputType: 'number',
     isSortable: true,
-    multiPlotType: 'area',
+    multiPlotType: 'line',
   },
   failure_rate: {
     parameters: [],

tests/sentry/integrations/slack/test_unfurl.py

@@ -292,7 +292,8 @@ class UnfurlTest(TestCase):
         )
         assert len(mock_generate_chart.mock_calls) == 1
 
-        assert mock_generate_chart.call_args[0][0] == ChartType.SLACK_DISCOVER_TOP5_PERIOD
+        # Line chart expected since yAxis is count_unique(user)
+        assert mock_generate_chart.call_args[0][0] == ChartType.SLACK_DISCOVER_TOP5_PERIOD_LINE
         chart_data = mock_generate_chart.call_args[0][1]
         assert chart_data["seriesName"] == "count_unique(user)"
         # 2 + 1 cause of Other