Browse Source

feat(mobile-vitals): Add mobile vitals cards to performance, again. (#27486)

* Revert "Revert "feat(mobile-vitals): Add mobile vitals cards to performance landing (#27252)" (#27361)"

This reverts commit 46ca84e23318246988ffe06b66c6307cf8c73824.
* Fixed function for apdex sometimes including threshold, also stop missing field from breaking the page.
k-fish 3 years ago
parent
commit
16089fde74

+ 2 - 0
src/sentry/search/events/constants.py

@@ -23,6 +23,8 @@ SEMVER_BUILD_ALIAS = "release.build"
 TIMESTAMP_TO_HOUR_ALIAS = "timestamp.to_hour"
 TIMESTAMP_TO_DAY_ALIAS = "timestamp.to_day"
 TRANSACTION_STATUS_ALIAS = "transaction.status"
+MEASUREMENTS_FRAMES_SLOW_RATE = "measurements.frames_slow_rate"
+MEASUREMENTS_FRAMES_FROZEN_RATE = "measurements.frames_frozen_rate"
 
 TAG_KEY_RE = re.compile(r"^tags\[(?P<tag>.*)\]$")
 # Based on general/src/protocol/tags.rs in relay

+ 45 - 1
src/sentry/search/events/fields.py

@@ -30,6 +30,8 @@ from sentry.search.events.constants import (
     ISSUE_ALIAS,
     ISSUE_ID_ALIAS,
     KEY_TRANSACTION_ALIAS,
+    MEASUREMENTS_FRAMES_FROZEN_RATE,
+    MEASUREMENTS_FRAMES_SLOW_RATE,
     PROJECT_ALIAS,
     PROJECT_NAME_ALIAS,
     PROJECT_THRESHOLD_CONFIG_ALIAS,
@@ -438,6 +440,32 @@ FIELD_ALIASES = {
             ),
             result_type="boolean",
         ),
+        PseudoField(
+            MEASUREMENTS_FRAMES_SLOW_RATE,
+            MEASUREMENTS_FRAMES_SLOW_RATE,
+            expression=[
+                "if",
+                [
+                    ["greater", ["measurements.frames_total", 0]],
+                    ["divide", ["measurements.frames_slow", "measurements.frames_total"]],
+                    None,
+                ],
+            ],
+            result_type="percentage",
+        ),
+        PseudoField(
+            MEASUREMENTS_FRAMES_FROZEN_RATE,
+            MEASUREMENTS_FRAMES_FROZEN_RATE,
+            expression=[
+                "if",
+                [
+                    ["greater", ["measurements.frames_total", 0]],
+                    ["divide", ["measurements.frames_frozen", "measurements.frames_total"]],
+                    None,
+                ],
+            ],
+            result_type="percentage",
+        ),
     ]
 }
 
@@ -1158,9 +1186,25 @@ class NumericColumnNoLookup(NumericColumn):
             if value in {"measurements_value", "span_op_breakdowns_value"}:
                 return ["arrayJoin", [value]]
 
+        if value in {MEASUREMENTS_FRAMES_SLOW_RATE, MEASUREMENTS_FRAMES_FROZEN_RATE}:
+            field = FIELD_ALIASES[value]
+            return field.get_expression(params)
+
         super().normalize(value, params)
         return value
 
+    def get_type(self, value):
+        # `measurements.frames_frozen_rate` and `measurements.frames_slow_rate` are aliases
+        # to a percentage value, since they are expressions rather than columns, we special
+        # case them here
+        if isinstance(value, list):
+            for name in {MEASUREMENTS_FRAMES_SLOW_RATE, MEASUREMENTS_FRAMES_FROZEN_RATE}:
+                field = FIELD_ALIASES[name]
+                expression = field.get_expression(None)
+                if expression == value:
+                    return field.result_type
+        return super().get_type(value)
+
 
 class DurationColumn(FunctionArg):
     def normalize(self, value, params):
@@ -1665,7 +1709,7 @@ FUNCTIONS = {
                                             for name in ["ok", "cancelled", "unknown"]
                                         ],
                                     ],
