feat(pipeline-ui): Display spark graph in destinations table (#19718)

* feat(pipeline-ui): Display spark graph in destinations table * Sync lint-staged paths with CI Otherwise couldn't even merge with master due to `bin/build-schema.mjs` being linted in the pre-commit hook despite not being included by `tsconfig.json`. CI didn't notice this because it never ran on that file - so we shouldn't run on it locally either. * Fix `pipelineAppMetricsLogic` * Generalize `Sparkline` * run prettier * Mock metrics in all stories * Speed Storybook up slightly * Update UI snapshots for `chromium` (1) --------- Co-authored-by: Michael Matloka <[email protected]> Co-authored-by: Thomas Obermüller <[email protected]> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Michael Matloka <[email protected]>
PostHog · Jan 16, 2024 · 88cd774 · 88cd774
1 parent d759793
commit 88cd774
Show file tree

Hide file tree

Showing 13 changed files with 93 additions and 100 deletions.
diff --git a/.storybook/main.ts b/.storybook/main.ts
@@ -38,7 +38,7 @@ const config: StorybookConfig = {
 
     framework: {
         name: '@storybook/react-webpack5',
-        options: {},
+        options: { builder: { useSWC: true } }
     },
 
     docs: {

diff --git a/frontend/__snapshots__/scenes-app-pipeline--pipeline-destinations-page--dark.png b/frontend/__snapshots__/scenes-app-pipeline--pipeline-destinations-page--dark.png
diff --git a/frontend/__snapshots__/scenes-app-pipeline--pipeline-destinations-page--light.png b/frontend/__snapshots__/scenes-app-pipeline--pipeline-destinations-page--light.png
diff --git a/frontend/__snapshots__/scenes-app-pipeline--pipeline-landing-page--dark.png b/frontend/__snapshots__/scenes-app-pipeline--pipeline-landing-page--dark.png
diff --git a/frontend/__snapshots__/scenes-app-pipeline--pipeline-landing-page--light.png b/frontend/__snapshots__/scenes-app-pipeline--pipeline-landing-page--light.png
diff --git a/frontend/src/lib/lemon-ui/LemonTable/TableCellSparkline.scss b/frontend/src/lib/lemon-ui/LemonTable/TableCellSparkline.scss
diff --git a/...emon-ui/LemonTable/TableCellSparkline.tsx → frontend/src/lib/lemon-ui/Sparkline.tsx b/...emon-ui/LemonTable/TableCellSparkline.tsx → frontend/src/lib/lemon-ui/Sparkline.tsx
@@ -1,12 +1,25 @@
-import './TableCellSparkline.scss'
-
 import { offset } from '@floating-ui/react'
 import { Chart, ChartItem, TooltipModel } from 'lib/Chart'
 import { getColorVar } from 'lib/colors'
 import { Popover } from 'lib/lemon-ui/Popover/Popover'
+import React from 'react'
 import { useEffect, useRef, useState } from 'react'
 
-export function TableCellSparkline({ labels, data }: { labels?: string[]; data: number[] }): JSX.Element {
+interface SparkLineTimeSeries {
+    name: string | null
+    /** Check vars.scss for available colors */
+    color: string
+    values: number[]
+}
+
+interface SparklineProps {
+    /** Optional labels for the X axis. */
+    labels?: string[]
+    /** Either a list of numbers for a muted graph or an array of time series */
+    data: number[] | SparkLineTimeSeries[]
+}
+
+export function Sparkline({ labels, data }: SparklineProps): JSX.Element {
     const canvasRef = useRef<HTMLCanvasElement | null>(null)
     const tooltipRef = useRef<HTMLDivElement | null>(null)
 
@@ -16,32 +29,37 @@ export function TableCellSparkline({ labels, data }: { labels?: string[]; data:
     useEffect(() => {
         // data should always be provided but React can render this without it,
         // so, fall back to an empty array for safety
-        if (data === undefined) {
+        if (data === undefined || data.length === 0) {
             return
         }
 
+        const adjustedData: SparkLineTimeSeries[] = !isSparkLineTimeSeries(data)
+            ? [{ name: null, color: 'muted', values: data }]
+            : data
+
         let chart: Chart
         if (canvasRef.current) {
             chart = new Chart(canvasRef.current?.getContext('2d') as ChartItem, {
                 type: 'bar',
                 data: {
-                    labels: labels || data.map(() => ''),
-                    datasets: [
-                        {
-                            data: data,
-                            minBarLength: 3,
-                            hoverBackgroundColor: getColorVar('primary'),
-                        },
-                    ],
+                    labels: labels || Object.values(adjustedData).map(() => ''),
+                    datasets: adjustedData.map((timeseries) => ({
+                        data: timeseries.values,
+                        minBarLength: 0,
+                        backgroundColor: getColorVar(timeseries.color),
+                        hoverBackgroundColor: getColorVar('primary'),
+                    })),
                 },
                 options: {
                     scales: {
                         x: {
                             display: false,
+                            stacked: true,
                         },
                         y: {
                             beginAtZero: true,
                             display: false,
+                            stacked: true,
                         },
                     },
                     plugins: {
@@ -51,16 +69,31 @@ export function TableCellSparkline({ labels, data }: { labels?: string[]; data:
                             display: false,
                         },
                         tooltip: {
+                            // TODO: use InsightsTooltip instead
                             enabled: false, // using external tooltip
                             external({ tooltip }: { chart: Chart; tooltip: TooltipModel<'bar'> }) {
                                 if (tooltip.opacity === 0) {
                                     setPopoverContent(null)
                                     return
                                 }
                                 const datapoint = tooltip.dataPoints[0]
-                                const tooltipLabel = datapoint.label ? `${datapoint.label}: ` : ''
-                                const tooltipContent = `${tooltipLabel} ${datapoint.formattedValue}`
-                                setPopoverContent(<>{tooltipContent}</>)
+                                const toolTipLabel = datapoint.label ? `${datapoint.label}: ` : ''
+                                if (tooltip.dataPoints.length === 1) {
+                                    const tooltipContent = toolTipLabel + datapoint.formattedValue
+                                    setPopoverContent(<>{tooltipContent}</>)
+                                } else {
+                                    const tooltipContent = [<React.Fragment key="-1">{toolTipLabel}</React.Fragment>]
+                                    for (let i = 0; i < tooltip.dataPoints.length; i++) {
+                                        const datapoint = tooltip.dataPoints[i]
+                                        tooltipContent.push(
+                                            <React.Fragment key={i}>
+                                                <br />
+                                                {adjustedData[i].name}: {datapoint.formattedValue}
+                                            </React.Fragment>
+                                        )
+                                    }
+                                    setPopoverContent(<>{tooltipContent}</>)
+                                }
                                 setPopoverOffset(tooltip.x)
                             },
                         },
@@ -80,8 +113,8 @@ export function TableCellSparkline({ labels, data }: { labels?: string[]; data:
     }, [labels, data])
 
     return (
-        <div className="TableCellSparkline">
-            <canvas ref={canvasRef} />
+        <div className="w-full">
+            <canvas ref={canvasRef} className="h-9" />
             <Popover
                 visible={!!popoverContent}
                 overlay={popoverContent}
@@ -93,3 +126,7 @@ export function TableCellSparkline({ labels, data }: { labels?: string[]; data:
         </div>
     )
 }
+
+function isSparkLineTimeSeries(data: number[] | SparkLineTimeSeries[]): data is SparkLineTimeSeries[] {
+    return typeof data[0] !== 'number'
+}
diff --git a/frontend/src/queries/nodes/DataTable/renderColumn.tsx b/frontend/src/queries/nodes/DataTable/renderColumn.tsx
@@ -4,9 +4,9 @@ import { JSONViewer } from 'lib/components/JSONViewer'
 import { Property } from 'lib/components/Property'
 import { PropertyKeyInfo } from 'lib/components/PropertyKeyInfo'
 import { TZLabel } from 'lib/components/TZLabel'
-import { TableCellSparkline } from 'lib/lemon-ui/LemonTable/TableCellSparkline'
 import { LemonTag } from 'lib/lemon-ui/LemonTag/LemonTag'
 import { Link } from 'lib/lemon-ui/Link'
+import { Sparkline } from 'lib/lemon-ui/Sparkline'
 import { Spinner } from 'lib/lemon-ui/Spinner/Spinner'
 import { Tooltip } from 'lib/lemon-ui/Tooltip'
 import { autoCaptureEventToDescription } from 'lib/utils'
@@ -83,7 +83,7 @@ export function renderColumn(
                         object[value[i]] = value[i + 1]
                     }
                     if ('results' in object && Array.isArray(object.results)) {
-                        return <TableCellSparkline data={object.results} />
+                        return <Sparkline data={object.results} />
                     }
                 }
 

diff --git a/frontend/src/scenes/data-management/ingestion-warnings/IngestionWarningsView.tsx b/frontend/src/scenes/data-management/ingestion-warnings/IngestionWarningsView.tsx
@@ -3,8 +3,8 @@ import { ReadingHog } from 'lib/components/hedgehogs'
 import { ProductIntroduction } from 'lib/components/ProductIntroduction/ProductIntroduction'
 import { TZLabel } from 'lib/components/TZLabel'
 import { LemonTable } from 'lib/lemon-ui/LemonTable'
-import { TableCellSparkline } from 'lib/lemon-ui/LemonTable/TableCellSparkline'
 import { Link } from 'lib/lemon-ui/Link'
+import { Sparkline } from 'lib/lemon-ui/Sparkline'
 import { urls } from 'scenes/urls'
 
 import { ProductKey } from '~/types'
@@ -172,7 +172,7 @@ export function IngestionWarningsView(): JSX.Element {
                             {
                                 title: 'Graph',
                                 render: function Render(_, summary: IngestionWarningSummary) {
-                                    return <TableCellSparkline labels={dates} data={summaryDatasets[summary.type]} />
+                                    return <Sparkline labels={dates} data={summaryDatasets[summary.type]} />
                                 },
                             },
                             {

diff --git a/frontend/src/scenes/pipeline/Destinations.tsx b/frontend/src/scenes/pipeline/Destinations.tsx
@@ -1,10 +1,10 @@
 import {
     LemonButton,
     LemonDivider,
+    LemonSkeleton,
     LemonTable,
     LemonTableColumn,
     LemonTag,
-    LemonTagType,
     Link,
     Tooltip,
 } from '@posthog/lemon-ui'
@@ -14,13 +14,15 @@ import { FEATURE_FLAGS } from 'lib/constants'
 import { More } from 'lib/lemon-ui/LemonButton/More'
 import { LemonMarkdown } from 'lib/lemon-ui/LemonMarkdown/LemonMarkdown'
 import { updatedAtColumn } from 'lib/lemon-ui/LemonTable/columnUtils'
+import { Sparkline } from 'lib/lemon-ui/Sparkline'
 import { featureFlagLogic } from 'lib/logic/featureFlagLogic'
 import { deleteWithUndo } from 'lib/utils/deleteWithUndo'
 
 import { PipelineAppKind, ProductKey } from '~/types'
 
-import { DestinationType, pipelineDestinationsLogic } from './destinationsLogic'
+import { DestinationType, PipelineAppBackend, pipelineDestinationsLogic } from './destinationsLogic'
 import { NewButton } from './NewButton'
+import { pipelineAppMetricsLogic } from './pipelineAppMetricsLogic'
 import { RenderApp } from './utils'
 
 export function Destinations(): JSX.Element {
@@ -98,42 +100,9 @@ function DestinationsTable(): JSX.Element {
                         },
                     },
                     {
-                        title: '24h', // TODO: two options 24h or 7d selected
-                        render: function Render24hDeliveryRate(_, destination) {
-                            if (destination.backend === 'plugin') {
-                                let tooltip = 'No events exported in the past 24 hours'
-                                let value = '-'
-                                let tagType: LemonTagType = 'muted'
-                                const deliveryRate = destination.success_rates['24h']
-                                if (deliveryRate !== null) {
-                                    value = `${Math.floor(deliveryRate * 100)}%`
-                                    tooltip = 'Success rate for past 24 hours'
-                                    if (deliveryRate >= 0.99) {
-                                        tagType = 'success'
-                                    } else if (deliveryRate >= 0.75) {
-                                        tagType = 'warning'
-                                    } else {
-                                        tagType = 'danger'
-                                    }
-                                }
-                                return (
-                                    <Tooltip title={tooltip}>
-                                        <Link to={destination.metrics_url}>
-                                            <LemonTag type={tagType}>{value}</LemonTag>
-                                        </Link>
-                                    </Tooltip>
-                                )
-                            } else {
-                                // Batch exports // TODO: fix this
-                                const tooltip = 'No events exported in the past 24 hours'
-                                return (
-                                    <Tooltip title={tooltip}>
-                                        <Link to={destination.metrics_url}>
-                                            <LemonTag type="muted">-</LemonTag>
-                                        </Link>
-                                    </Tooltip>
-                                )
-                            }
+                        title: 'Success rate',
+                        render: function RenderSuccessRate(_, destination) {
+                            return <DestinationSparkLine destination={destination} />
                         },
                     },
                     updatedAtColumn() as LemonTableColumn<DestinationType, any>,
@@ -241,3 +210,26 @@ function DestinationsTable(): JSX.Element {
         </>
     )
 }
+
+function DestinationSparkLine({ destination }: { destination: DestinationType }): JSX.Element {
+    if (destination.backend === PipelineAppBackend.BatchExport) {
+        return <></> // TODO: not ready yet
+    } else {
+        const logic = pipelineAppMetricsLogic({ pluginConfigId: destination.id })
+        const { appMetricsResponse } = useValues(logic)
+
+        if (appMetricsResponse === null) {
+            return <LemonSkeleton className="w-full h-9" />
+        }
+
+        return (
+            <Sparkline
+                labels={appMetricsResponse.metrics.dates}
+                data={[
+                    { color: 'danger', name: 'failures', values: appMetricsResponse.metrics.failures },
+                    { color: 'success', name: 'sucesses', values: appMetricsResponse.metrics.successes },
+                ]}
+            />
+        )
+    }
+}
diff --git a/frontend/src/scenes/pipeline/Pipeline.stories.tsx b/frontend/src/scenes/pipeline/Pipeline.stories.tsx
@@ -29,6 +29,8 @@ export default {
                 // TODO: Differentiate between transformation and destination mocks for nicer mocks
                 '/api/organizations/@current/pipeline_destinations/': plugins,
                 '/api/projects/:team_id/pipeline_destination_configs/': pluginConfigs,
+                '/api/projects/:team_id/app_metrics/:plugin_config_id?date_from=-7d': require('./__mocks__/pluginMetrics.json'),
+                '/api/projects/:team_id/app_metrics/:plugin_config_id/error_details?error_type=Error': require('./__mocks__/pluginErrorDetails.json'),
             },
         }),
     ],
@@ -117,12 +119,6 @@ export function PipelineAppConfiguration404(): JSX.Element {
 }
 
 export function PipelineAppMetrics(): JSX.Element {
-    useStorybookMocks({
-        get: {
-            '/api/projects/:team_id/app_metrics/:plugin_config_id?date_from=-7d': require('./__mocks__/pluginMetrics.json'),
-            '/api/projects/:team_id/app_metrics/:plugin_config_id/error_details?error_type=Error': require('./__mocks__/pluginErrorDetails.json'),
-        },
-    })
     useEffect(() => {
         router.actions.push(urls.pipelineApp(PipelineAppKind.Destination, geoIpConfigId, PipelineAppTab.Metrics))
         pipelineAppMetricsLogic({ pluginConfigId: geoIpConfigId }).mount()
@@ -131,12 +127,6 @@ export function PipelineAppMetrics(): JSX.Element {
 }
 
 export function PipelineAppMetricsErrorModal(): JSX.Element {
-    useStorybookMocks({
-        get: {
-            '/api/projects/:team_id/app_metrics/:plugin_config_id?date_from=-7d': require('./__mocks__/pluginMetrics.json'),
-            '/api/projects/:team_id/app_metrics/:plugin_config_id/error_details?error_type=Error': require('./__mocks__/pluginErrorDetails.json'),
-        },
-    })
     useEffect(() => {
         router.actions.push(urls.pipelineApp(PipelineAppKind.Destination, geoIpConfigId, PipelineAppTab.Metrics))
         const logic = pipelineAppMetricsLogic({ pluginConfigId: geoIpConfigId })

diff --git a/frontend/src/scenes/pipeline/destinationsLogic.tsx b/frontend/src/scenes/pipeline/destinationsLogic.tsx
@@ -20,15 +20,6 @@ import {
 import type { pipelineDestinationsLogicType } from './destinationsLogicType'
 import { captureBatchExportEvent, capturePluginEvent } from './utils'
 
-interface WebhookSuccessRate {
-    '24h': number | null
-    '7d': number | null
-}
-interface BatchExportSuccessRate {
-    '24h': [successes: number, failures: number]
-    '7d': [successes: number, failures: number]
-}
-
 interface DestinationTypeBase {
     name: string
     description?: string
@@ -47,15 +38,13 @@ export enum PipelineAppBackend {
 export interface BatchExportDestination extends DestinationTypeBase {
     backend: PipelineAppBackend.BatchExport
     id: string
-    success_rates: BatchExportSuccessRate
     app_source_code_url?: never
 }
 export interface WebhookDestination extends DestinationTypeBase {
     backend: PipelineAppBackend.Plugin
     id: number
     plugin: PluginType
     app_source_code_url?: string
-    success_rates: WebhookSuccessRate
 }
 export type DestinationType = BatchExportDestination | WebhookDestination
 
@@ -189,10 +178,6 @@ export const pipelineDestinationsLogic = kea<pipelineDestinationsLogicType>([
                     logs_url: urls.pipelineApp(PipelineAppKind.Destination, pluginConfig.id, PipelineAppTab.Logs),
                     app_source_code_url: '',
                     plugin: plugins[pluginConfig.plugin],
-                    success_rates: {
-                        '24h': pluginConfig.delivery_rate_24h === undefined ? null : pluginConfig.delivery_rate_24h,
-                        '7d': null, // TODO: start populating real data for this
-                    },
                     updated_at: pluginConfig.updated_at,
                 }))
                 const batchDests = Object.values(batchExportConfigs).map<DestinationType>((batchExport) => ({
@@ -209,10 +194,6 @@ export const pipelineDestinationsLogic = kea<pipelineDestinationsLogicType>([
                     ),
                     metrics_url: urls.pipelineApp(PipelineAppKind.Destination, batchExport.id, PipelineAppTab.Metrics),
                     logs_url: urls.pipelineApp(PipelineAppKind.Destination, batchExport.id, PipelineAppTab.Logs),
-                    success_rates: {
-                        '24h': [5, 17],
-                        '7d': [12, 100043],
-                    },
                     updated_at: batchExport.created_at, // TODO: Add updated_at to batch exports in the backend
                 }))
                 const enabledFirst = [...appDests, ...batchDests].sort((a, b) => Number(b.enabled) - Number(a.enabled))

diff --git a/package.json b/package.json
@@ -312,11 +312,11 @@
             "stylelint --fix",
             "prettier --write"
         ],
-        "(!(plugin-server)/**).{js,jsx,mjs,ts,tsx}": [
+        "frontend/src/**.{js,jsx,mjs,ts,tsx}": [
             "eslint -c .eslintrc.js --fix",
             "prettier --write"
         ],
-        "(plugin-server/**).{js,jsx,mjs,ts,tsx}": [
+        "plugin-server/**.{js,jsx,mjs,ts,tsx}": [
             "pnpm --dir plugin-server exec eslint --fix",
             "pnpm --dir plugin-server exec prettier --write"
         ],