Browse Source

feat(app-start): Change widgets to two columns (#64070)

Uses two columns for the widgets where the left is cold start data and
the right is warm start data.

I adapted the count widget to display the line chart and removed the
files related to widgets we aren't displaying anymore.

![Screenshot 2024-01-29 at 12 24
14 PM](https://github.com/getsentry/sentry/assets/22846452/8eb751c4-aecd-4034-9a9a-b195083b5a32)

---------

Co-authored-by: getsantry[bot] <66042841+getsantry[bot]@users.noreply.github.com>
Nar Saynorath 1 year ago
parent
commit
67ebd07c1c

+ 8 - 0
static/app/views/starfish/colours.tsx

@@ -5,3 +5,11 @@ export const P50_COLOR = CHART_PALETTE[3][1];
 export const P95_COLOR = CHART_PALETTE[0][0];
 export const AVG_COLOR = CHART_PALETTE[0][0];
 export const ERRORS_COLOR = CHART_PALETTE[5][3];
+
+export const COLD_START_COLOR = CHART_PALETTE[5][4];
+export const WARM_START_COLOR = CHART_PALETTE[5][5];
+
+export const RELEASE_COMPARISON = {
+  PRIMARY_RELEASE_COLOR: CHART_PALETTE[0][0],
+  SECONDARY_RELEASE_COLOR: CHART_PALETTE[5][3],
+};

+ 0 - 189
static/app/views/starfish/views/appStartup/screenSummary/appStartBreakdownWidget.tsx

@@ -1,189 +0,0 @@
-import styled from '@emotion/styled';
-
-import {getInterval} from 'sentry/components/charts/utils';
-import EmptyStateWarning from 'sentry/components/emptyStateWarning';
-import {OpsDot} from 'sentry/components/events/opsBreakdown';
-import LoadingContainer from 'sentry/components/loading/loadingContainer';
-import TextOverflow from 'sentry/components/textOverflow';
-import {t} from 'sentry/locale';
-import {space} from 'sentry/styles/space';
-import EventView from 'sentry/utils/discover/eventView';
-import {DiscoverDatasets} from 'sentry/utils/discover/types';
-import {formatVersion} from 'sentry/utils/formatters';
-import {decodeScalar} from 'sentry/utils/queryString';
-import {MutableSearch} from 'sentry/utils/tokenizeSearch';
-import {useLocation} from 'sentry/utils/useLocation';
-import usePageFilters from 'sentry/utils/usePageFilters';
-import {prepareQueryForLandingPage} from 'sentry/views/performance/data';
-import MiniChartPanel from 'sentry/views/starfish/components/miniChartPanel';
-import {useReleaseSelection} from 'sentry/views/starfish/queries/useReleases';
-import {formatVersionAndCenterTruncate} from 'sentry/views/starfish/utils/centerTruncate';
-import {STARFISH_CHART_INTERVAL_FIDELITY} from 'sentry/views/starfish/utils/constants';
-import {appendReleaseFilters} from 'sentry/views/starfish/utils/releaseComparison';
-import Breakdown from 'sentry/views/starfish/views/appStartup/breakdown';
-import {useTableQuery} from 'sentry/views/starfish/views/screens/screensTable';
-
-export const COLD_START_COLOR = '#F58C46';
-export const WARM_START_COLOR = '#F2B712';
-
-function AppStartBreakdownWidget({additionalFilters}) {
-  const pageFilter = usePageFilters();
-  const location = useLocation();
-  const {query: locationQuery} = location;
-
-  const {
-    primaryRelease,
-    secondaryRelease,
-    isLoading: isReleasesLoading,
-  } = useReleaseSelection();
-
-  const query = new MutableSearch([
-    'span.op:[app.start.warm,app.start.cold]',
-    ...(additionalFilters ?? []),
-  ]);
-
-  const searchQuery = decodeScalar(locationQuery.query, '');
-  if (searchQuery) {
-    query.addStringFilter(prepareQueryForLandingPage(searchQuery, false));
-  }
-
-  const queryString = `${appendReleaseFilters(
-    query,
-    primaryRelease,
-    secondaryRelease
-  )} span.description:["Cold Start","Warm Start"]`;
-
-  const {data, isLoading} = useTableQuery({
-    eventView: EventView.fromNewQueryWithPageFilters(
-      {
-        name: '',
-        fields: ['release', 'span.op', 'count()'],
-        topEvents: '2',
-        query: queryString,
-        dataset: DiscoverDatasets.SPANS_METRICS,
-        version: 2,
-        interval: getInterval(
-          pageFilter.selection.datetime,
-          STARFISH_CHART_INTERVAL_FIDELITY
-        ),
-      },
-      pageFilter.selection
-    ),
-    enabled: !isReleasesLoading,
-    referrer: 'api.starfish.mobile-startup-breakdown',
-    initialData: {data: []},
-  });
-
-  if (isLoading) {
-    return <LoadingContainer isLoading />;
-  }
-
-  if (!data) {
-    return (
-      <EmptyStateWarning small>
-        <p>{t('There was no app start data found for these two releases')}</p>
-      </EmptyStateWarning>
-    );
-  }
-
-  const startsByReleaseSeries = data.data.reduce((acc, row) => {
-    acc[row.release] = {...acc[row.release], [row['span.op']]: row['count()']};
-
-    return acc;
-  }, {});
-
-  const breakdownGroups = [
-    {
-      key: 'app.start.cold',
-      color: COLD_START_COLOR,
-      name: t('Cold Start'),
-    },
-    {
-      key: 'app.start.warm',
-      color: WARM_START_COLOR,
-      name: t('Warm Start'),
-    },
-  ];
-
-  return (
-    <MiniChartPanel
-      title={t('App Start')}
-      subtitle={
-        primaryRelease
-          ? t(
-              '%s v. %s',
-              formatVersionAndCenterTruncate(primaryRelease, 12),
-              secondaryRelease ? formatVersionAndCenterTruncate(secondaryRelease, 12) : ''
-            )
-          : ''
-      }
-    >
-      <Legend>
-        <LegendEntry>
-          <StyledStartTypeDot style={{backgroundColor: COLD_START_COLOR}} />
-          {t('Cold Start')}
-        </LegendEntry>
-        <LegendEntry>
-          <StyledStartTypeDot style={{backgroundColor: WARM_START_COLOR}} />
-          {t('Warm Start')}
-        </LegendEntry>
-      </Legend>
-      <AppStartBreakdownContent>
-        {primaryRelease && (
-          <ReleaseAppStartBreakdown>
-            <TextOverflow>{formatVersion(primaryRelease)}</TextOverflow>
-            <Breakdown
-              row={startsByReleaseSeries[primaryRelease]}
-              breakdownGroups={breakdownGroups}
-            />
-          </ReleaseAppStartBreakdown>
-        )}
-        {secondaryRelease && (
-          <ReleaseAppStartBreakdown>
-            <TextOverflow>{formatVersion(secondaryRelease)}</TextOverflow>
-            <Breakdown
-              row={startsByReleaseSeries[secondaryRelease]}
-              breakdownGroups={breakdownGroups}
-            />
-          </ReleaseAppStartBreakdown>
-        )}
-      </AppStartBreakdownContent>
-    </MiniChartPanel>
-  );
-}
-
-export default AppStartBreakdownWidget;
-
-const ReleaseAppStartBreakdown = styled('div')`
-  display: grid;
-  grid-template-columns: 20% auto;
-  gap: ${space(1)};
-  color: ${p => p.theme.subText};
-`;
-
-const AppStartBreakdownContent = styled('div')`
-  display: flex;
-  flex-direction: column;
-  gap: ${space(1)};
-  margin-top: ${space(1)};
-`;
-
-const Legend = styled('div')`
-  display: flex;
-  gap: ${space(1.5)};
-  position: absolute;
-  top: ${space(1.5)};
-  right: ${space(2)};
-`;
-
-const LegendEntry = styled('div')`
-  display: flex;
-  justify-content: center;
-  align-items: center;
-  font-size: ${p => p.theme.fontSizeSmall};
-`;
-
-const StyledStartTypeDot = styled(OpsDot)`
-  position: relative;
-  top: -1px;
-`;

+ 160 - 0
static/app/views/starfish/views/appStartup/screenSummary/startDurationWidget.spec.tsx

@@ -0,0 +1,160 @@
+import {OrganizationFixture} from 'sentry-fixture/organization';
+import {ProjectFixture} from 'sentry-fixture/project';
+
+import {render, screen} from 'sentry-test/reactTestingLibrary';
+
+import type {MultiSeriesEventsStats} from 'sentry/types';
+import usePageFilters from 'sentry/utils/usePageFilters';
+
+import StartDurationWidget, {transformData} from './startDurationWidget';
+
+jest.mock('sentry/utils/usePageFilters');
+
+describe('StartDurationWidget', () => {
+  const organization = OrganizationFixture();
+  const project = ProjectFixture();
+
+  beforeEach(function () {
+    jest.mocked(usePageFilters).mockReturnValue({
+      isReady: true,
+      desyncedFilters: new Set(),
+      pinnedFilters: new Set(),
+      shouldPersist: true,
+      selection: {
+        datetime: {
+          period: '10d',
+          start: null,
+          end: null,
+          utc: false,
+        },
+        environments: [],
+        projects: [parseInt(project.id, 10)],
+      },
+    });
+    MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/releases/`,
+      body: [
+        {
+          id: 970136705,
+          version: 'com.example.vu.android@2.10.5',
+          dateCreated: '2023-12-19T21:37:53.895495Z',
+        },
+        {
+          id: 969902997,
+          version: 'com.example.vu.android@2.10.3+42',
+          dateCreated: '2023-12-19T18:04:06.953025Z',
+        },
+      ],
+    });
+    MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/events-stats/`,
+      body: {
+        data: {},
+      },
+    });
+    MockApiClient.addMockResponse({
+      url: `/organizations/${organization.slug}/events/`,
+      body: {
+        data: [],
+      },
+    });
+  });
+
+  afterEach(function () {
+    MockApiClient.clearMockResponses();
+    jest.clearAllMocks();
+  });
+
+  it('renders correct title for cold start duration', async () => {
+    render(<StartDurationWidget chartHeight={200} type="cold" />);
+    expect(await screen.findByText('Avg. Cold Start Duration')).toBeInTheDocument();
+  });
+
+  it('renders correct title for warm start duration', async () => {
+    render(<StartDurationWidget chartHeight={200} type="warm" />);
+    expect(await screen.findByText('Avg. Warm Start Duration')).toBeInTheDocument();
+  });
+
+  describe('transformData', () => {
+    it('properly sets the release color and transforms timestamps', () => {
+      const mockData = {
+        'com.example.vu.android@2.10.5': {
+          data: [
+            [
+              1703937600,
+              [
+                {
+                  count: 100,
+                },
+              ],
+            ],
+          ],
+          order: 0,
+          isMetricsData: false,
+          start: 1703937600,
+          end: 1706529600,
+          meta: {
+            fields: {},
+            units: {},
+            isMetricsData: false,
+            isMetricsExtractedData: false,
+            tips: {},
+            datasetReason: 'unchanged',
+            dataset: 'spansMetrics',
+          },
+        },
+        'com.example.vu.android@2.10.3+42': {
+          data: [
+            [
+              1703937600,
+              [
+                {
+                  count: 200,
+                },
+              ],
+            ],
+          ],
+          order: 1,
+          isMetricsData: false,
+          start: 1703937600,
+          end: 1706529600,
+          meta: {
+            fields: {},
+            units: {},
+            isMetricsData: false,
+            isMetricsExtractedData: false,
+            tips: {},
+            datasetReason: 'unchanged',
+            dataset: 'spansMetrics',
+          },
+        },
+      } as MultiSeriesEventsStats;
+
+      // com.example.vu.android@2.10.5 is noted as the primary, so the series with
+      // com.example.vu.android@2.10.3+42 should be colored differently.
+      const transformedData = transformData(mockData, 'com.example.vu.android@2.10.5');
+      expect(transformedData).toEqual({
+        'com.example.vu.android@2.10.5': {
+          seriesName: 'com.example.vu.android@2.10.5',
+          color: '#444674',
+          data: [
+            {
+              name: 1703937600000,
+              value: 100,
+            },
+          ],
+        },
+        'com.example.vu.android@2.10.3+42': {
+          seriesName: 'com.example.vu.android@2.10.3+42',
+          color: '#e9626e',
+          data: [
+            {
+              name: 1703937600000,
+              value: 200,
+            },
+          ],
+        },
+      });
+    });
+  });
+});

