Browse Source

fix(trends): add trendsParameter to query to show correct parameter graph (#50305)

<!-- Describe your PR here. -->
Issue that was brought up in @udameli bug bash where switching the
trends parameter didn't switch the trends graph.
### Examples

![image](https://github.com/getsentry/sentry/assets/72356613/68a4a0d9-1f79-4b8e-aa3e-a408baba9e00)

![image](https://github.com/getsentry/sentry/assets/72356613/38d29b62-e0b9-44f1-bca1-585c924cce2d)

### The Fix
The graph using metrics data gets this information via the
trendsParameter which was not being updated in the query. The
trendsParameter is now visible in the query and is being updated as the
user changes the parameter. Now the the graph gets updated with the
correct data when the trend parameter is changed.
nikkikapadia 1 year ago
parent
commit
0d007ebb3e

+ 22 - 9
static/app/views/performance/transactionSummary/transactionOverview/charts.tsx

@@ -21,7 +21,11 @@ import {getTransactionMEPParamsIfApplicable} from 'sentry/views/performance/tran
 import {DisplayModes} from 'sentry/views/performance/transactionSummary/utils';
 import {TransactionsListOption} from 'sentry/views/releases/detail/overview';
 
-import {TrendColumnField, TrendFunctionField} from '../../trends/types';
+import {
+  TrendFunctionField,
+  TrendParameterColumn,
+  TrendParameterLabel,
+} from '../../trends/types';
 import {TRENDS_FUNCTIONS, TRENDS_PARAMETERS} from '../../trends/utils';
 import {SpanOperationBreakdownFilter} from '../filter';
 
@@ -112,13 +116,16 @@ function TransactionSummaryCharts({
   function handleTrendColumnChange(value: string) {
     browserHistory.push({
       pathname: location.pathname,
-      query: {...location.query, trendColumn: value},
+      query: {
+        ...location.query,
+        trendParameter: value,
+      },
     });
   }
 
   const TREND_PARAMETERS_OPTIONS: SelectValue<string>[] = TRENDS_PARAMETERS.map(
-    ({column, label}) => ({
-      value: column,
+    ({label}) => ({
+      value: label,
       label,
     })
   );
@@ -128,8 +135,8 @@ function TransactionSummaryCharts({
     location.query.trendFunction,
     TREND_FUNCTIONS_OPTIONS[0].value
   ) as TrendFunctionField;
-  let trendColumn = decodeScalar(
-    location.query.trendColumn,
+  let trendParameter = decodeScalar(
+    location.query.trendParameter,
     TREND_PARAMETERS_OPTIONS[0].value
   );
 
@@ -139,10 +146,16 @@ function TransactionSummaryCharts({
   if (!Object.values(TrendFunctionField).includes(trendFunction)) {
     trendFunction = TrendFunctionField.P50;
   }
-  if (!Object.values(TrendColumnField).includes(trendColumn as TrendColumnField)) {
-    trendColumn = TrendColumnField.DURATION;
+  if (
+    !Object.values(TrendParameterLabel).includes(trendParameter as TrendParameterLabel)
+  ) {
+    trendParameter = TrendParameterLabel.DURATION;
   }
 
+  const trendColumn =
+    TRENDS_PARAMETERS.find(parameter => parameter.label === trendParameter)?.column ||
+    TrendParameterColumn.DURATION;
+
   const releaseQueryExtra = {
     yAxis: display === DisplayModes.VITALS ? 'countVital' : 'countDuration',
     showTransactions:
@@ -267,7 +280,7 @@ function TransactionSummaryCharts({
           {display === DisplayModes.TREND && (
             <OptionSelector
               title={t('Parameter')}
-              selected={trendColumn}
+              selected={trendParameter}
               options={TREND_PARAMETERS_OPTIONS}
               onChange={handleTrendColumnChange}
             />

+ 6 - 4
static/app/views/performance/trends/changedTransactions.tsx

@@ -43,8 +43,8 @@ import Chart from './chart';
 import {
   NormalizedTrendsTransaction,
   TrendChangeType,
-  TrendColumnField,
   TrendFunctionField,
+  TrendParameterColumn,
   TrendsStats,
   TrendView,
 } from './types';
@@ -68,7 +68,7 @@ type Props = {
   setError: (msg: string | undefined) => void;
   trendChangeType: TrendChangeType;
   trendView: TrendView;
-  previousTrendColumn?: TrendColumnField;
+  previousTrendColumn?: TrendParameterColumn;
   previousTrendFunction?: TrendFunctionField;
   withBreakpoint?: boolean;
 };
@@ -573,8 +573,8 @@ function TransactionSummaryLink(props: TransactionSummaryLinkProps) {
     trendView: eventView,
     transaction,
     projects,
+    location,
     currentTrendFunction,
-    currentTrendColumn,
   } = props;
   const summaryView = eventView.clone();
   const projectID = getTrendProjectId(transaction, projects);
@@ -585,7 +585,9 @@ function TransactionSummaryLink(props: TransactionSummaryLinkProps) {
     projectID,
     display: DisplayModes.TREND,
     trendFunction: currentTrendFunction,
-    trendColumn: currentTrendColumn,
+    additionalQuery: {
+      trendParameter: location.query.trendParameter?.toString(),
+    },
   });
 
   return (

+ 2 - 2
static/app/views/performance/trends/index.spec.tsx

@@ -304,7 +304,7 @@ describe('Performance > Trends', function () {
 
     expect(summaryLink.closest('a')).toHaveAttribute(
       'href',
-      '/organizations/org-slug/performance/summary/?display=trend&project=1&query=tpm%28%29%3A%3E0.01%20transaction.duration%3A%3E0%20transaction.duration%3A%3C15min&referrer=performance-transaction-summary&statsPeriod=14d&transaction=%2Forganizations%2F%3AorgId%2Fperformance%2F&trendColumn=transaction.duration&trendFunction=p50&unselectedSeries=p100%28%29'
+      '/organizations/org-slug/performance/summary/?display=trend&project=1&query=tpm%28%29%3A%3E0.01%20transaction.duration%3A%3E0%20transaction.duration%3A%3C15min&referrer=performance-transaction-summary&statsPeriod=14d&transaction=%2Forganizations%2F%3AorgId%2Fperformance%2F&trendFunction=p50&unselectedSeries=p100%28%29'
     );
   });
 
@@ -545,7 +545,7 @@ describe('Performance > Trends', function () {
     );
 
     for (const parameter of TRENDS_PARAMETERS) {
-      if (Object.values(WebVital).includes(parameter.column as WebVital)) {
+      if (Object.values(WebVital).includes(parameter.column as string as WebVital)) {
         trendsStatsMock.mockReset();
 
         const newLocation = {

+ 15 - 3
static/app/views/performance/trends/types.tsx

@@ -27,8 +27,8 @@ export type TrendFunction = {
 };
 
 export type TrendParameter = {
-  column: string;
-  label: string;
+  column: TrendParameterColumn;
+  label: TrendParameterLabel;
 };
 
 export enum TrendChangeType {
@@ -44,7 +44,7 @@ export enum TrendFunctionField {
   AVG = 'avg',
 }
 
-export enum TrendColumnField {
+export enum TrendParameterColumn {
   DURATION = 'transaction.duration',
   LCP = 'measurements.lcp',
   FCP = 'measurements.fcp',
@@ -56,6 +56,18 @@ export enum TrendColumnField {
   SPANS_RESOURCE = 'spans.resource',
 }
 
+export enum TrendParameterLabel {
+  DURATION = 'Duration',
+  LCP = 'LCP',
+  FCP = 'FCP',
+  FID = 'FID',
+  CLS = 'CLS',
+  SPANS_DB = 'Spans (db)',
+  SPANS_HTTP = 'Spans (http)',
+  SPANS_BROWSER = 'Spans (browser)',
+  SPANS_RESOURCE = 'Spans (resource)',
+}
+
 export type TrendStat = {
   data: EventsStatsData;
   order: number;

+ 12 - 9
static/app/views/performance/trends/utils.spec.tsx

@@ -1,4 +1,7 @@
-import {TrendColumnField} from 'sentry/views/performance/trends/types';
+import {
+  TrendParameterColumn,
+  TrendParameterLabel,
+} from 'sentry/views/performance/trends/types';
 import {
   getCurrentTrendParameter,
   performanceTypeToTrendParameterLabel,
@@ -9,13 +12,13 @@ describe('Trend parameter utils', function () {
   describe('performanceTypeToTrendParameterLabel', function () {
     it('returns correct trend parameter label based on performance type', function () {
       const lcp = {
-        label: 'LCP',
-        column: TrendColumnField.LCP,
+        label: TrendParameterLabel.LCP,
+        column: TrendParameterColumn.LCP,
       };
 
       const duration = {
-        label: 'Duration',
-        column: TrendColumnField.DURATION,
+        label: TrendParameterLabel.DURATION,
+        column: TrendParameterColumn.DURATION,
       };
 
       const frontendProjectOutput = performanceTypeToTrendParameterLabel(
@@ -49,8 +52,8 @@ describe('Trend parameter utils', function () {
     it('returns trend parameter from location', () => {
       const location = TestStubs.location({query: {trendParameter: 'FCP'}});
       const expectedTrendParameter = {
-        label: 'FCP',
-        column: TrendColumnField.FCP,
+        label: TrendParameterLabel.FCP,
+        column: TrendParameterColumn.FCP,
       };
       // project with performance type 'any'
       const projects = [TestStubs.Project({id: 1, platform: null})];
@@ -62,8 +65,8 @@ describe('Trend parameter utils', function () {
     it('returns default trend parameter based on project type if no trend parameter set in location', function () {
       const location = TestStubs.location();
       const expectedTrendParameter = {
-        label: 'Duration',
-        column: TrendColumnField.DURATION,
+        label: TrendParameterLabel.DURATION,
+        column: TrendParameterColumn.DURATION,
       };
       // project with performance type 'any'
       const projects = [TestStubs.Project({id: 1, platform: null})];

+ 24 - 23
static/app/views/performance/trends/utils.tsx

@@ -22,10 +22,11 @@ import {platformToPerformanceType, PROJECT_PERFORMANCE_TYPE} from '../utils';
 import {
   NormalizedTrendsTransaction,
   TrendChangeType,
-  TrendColumnField,
   TrendFunction,
   TrendFunctionField,
   TrendParameter,
+  TrendParameterColumn,
+  TrendParameterLabel,
   TrendsTransaction,
   TrendView,
 } from './types';
@@ -68,40 +69,40 @@ export const TRENDS_FUNCTIONS: TrendFunction[] = [
 
 export const TRENDS_PARAMETERS: TrendParameter[] = [
   {
-    label: 'Duration',
-    column: TrendColumnField.DURATION,
+    label: TrendParameterLabel.DURATION,
+    column: TrendParameterColumn.DURATION,
   },
   {
-    label: 'LCP',
-    column: TrendColumnField.LCP,
+    label: TrendParameterLabel.LCP,
+    column: TrendParameterColumn.LCP,
   },
   {
-    label: 'FCP',
-    column: TrendColumnField.FCP,
+    label: TrendParameterLabel.FCP,
+    column: TrendParameterColumn.FCP,
   },
   {
-    label: 'FID',
-    column: TrendColumnField.FID,
+    label: TrendParameterLabel.FID,
+    column: TrendParameterColumn.FID,
   },
   {
-    label: 'CLS',
-    column: TrendColumnField.CLS,
+    label: TrendParameterLabel.CLS,
+    column: TrendParameterColumn.CLS,
   },
   {
-    label: 'Spans (http)',
-    column: TrendColumnField.SPANS_HTTP,
+    label: TrendParameterLabel.SPANS_HTTP,
+    column: TrendParameterColumn.SPANS_HTTP,
   },
   {
-    label: 'Spans (db)',
-    column: TrendColumnField.SPANS_DB,
+    label: TrendParameterLabel.SPANS_DB,
+    column: TrendParameterColumn.SPANS_DB,
   },
   {
-    label: 'Spans (browser)',
-    column: TrendColumnField.SPANS_BROWSER,
+    label: TrendParameterLabel.SPANS_BROWSER,
+    column: TrendParameterColumn.SPANS_BROWSER,
   },
   {
-    label: 'Spans (resource)',
-    column: TrendColumnField.SPANS_RESOURCE,
+    label: TrendParameterLabel.SPANS_RESOURCE,
+    column: TrendParameterColumn.SPANS_RESOURCE,
   },
 ];
 
@@ -181,16 +182,16 @@ export function performanceTypeToTrendParameterLabel(
   switch (performanceType) {
     case PROJECT_PERFORMANCE_TYPE.FRONTEND:
       return {
-        label: 'LCP',
-        column: TrendColumnField.LCP,
+        label: TrendParameterLabel.LCP,
+        column: TrendParameterColumn.LCP,
       };
     case PROJECT_PERFORMANCE_TYPE.ANY:
     case PROJECT_PERFORMANCE_TYPE.BACKEND:
     case PROJECT_PERFORMANCE_TYPE.FRONTEND_OTHER:
     default:
       return {
-        label: 'Duration',
-        column: TrendColumnField.DURATION,
+        label: TrendParameterLabel.DURATION,
+        column: TrendParameterColumn.DURATION,
       };
   }
 }