Browse Source

ref(starfish): Simplify `EventView` creation (#52376)

It's redundant to pass both a `location` and `pageFilters` object when
creating event views for Starfish. The reason we pass both is that
there's no way to auto-determine an interval from a `location`, and no
way to create an event view from a `pageFilters`. So, I added a way to
create an `EventView` from a `pageFilters`, and now we don't need to
pass `location` around anymore in places where we both have to create
event views and determine the intervals.

Also, `location` is a primitive, IMO it's better to use `pageFilters`,
it contains the stuff we want. This is a bit of cleanup post removing
hard-coded project IDs.

Co-authored-by: Dominik Buszowiecki <44422760+DominikB2014@users.noreply.github.com>
George Gritsouk 1 year ago
parent
commit
7a362f2f11

+ 15 - 0
fixtures/js-stubs/pageFilters.ts

@@ -0,0 +1,15 @@
+import type {PageFilters as PageFilterType} from 'sentry/types';
+
+export function PageFilters(params: Partial<PageFilterType> = {}): PageFilterType {
+  return {
+    datetime: {
+      end: null,
+      period: null,
+      start: null,
+      utc: null,
+    },
+    environments: [],
+    projects: [],
+    ...params,
+  };
+}

+ 1 - 0
fixtures/js-stubs/types.tsx

@@ -104,6 +104,7 @@ type TestStubFixtures = {
   OutcomesWithLowProcessedEvents: SimpleStub;
   OutcomesWithReason: SimpleStub;
   OutcomesWithoutClientDiscarded: SimpleStub;
+  PageFilters: OverridableStub;
   PhabricatorCreate: SimpleStub;
   PhabricatorPlugin: SimpleStub;
   PlatformExternalIssue: OverridableStub;

+ 58 - 0
static/app/utils/discover/eventView.spec.tsx

@@ -503,6 +503,64 @@ describe('EventView.fromSavedQuery()', function () {
   });
 });
 