+ 40 - 33
static/app/views/starfish/views/appStartup/screenSummary/countWidget.tsx → static/app/views/starfish/views/appStartup/screenSummary/startDurationWidget.tsx

@@ -7,33 +7,32 @@ import {defined} from 'sentry/utils';
 import {tooltipFormatterUsingAggregateOutputType} from 'sentry/utils/discover/charts';
 import EventView from 'sentry/utils/discover/eventView';
 import {DiscoverDatasets} from 'sentry/utils/discover/types';
+import {formatVersion} from 'sentry/utils/formatters';
 import {MutableSearch} from 'sentry/utils/tokenizeSearch';
 import usePageFilters from 'sentry/utils/usePageFilters';
+import {RELEASE_COMPARISON} from 'sentry/views/starfish/colours';
 import Chart from 'sentry/views/starfish/components/chart';
 import MiniChartPanel from 'sentry/views/starfish/components/miniChartPanel';
 import {useReleaseSelection} from 'sentry/views/starfish/queries/useReleases';
 import {STARFISH_CHART_INTERVAL_FIDELITY} from 'sentry/views/starfish/utils/constants';
 import {appendReleaseFilters} from 'sentry/views/starfish/utils/releaseComparison';
 import {useEventsStatsQuery} from 'sentry/views/starfish/utils/useEventsStatsQuery';
