Browse Source

feat(perf): Improve suspect tags to change column layout (#25371)

* feat(perf): Improve suspect tags to change column layout

This will switch to more simplified view of suspect tags, removing the facets visualization as well as the sort dropdown as we can add column level sorting in a future PR.

Other:
- Fixed frequency percents when sampling
- Increased min transaction count before adjusting sampling
- Added ops breakdown to column dropdown (will add inferred column from platform later)
k-fish 3 years ago
parent
commit
1de63b9846

+ 10 - 6
src/sentry/snuba/discover.py

@@ -817,7 +817,10 @@ def get_performance_facets(
             referrer="{}.{}".format(referrer, "all_transactions"),
         )
         counts = [r["count"] for r in key_names["data"]]
-        if len(counts) != 1 or counts[0] == 0:
+        aggregates = [r["aggregate"] for r in key_names["data"]]
+
+        # Return early to avoid doing more queries with 0 count transactions or aggregates for columns that dont exist
+        if len(counts) != 1 or counts[0] == 0 or aggregates[0] is None:
             return []
 
     results = []
@@ -828,12 +831,13 @@ def get_performance_facets(
 
     # Dynamically sample so at least 10000 transactions are selected
     transaction_count = key_names["data"][0]["count"]
-    sampling_enabled = transaction_count > 10000
-    # Log growth starting at 10,000
-    target_sample = 10000 * (math.log(transaction_count, 10) - 3)
+    sampling_enabled = transaction_count > 50000
+    # Log growth starting at 50,000
+    target_sample = 50000 * (math.log(transaction_count, 10) - 3)
 
     dynamic_sample_rate = 0 if transaction_count <= 0 else (target_sample / transaction_count)
     sample_rate = dynamic_sample_rate if sampling_enabled else None
+    frequency_sample_rate = sample_rate if sample_rate else 1
 
     excluded_tags = [
         "tags_key",
@@ -882,7 +886,7 @@ def get_performance_facets(
             referrer="{}.{}".format(referrer, "tag_values"),
             sample=sample_rate,
             turbo=sample_rate is not None,
-            limitby=[5, "tags_key"],
+            limitby=[1, "tags_key"],
         )
         results.extend(
             [
@@ -890,7 +894,7 @@ def get_performance_facets(
                     key=r["tags_key"],
                     value=r["tags_value"],
                     performance=float(r["aggregate"]),
-                    frequency=float(r["cnt"] / transaction_count),
+                    frequency=float((r["cnt"] / frequency_sample_rate) / transaction_count),
                     comparison=float(r["aggregate"] / transaction_aggregate),
                     sumdelta=float(r["sumdelta"]),
                 )

+ 5 - 19
static/app/utils/performance/segmentExplorer/segmentExplorerQuery.tsx

@@ -21,6 +21,7 @@ type IncomingTopValue = {
   aggregate: number;
   count: number;
   comparison: number;
+  sumdelta: number;
   isOther?: boolean;
 };
 
@@ -35,6 +36,7 @@ export type TableDataRow = IncomingDataRow & {
   frequency: number;
   comparison: number;
   otherValues: TopValue[];
+  totalTimeLost: number;
 };
 
 export type TagHint = {
@@ -54,7 +56,6 @@ type ChildrenProps = Omit<GenericChildrenProps<TableData>, 'tableData'> & {
 };
 
 type QueryProps = DiscoverQueryProps & {
-  tagOrder: string;
   aggregateColumn: string;
   children: (props: ChildrenProps) => React.ReactNode;
 };
@@ -66,11 +67,10 @@ type FacetQuery = LocationQuery &
   };
 
 export function getRequestFunction(_props: QueryProps) {
-  const {tagOrder, aggregateColumn} = _props;
+  const {aggregateColumn} = _props;
   function getTagExplorerRequestPayload(props: DiscoverQueryProps) {
     const {eventView} = props;
     const apiPayload: FacetQuery = eventView.getEventsAPIPayload(props.location);
-    apiPayload.order = tagOrder;
     apiPayload.aggregateColumn = aggregateColumn;
     return apiPayload;
   }
@@ -78,10 +78,7 @@ export function getRequestFunction(_props: QueryProps) {
 }
 
 function shouldRefetchData(prevProps: QueryProps, nextProps: QueryProps) {
-  return (
-    prevProps.tagOrder !== nextProps.tagOrder ||
-    prevProps.aggregateColumn !== nextProps.aggregateColumn
-  );
+  return prevProps.aggregateColumn !== nextProps.aggregateColumn;
 }
 
 function afterFetch(data: IncomingTableData) {
@@ -92,18 +89,7 @@ function afterFetch(data: IncomingTableData) {
     row.aggregate = firstItem.aggregate;
     row.frequency = firstItem.count;
     row.comparison = firstItem.comparison;
-    row.otherValues = row.topValues.slice(0);
-    const otherEventValue = row.topValues.reduce((acc, curr) => acc - curr.count, 1);
-    if (otherEventValue > 0.01) {
-      row.otherValues.push({
-        name: 'other',
-        value: 'other',
-        isOther: true,
-        aggregate: 0,
-        count: otherEventValue,
-        comparison: 0,
-      });
-    }
+    row.totalTimeLost = firstItem.sumdelta;
     return row;
   });
 }

+ 46 - 167
static/app/views/performance/transactionSummary/tagExplorer.tsx

@@ -1,9 +1,8 @@
 import React from 'react';
 import {browserHistory} from 'react-router';
 import styled from '@emotion/styled';
-import {Location, LocationDescriptor} from 'history';
+import {Location} from 'history';
 
-import {TagSegment} from 'app/actionCreators/events';
 import DropdownButton from 'app/components/dropdownButton';
 import DropdownControl, {DropdownItem} from 'app/components/dropdownControl';
 import GridEditable from 'app/components/gridEditable';
@@ -43,6 +42,14 @@ const COLUMN_ORDER = [
       kind: 'field',
     },
   },
+  {
+    key: 'aggregate',
+    name: 'Avg Duration',
+    width: -1,
+    column: {
+      kind: 'field',
+    },
+  },
   {
     key: 'frequency',
     name: 'Frequency',
@@ -53,15 +60,15 @@ const COLUMN_ORDER = [
   },
   {
     key: 'comparison',
-    name: 'Comparison',
+    name: 'Comparison To Avg',
     width: -1,
     column: {
       kind: 'field',
     },
   },
   {
-    key: 'otherValues',
-    name: 'Facet Map',
+    key: 'totalTimeLost',
+    name: 'Total Time Lost',
     width: -1,
     column: {
       kind: 'field',
@@ -69,29 +76,30 @@ const COLUMN_ORDER = [
   },
 ];
 
-const HEADER_OPTIONS: DropdownOption[] = [
+const DURATION_OPTIONS: DropdownOption[] = [
   {
-    label: 'Suspect Tag Values',
-    value: '-sumdelta',
+    label: 'transaction.duration',
+    value: 'duration',
   },
   {
-    label: 'Slowest Tag Values',
-    value: '-aggregate',
+    label: 'measurements.lcp',
+    value: 'measurements[lcp]',
   },
   {
-    label: 'Fastest Tag Values',
-    value: 'aggregate',
+    label: 'spans.browser',
+    value: 'span_op_breakdowns[ops.browser]',
   },
-];
-
-const DURATION_OPTIONS: DropdownOption[] = [
   {
-    label: 'transaction.duration',
-    value: 'duration',
+    label: 'spans.db',
+    value: 'span_op_breakdowns[ops.db]',
   },
   {
-    label: 'measurements.lcp',
-    value: 'measurements[lcp]',
+    label: 'spans.http',
+    value: 'span_op_breakdowns[ops.http]',
+  },
+  {
+    label: 'spans.resource',
+    value: 'span_op_breakdowns[ops.resource]',
   },
 ];
 
@@ -154,106 +162,16 @@ const renderBodyCell = (
       </Tooltip>
     );
   }
+  if (column.key === 'aggregate') {
+    return <PerformanceDuration abbreviation milliseconds={dataRow.aggregate} />;
+  }
 
-  if (column.key === 'otherValues' && Array.isArray(value)) {
-    const converted = dataRow.otherValues;
-    const segments = converted.map(val => {
-      return {
-        count: val.count,
-        name: val.name,
-        key: dataRow.key,
-        value: val.value,
-        isOther: val.isOther,
-        url: '',
-      };
-    });
-
-    return <SegmentVisualization location={location} segments={segments} />;
+  if (column.key === 'totalTimeLost') {
+    return <PerformanceDuration abbreviation milliseconds={dataRow.totalTimeLost} />;
   }
   return value;
 };
 
-type SegmentValue = {
-  to: LocationDescriptor;
-  onClick: () => void;
-  index: number;
-};
-type SegmentProps = {
-  location: Location;
-  segments: TagSegment[];
-};
-const SegmentVisualization = (props: SegmentProps) => {
-  const {location, segments} = props;
-  return (
-    <SegmentBar>
-      {segments.map((value, index) => {
-        const pct = value.count * 100;
-        const pctLabel = Math.floor(pct);
-        const renderTooltipValue = () => {
-          return value.name || t('n/a');
-        };
-
-        const tooltipHtml = (
-          <React.Fragment>
-            <div className="truncate">{renderTooltipValue()}</div>
-            {pctLabel}%
-          </React.Fragment>
-        );
-
-        const segmentProps: SegmentValue = {
-          index,
-          to: value.url,
-          onClick: () => handleTagValueClick(location, value.key ?? '', value.value),
-        };
-
-        return (
-          <div key={value.value} style={{width: pct + '%'}}>
-            <Tooltip title={tooltipHtml} containerDisplayMode="block">
-              {value.isOther ? <OtherSegment /> : <Segment {...segmentProps} />}
-            </Tooltip>
-          </div>
-        );
-      })}
-    </SegmentBar>
-  );
-};
-
-const SegmentBar = styled('div')`
-  display: flex;
-  overflow: hidden;
-  border-radius: 2px;
-`;
-
-const COLORS = [
-  '#3A3387',
-  '#5F40A3',
-  '#8C4FBD',
-  '#B961D3',
-  '#DE76E4',
-  '#EF91E8',
-  '#F7B2EC',
-  '#FCD8F4',
-  '#FEEBF9',
-];
-
-const OtherSegment = styled('span')`
-  display: block;
-  width: 100%;
-  height: 16px;
-  color: inherit;
-  outline: none;
-  background-color: ${COLORS[COLORS.length - 1]};
-`;
-
-const Segment = styled(Link)<SegmentValue>`
-  display: block;
-  width: 100%;
-  height: 16px;
-  color: inherit;
-  outline: none;
-  background-color: ${p => COLORS[p.index]};
-`;
-
 const renderBodyCellWithData = (parentProps: Props) => {
   return (
     column: TableColumn<keyof TableDataRow>,
@@ -279,22 +197,14 @@ type Props = {
 };
 
 type State = {
-  tagOrder: string;
   aggregateColumn: string;
 };
 
 class _TagExplorer extends React.Component<Props, State> {
   state: State = {
-    tagOrder: HEADER_OPTIONS[0].value,
     aggregateColumn: DURATION_OPTIONS[0].value,
   };
 
-  setTagOrder(value: string) {
-    this.setState({
-      tagOrder: value,
-    });
-  }
-
   setAggregateColumn(value: string) {
     this.setState({
       aggregateColumn: value,
@@ -303,14 +213,10 @@ class _TagExplorer extends React.Component<Props, State> {
 
   render() {
     const {eventView, organization, location} = this.props;
-    const {tagOrder, aggregateColumn} = this.state;
+    const {aggregateColumn} = this.state;
     const handleCursor = () => {};
 
-    const sortDropdownOptions = HEADER_OPTIONS;
     const columnDropdownOptions = DURATION_OPTIONS;
-
-    const selectedSort =
-      sortDropdownOptions.find(o => o.value === tagOrder) || sortDropdownOptions[0];
     const selectedColumn =
       columnDropdownOptions.find(o => o.value === aggregateColumn) ||
       columnDropdownOptions[0];
@@ -320,7 +226,6 @@ class _TagExplorer extends React.Component<Props, State> {
         eventView={eventView}
         orgSlug={organization.slug}
         location={location}
-        tagOrder={tagOrder}
         aggregateColumn={aggregateColumn}
         limit={5}
       >
@@ -328,11 +233,8 @@ class _TagExplorer extends React.Component<Props, State> {
           return (
             <React.Fragment>
               <TagsHeader
-                selectedSort={selectedSort}
-                sortOptions={sortDropdownOptions}
                 selectedColumn={selectedColumn}
                 columnOptions={columnDropdownOptions}
-                handleSortDropdownChange={(v: string) => this.setTagOrder(v)}
                 handleColumnDropdownChange={(v: string) => this.setAggregateColumn(v)}
               />
               <GridEditable
@@ -364,49 +266,15 @@ type DropdownOption = {
 };
 
 type HeaderProps = {
-  selectedSort: DropdownOption;
-  sortOptions: DropdownOption[];
   selectedColumn: DropdownOption;
   columnOptions: DropdownOption[];
-  handleSortDropdownChange: (k: string) => void;
   handleColumnDropdownChange: (k: string) => void;
 };
 function TagsHeader(props: HeaderProps) {
-  const {
-    selectedSort,
-    sortOptions,
-    selectedColumn,
-    columnOptions,
-    handleSortDropdownChange,
-    handleColumnDropdownChange,
-  } = props;
+  const {selectedColumn, columnOptions, handleColumnDropdownChange} = props;
   return (
     <Header>
-      <DropdownControl
-        data-test-id="sort-tag-values"
-        button={({isOpen, getActorProps}) => (
-          <StyledDropdownButton
-            {...getActorProps()}
-            isOpen={isOpen}
-            prefix={t('Sort')}
-            size="small"
-          >
-            {selectedSort.label}
-          </StyledDropdownButton>
-        )}
-      >
-        {sortOptions.map(({value, label}) => (
-          <DropdownItem
-            data-test-id={`option-${value}`}
-            key={value}
-            onSelect={handleSortDropdownChange}
-            eventKey={value}
-            isActive={value === selectedSort.value}
-          >
-            {label}
-          </DropdownItem>
-        ))}
-      </DropdownControl>
+      <SectionHeading>{t('Suspect Tags')}</SectionHeading>
       <DropdownControl
         data-test-id="tag-column-performance"
         button={({isOpen, getActorProps}) => (
@@ -436,6 +304,17 @@ function TagsHeader(props: HeaderProps) {
   );
 }
 
+export const SectionHeading = styled('h4')`
+  display: inline-grid;
+  grid-auto-flow: column;
+  grid-gap: ${space(1)};
+  align-items: center;
+  color: ${p => p.theme.subText};
+  font-size: ${p => p.theme.fontSizeMedium};
+  margin: ${space(1)} 0;
+  line-height: 1.3;
+`;
+
 const Header = styled('div')`
   display: flex;
   justify-content: space-between;

+ 8 - 3
static/app/views/performance/utils.tsx

@@ -88,8 +88,9 @@ export function getTransactionName(location: Location): string | undefined {
   return decodeScalar(transaction);
 }
 
-type SecondsProps = {seconds: number};
-type MillisecondsProps = {milliseconds: number};
+type DurationProps = {abbreviation?: boolean};
+type SecondsProps = {seconds: number} & DurationProps;
+type MillisecondsProps = {milliseconds: number} & DurationProps;
 type PerformanceDurationProps = SecondsProps | MillisecondsProps;
 const hasMilliseconds = (props: PerformanceDurationProps): props is MillisecondsProps => {
   return defined((props as MillisecondsProps).milliseconds);
@@ -101,6 +102,10 @@ export function PerformanceDuration(props: PerformanceDurationProps) {
     ? props.milliseconds / 1000
     : props.seconds;
   return (
-    <Duration seconds={normalizedSeconds} fixedDigits={normalizedSeconds > 1 ? 2 : 0} />
+    <Duration
+      abbreviation={props.abbreviation}
+      seconds={normalizedSeconds}
+      fixedDigits={normalizedSeconds > 1 ? 2 : 0}
+    />
   );
 }

+ 1 - 1
tests/sentry/snuba/test_discover.py

@@ -2973,7 +2973,7 @@ class GetPerformanceFacetsTest(SnubaTestCase, TestCase):
 
         result = discover.get_performance_facets("", params)
 
-        assert len(result) == 11
+        assert len(result) == 10
         for r in result:
             if r.key == "color" and r.value == "red":
                 assert r.frequency == 0.5