feat: Improve pie chart implementation (#1773)

Closes HDX-3479

# Summary

This PR makes a number of improvements to new pie chart implementation (#1704)

1. Pie charts are now limited to 1 series. Previously, the pie chart summed the values of each series by group, and used the sum as the slice value. This is non-obvious and probably not what users expect. With a one-series limit, this problem is eliminated. Further, the logic for formatting the pie chart data from the clickhouse response is dramatically simpler.
2. Slices are now ordered by value decreasing, to avoid randomly changing slice order on refresh
3. Instead of being randomly generated, slice colors are now consistent with the theme colors and auto-detect log and trace severity levels, matching line/bar chart behavior
4. The external dashboards API now supports reading and writing pie charts. The transformation code has been updated so that there will be a type error if any new chart types are added in the future without updating the external API code.
5. The pie chart's tooltip now matches the style of the line chart tooltip, and is updated appropriately based on the app theme and light/dark mode.
6. The chart's number format is now applied to values in the pie chart tooltip
7. Slice labels are now correctly populated when a map is accessed in the Group By (eg. when grouping by `ResourceAttributes['app']`, the slice labels include the `app` value instead of being empty).
8. Also, added some unit tests for the pie chart data transformation, and moved it to ChartUtils with the other similar chart data transformation code.

Author: pulpdrew

.changeset/twenty-hats-deny.md

@@ -0,0 +1,6 @@
+---
+"@hyperdx/api": patch
+"@hyperdx/app": patch
+---
+
+fix: Improve Pie Chart implemententation

packages/api/openapi.json

@@ -1178,6 +1178,47 @@
           }
         }
       },
+      "PieChartConfig": {
+        "type": "object",
+        "required": [
+          "displayType",
+          "sourceId",
+          "select"
+        ],
+        "description": "Configuration for a pie chart tile. Each slice represents one group value.",
+        "properties": {
+          "displayType": {
+            "type": "string",
+            "enum": [
+              "pie"
+            ],
+            "example": "pie"
+          },
+          "sourceId": {
+            "type": "string",
+            "description": "ID of the data source to query.",
+            "example": "65f5e4a3b9e77c001a111111"
+          },
+          "select": {
+            "type": "array",
+            "minItems": 1,
+            "maxItems": 1,
+            "description": "Exactly one aggregated value used to size each pie slice.",
+            "items": {
+              "$ref": "#/components/schemas/SelectItem"
+            }
+          },
+          "groupBy": {
+            "type": "string",
+            "maxLength": 10000,
+            "description": "Field expression to group results by (one slice per group value).",
+            "example": "service"
+          },
+          "numberFormat": {
+            "$ref": "#/components/schemas/NumberFormat"
+          }
+        }
+      },
       "SearchChartConfig": {
         "type": "object",
         "required": [
@@ -1255,6 +1296,9 @@
           {
             "$ref": "#/components/schemas/NumberChartConfig"
           },
+          {
+            "$ref": "#/components/schemas/PieChartConfig"
+          },
           {
             "$ref": "#/components/schemas/SearchChartConfig"
           },
@@ -1269,6 +1313,7 @@
             "stacked_bar": "#/components/schemas/BarChartConfig",
             "table": "#/components/schemas/TableChartConfig",
             "number": "#/components/schemas/NumberChartConfig",
+            "pie": "#/components/schemas/PieChartConfig",
             "search": "#/components/schemas/SearchChartConfig",
             "markdown": "#/components/schemas/MarkdownChartConfig"
           }

packages/api/src/routers/external-api/__tests__/dashboards.test.ts

@@ -1648,6 +1648,26 @@ describe('External API v2 Dashboards - new format', () => {
     },
   });
 
+  const createPieChart = (sourceId: string): ExternalDashboardTileWithId => ({
+    name: 'Pie Chart',
+    x: 6,
+    y: 3,
+    w: 3,
+    h: 3,
+    id: new ObjectId().toString(),
+    config: {
+      displayType: 'pie',
+      sourceId,
+      select: [
+        {
+          aggFn: 'count',
+          where: '',
+        },
+      ],
+      groupBy: 'service.name',
+    },
+  });
+
   const server = getServer();
   let agent, team, user, traceSource, metricSource;
 
@@ -1912,6 +1932,7 @@ describe('External API v2 Dashboards - new format', () => {
           createTableChart(traceSource._id.toString()),
           createNumberChart(traceSource._id.toString()),
           createMarkdownChart(),
+          createPieChart(traceSource._id.toString()),
         ],
         tags: ['test', 'chart-types'],
       };
