feat(logs): Use query params to write aggregate fields (#97739)

This switches the writes for aggregate fields to use query params
context.

Author: Zylphrex

static/app/types/utils.tsx

@@ -20,7 +20,3 @@ export type DeepPartial<T> =
         [P in keyof T]?: DeepPartial<T[P]>;
       }
     : T;
-
-export type Nullable<T> = {
-  [P in keyof T]: T[P] | null;
-};

static/app/views/explore/contexts/logs/logsPageParams.tsx

@@ -31,7 +31,6 @@ import {
 } from 'sentry/views/explore/contexts/logs/sortBys';
 import {
   getModeFromLocation,
-  updateLocationWithMode,
   type Mode,
 } from 'sentry/views/explore/contexts/pageParamsContext/mode';
 import {OurLogKnownFieldKey} from 'sentry/views/explore/logs/types';
@@ -114,7 +113,8 @@ interface LogsPageParams {
 type NullablePartial<T> = {
   [P in keyof T]?: T[P] | null;
 };
-type LogPageParamsUpdate = NullablePartial<LogsPageParams>;
+type NonUpdatableParams = 'mode' | 'aggregateFn' | 'aggregateParam' | 'groupBy';
+type LogPageParamsUpdate = NullablePartial<Omit<LogsPageParams, NonUpdatableParams>>;
 
 const [_LogsPageParamsProvider, _useLogsPageParams, LogsPageParamsContext] =
   createDefinedContext<LogsPageParams>({
@@ -254,10 +254,6 @@ function setLogsPageParams(location: Location, pageParams: LogPageParamsUpdate)
   updateNullableLocation(target, LOGS_CURSOR_KEY, pageParams.cursor);
   updateNullableLocation(target, LOGS_AGGREGATE_CURSOR_KEY, pageParams.aggregateCursor);
   updateNullableLocation(target, LOGS_FIELDS_KEY, pageParams.fields);
-  updateNullableLocation(target, LOGS_GROUP_BY_KEY, pageParams.groupBy);
-  updateNullableLocation(target, LOGS_AGGREGATE_FN_KEY, pageParams.aggregateFn);
-  updateNullableLocation(target, LOGS_AGGREGATE_PARAM_KEY, pageParams.aggregateParam);
-  updateLocationWithMode(target, pageParams.mode); // Can be swapped with updateNullableLocation if we merge page params.
   if (!pageParams.isTableFrozen) {
     updateLocationWithLogSortBys(target, pageParams.sortBys);
     updateLocationWithAggregateSortBys(target, pageParams.aggregateSortBys);

static/app/views/explore/contexts/pageParamsContext/visualizes.tsx

@@ -42,7 +42,7 @@ export class Visualize {
   chartType: ChartType;
   yAxis: string;
   stack?: string;
-  private selectedChartType?: ChartType;
+  selectedChartType?: ChartType;
 
   constructor(yAxis: string, options?: VisualizeOptions) {
     this.yAxis = yAxis;

static/app/views/explore/hooks/useSaveQuery.tsx

@@ -1,15 +1,16 @@
 import {useCallback, useMemo} from 'react';
 
 import type {DateString} from 'sentry/types/core';
+import {defined} from 'sentry/utils';
 import {encodeSort} from 'sentry/utils/discover/eventView';
 import useApi from 'sentry/utils/useApi';
 import useOrganization from 'sentry/utils/useOrganization';
 import usePageFilters from 'sentry/utils/usePageFilters';
 import {useLogsPageParams} from 'sentry/views/explore/contexts/logs/logsPageParams';
 import {useExplorePageParams} from 'sentry/views/explore/contexts/pageParamsContext';
 import {
-  isGroupBy,
-  isVisualize,
+  isGroupBy as isLegacyGroupBy,
+  isVisualize as isLegacyVisualize,
 } from 'sentry/views/explore/contexts/pageParamsContext/aggregateFields';
 import type {Mode} from 'sentry/views/explore/contexts/pageParamsContext/mode';
 import {useChartInterval} from 'sentry/views/explore/hooks/useChartInterval';
@@ -18,6 +19,10 @@ import {
   useInvalidateSavedQuery,
   type SavedQuery,
 } from 'sentry/views/explore/hooks/useGetSavedQueries';
+import {useQueryParams} from 'sentry/views/explore/queryParams/context';
+import {isGroupBy} from 'sentry/views/explore/queryParams/groupBy';
+import type {ReadableQueryParams} from 'sentry/views/explore/queryParams/readableQueryParams';
+import {isVisualize} from 'sentry/views/explore/queryParams/visualize';
 
 // Request payload type that matches the backend ExploreSavedQuerySerializer
 type ExploreSavedQueryRequest = {
@@ -170,13 +175,20 @@ export function useLogsSaveQuery() {
   const pageFilters = usePageFilters();
   const [interval] = useChartInterval();
   const logsParams = useLogsPageParams();
+  const queryParams = useQueryParams();
   const {id, title} = logsParams;
 
   const {saveQueryFromSavedQuery, updateQueryFromSavedQuery} = useFromSavedQuery();
 
   const requestData = useMemo((): ExploreSavedQueryRequest => {
-    return convertLogsPageParamsToRequest(logsParams, pageFilters, interval, title ?? '');
-  }, [logsParams, pageFilters, interval, title]);
+    return convertLogsPageParamsToRequest({
+      logsParams,
+      queryParams,
+      pageFilters,
+      interval,
+      title: title ?? '',
+    });
+  }, [logsParams, queryParams, pageFilters, interval, title]);
 
   const {saveQueryApi, updateQueryApi} = useCreateOrUpdateSavedQuery(id);
 
@@ -208,17 +220,21 @@ function convertExplorePageParamsToRequest(
 
   const transformedAggregateFields = aggregateFields
     .filter(aggregateField => {
-      if (isGroupBy(aggregateField)) {
+      if (isLegacyGroupBy(aggregateField)) {
         return aggregateField.groupBy !== '';
       }
       return true;
     })
     .map(aggregateField => {
-      return isVisualize(aggregateField)
-        ? {
-            yAxes: [aggregateField.yAxis],
-            chartType: aggregateField.chartType,
-          }
+      return isLegacyVisualize(aggregateField)
+        ? defined(aggregateField.selectedChartType)
+          ? {
+              yAxes: [aggregateField.yAxis],
+              chartType: aggregateField.selectedChartType,
+            }
+          : {
+              yAxes: [aggregateField.yAxis],
+            }
         : {groupBy: aggregateField.groupBy};
     });
 
@@ -243,29 +259,45 @@ function convertExplorePageParamsToRequest(
   };
 }
 
-function convertLogsPageParamsToRequest(
-  logsParams: ReturnType<typeof useLogsPageParams>,
-  pageFilters: ReturnType<typeof usePageFilters>,
-  interval: string,
-  title: string
-): ExploreSavedQueryRequest {
+function convertLogsPageParamsToRequest({
+  logsParams,
+  queryParams,
+  pageFilters,
+  interval,
+  title,
+}: {
+  interval: string;
+  logsParams: ReturnType<typeof useLogsPageParams>;
+  pageFilters: ReturnType<typeof usePageFilters>;
+  queryParams: ReadableQueryParams;
+  title: string;
+}): ExploreSavedQueryRequest {
   const {selection} = pageFilters;
   const {datetime, projects, environments} = selection;
   const {start, end, period} = datetime;
 
-  const {sortBys, fields, search, mode, groupBy, aggregateFn, aggregateParam} =
-    logsParams;
+  const {sortBys, fields, search, mode} = logsParams;
   const query = search?.formatString() ?? '';
 
-  const aggregate =
-    aggregateFn && aggregateParam ? `${aggregateFn}(${aggregateParam})` : undefined;
-  const visualize = aggregate
-    ? [
-        {
-          yAxes: [aggregate],
-        },
-      ]
-    : undefined;
+  const aggregateFields = queryParams.aggregateFields.map(aggregateField => {
+    if (isGroupBy(aggregateField)) {
+      return {groupBy: aggregateField.groupBy};
+    }
+
+    if (isVisualize(aggregateField)) {
+      if (defined(aggregateField.selectedChartType)) {
+        return {
+          yAxes: [aggregateField.yAxis],
+          chartType: aggregateField.selectedChartType,
+        };
+      }
+      return {
+        yAxes: [aggregateField.yAxis],
+      };
+    }
+
+    throw new Error(`Unknown aggregate field: ${JSON.stringify(aggregateField)}`);
+  });
 
   return {
     name: title,
@@ -281,9 +313,8 @@ function convertLogsPageParamsToRequest(
         fields,
         orderby: sortBys[0] ? encodeSort(sortBys[0]) : undefined,
         query,
-        groupby: groupBy ? [groupBy] : undefined,
         mode,
-        visualize,
+        aggregateField: aggregateFields,
       },
     ],
   };

static/app/views/explore/logs/logsQueryParams.spec.tsx

@@ -6,6 +6,7 @@ import {Mode} from 'sentry/views/explore/queryParams/mode';
 import type {ReadableQueryParamsOptions} from 'sentry/views/explore/queryParams/readableQueryParams';
 import {ReadableQueryParams} from 'sentry/views/explore/queryParams/readableQueryParams';
 import {VisualizeFunction} from 'sentry/views/explore/queryParams/visualize';
+import {ChartType} from 'sentry/views/insights/common/components/chart';
 
 function locationFixture(query: Location['query']): Location {
   return LocationFixture({query});
@@ -201,6 +202,70 @@ describe('getReadableQueryParamsFromLocation', function () {
     );
   });
 
+  it('decodes aggregate fields correctly', function () {
+    const location = locationFixture({
+      aggregateField: [
+        {yAxes: ['count(message)'], chartType: ChartType.AREA},
+        {groupBy: 'message.template'},
+        {yAxes: ['p50(foo)', 'p75(bar)']},
+      ].map(aggregateField => JSON.stringify(aggregateField)),
+    });
+    const queryParams = getReadableQueryParamsFromLocation(location);
+    expect(queryParams).toEqual(
+      new ReadableQueryParams(
+        readableQueryParamOptions({
+          aggregateFields: [
+            new VisualizeFunction('count(message)', {chartType: ChartType.AREA}),
+            {groupBy: 'message.template'},
+            new VisualizeFunction('p50(foo)'),
+            new VisualizeFunction('p75(bar)'),
+          ],
+        })
+      )
+    );
+  });
+
+  it('decodes custom aggregatefields and inserts default group bys', function () {
+    const location = locationFixture({
+      aggregateField: [
+        JSON.stringify({yAxes: ['count(message)'], chartType: ChartType.LINE}),
+      ],
+    });
+    const queryParams = getReadableQueryParamsFromLocation(location);
+    expect(queryParams).toEqual(
+      new ReadableQueryParams(
+        readableQueryParamOptions({
+          aggregateFields: [
+            new VisualizeFunction('count(message)', {chartType: ChartType.LINE}),
+            {groupBy: ''},
+          ],
+        })
+      )
+    );
+  });
+
+  it('decodes custom aggregatefields and inserts default visualizes', function () {
+    const location = locationFixture({
+      aggregateField: [JSON.stringify({groupBy: 'message.template'})],
+    });
+    const queryParams = getReadableQueryParamsFromLocation(location);
+    expect(queryParams.aggregateFields).toHaveLength(2);
+    expect(queryParams.aggregateFields[0]).toEqual({groupBy: 'message.template'});
+    expect(queryParams.aggregateFields[1]).toEqual(
+      new VisualizeFunction('count(message)')
+    );
+    expect(queryParams).toEqual(
+      new ReadableQueryParams(
+        readableQueryParamOptions({
+          aggregateFields: [
+            {groupBy: 'message.template'},
+            new VisualizeFunction('count(message)'),
+          ],
+        })
+      )
+    );
+  });
+
   it('decodes custom aggregate sort bys correctly', function () {
     const location = locationFixture({
       logsGroupBy: 'severity',
@@ -218,4 +283,22 @@ describe('getReadableQueryParamsFromLocation', function () {
       )
     );
   });
+
+  it('decodes custom aggregate sort bys correctly with aggregate fields', function () {
+    const location = locationFixture({
+      logsAggregateSortBys: '-severity',
+      aggregateField: [{groupBy: 'severity'}, {yAxes: ['avg(foo)']}].map(aggregateField =>
+        JSON.stringify(aggregateField)
+      ),
+    });
+    const queryParams = getReadableQueryParamsFromLocation(location);
+    expect(queryParams).toEqual(
+      new ReadableQueryParams(
+        readableQueryParamOptions({
+          aggregateFields: [{groupBy: 'severity'}, new VisualizeFunction('avg(foo)')],
+          aggregateSortBys: [{field: 'severity', kind: 'desc'}],
+        })
+      )
+    );
+  });
 });