Browse Source

feat(dashboards): Add timeseries support for EAP widgets (#80188)

Adds timeseries support for EAP widgets. This involves:

- Updating the helper of the config to conduct the events-stats request
using the spans dataset (relies on #80159)
- Ensures that the correct primary options and aggregate options are
displayed for timeseries charts
- Pass down the right filtering helpers. i.e. y-axes are only functions,
the parameters are only number tags; we already had this implemented for
table aggregates
- Ensures the correct options are displayed in the sort field
- Instead of allowing arbitrary selections, I allowed EAP widgets to use
the "table" sorting, because in Explore we only show the fields that are
selected. This maintains the same behaviour in dashboards

Contributes to #78264
Nar Saynorath 4 months ago
parent
commit
96c5591c59

+ 66 - 1
static/app/views/dashboards/datasetConfig/spans.spec.tsx

@@ -1,13 +1,22 @@
 import {OrganizationFixture} from 'sentry-fixture/organization';
+import {PageFiltersFixture} from 'sentry-fixture/pageFilters';
+import {WidgetFixture} from 'sentry-fixture/widget';
 
+import {waitFor} from 'sentry-test/reactTestingLibrary';
+
+import type {Client} from 'sentry/api';
 import type {Organization} from 'sentry/types/organization';
-import {ALLOWED_EXPLORE_VISUALIZE_AGGREGATES} from 'sentry/utils/fields';
+import {DiscoverDatasets} from 'sentry/utils/discover/types';
+import {ALLOWED_EXPLORE_VISUALIZE_AGGREGATES, FieldKind} from 'sentry/utils/fields';
 import {SpansConfig} from 'sentry/views/dashboards/datasetConfig/spans';
+import {WidgetType} from 'sentry/views/dashboards/types';
 
 describe('SpansConfig', () => {
   let organization: Organization;
+  const api: Client = new MockApiClient();
 
   beforeEach(() => {
+    MockApiClient.clearMockResponses();
     organization = OrganizationFixture({
       features: ['performance-view'],
     });
@@ -22,4 +31,60 @@ describe('SpansConfig', () => {
 
     expect(functionOptions).toEqual(ALLOWED_EXPLORE_VISUALIZE_AGGREGATES);
   });
+
+  it('can make a series request with the expected dataset', async () => {
+    const eventsStatsMock = MockApiClient.addMockResponse({
+      url: '/organizations/org-slug/events-stats/',
+      body: [],
+    });
+
+    const widget = WidgetFixture({
+      widgetType: WidgetType.SPANS,
+    });
+
+    // Trigger request
+    SpansConfig.getSeriesRequest!(
+      api,
+      widget,
+      0,
+      organization,
+      PageFiltersFixture(),
+      undefined,
+      'test-referrer',
+      undefined
+    );
+
+    expect(eventsStatsMock).toHaveBeenCalled();
+    await waitFor(() => {
+      expect(eventsStatsMock).toHaveBeenCalledWith(
+        '/organizations/org-slug/events-stats/',
+        expect.objectContaining({
+          query: expect.objectContaining({dataset: DiscoverDatasets.SPANS_EAP}),
+        })
+      );
+    });
+  });
+
+  it('returns the string tags as group by options', () => {
+    const mockTags = {
+      ['transaction']: {
+        name: 'transaction',
+        key: 'transaction',
+        kind: FieldKind.TAG,
+      },
+      ['platform']: {
+        name: 'platform',
+        key: 'platform',
+        kind: FieldKind.TAG,
+      },
+      ['span.duration']: {
+        name: 'span.duration',
+        key: 'span.duration',
+        kind: FieldKind.MEASUREMENT,
+      },
+    };
+    const groupByOptions = SpansConfig.getGroupByFieldOptions!(organization, mockTags);
+
+    expect(Object.keys(groupByOptions)).toEqual(['tag:transaction', 'tag:platform']);
+  });
 });

+ 66 - 17
static/app/views/dashboards/datasetConfig/spans.tsx

@@ -1,3 +1,6 @@
+import pickBy from 'lodash/pickBy';
+
+import {doEventsRequest} from 'sentry/actionCreators/events';
 import type {Client} from 'sentry/api';
 import type {PageFilters} from 'sentry/types/core';
 import type {TagCollection} from 'sentry/types/group';
@@ -23,11 +26,11 @@ import {
   handleOrderByReset,
 } from 'sentry/views/dashboards/datasetConfig/base';
 import {
-  getCustomEventsFieldRenderer,
   getTableSortOptions,
   transformEventsResponseToSeries,
   transformEventsResponseToTable,
 } from 'sentry/views/dashboards/datasetConfig/errorsAndTransactions';
+import {getSeriesRequestData} from 'sentry/views/dashboards/datasetConfig/utils/getSeriesRequestData';
 import {DisplayType, type Widget, type WidgetQuery} from 'sentry/views/dashboards/types';
 import {eventViewFromWidget} from 'sentry/views/dashboards/utils';
 import SpansSearchBar from 'sentry/views/dashboards/widgetBuilder/buildSteps/filterResultsStep/spansSearchBar';
@@ -61,30 +64,29 @@ const EAP_AGGREGATIONS = ALLOWED_EXPLORE_VISUALIZE_AGGREGATES.reduce((acc, aggre
   return acc;
 }, {});
 
+// getTimeseriesSortOptions is undefined because we want to restrict the
+// sort options to the same behaviour as tables. i.e. we are only able
+// to sort by fields that have already been selected
 export const SpansConfig: DatasetConfig<
   EventsStats | MultiSeriesEventsStats,
   TableData | EventsTableData
 > = {
   defaultWidgetQuery: DEFAULT_WIDGET_QUERY,
   enableEquations: false,
-  getCustomFieldRenderer: getCustomEventsFieldRenderer,
   SearchBar: SpansSearchBar,
-  filterSeriesSortOptions: () => () => true,
-  filterYAxisAggregateParams: () => () => true,
-  filterYAxisOptions: () => () => true,
-  getTableFieldOptions: getEventsTableFieldOptions,
-  // getTimeseriesSortOptions: (organization, widgetQuery, tags) =>
-  //  getTimeseriesSortOptions(organization, widgetQuery, tags, getEventsTableFieldOptions),
+  filterYAxisAggregateParams: () => filterAggregateParams,
+  filterYAxisOptions,
+  getTableFieldOptions: getPrimaryFieldOptions,
   getTableSortOptions: getTableSortOptions,
-  getGroupByFieldOptions: getEventsTableFieldOptions,
+  getGroupByFieldOptions,
   handleOrderByReset,
   supportedDisplayTypes: [
-    // DisplayType.AREA,
-    // DisplayType.BAR,
-    // DisplayType.BIG_NUMBER,
-    // DisplayType.LINE,
+    DisplayType.AREA,
+    DisplayType.BAR,
+    DisplayType.BIG_NUMBER,
+    DisplayType.LINE,
     DisplayType.TABLE,
-    // DisplayType.TOP_N,
+    DisplayType.TOP_N,
   ],
   getTableRequest: (
     api: Client,
@@ -108,18 +110,18 @@ export const SpansConfig: DatasetConfig<
       referrer
     );
   },
-  // getSeriesRequest: getErrorsSeriesRequest,
+  getSeriesRequest,
   transformTable: transformEventsResponseToTable,
   transformSeries: transformEventsResponseToSeries,
   filterTableOptions,
   filterAggregateParams,
 };
 
-function getEventsTableFieldOptions(
+function getPrimaryFieldOptions(
   organization: Organization,
   tags?: TagCollection,
   _customMeasurements?: CustomMeasurementCollection
-) {
+): Record<string, FieldValueOption> {
   const baseFieldOptions = generateFieldOptions({
     organization,
     tagKeys: [],
@@ -170,6 +172,12 @@ function filterAggregateParams(option: FieldValueOption) {
   return true;
 }
 
+function filterYAxisOptions() {
+  return function (option: FieldValueOption) {
+    return option.value.kind === FieldValueKind.FUNCTION;
+  };
+}
+
 function getEventsRequest(
   api: Client,
   query: WidgetQuery,
@@ -212,3 +220,44 @@ function getEventsRequest(
     }
   );
 }
+
+function getGroupByFieldOptions(
+  organization: Organization,
+  tags?: TagCollection,
+  customMeasurements?: CustomMeasurementCollection
+) {
+  const primaryFieldOptions = getPrimaryFieldOptions(
+    organization,
+    tags,
+    customMeasurements
+  );
+  const yAxisFilter = filterYAxisOptions();
+
+  // The only options that should be returned as valid group by options
+  // are string tags
+  const filterGroupByOptions = (option: FieldValueOption) =>
+    filterTableOptions(option) && !yAxisFilter(option);
+
+  return pickBy(primaryFieldOptions, filterGroupByOptions);
+}
+
+function getSeriesRequest(
+  api: Client,
+  widget: Widget,
+  queryIndex: number,
+  organization: Organization,
+  pageFilters: PageFilters,
+  _onDemandControlContext?: OnDemandControlContext,
+  referrer?: string,
+  _mepSetting?: MEPState | null
+) {
+  const requestData = getSeriesRequestData(
+    widget,
+    queryIndex,
+    organization,
+    pageFilters,
+    DiscoverDatasets.SPANS_EAP,
+    referrer
+  );
+  return doEventsRequest<true>(api, requestData);
+}

+ 1 - 1
static/app/views/dashboards/widgetBuilder/buildSteps/sortByStep/sortBySelectors.tsx

@@ -103,7 +103,7 @@ export function SortBySelectors({
         title={disableSortReason}
         disabled={!disableSort || (disableSortDirection && disableSort)}
       >
-        {displayType === DisplayType.TABLE ? (
+        {displayType === DisplayType.TABLE || widgetType === WidgetType.SPANS ? (
           <SelectControl
             name="sortBy"
             aria-label={t('Sort by')}

+ 55 - 3
static/app/views/dashboards/widgetBuilder/widgetBuilderSortBy.spec.tsx

@@ -12,9 +12,14 @@ import selectEvent from 'sentry-test/selectEvent';
 import ProjectsStore from 'sentry/stores/projectsStore';
 import TagStore from 'sentry/stores/tagStore';
 import type {DashboardDetails, Widget} from 'sentry/views/dashboards/types';
-import {DashboardWidgetSource, DisplayType} from 'sentry/views/dashboards/types';
-import type {WidgetBuilderProps} from 'sentry/views/dashboards/widgetBuilder';
-import WidgetBuilder from 'sentry/views/dashboards/widgetBuilder';
+import {
+  DashboardWidgetSource,
+  DisplayType,
+  WidgetType,
+} from 'sentry/views/dashboards/types';
+import WidgetBuilder, {
+  type WidgetBuilderProps,
+} from 'sentry/views/dashboards/widgetBuilder';
 
 import WidgetLegendSelectionState from '../widgetLegendSelectionState';
 
@@ -926,4 +931,51 @@ describe('WidgetBuilder', function () {
       );
     });
   });
+
+  describe('spans dataset timeseries', function () {
+    it('returns only the selected aggregates and group by as options', async function () {
+      const widget: Widget = {
+        id: '1',
+        title: 'Test Widget',
+        interval: '5m',
+        displayType: DisplayType.LINE,
+        widgetType: WidgetType.SPANS,
+        queries: [
+          {
+            name: '',
+            conditions: '',
+            fields: ['count(span.duration)', 'avg(span.duration)', 'transaction'],
+            aggregates: ['count(span.duration)', 'avg(span.duration)'],
+            columns: ['transaction'],
+            orderby: '-count(span.duration)',
+          },
+        ],
+      };
+
+      const dashboard = mockDashboard({widgets: [widget]});
+
+      renderTestComponent({
+        dashboard,
+        params: {
+          widgetIndex: '0',
+        },
+        orgFeatures: [...defaultOrgFeatures, 'dashboards-eap'],
+      });
+
+      await screen.findByText('Sort by a y-axis');
+      await selectEvent.openMenu(await screen.findByText('count(span.duration)'));
+
+      // 3 options in the dropdown
+      expect(screen.queryAllByTestId('menu-list-item-label')).toHaveLength(3);
+
+      // Appears once in the dropdown and once in the sort by field
+      expect(await screen.findAllByText('count(span.duration)')).toHaveLength(2);
+
+      // Appears once in the dropdown
+      expect(await screen.findAllByText('avg(span.duration)')).toHaveLength(1);
+
+      // Appears once in the dropdown and once in the group by field
+      expect(await screen.findAllByText('transaction')).toHaveLength(2);
+    });
+  });
 });