+describe('EventView.fromNewQueryWithPageFilters()', function () {
+  const prebuiltQuery: NewQuery = {
+    id: undefined,
+    name: 'Page Filter Events',
+    query: '',
+    projects: undefined,
+    fields: ['title', 'project', 'timestamp'],
+    orderby: '-timestamp',
+    version: 2,
+  };
+
+  it('maps basic properties of a prebuilt query', function () {
+    const pageFilters = TestStubs.PageFilters();
+
+    const eventView = EventView.fromNewQueryWithPageFilters(prebuiltQuery, pageFilters);
+
+    expect(eventView).toMatchObject({
+      id: undefined,
+      name: 'Page Filter Events',
+      fields: [{field: 'title'}, {field: 'project'}, {field: 'timestamp'}],
+      sorts: [{field: 'timestamp', kind: 'desc'}],
+      query: '',
+      project: [],
+      start: undefined,
+      end: undefined,
+      statsPeriod: '14d',
+      environment: [],
+      yAxis: undefined,
+    });
+  });
+
+  it('merges page filter values', function () {
+    const pageFilters = TestStubs.PageFilters({
+      datetime: {
+        period: '3d',
+      },
+      projects: [42],
+      environments: ['prod'],
+    });
+
+    const eventView = EventView.fromNewQueryWithPageFilters(prebuiltQuery, pageFilters);
+
+    expect(eventView).toMatchObject({
+      id: undefined,
+      name: 'Page Filter Events',
+      fields: [{field: 'title'}, {field: 'project'}, {field: 'timestamp'}],
+      sorts: [{field: 'timestamp', kind: 'desc'}],
+      query: '',
+      project: [42],
+      start: undefined,
+      end: undefined,
+      statsPeriod: '3d',
+      environment: ['prod'],
+      yAxis: undefined,
+    });
+  });
+});
+
 describe('EventView.fromNewQueryWithLocation()', function () {
   const prebuiltQuery: NewQuery = {
     id: undefined,

+ 12 - 0
static/app/utils/discover/eventView.tsx

@@ -433,6 +433,18 @@ class EventView {
     return EventView.fromSavedQuery(saved);
   }
 
+  static fromNewQueryWithPageFilters(newQuery: NewQuery, pageFilters: PageFilters) {
+    return EventView.fromSavedQuery({
+      ...newQuery,
+      environment: newQuery.environment ?? pageFilters.environments,
+      projects: newQuery.projects ?? pageFilters.projects,
+      start: newQuery.start ?? pageFilters.datetime.start ?? undefined,
+      end: newQuery.end ?? pageFilters.datetime.end ?? undefined,
+      range: newQuery.range ?? pageFilters.datetime.period ?? undefined,
+      utc: newQuery.utc ?? pageFilters.datetime.utc ?? undefined,
+    });
+  }
+
   static getFields(saved: NewQuery | SavedQuery) {
     return saved.fields.map((field, i) => {
       const width =

+ 3 - 7
static/app/views/starfish/queries/useSpanMetricsSeries.tsx

@@ -1,4 +1,3 @@
-import {Location} from 'history';
 import keyBy from 'lodash/keyBy';
 
 import {getInterval} from 'sentry/components/charts/utils';
@@ -6,7 +5,6 @@ import {PageFilters} from 'sentry/types';
 import {Series} from 'sentry/types/echarts';
 import EventView from 'sentry/utils/discover/eventView';
 import {DiscoverDatasets} from 'sentry/utils/discover/types';
-import {useLocation} from 'sentry/utils/useLocation';
 import usePageFilters from 'sentry/utils/usePageFilters';
 import type {IndexedSpan} from 'sentry/views/starfish/queries/types';
 import {SpanSummaryQueryFilters} from 'sentry/views/starfish/queries/useSpanMetrics';
@@ -30,11 +28,10 @@ export const useSpanMetricsSeries = (
   yAxis: string[] = [],
   referrer = 'span-metrics-series'
 ) => {
-  const location = useLocation();
   const pageFilters = usePageFilters();
 
   const eventView = span
-    ? getEventView(span, location, pageFilters.selection, yAxis, queryFilters)
+    ? getEventView(span, pageFilters.selection, yAxis, queryFilters)
     : undefined;
 
   const enabled =
@@ -68,14 +65,13 @@ export const useSpanMetricsSeries = (
 
 function getEventView(
   span: {group: string},
-  location: Location,
   pageFilters: PageFilters,
   yAxis: string[],
   queryFilters: SpanSummaryQueryFilters
 ) {
   const cleanGroupId = span.group.replaceAll('-', '').slice(-16);
 
-  return EventView.fromNewQueryWithLocation(
+  return EventView.fromNewQueryWithPageFilters(
     {
       name: '',
       query: `${SPAN_GROUP}:${cleanGroupId}${
@@ -93,6 +89,6 @@ function getEventView(
       interval: getInterval(pageFilters.datetime, STARFISH_CHART_INTERVAL_FIDELITY),
       version: 2,
     },
-    location
+    pageFilters
   );
 }

+ 6 - 5
static/app/views/starfish/views/spans/queries.tsx

@@ -4,14 +4,12 @@ import {getInterval} from 'sentry/components/charts/utils';
 import {NewQuery} from 'sentry/types';
 import EventView from 'sentry/utils/discover/eventView';
 import {DiscoverDatasets} from 'sentry/utils/discover/types';
-import {useLocation} from 'sentry/utils/useLocation';
 import usePageFilters from 'sentry/utils/usePageFilters';
 import {STARFISH_CHART_INTERVAL_FIDELITY} from 'sentry/views/starfish/utils/constants';
 import {useSpansQuery} from 'sentry/views/starfish/utils/useSpansQuery';
 
 export const useErrorRateQuery = (queryString: string) => {
-  const location = useLocation();
-  const pageFilter = usePageFilters();
+  const pageFilters = usePageFilters();
 
   const discoverQuery: NewQuery = {
     id: undefined,
@@ -22,13 +20,16 @@ export const useErrorRateQuery = (queryString: string) => {
     topEvents: '5',
     dataset: DiscoverDatasets.SPANS_METRICS,
     interval: getInterval(
-      pageFilter.selection.datetime,
+      pageFilters.selection.datetime,
       STARFISH_CHART_INTERVAL_FIDELITY
     ),
     yAxis: ['http_error_count()'],
   };
 
-  const eventView = EventView.fromNewQueryWithLocation(discoverQuery, location);
+  const eventView = EventView.fromNewQueryWithPageFilters(
+    discoverQuery,
+    pageFilters.selection
+  );
 
   const result = useSpansQuery<{'http_error_count()': number; interval: number}[]>({
     eventView,

+ 6 - 18
static/app/views/starfish/views/spans/spanTimeCharts.tsx

@@ -1,5 +1,4 @@
 import styled from '@emotion/styled';
-import {Location} from 'history';
 
 import {getInterval} from 'sentry/components/charts/utils';
 import {space} from 'sentry/styles/space';
@@ -7,7 +6,6 @@ import {PageFilters} from 'sentry/types';
 import {Series} from 'sentry/types/echarts';
 import EventView from 'sentry/utils/discover/eventView';
 import {DiscoverDatasets} from 'sentry/utils/discover/types';
-import {useLocation} from 'sentry/utils/useLocation';
 import usePageFilters from 'sentry/utils/usePageFilters';
 import {ERRORS_COLOR, P95_COLOR, THROUGHPUT_COLOR} from 'sentry/views/starfish/colours';
 import Chart, {useSynchronizeCharts} from 'sentry/views/starfish/components/chart';
@@ -46,15 +44,8 @@ function getSegmentLabel(moduleName: ModuleName) {
 
 export function SpanTimeCharts({moduleName, appliedFilters, spanCategory}: Props) {
   const {selection} = usePageFilters();
-  const location = useLocation();
-
-  const eventView = getEventView(
-    moduleName,
-    location,
-    selection,
-    appliedFilters,
-    spanCategory
-  );
+
+  const eventView = getEventView(moduleName, selection, appliedFilters, spanCategory);
 
   const {isLoading} = useSpansQuery({
     eventView,
@@ -96,8 +87,7 @@ export function SpanTimeCharts({moduleName, appliedFilters, spanCategory}: Props
 
 function ThroughputChart({moduleName, filters}: ChartProps): JSX.Element {
   const pageFilters = usePageFilters();
-  const location = useLocation();
-  const eventView = getEventView(moduleName, location, pageFilters.selection, filters);
+  const eventView = getEventView(moduleName, pageFilters.selection, filters);
 
   const label = getSegmentLabel(moduleName);
   const {isLoading, data} = useSpansQuery({
@@ -147,8 +137,7 @@ function ThroughputChart({moduleName, filters}: ChartProps): JSX.Element {
 
 function DurationChart({moduleName, filters}: ChartProps): JSX.Element {
   const pageFilters = usePageFilters();
-  const location = useLocation();
-  const eventView = getEventView(moduleName, location, pageFilters.selection, filters);
+  const eventView = getEventView(moduleName, pageFilters.selection, filters);
 
   const label = `p95(${SPAN_SELF_TIME})`;
 
@@ -234,14 +223,13 @@ const SPAN_FILTER_KEYS = ['span_operation', 'domain', 'action'];
 
 const getEventView = (
   moduleName: ModuleName,
-  location: Location,
   pageFilters: PageFilters,
   appliedFilters: AppliedFilters,
   spanCategory?: string
 ) => {
   const query = buildDiscoverQueryConditions(moduleName, appliedFilters, spanCategory);
 
-  return EventView.fromNewQueryWithLocation(
+  return EventView.fromNewQueryWithPageFilters(
     {
       name: '',
       fields: [''],
@@ -251,7 +239,7 @@ const getEventView = (
       interval: getInterval(pageFilters.datetime, STARFISH_CHART_INTERVAL_FIDELITY),
       version: 2,
     },
-    location
+    pageFilters
   );
 };
 

+ 7 - 10
static/app/views/starfish/views/webServiceView/spanGroupBreakdownContainer.tsx

@@ -1,7 +1,6 @@
 import {useState} from 'react';
 import {useTheme} from '@emotion/react';
 import styled from '@emotion/styled';
-import {Location} from 'history';
 
 import {getInterval} from 'sentry/components/charts/utils';
 import {SelectOption} from 'sentry/components/compactSelect';
@@ -66,7 +65,7 @@ export function SpanGroupBreakdownContainer({transaction, transactionMethod}: Pr
 
   const {data: segments, isLoading: isSegmentsLoading} = useDiscoverQuery({
     eventView: getCumulativeTimeEventView(
-      location,
+      selection,
       `transaction.op:http.server ${transaction ? `transaction:${transaction}` : ''} ${
         transactionMethod ? `http.method:${transactionMethod}` : ''
       }`,
@@ -80,7 +79,7 @@ export function SpanGroupBreakdownContainer({transaction, transactionMethod}: Pr
 
   const {data: cumulativeTime, isLoading: isCumulativeDataLoading} = useDiscoverQuery({
     eventView: getCumulativeTimeEventView(
-      location,
+      selection,
       `transaction.op:http.server ${transaction ? `transaction:${transaction}` : ''} ${
         transactionMethod ? `http.method:${transactionMethod}` : ''
       }`,
@@ -97,7 +96,6 @@ export function SpanGroupBreakdownContainer({transaction, transactionMethod}: Pr
     isError,
   } = useEventsStatsQuery({
     eventView: getEventView(
-      location,
       selection,
       `transaction.op:http.server ${transaction ? `transaction:${transaction}` : ''} ${
         transactionMethod ? `http.method:${transactionMethod}` : ''
@@ -200,7 +198,6 @@ const StyledPanel = styled(Panel)`
 `;
 
 const getEventView = (
-  location: Location,
   pageFilters: PageFilters,
   query: string,
   groups: string[],
@@ -212,7 +209,7 @@ const getEventView = (
       ? `p95(${SPAN_SELF_TIME})`
       : `sum(${SPAN_SELF_TIME})`;
 
-  return EventView.fromNewQueryWithLocation(
+  return EventView.fromNewQueryWithPageFilters(
     {
       name: '',
       fields: [`sum(${SPAN_SELF_TIME})`, `p95(${SPAN_SELF_TIME})`, ...groups],
@@ -226,16 +223,16 @@ const getEventView = (
         ? getInterval(pageFilters.datetime, STARFISH_CHART_INTERVAL_FIDELITY)
         : undefined,
     },
-    location
+    pageFilters
   );
 };
 
 const getCumulativeTimeEventView = (
-  location: Location,
+  pageFilters: PageFilters,
   query: string,
   groups: string[]
 ) => {
-  return EventView.fromNewQueryWithLocation(
+  return EventView.fromNewQueryWithPageFilters(
     {
       name: '',
       fields: [`sum(${SPAN_SELF_TIME})`, ...groups],
@@ -245,6 +242,6 @@ const getCumulativeTimeEventView = (
       version: 2,
       topEvents: groups.length > 0 ? '4' : undefined,
     },
-    location
+    pageFilters
   );
 };