diff --git a/static/app/views/dashboards/datasetConfig/spans.tsx b/static/app/views/dashboards/datasetConfig/spans.tsx index b43849e8dc46ee..57d939546312d1 100644 --- a/static/app/views/dashboards/datasetConfig/spans.tsx +++ b/static/app/views/dashboards/datasetConfig/spans.tsx @@ -1,4 +1,5 @@ import pickBy from 'lodash/pickBy'; +import trimStart from 'lodash/trimStart'; import {doEventsRequest} from 'sentry/actionCreators/events'; import type {Client} from 'sentry/api'; @@ -17,7 +18,13 @@ import type {EventsTableData, TableData} from 'sentry/utils/discover/discoverQue import type {EventData} from 'sentry/utils/discover/eventView'; import type {RenderFunctionBaggage} from 'sentry/utils/discover/fieldRenderers'; import {emptyStringValue, getFieldRenderer} from 'sentry/utils/discover/fieldRenderers'; -import type {Aggregation, QueryFieldValue} from 'sentry/utils/discover/fields'; +import { + getEquationAliasIndex, + isEquation, + isEquationAlias, + type Aggregation, + type QueryFieldValue, +} from 'sentry/utils/discover/fields'; import { doDiscoverQuery, type DiscoverQueryExtras, @@ -129,7 +136,7 @@ export const SpansConfig: DatasetConfig< > = { defaultField: DEFAULT_FIELD, defaultWidgetQuery: DEFAULT_WIDGET_QUERY, - enableEquations: false, + enableEquations: true, SearchBar: SpansSearchBar, filterYAxisAggregateParams: () => filterAggregateParams, filterYAxisOptions, @@ -287,8 +294,19 @@ function getEventsRequest( ...queryExtras, }; - if (query.orderby) { - params.sort = toArray(query.orderby); + let orderBy = query.orderby; + + if (orderBy) { + if (isEquationAlias(trimStart(orderBy, '-'))) { + const equations = query.fields?.filter(isEquation) ?? []; + const equationIndex = getEquationAliasIndex(trimStart(orderBy, '-')); + + const orderby = equations[equationIndex]; + if (orderby) { + orderBy = orderBy.startsWith('-') ? `-${orderby}` : orderby; + } + } + params.sort = toArray(orderBy); } return doDiscoverQuery( @@ -359,7 +377,10 @@ function getSeriesRequest( // Filters the primary options in the sort by selector function filterSeriesSortOptions(columns: Set) { return (option: FieldValueOption) => { - if (option.value.kind === FieldValueKind.FUNCTION) { + if ( + option.value.kind === FieldValueKind.FUNCTION || + option.value.kind === FieldValueKind.EQUATION + ) { return true; } diff --git a/static/app/views/dashboards/datasetConfig/utils/getSeriesRequestData.tsx b/static/app/views/dashboards/datasetConfig/utils/getSeriesRequestData.tsx index 8321084cc8d1a7..693804ae7e8a3b 100644 --- a/static/app/views/dashboards/datasetConfig/utils/getSeriesRequestData.tsx +++ b/static/app/views/dashboards/datasetConfig/utils/getSeriesRequestData.tsx @@ -5,11 +5,11 @@ import type {PageFilters} from 'sentry/types/core'; import type {Organization} from 'sentry/types/organization'; import { getAggregateAlias, + getEquationAliasIndex, isEquation, isEquationAlias, } from 'sentry/utils/discover/fields'; -import type {DiscoverDatasets} from 'sentry/utils/discover/types'; -import {TOP_N} from 'sentry/utils/discover/types'; +import {DiscoverDatasets, TOP_N} from 'sentry/utils/discover/types'; import {DisplayType, type Widget} from 'sentry/views/dashboards/types'; import {getNumEquations, getWidgetInterval} from 'sentry/views/dashboards/utils'; @@ -93,18 +93,43 @@ export function getSeriesRequestData( requestData.excludeOther = widgetQuery.aggregates.length !== 1 || widget.queries.length !== 1; - if (isEquation(trimStart(widgetQuery.orderby, '-'))) { - const nextEquationIndex = getNumEquations(widgetQuery.aggregates); - const isDescending = widgetQuery.orderby.startsWith('-'); - const prefix = isDescending ? '-' : ''; + if ([DiscoverDatasets.OURLOGS, DiscoverDatasets.SPANS].includes(dataset)) { + if ( + isEquation(trimStart(widgetQuery.orderby, '-')) && + !requestData.field?.includes(trimStart(widgetQuery.orderby, '-')) + ) { + requestData.field = [ + ...widgetQuery.columns, + ...widgetQuery.aggregates, + trimStart(widgetQuery.orderby, '-'), + ]; + } else if (isEquationAlias(trimStart(widgetQuery.orderby, '-'))) { + const equations = widgetQuery.fields?.filter(isEquation) ?? []; + const equationIndex = getEquationAliasIndex( + trimStart(widgetQuery.orderby, '-') + ); - // Construct the alias form of the equation and inject it into the request - requestData.orderby = `${prefix}equation[${nextEquationIndex}]`; - requestData.field = [ - ...widgetQuery.columns, - ...widgetQuery.aggregates, - trimStart(widgetQuery.orderby, '-'), - ]; + const equationOrderBy = equations[equationIndex]; + if (equationOrderBy) { + requestData.orderby = widgetQuery.orderby.startsWith('-') + ? `-${equationOrderBy}` + : equationOrderBy; + } + } + } else { + if (isEquation(trimStart(widgetQuery.orderby, '-'))) { + const nextEquationIndex = getNumEquations(widgetQuery.aggregates); + const isDescending = widgetQuery.orderby.startsWith('-'); + const prefix = isDescending ? '-' : ''; + + // Construct the alias form of the equation and inject it into the request + requestData.orderby = `${prefix}equation[${nextEquationIndex}]`; + requestData.field = [ + ...widgetQuery.columns, + ...widgetQuery.aggregates, + trimStart(widgetQuery.orderby, '-'), + ]; + } } } } diff --git a/static/app/views/dashboards/utils/getWidgetExploreUrl.spec.tsx b/static/app/views/dashboards/utils/getWidgetExploreUrl.spec.tsx index 007d09b263e629..7a0d264725d8ff 100644 --- a/static/app/views/dashboards/utils/getWidgetExploreUrl.spec.tsx +++ b/static/app/views/dashboards/utils/getWidgetExploreUrl.spec.tsx @@ -41,6 +41,41 @@ describe('getWidgetExploreUrl', () => { }); }); + it('returns the correct aggregate mode url for table widgets with equations sort', () => { + const widget = WidgetFixture({ + displayType: DisplayType.TABLE, + queries: [ + { + fields: ['span.description', 'equation|avg(span.duration) + 100'], + aggregates: ['equation|avg(span.duration) + 100'], + columns: ['span.description'], + conditions: '', + orderby: '-equation[0]', + name: '', + }, + ], + }); + + const url = getWidgetExploreUrl(widget, undefined, selection, organization); + + // Note: for table widgets the mode is set to samples and the fields are propagated + expectUrl(url).toMatch({ + path: '/organizations/org-slug/explore/traces/', + params: [ + ['field', 'span.description'], + ['groupBy', 'span.description'], + ['interval', '30m'], + ['mode', 'aggregate'], + ['statsPeriod', '14d'], + ['sort', '-equation|avg(span.duration) + 100'], + [ + 'visualize', + JSON.stringify({chartType: 1, yAxes: ['equation|avg(span.duration) + 100']}), + ], + ], + }); + }); + it('returns the correct samples mode url for table widgets without aggregation', () => { const widget = WidgetFixture({ displayType: DisplayType.TABLE, diff --git a/static/app/views/dashboards/utils/getWidgetExploreUrl.tsx b/static/app/views/dashboards/utils/getWidgetExploreUrl.tsx index 65f033cd255f32..b2c7a110c2a14a 100644 --- a/static/app/views/dashboards/utils/getWidgetExploreUrl.tsx +++ b/static/app/views/dashboards/utils/getWidgetExploreUrl.tsx @@ -6,8 +6,11 @@ import type {Organization} from 'sentry/types/organization'; import {defined} from 'sentry/utils'; import { getAggregateAlias, + getEquationAliasIndex, isAggregateField, isAggregateFieldOrEquation, + isEquation, + isEquationAlias, parseFunction, } from 'sentry/utils/discover/fields'; import {FieldKind, getFieldDefinition} from 'sentry/utils/fields'; @@ -336,6 +339,14 @@ function _getWidgetExploreUrl( } else if (exploreMode === Mode.AGGREGATE) { sort = widget.queries[0]?.orderby; } + } else if (isEquationAlias(sortColumn) && exploreMode === Mode.AGGREGATE) { + const equations = query.fields?.filter(isEquation) ?? []; + const equationIndex = getEquationAliasIndex(sortColumn); + + const orderby = equations[equationIndex]; + if (orderby) { + sort = `${sortDirection}${orderby}`; + } } else if (!isAggregateFieldOrEquation(sortColumn)) { sort = widget.queries[0]?.orderby; } diff --git a/static/app/views/dashboards/widgetBuilder/components/exploreArithmeticBuilder.tsx b/static/app/views/dashboards/widgetBuilder/components/exploreArithmeticBuilder.tsx new file mode 100644 index 00000000000000..e10bc69aeba53b --- /dev/null +++ b/static/app/views/dashboards/widgetBuilder/components/exploreArithmeticBuilder.tsx @@ -0,0 +1,74 @@ +import {useCallback, useMemo} from 'react'; + +import {ArithmeticBuilder} from 'sentry/components/arithmeticBuilder'; +import type {Expression} from 'sentry/components/arithmeticBuilder/expression'; +import type {FunctionArgument} from 'sentry/components/arithmeticBuilder/types'; +import {stripEquationPrefix} from 'sentry/utils/discover/fields'; +import { + ALLOWED_EXPLORE_EQUATION_AGGREGATES, + FieldKind, + getFieldDefinition, +} from 'sentry/utils/fields'; +import {useTraceItemTags} from 'sentry/views/explore/contexts/spanTagsContext'; +import {useExploreSuggestedAttribute} from 'sentry/views/explore/hooks/useExploreSuggestedAttribute'; + +type Props = { + equation: string; + onUpdate: (value: string) => void; +}; + +export function ExploreArithmeticBuilder({equation, onUpdate}: Props) { + const expression = stripEquationPrefix(equation); + const {tags: numberTags} = useTraceItemTags('number'); + const {tags: stringTags} = useTraceItemTags('string'); + + const functionArguments: FunctionArgument[] = useMemo(() => { + return [ + ...Object.entries(numberTags).map(([key, tag]) => { + return { + kind: FieldKind.MEASUREMENT, + name: key, + label: tag.name, + }; + }), + ...Object.entries(stringTags).map(([key, tag]) => { + return { + kind: FieldKind.TAG, + name: key, + label: tag.name, + }; + }), + ]; + }, [numberTags, stringTags]); + + const getSpanFieldDefinition = useCallback( + (key: string) => { + const tag = numberTags[key] ?? stringTags[key]; + return getFieldDefinition(key, 'span', tag?.kind); + }, + [numberTags, stringTags] + ); + + const handleExpressionChange = useCallback( + (newExpression: Expression) => { + onUpdate(stripEquationPrefix(newExpression.text)); + }, + [onUpdate] + ); + + const getSuggestedAttribute = useExploreSuggestedAttribute({ + numberAttributes: numberTags, + stringAttributes: stringTags, + }); + + return ( + + ); +} diff --git a/static/app/views/dashboards/widgetBuilder/components/sortBySelector.spec.tsx b/static/app/views/dashboards/widgetBuilder/components/sortBySelector.spec.tsx index 9531c2e67147ab..a1c429ad505af7 100644 --- a/static/app/views/dashboards/widgetBuilder/components/sortBySelector.spec.tsx +++ b/static/app/views/dashboards/widgetBuilder/components/sortBySelector.spec.tsx @@ -1,3 +1,4 @@ +import {OrganizationFixture} from 'sentry-fixture/organization'; import {RouterFixture} from 'sentry-fixture/routerFixture'; import {initializeOrg} from 'sentry-test/initializeOrg'; @@ -294,4 +295,124 @@ describe('WidgetBuilderSortBySelector', () => { expect.anything() ); }); + + it('sorts by equations line chart', async () => { + const mockNavigate = jest.fn(); + mockUseNavigate.mockReturnValue(mockNavigate); + + const organizationWithFlag = OrganizationFixture(); + organizationWithFlag.features.push('visibility-explore-equations'); + + const equationRouter = RouterFixture({ + ...router, + location: { + ...router.location, + query: { + ...router.location.query, + yAxis: ['count()', 'equation|count_unique(transaction.duration) + 100'], + }, + }, + }); + + render( + + + + + , + { + router: equationRouter, + organization: organizationWithFlag, + deprecatedRouterMocks: true, + } + ); + + const sortDirectionSelector = await screen.findByText('High to low'); + const sortFieldSelector = await screen.findByText('(Required)'); + + expect(sortFieldSelector).toBeInTheDocument(); + + await userEvent.click(sortFieldSelector); + await userEvent.click( + await screen.findByText('count_unique(transaction.duration) + 100') + ); + + expect(mockNavigate).toHaveBeenLastCalledWith( + expect.objectContaining({ + ...router.location, + query: expect.objectContaining({sort: ['-equation[0]']}), + }), + expect.anything() + ); + + await userEvent.click(sortDirectionSelector); + await userEvent.click(await screen.findByText('Low to high')); + expect(mockNavigate).toHaveBeenLastCalledWith( + expect.objectContaining({ + ...router.location, + query: expect.objectContaining({sort: ['equation[0]']}), + }), + expect.anything() + ); + }); + it('sorts by equations table', async () => { + const mockNavigate = jest.fn(); + mockUseNavigate.mockReturnValue(mockNavigate); + + const organizationWithFlag = OrganizationFixture(); + organizationWithFlag.features.push('visibility-explore-equations'); + + const equationRouter = RouterFixture({ + ...router, + location: { + ...router.location, + query: { + ...router.location.query, + displayType: 'table', + yAxis: ['count()', 'equation|count_unique(transaction.duration) + 100'], + }, + }, + }); + + render( + + + + + , + { + router: equationRouter, + organization: organizationWithFlag, + deprecatedRouterMocks: true, + } + ); + + const sortDirectionSelector = await screen.findByText('High to low'); + const sortFieldSelector = await screen.findByText(`Select a column\u{2026}`); + + expect(sortFieldSelector).toBeInTheDocument(); + + await userEvent.click(sortFieldSelector); + await userEvent.click( + await screen.findByText('count_unique(transaction.duration) + 100') + ); + + expect(mockNavigate).toHaveBeenLastCalledWith( + expect.objectContaining({ + ...router.location, + query: expect.objectContaining({sort: ['-equation[0]']}), + }), + expect.anything() + ); + + await userEvent.click(sortDirectionSelector); + await userEvent.click(await screen.findByText('Low to high')); + expect(mockNavigate).toHaveBeenLastCalledWith( + expect.objectContaining({ + ...router.location, + query: expect.objectContaining({sort: ['equation[0]']}), + }), + expect.anything() + ); + }); }); diff --git a/static/app/views/dashboards/widgetBuilder/components/sortBySelectors.tsx b/static/app/views/dashboards/widgetBuilder/components/sortBySelectors.tsx index 93bc5ee1166513..c34b2e07f99166 100644 --- a/static/app/views/dashboards/widgetBuilder/components/sortBySelectors.tsx +++ b/static/app/views/dashboards/widgetBuilder/components/sortBySelectors.tsx @@ -22,6 +22,7 @@ import useOrganization from 'sentry/utils/useOrganization'; import {getDatasetConfig} from 'sentry/views/dashboards/datasetConfig/base'; import type {WidgetQuery} from 'sentry/views/dashboards/types'; import {DisplayType, WidgetType} from 'sentry/views/dashboards/types'; +import {ExploreArithmeticBuilder} from 'sentry/views/dashboards/widgetBuilder/components/exploreArithmeticBuilder'; import {getColumnOptions} from 'sentry/views/dashboards/widgetBuilder/components/visualize'; import { sortDirections, @@ -241,21 +242,35 @@ export function SortBySelectors({ {showCustomEquation && ( - { - const newValue = { - sortBy: `${EQUATION_PREFIX}${value}`, - sortDirection: values.sortDirection, - }; - onChange(newValue); - setCustomEquation(newValue); - }} - hideFieldOptions - /> + {widgetType === WidgetType.SPANS ? ( + { + const newValue = { + sortBy: `${EQUATION_PREFIX}${value}`, + sortDirection: values.sortDirection, + }; + onChange(newValue); + setCustomEquation(newValue); + }} + /> + ) : ( + { + const newValue = { + sortBy: `${EQUATION_PREFIX}${value}`, + sortDirection: values.sortDirection, + }; + onChange(newValue); + setCustomEquation(newValue); + }} + hideFieldOptions + /> + )} )} diff --git a/static/app/views/dashboards/widgetBuilder/components/visualize/index.spec.tsx b/static/app/views/dashboards/widgetBuilder/components/visualize/index.spec.tsx index e110d5f151466e..9f678888ef0126 100644 --- a/static/app/views/dashboards/widgetBuilder/components/visualize/index.spec.tsx +++ b/static/app/views/dashboards/widgetBuilder/components/visualize/index.spec.tsx @@ -2,7 +2,13 @@ import {LocationFixture} from 'sentry-fixture/locationFixture'; import {OrganizationFixture} from 'sentry-fixture/organization'; import {RouterFixture} from 'sentry-fixture/routerFixture'; -import {render, screen, userEvent, within} from 'sentry-test/reactTestingLibrary'; +import { + render, + screen, + userEvent, + waitFor, + within, +} from 'sentry-test/reactTestingLibrary'; import type {TagCollection} from 'sentry/types/group'; import {FieldKind} from 'sentry/utils/fields'; @@ -1464,6 +1470,116 @@ describe('Visualize', () => { await screen.findByRole('button', {name: 'Aggregate Selection'}) ).toHaveTextContent(/^count$/); }); + + it('adds equations', async () => { + const organizationWithFlag = OrganizationFixture(); + organizationWithFlag.features.push('visibility-explore-equations'); + + render( + + + , + { + organization: organizationWithFlag, + router: RouterFixture({ + location: LocationFixture({ + query: { + dataset: WidgetType.SPANS, + displayType: DisplayType.TABLE, + yAxis: ['count(span.duration)'], + }, + }), + }), + + deprecatedRouterMocks: true, + } + ); + + await userEvent.click(screen.getByRole('button', {name: 'Add Equation'})); + + expect(screen.getByLabelText('Enter an equation')).toBeInTheDocument(); + + const input = screen.getByRole('combobox', { + name: 'Add a term', + }); + expect(input).toBeInTheDocument(); + + await userEvent.click(input); + await userEvent.type(input, 'avg('); + + expect( + await screen.findByRole('row', { + name: 'avg(span.duration)', + }) + ).toBeInTheDocument(); + + await waitFor(() => { + expect(screen.getByLabelText('Select an attribute')).toHaveFocus(); + }); + await userEvent.type(input, '{ArrowDown}{Enter}'); + + expect(mockNavigate).toHaveBeenCalledWith( + expect.objectContaining({ + query: expect.objectContaining({field: ['equation|( avg(span.duration)']}), + }), + expect.anything() + ); + }); + + it('adds equations line chart', async () => { + const organizationWithFlag = OrganizationFixture(); + organizationWithFlag.features.push('visibility-explore-equations'); + + render( + + + , + { + organization: organizationWithFlag, + router: RouterFixture({ + location: LocationFixture({ + query: { + dataset: WidgetType.SPANS, + displayType: DisplayType.TABLE, + yAxis: ['count(span.duration)'], + }, + }), + }), + + deprecatedRouterMocks: true, + } + ); + + await userEvent.click(screen.getByRole('button', {name: 'Add Equation'})); + + expect(screen.getByLabelText('Enter an equation')).toBeInTheDocument(); + + const input = screen.getByRole('combobox', { + name: 'Add a term', + }); + expect(input).toBeInTheDocument(); + + await userEvent.click(input); + await userEvent.type(input, 'avg('); + + expect( + await screen.findByRole('row', { + name: 'avg(span.duration)', + }) + ).toBeInTheDocument(); + + await waitFor(() => { + expect(screen.getByLabelText('Select an attribute')).toHaveFocus(); + }); + await userEvent.type(input, '{ArrowDown}{Enter}'); + + expect(mockNavigate).toHaveBeenCalledWith( + expect.objectContaining({ + query: expect.objectContaining({field: ['equation|( avg(span.duration)']}), + }), + expect.anything() + ); + }); }); it('disables changing visualize fields for count', async () => { diff --git a/static/app/views/dashboards/widgetBuilder/components/visualize/index.tsx b/static/app/views/dashboards/widgetBuilder/components/visualize/index.tsx index ff156f7bb497d6..46dc6c8621fb1c 100644 --- a/static/app/views/dashboards/widgetBuilder/components/visualize/index.tsx +++ b/static/app/views/dashboards/widgetBuilder/components/visualize/index.tsx @@ -37,6 +37,7 @@ import {getDatasetConfig} from 'sentry/views/dashboards/datasetConfig/base'; import {DisplayType, WidgetType} from 'sentry/views/dashboards/types'; import {SectionHeader} from 'sentry/views/dashboards/widgetBuilder/components/common/sectionHeader'; import SortableVisualizeFieldWrapper from 'sentry/views/dashboards/widgetBuilder/components/common/sortableFieldWrapper'; +import {ExploreArithmeticBuilder} from 'sentry/views/dashboards/widgetBuilder/components/exploreArithmeticBuilder'; import {AggregateParameterField} from 'sentry/views/dashboards/widgetBuilder/components/visualize/aggregateParameterField'; import { ColumnCompactSelect, @@ -358,6 +359,10 @@ function Visualize({error, setError}: VisualizeProps) { const draggableFieldIds = fields?.map((_field, index) => index.toString()) ?? []; + const hasExploreEquations = organization.features.includes( + 'visibility-explore-equations' + ); + return ( {field.kind === FieldValueKind.EQUATION ? ( - { - dispatch({ - type: updateAction, - payload: fields.map((_field, i) => - i === index ? {..._field, field: value} : _field - ), - }); - setError?.({...error, queries: []}); - trackAnalytics('dashboards_views.widget_builder.change', { - builder_version: WidgetBuilderVersion.SLIDEOUT, - field: 'visualize.updateEquation', - from: source, - new_widget: !isEditing, - value: '', - widget_type: state.dataset ?? '', - organization, - }); - }} - options={fields} - placeholder={t('Equation')} - aria-label={t('Equation')} - /> + state.dataset === WidgetType.SPANS ? ( + { + dispatch({ + type: updateAction, + payload: fields.map((_field, i) => + i === index ? {..._field, field: value} : _field + ), + }); + setError?.({...error, queries: []}); + trackAnalytics( + 'dashboards_views.widget_builder.change', + { + builder_version: WidgetBuilderVersion.SLIDEOUT, + field: 'visualize.updateEquation', + from: source, + new_widget: !isEditing, + value: '', + widget_type: state.dataset ?? '', + organization, + } + ); + }} + /> + ) : ( + { + dispatch({ + type: updateAction, + payload: fields.map((_field, i) => + i === index ? {..._field, field: value} : _field + ), + }); + setError?.({...error, queries: []}); + trackAnalytics( + 'dashboards_views.widget_builder.change', + { + builder_version: WidgetBuilderVersion.SLIDEOUT, + field: 'visualize.updateEquation', + from: source, + new_widget: !isEditing, + value: '', + widget_type: state.dataset ?? '', + organization, + } + ); + }} + options={fields} + placeholder={t('Equation')} + aria-label={t('Equation')} + /> + ) ) : ( - {datasetConfig.enableEquations && ( - { - dispatch({ - type: updateAction, - payload: [...(fields ?? []), {kind: FieldValueKind.EQUATION, field: ''}], - }); - - trackAnalytics('dashboards_views.widget_builder.change', { - builder_version: WidgetBuilderVersion.SLIDEOUT, - field: 'visualize.addEquation', - from: source, - new_widget: !isEditing, - value: '', - widget_type: state.dataset ?? '', - organization, - }); - }} - > - {t('+ Add Equation')} - - )} + {datasetConfig.enableEquations && + (state.dataset !== WidgetType.SPANS || + (state.dataset === WidgetType.SPANS && hasExploreEquations)) && ( + { + dispatch({ + type: updateAction, + payload: [ + ...(fields ?? []), + {kind: FieldValueKind.EQUATION, field: ''}, + ], + }); + + trackAnalytics('dashboards_views.widget_builder.change', { + builder_version: WidgetBuilderVersion.SLIDEOUT, + field: 'visualize.addEquation', + from: source, + new_widget: !isEditing, + value: '', + widget_type: state.dataset ?? '', + organization, + }); + }} + > + {t('+ Add Equation')} + + )} );