-import {
-  COLD_START_COLOR,
-  WARM_START_COLOR,
-} from 'sentry/views/starfish/views/appStartup/screenSummary/appStartBreakdownWidget';
-import {OUTPUT_TYPE, YAxis} from 'sentry/views/starfish/views/screens';
 
-const SPAN_OP_TO_STRING = {
-  'app.start.cold': t('Cold Start'),
-  'app.start.warm': t('Warm Start'),
-};
+const COLD_START_CONDITIONS = ['span.op:app.start.cold', 'span.description:"Cold Start"'];
+const WARM_START_CONDITIONS = ['span.op:app.start.warm', 'span.description:"Warm Start"'];
 
-function transformData(data?: MultiSeriesEventsStats) {
-  const transformedSeries: {[yAxisName: string]: Series} = {};
+export function transformData(data?: MultiSeriesEventsStats, primaryRelease?: string) {
+  const transformedSeries: {[releaseName: string]: Series} = {};
   if (defined(data)) {
-    Object.keys(data).forEach(yAxis => {
-      transformedSeries[yAxis] = {
-        seriesName: yAxis,
+    Object.keys(data).forEach(releaseName => {
+      transformedSeries[releaseName] = {
+        seriesName: releaseName,
+        color:
+          releaseName === primaryRelease
+            ? RELEASE_COMPARISON.PRIMARY_RELEASE_COLOR
+            : RELEASE_COMPARISON.SECONDARY_RELEASE_COLOR,
         data:
-          data[yAxis]?.data.map(datum => {
+          data[releaseName]?.data?.map(datum => {
             return {
               name: datum[0] * 1000,
               value: datum[1][0].count,
@@ -47,10 +46,11 @@ function transformData(data?: MultiSeriesEventsStats) {
 
 interface Props {
   chartHeight: number;
+  type: 'cold' | 'warm';
   additionalFilters?: string[];
 }
 
-function CountWidget({additionalFilters, chartHeight}: Props) {
+function StartDurationWidget({additionalFilters, chartHeight, type}: Props) {
   const pageFilter = usePageFilters();
   const {
     primaryRelease,
@@ -59,14 +59,10 @@ function CountWidget({additionalFilters, chartHeight}: Props) {
   } = useReleaseSelection();
 
   const query = new MutableSearch([
-    'span.op:[app.start.warm,app.start.cold]',
+    ...(type === 'cold' ? COLD_START_CONDITIONS : WARM_START_CONDITIONS),
     ...(additionalFilters ?? []),
   ]);
-  const queryString = `${appendReleaseFilters(
-    query,
-    primaryRelease,
-    secondaryRelease
-  )} span.description:["Cold Start","Warm Start"]`;
+  const queryString = appendReleaseFilters(query, primaryRelease, secondaryRelease);
 
   const {
     data: series,
@@ -77,8 +73,8 @@ function CountWidget({additionalFilters, chartHeight}: Props) {
       {
         name: '',
         topEvents: '2',
-        fields: ['span.op', 'count()'],
-        yAxis: ['count()'],
+        fields: ['release', 'avg(span.duration)'],
+        yAxis: ['avg(span.duration)'],
         query: queryString,
         dataset: DiscoverDatasets.SPANS_METRICS,
         version: 2,
@@ -98,13 +94,24 @@ function CountWidget({additionalFilters, chartHeight}: Props) {
     return <LoadingContainer isLoading />;
   }
 
-  const transformedSeries = transformData(series);
+  // The expected response is a multi series response, but if there is no data
+  // then we get an object representing a single series with all empty values
+  // (i.e without being grouped by release)
+  const hasReleaseData = series && !('data' in series);
+
+  // Only transform the data is we know there's at least one release
+  const transformedSeries = hasReleaseData
+    ? Object.values(transformData(series, primaryRelease)).sort()
+    : [];
 
   return (
-    <MiniChartPanel title={t('Count')}>
+    <MiniChartPanel
+      title={
+        type === 'cold' ? t('Avg. Cold Start Duration') : t('Avg. Warm Start Duration')
+      }
+    >
       <Chart
-        chartColors={[COLD_START_COLOR, WARM_START_COLOR]}
-        data={Object.values(transformedSeries)}
+        data={transformedSeries}
         height={chartHeight}
         loading={isSeriesLoading}
         grid={{
@@ -116,17 +123,17 @@ function CountWidget({additionalFilters, chartHeight}: Props) {
         showLegend
         definedAxisTicks={2}
         isLineChart
-        aggregateOutputFormat={OUTPUT_TYPE[YAxis.COUNT]}
+        aggregateOutputFormat="duration"
         tooltipFormatterOptions={{
           valueFormatter: value =>
-            tooltipFormatterUsingAggregateOutputType(value, OUTPUT_TYPE[YAxis.COUNT]),
-          nameFormatter: value => SPAN_OP_TO_STRING[value],
+            tooltipFormatterUsingAggregateOutputType(value, 'duration'),
+          nameFormatter: value => formatVersion(value),
         }}
-        legendFormatter={value => SPAN_OP_TO_STRING[value]}
+        legendFormatter={value => formatVersion(value)}
         errored={isError}
       />
     </MiniChartPanel>
   );
 }
 
-export default CountWidget;
+export default StartDurationWidget;

+ 16 - 19
static/app/views/starfish/views/appStartup/screenSummary/widgets.tsx

@@ -14,15 +14,12 @@ import {prepareQueryForLandingPage} from 'sentry/views/performance/data';
 import {useReleaseSelection} from 'sentry/views/starfish/queries/useReleases';
 import {STARFISH_CHART_INTERVAL_FIDELITY} from 'sentry/views/starfish/utils/constants';
 import {appendReleaseFilters} from 'sentry/views/starfish/utils/releaseComparison';
-import CountWidget from 'sentry/views/starfish/views/appStartup/screenSummary/countWidget';
 import DeviceClassBreakdownBarChart from 'sentry/views/starfish/views/appStartup/screenSummary/deviceClassBreakdownBarChart';
-import SystemApplicationBreakdown from 'sentry/views/starfish/views/appStartup/screenSummary/systemApplicationBreakdown';
+import StartDurationWidget from 'sentry/views/starfish/views/appStartup/screenSummary/startDurationWidget';
 import {YAxis, YAXIS_COLUMNS} from 'sentry/views/starfish/views/screens';
 import {useTableQuery} from 'sentry/views/starfish/views/screens/screensTable';
 import {transformDeviceClassEvents} from 'sentry/views/starfish/views/screens/utils';
 
-import AppStartBreakdownWidget from './appStartBreakdownWidget';
-
 const YAXES = [YAxis.COLD_START, YAxis.WARM_START];
 
 function SummaryWidgets({additionalFilters}) {
@@ -84,37 +81,38 @@ function SummaryWidgets({additionalFilters}) {
 
   return (
     <WidgetLayout>
-      <div style={{gridArea: '1 / 1 / 1 / 1'}}>
-        <AppStartBreakdownWidget additionalFilters={additionalFilters} />
+      <div style={{gridArea: '1 / 1'}}>
+        <StartDurationWidget
+          additionalFilters={additionalFilters}
+          chartHeight={100}
+          type="cold"
+        />
       </div>
-      <div style={{gridArea: '1 / 3 / 3 / 3'}}>
-        <CountWidget
+      <div style={{gridArea: '1 / 2'}}>
+        <StartDurationWidget
           additionalFilters={additionalFilters}
-          // 238 aligns the x-axis with other widgets
-          chartHeight={238}
+          chartHeight={100}
+          type="warm"
         />
       </div>
-      <div style={{gridArea: '1 / 2 / 1 / 2'}}>
+      <div style={{gridArea: '2 / 1'}}>
         <DeviceClassBreakdownBarChart
-          title={t('Cold Start')}
+          title={t('Device Classification')}
           isLoading={isLoading}
           isError={isError}
           data={Object.values(transformedData[YAXIS_COLUMNS[YAxis.COLD_START]])}
           yAxis={YAXIS_COLUMNS[YAxis.COLD_START]}
         />
       </div>
-      <div style={{gridArea: '2 / 2 / 2 / 2'}}>
+      <div style={{gridArea: '2 / 2'}}>
         <DeviceClassBreakdownBarChart
-          title={t('Warm Start')}
+          title={t('Device Classification')}
           isLoading={isLoading}
           isError={isError}
           data={Object.values(transformedData[YAXIS_COLUMNS[YAxis.WARM_START]])}
           yAxis={YAXIS_COLUMNS[YAxis.WARM_START]}
         />
       </div>
-      <div style={{gridArea: '2 / 1 / 2 / 1'}}>
-        <SystemApplicationBreakdown additionalFilters={additionalFilters} />
-      </div>
     </WidgetLayout>
   );
 }
@@ -123,9 +121,8 @@ export default SummaryWidgets;
 
 const WidgetLayout = styled('div')`
   display: grid;
-  grid-template-columns: 33% 33% 33%;
   grid-template-rows: 140px 140px;
-  gap: ${space(1)};
+  gap: ${space(1.5)};
 
   ${Panel} {
     height: 100%;

+ 1 - 4
static/app/views/starfish/views/appStartup/screensTable.tsx

@@ -17,13 +17,10 @@ import {useLocation} from 'sentry/utils/useLocation';
 import useOrganization from 'sentry/utils/useOrganization';
 import {normalizeUrl} from 'sentry/utils/withDomainRequired';
 import TopResultsIndicator from 'sentry/views/discover/table/topResultsIndicator';
+import {COLD_START_COLOR, WARM_START_COLOR} from 'sentry/views/starfish/colours';
 import {useReleaseSelection} from 'sentry/views/starfish/queries/useReleases';
 import {formatVersionAndCenterTruncate} from 'sentry/views/starfish/utils/centerTruncate';
 import Breakdown from 'sentry/views/starfish/views/appStartup/breakdown';
-import {
-  COLD_START_COLOR,
-  WARM_START_COLOR,
-} from 'sentry/views/starfish/views/appStartup/screenSummary/appStartBreakdownWidget';
 import {TOP_SCREENS} from 'sentry/views/starfish/views/screens';
 
 const MAX_TABLE_RELEASE_CHARS = 15;