@@ -1922,18 +1943,45 @@ describe('External API v2 Dashboards - new format', () => {
 
       const { id } = response.body.data;
       expect(response.body.data).toHaveProperty('id');
-      expect(response.body.data.tiles.length).toBe(4);
+      expect(response.body.data.tiles.length).toBe(5);
 
       // Verify by retrieving the dashboard
       const retrieveResponse = await authRequest('get', `${BASE_URL}/${id}`);
 
       expect(retrieveResponse.status).toBe(200);
-      expect(retrieveResponse.body.data.tiles.length).toBe(4);
+      expect(retrieveResponse.body.data.tiles.length).toBe(5);
       expect(retrieveResponse.body.data.tags).toEqual(['test', 'chart-types']);
     });
 
     it('can round-trip all supported chart types and all supported fields on each chart type', async () => {
       // Arrange
+      const pieChart: ExternalDashboardTile = {
+        name: 'Pie Chart',
+        x: 6,
+        y: 3,
+        w: 6,
+        h: 3,
+        config: {
+          displayType: 'pie',
+          sourceId: traceSource._id.toString(),
+          select: [
+            {
+              aggFn: 'quantile',
+              level: 0.5,
+              valueExpression: 'Duration',
+              alias: 'Median Duration',
+              where: "env = 'production'",
+              whereLanguage: 'sql',
+            },
+          ],
+          groupBy: 'service.name',
+          numberFormat: {
+            output: 'number',
+            mantissa: 2,
+          },
+        },
+      };
+
       const lineChart: ExternalDashboardTile = {
         name: 'Line Chart',
         x: 0,
@@ -2087,7 +2135,14 @@ describe('External API v2 Dashboards - new format', () => {
       const response = await authRequest('post', BASE_URL)
         .send({
           name: 'Dashboard with All Chart Types',
-          tiles: [lineChart, barChart, tableChart, numberChart, markdownChart],
+          tiles: [
+            lineChart,
+            barChart,
+            tableChart,
+            numberChart,
+            markdownChart,
+            pieChart,
+          ],
           tags: ['round-trip-test'],
         })
         .expect(200);
@@ -2098,6 +2153,7 @@ describe('External API v2 Dashboards - new format', () => {
       expect(omit(response.body.data.tiles[2], ['id'])).toEqual(tableChart);
       expect(omit(response.body.data.tiles[3], ['id'])).toEqual(numberChart);
       expect(omit(response.body.data.tiles[4], ['id'])).toEqual(markdownChart);
+      expect(omit(response.body.data.tiles[5], ['id'])).toEqual(pieChart);
     });
 
     it('should return 400 when source IDs do not exist', async () => {

packages/api/src/routers/external-api/v2/dashboards.ts

@@ -585,6 +585,37 @@ async function getMissingSources(
  *         numberFormat:
  *           $ref: '#/components/schemas/NumberFormat'
  *
+ *     PieChartConfig:
+ *       type: object
+ *       required:
+ *         - displayType
+ *         - sourceId
+ *         - select
+ *       description: Configuration for a pie chart tile. Each slice represents one group value.
+ *       properties:
+ *         displayType:
+ *           type: string
+ *           enum: [pie]
+ *           example: "pie"
+ *         sourceId:
+ *           type: string
+ *           description: ID of the data source to query.
+ *           example: "65f5e4a3b9e77c001a111111"
+ *         select:
+ *           type: array
+ *           minItems: 1
+ *           maxItems: 1
+ *           description: Exactly one aggregated value used to size each pie slice.
+ *           items:
+ *             $ref: '#/components/schemas/SelectItem'
+ *         groupBy:
+ *           type: string
+ *           maxLength: 10000
+ *           description: Field expression to group results by (one slice per group value).
+ *           example: "service"
+ *         numberFormat:
+ *           $ref: '#/components/schemas/NumberFormat'
+ *
  *     SearchChartConfig:
  *       type: object
  *       required:
@@ -641,6 +672,7 @@ async function getMissingSources(
  *         - $ref: '#/components/schemas/BarChartConfig'
  *         - $ref: '#/components/schemas/TableChartConfig'
  *         - $ref: '#/components/schemas/NumberChartConfig'
+ *         - $ref: '#/components/schemas/PieChartConfig'
  *         - $ref: '#/components/schemas/SearchChartConfig'
  *         - $ref: '#/components/schemas/MarkdownChartConfig'
  *       discriminator:
@@ -650,6 +682,7 @@ async function getMissingSources(
  *           stacked_bar: '#/components/schemas/BarChartConfig'
  *           table: '#/components/schemas/TableChartConfig'
  *           number: '#/components/schemas/NumberChartConfig'
+ *           pie: '#/components/schemas/PieChartConfig'
  *           search: '#/components/schemas/SearchChartConfig'
  *           markdown: '#/components/schemas/MarkdownChartConfig'
  *

packages/api/src/routers/external-api/v2/utils/dashboards.ts

@@ -91,8 +91,8 @@ const convertToExternalTileChartConfig = (
   };
 
   switch (config.displayType) {
-    case 'line':
-    case 'stacked_bar':
+    case DisplayType.Line:
+    case DisplayType.StackedBar:
       return {
         displayType: config.displayType,
         sourceId,
@@ -106,12 +106,12 @@ const convertToExternalTileChartConfig = (
         select: Array.isArray(config.select)
           ? config.select.map(convertToExternalSelectItem)
           : [DEFAULT_SELECT_ITEM],
-        ...(config.displayType === 'line'
+        ...(config.displayType === DisplayType.Line
           ? { compareToPreviousPeriod: config.compareToPreviousPeriod }
           : {}),
         numberFormat: config.numberFormat,
       };
-    case 'number':
+    case DisplayType.Number:
       return {
         displayType: config.displayType,
         sourceId,
@@ -120,7 +120,17 @@ const convertToExternalTileChartConfig = (
           : [DEFAULT_SELECT_ITEM],
         numberFormat: config.numberFormat,
       };
-    case 'table':
+    case DisplayType.Pie:
+      return {
+        displayType: config.displayType,
+        sourceId,
+        select: Array.isArray(config.select)
+          ? [convertToExternalSelectItem(config.select[0])]
+          : [DEFAULT_SELECT_ITEM],
+        groupBy: stringValueOrDefault(config.groupBy, undefined),
+        numberFormat: config.numberFormat,
+      };
+    case DisplayType.Table:
       return {
         ...pick(config, ['having', 'numberFormat']),
         displayType: config.displayType,
@@ -135,25 +145,28 @@ const convertToExternalTileChartConfig = (
           : [DEFAULT_SELECT_ITEM],
         orderBy: stringValueOrDefault(config.orderBy, undefined),
       };
-    case 'search':
+    case DisplayType.Search:
       return {
         displayType: config.displayType,
         sourceId,
         select: stringValueOrDefault(config.select, ''),
         where: config.where,
         whereLanguage: config.whereLanguage ?? 'lucene',
       };
-    case 'markdown':
+    case DisplayType.Markdown:
       return {
         displayType: config.displayType,
         markdown: stringValueOrDefault(config.markdown, ''),
       };
-    default:
+    case DisplayType.Heatmap:
+    case undefined:
       logger.error(
         { config },
-        'Error converting chart config to external chart - unrecognized display type',
+        'Error converting chart config to external chart - unsupported display type',
       );
       return undefined;
+    default:
+      config.displayType satisfies never;
   }
 };
 
@@ -257,6 +270,16 @@ export function convertToInternalTileConfig(
         name,
       };
       break;
+    case 'pie':
+      internalConfig = {
+        ...pick(externalConfig, ['groupBy', 'numberFormat']),
+        displayType: DisplayType.Pie,
+        select: [convertToInternalSelectItem(externalConfig.select[0])],
+        source: externalConfig.sourceId,
+        where: '',
+        name,
+      };
+      break;
     case 'search':
       internalConfig = {
         ...pick(externalConfig, ['select', 'where']),
@@ -276,6 +299,13 @@ export function convertToInternalTileConfig(
         name,
       };
       break;
+    default:
+      // Typecheck to ensure all display types are handled
+      externalConfig satisfies never;
+
+      // We should never hit this due to the typecheck above.
+      // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion
+      internalConfig = {} as SavedChartConfig;
   }
 
   // Omit keys that are null/undefined, so that they're not saved as null in Mongo.

packages/api/src/utils/zod.ts

@@ -234,6 +234,14 @@ const externalDashboardNumberChartConfigSchema = z.object({
   numberFormat: NumberFormatSchema.optional(),
 });
 
+const externalDashboardPieChartConfigSchema = z.object({
+  displayType: z.literal('pie'),
+  sourceId: objectIdSchema,
+  select: z.array(externalDashboardSelectItemSchema).length(1),
+  groupBy: z.string().max(10000).optional(),
+  numberFormat: NumberFormatSchema.optional(),
+});
+
 const externalDashboardSearchChartConfigSchema = z.object({
   displayType: z.literal('search'),
   sourceId: objectIdSchema,
@@ -253,6 +261,7 @@ export const externalDashboardTileConfigSchema = z
     externalDashboardBarChartConfigSchema,
     externalDashboardTableChartConfigSchema,
     externalDashboardNumberChartConfigSchema,
+    externalDashboardPieChartConfigSchema,
     externalDashboardMarkdownChartConfigSchema,
     externalDashboardSearchChartConfigSchema,
   ])