-                                    "transaction_status",
+                                    "transaction.status",
                                 ],
                             ],
                         ],

+ 10 - 1
src/sentry/utils/snuba.py

@@ -1247,7 +1247,16 @@ def resolve_snuba_aliases(snuba_filter, resolve_func, function_translations=None
                 if func_index is not None:
                     # Resolve the columns on the nested function, and add a wrapping
                     # list to become a valid query expression.
-                    formatted.append([argument[0], [resolve_func(col) for col in argument[1]]])
+                    resolved_args = []
+                    for col in argument[1]:
+                        if col is None or isinstance(col, float):
+                            resolved_args.append(col)
+                        elif isinstance(col, list):
+                            resolve_complex_column(col, resolve_func, aggregation_aliases)
+                            resolved_args.append(col)
+                        else:
+                            resolved_args.append(resolve_func(col))
+                    formatted.append([argument[0], resolved_args])
                 else:
                     # Parameter is a list of fields.
                     formatted.append(

+ 5 - 2
static/app/views/performance/data.tsx

@@ -46,6 +46,8 @@ export enum PERFORMANCE_TERM {
   APDEX_NEW = 'apdexNew',
   APP_START_COLD = 'appStartCold',
   APP_START_WARM = 'appStartWarm',
+  SLOW_FRAMES = 'slowFrames',
+  FROZEN_FRAMES = 'frozenFrames',
 }
 
 export type TooltipOption = SelectValue<string> & {
@@ -380,6 +382,8 @@ const PERFORMANCE_TERMS: Record<PERFORMANCE_TERM, TermFormatter> = {
     t('Cold start is a measure of the application start up time from scratch.'),
   appStartWarm: () =>
     t('Warm start is a measure of the application start up time while still in memory.'),
+  slowFrames: () => t('The count of the number of slow frames in the transaction.'),
+  frozenFrames: () => t('The count of the number of frozen frames in the transaction.'),
 };
 
 export function getTermHelp(
@@ -552,9 +556,8 @@ function generateMobilePerformanceEventView(
   ];
 
   const featureFields = organization.features.includes('project-transaction-threshold')
-    ? ['apdex()', 'count_unique(user)', 'count_miserable(user)', 'user_misery()']
+    ? ['count_unique(user)', 'count_miserable(user)', 'user_misery()']
     : [
-        `apdex(${organization.apdexThreshold})`,
         'count_unique(user)',
         `count_miserable(user,${organization.apdexThreshold})`,
         `user_misery(${organization.apdexThreshold})`,

+ 6 - 1
static/app/views/performance/landing/content.tsx

@@ -46,7 +46,7 @@ import {
   LEFT_AXIS_QUERY_KEY,
   RIGHT_AXIS_QUERY_KEY,
 } from './utils';
-import {BackendCards, FrontendCards} from './vitalsCards';
+import {BackendCards, FrontendCards, MobileCards} from './vitalsCards';
 
 type Props = {
   organization: Organization;
@@ -209,6 +209,11 @@ class LandingContent extends Component<Props, State> {
 
     return (
       <Fragment>
+        <MobileCards
+          eventView={eventView}
+          organization={organization}
+          location={location}
+        />
         <DoubleAxisDisplay
           eventView={eventView}
           organization={organization}

+ 33 - 36
static/app/views/performance/landing/utils.tsx

@@ -3,7 +3,6 @@ import {Location} from 'history';
 import {t} from 'app/locale';
 import {LightWeightOrganization, Organization, Project} from 'app/types';
 import EventView from 'app/utils/discover/eventView';
-import {AggregationKey, Column} from 'app/utils/discover/fields';
 import {
   formatAbbreviatedNumber,
   formatFloat,
@@ -86,36 +85,6 @@ export function getChartWidth(chartData: HistogramData, refPixelRect: Rectangle
   };
 }
 
-export function getBackendFunction(
-  functionName: AggregationKey,
-  organization: Organization
-): Column {
-  switch (functionName) {
-    case 'p75':
-      return {
-        kind: 'function',
-        function: ['p75', 'transaction.duration', undefined, undefined],
-      };
-    case 'tpm':
-      return {kind: 'function', function: ['tpm', '', undefined, undefined]};
-    case 'failure_rate':
-      return {kind: 'function', function: ['failure_rate', '', undefined, undefined]};
-    case 'apdex':
-      if (organization.features.includes('project-transaction-threshold')) {
-        return {
-          kind: 'function',
-          function: ['apdex' as AggregationKey, '', undefined, undefined],
-        };
-      }
-      return {
-        kind: 'function',
-        function: ['apdex', `${organization.apdexThreshold}`, undefined, undefined],
-      };
-    default:
-      throw new Error(`Unsupported backend function: ${functionName}`);
-  }
-}
-
 export function getDefaultDisplayFieldForPlatform(
   projects: Project[],
   eventView?: EventView
@@ -136,30 +105,58 @@ export function getDefaultDisplayFieldForPlatform(
   return landingField;
 }
 
-export const backendCardDetails = (organization: LightWeightOrganization) => {
+type VitalCardDetail = {
+  title: string;
+  tooltip: string;
+  formatter: (value: number) => string | number;
+};
+
+export const vitalCardDetails = (
+  organization: LightWeightOrganization
+): {[key: string]: VitalCardDetail | undefined} => {
   return {
-    p75: {
+    'p75(transaction.duration)': {
       title: t('Duration (p75)'),
       tooltip: getTermHelp(organization, PERFORMANCE_TERM.P75),
       formatter: value => getDuration(value / 1000, value >= 1000 ? 3 : 0, true),
     },
-    tpm: {
+    'tpm()': {
       title: t('Throughput'),
       tooltip: getTermHelp(organization, PERFORMANCE_TERM.THROUGHPUT),
       formatter: formatAbbreviatedNumber,
     },
-    failure_rate: {
+    'failure_rate()': {
       title: t('Failure Rate'),
       tooltip: getTermHelp(organization, PERFORMANCE_TERM.FAILURE_RATE),
       formatter: value => formatPercentage(value, 2),
     },
-    apdex: {
+    'apdex()': {
       title: t('Apdex'),
       tooltip: organization.features.includes('project-transaction-threshold')
         ? getTermHelp(organization, PERFORMANCE_TERM.APDEX_NEW)
         : getTermHelp(organization, PERFORMANCE_TERM.APDEX),
       formatter: value => formatFloat(value, 4),
     },
+    'p75(measurements.frames_slow_rate)': {
+      title: t('Slow Frames (p75)'),
+      tooltip: getTermHelp(organization, PERFORMANCE_TERM.SLOW_FRAMES),
+      formatter: value => formatPercentage(value, 2),
+    },
+    'p75(measurements.frames_frozen_rate)': {
+      title: t('Frozen Frames (p75)'),
+      tooltip: getTermHelp(organization, PERFORMANCE_TERM.FROZEN_FRAMES),
+      formatter: value => formatPercentage(value, 2),
+    },
+    'p75(measurements.app_start_cold)': {
+      title: t('Cold Start (p75)'),
+      tooltip: getTermHelp(organization, PERFORMANCE_TERM.APP_START_COLD),
+      formatter: value => getDuration(value / 1000, value >= 1000 ? 3 : 0, true),
+    },
+    'p75(measurements.app_start_warm)': {
+      title: t('Warm Start (p75)'),
+      tooltip: getTermHelp(organization, PERFORMANCE_TERM.APP_START_WARM),
+      formatter: value => getDuration(value / 1000, value >= 1000 ? 3 : 0, true),
+    },
   };
 };
 

+ 74 - 22
static/app/views/performance/landing/vitalsCards.tsx

@@ -1,5 +1,6 @@
 import * as React from 'react';
 import styled from '@emotion/styled';
+import * as Sentry from '@sentry/react';
 import {Location} from 'history';
 
 import {Client} from 'app/api';
@@ -20,7 +21,7 @@ import {Organization, Project} from 'app/types';
 import {getUtcToLocalDateObject} from 'app/utils/dates';
 import DiscoverQuery from 'app/utils/discover/discoverQuery';
 import EventView from 'app/utils/discover/eventView';
-import {getAggregateAlias, WebVital} from 'app/utils/discover/fields';
+import {Column, getAggregateAlias, WebVital} from 'app/utils/discover/fields';
 import {WEB_VITAL_DETAILS} from 'app/utils/performance/vitals/constants';
 import VitalsCardsDiscoverQuery, {
   VitalData,
@@ -41,10 +42,9 @@ import {
 import VitalPercents from '../vitalDetail/vitalPercents';
 
 import {
-  backendCardDetails,
-  getBackendFunction,
   getDefaultDisplayFieldForPlatform,
   LandingDisplayField,
+  vitalCardDetails,
 } from './utils';
 
 type FrontendCardsProps = {
@@ -123,22 +123,19 @@ const VitalBarContainer = styled('div')`
   margin-top: ${space(1.5)};
 `;
 
-type BackendCardsProps = {
+type BaseCardsProps = {
   api: Client;
   eventView: EventView;
   location: Location;
   organization: Organization;
 };
 
-function _BackendCards(props: BackendCardsProps) {
-  const {api, eventView: baseEventView, location, organization} = props;
-  const functionNames = [
-    'p75' as const,
-    'tpm' as const,
-    'failure_rate' as const,
-    'apdex' as const,
-  ];
-  const functions = functionNames.map(fn => getBackendFunction(fn, organization));
+type GenericCardsProps = BaseCardsProps & {
+  functions: Column[];
+};
+
+function GenericCards(props: GenericCardsProps) {
+  const {api, eventView: baseEventView, location, organization, functions} = props;
   const eventView = baseEventView.withColumns(functions);
 
   // construct request parameters for fetching chart data
@@ -184,17 +181,26 @@ function _BackendCards(props: BackendCardsProps) {
               allSeries[oneSeries.seriesName] = oneSeries.data.map(item => item.value);
               return allSeries;
             }, {});
-            const fields = eventView
-              .getFields()
-              .map((fn, i) => [functionNames[i], fn, series?.[fn]]);
+            const details = vitalCardDetails(organization);
 
             return (
               <VitalsContainer>
-                {fields.map(([name, fn, data]) => {
-                  const {title, tooltip, formatter} =
-                    backendCardDetails(organization)[name];
-                  const alias = getAggregateAlias(fn);
+                {eventView.getFields().map(fieldName => {
+                  if (fieldName.includes('apdex')) {
+                    // Replace apdex with explicit thresholds with a generic one for lookup
+                    fieldName = 'apdex()';
+                  }
+
+                  const cardDetail = details[fieldName];
+                  if (!cardDetail) {
+                    Sentry.captureMessage(`Missing field '${fieldName}' in vital cards.`);
+                    return null;
+                  }
+
+                  const {title, tooltip, formatter} = cardDetail;
+                  const alias = getAggregateAlias(fieldName);
                   const rawValue = tableData?.data?.[0]?.[alias];
+                  const data = series?.[fieldName];
                   const value =
                     isSummaryLoading || rawValue === undefined
                       ? '\u2014'
@@ -202,7 +208,7 @@ function _BackendCards(props: BackendCardsProps) {
                   const chart = <SparklineChart data={data} />;
                   return (
                     <VitalCard
-                      key={name}
+                      key={fieldName}
                       title={title}
                       tooltip={tooltip}
                       value={value}
@@ -222,8 +228,54 @@ function _BackendCards(props: BackendCardsProps) {
   );
 }
 
+function _BackendCards(props: BaseCardsProps) {
+  const {organization} = props;
+  const functions: Column[] = [
+    {
+      kind: 'function',
+      function: ['p75', 'transaction.duration', undefined, undefined],
+    },
+    {kind: 'function', function: ['tpm', '', undefined, undefined]},
+    {kind: 'function', function: ['failure_rate', '', undefined, undefined]},
+    organization.features.includes('project-transaction-threshold')
+      ? {
+          kind: 'function',
+          function: ['apdex', '', undefined, undefined],
+        }
+      : {
+          kind: 'function',
+          function: ['apdex', `${organization.apdexThreshold}`, undefined, undefined],
+        },
+  ];
+  return <GenericCards {...props} functions={functions} />;
+}
+
 export const BackendCards = withApi(_BackendCards);
 
+function _MobileCards(props: BaseCardsProps) {
+  const functions: Column[] = [
+    {
+      kind: 'function',
+      function: ['p75', 'measurements.app_start_cold', undefined, undefined],
+    },
+    {
+      kind: 'function',
+      function: ['p75', 'measurements.app_start_warm', undefined, undefined],
+    },
+    {
+      kind: 'function',
+      function: ['p75', 'measurements.frames_slow_rate', undefined, undefined],
+    },
+    {
+      kind: 'function',
+      function: ['p75', 'measurements.frames_frozen_rate', undefined, undefined],
+    },
+  ];
+  return <GenericCards {...props} functions={functions} />;
+}
+
+export const MobileCards = withApi(_MobileCards);
+
 type SparklineChartProps = {
   data: number[];
 };
@@ -353,7 +405,7 @@ const EmptyVitalBar = styled(EmptyStateWarning)`
 type VitalCardProps = {
   title: string;
   tooltip: string;
-  value: string;
+  value: string | number;
   chart: React.ReactNode;
   minHeight?: number;
   horizontal?: boolean;

+ 1 - 1
tests/sentry/search/events/test_fields.py

@@ -1416,7 +1416,7 @@ class ResolveFieldListTest(unittest.TestCase):
                                             for name in ["ok", "cancelled", "unknown"]
                                         ],
                                     ],
-                                    "transaction_status",
+                                    "transaction.status",
                                 ],
                             ],
                         ],

+ 49 - 0
tests/snuba/api/endpoints/test_organization_events_v2.py

@@ -4563,3 +4563,52 @@ class OrganizationEventsV2EndpointTest(APITestCase, SnubaTestCase):
         response = self.do_request(query)
         assert response.status_code == 200
         assert len(response.data["data"]) == 1
+
+    def test_measurements_frame_rates(self):
+        data = load_data("transaction", timestamp=before_now(minutes=1))
+        data["measurements"]["frames_total"] = {"value": 100}
+        data["measurements"]["frames_slow"] = {"value": 10}
+        data["measurements"]["frames_frozen"] = {"value": 5}
+        self.store_event(data, project_id=self.project.id)
+
+        query = {
+            "field": [
+                "measurements.frames_slow_rate",
+                "measurements.frames_frozen_rate",
+            ],
+            "query": "",
+            "project": [self.project.id],
+        }
+        response = self.do_request(query)
+        assert response.status_code == 200
+        data = response.data["data"]
+        assert len(data) == 1
+        assert data[0]["measurements.frames_slow_rate"] == 0.1
+        assert data[0]["measurements.frames_frozen_rate"] == 0.05
+        meta = response.data["meta"]
+        assert meta["measurements.frames_slow_rate"] == "percentage"
+        assert meta["measurements.frames_frozen_rate"] == "percentage"
+
+        query = {
+            "field": [
+                "p75(measurements.frames_slow_rate)",
+                "p75(measurements.frames_frozen_rate)",
+                "percentile(measurements.frames_slow_rate,0.5)",
+                "percentile(measurements.frames_frozen_rate,0.5)",
+            ],
+            "query": "",
+            "project": [self.project.id],
+        }
+        response = self.do_request(query)
+        assert response.status_code == 200
+        data = response.data["data"]
+        assert len(data) == 1
+        assert data[0]["p75_measurements_frames_slow_rate"] == 0.1
+        assert data[0]["p75_measurements_frames_frozen_rate"] == 0.05
+        assert data[0]["percentile_measurements_frames_slow_rate_0_5"] == 0.1
+        assert data[0]["percentile_measurements_frames_frozen_rate_0_5"] == 0.05
+        meta = response.data["meta"]
+        assert meta["p75_measurements_frames_slow_rate"] == "percentage"
+        assert meta["p75_measurements_frames_frozen_rate"] == "percentage"
+        assert meta["percentile_measurements_frames_slow_rate_0_5"] == "percentage"
+        assert meta["percentile_measurements_frames_frozen_rate_0_5"] == "percentage"