ui: Make handling of selections more robust

ui/packages/shared/parser/package.json

@@ -4,7 +4,7 @@
   "description": "JavaScript parser implementation for the continuous profiling language",
   "main": "dist/index.js",
   "scripts": {
-    "test": "jest --coverage --config ../../../jest.config.js ./src/*",
+    "test": "jest --coverage --config ../../../jest.config.cjs ./src/*",
     "gen": "nearleyc -g -o ./src/selector.js ./src/selector.ne",
     "watch": "tsc-watch",
     "build": "tsc"

ui/packages/shared/parser/src/index.test.ts

@@ -41,6 +41,12 @@ test('Query.toString', () => {
   );
 });
 
+test('Query.toString With Comma', () => {
+  expect(Query.parse('memory:inuse_objects:count:space:bytes{instance="abc,def"}').toString()).toBe(
+    'memory:inuse_objects:count:space:bytes{instance="abc,def"}'
+  );
+});
+
 test('Partial Parsing ProfileName and rest', () => {
   [
     {

ui/packages/shared/profile/src/ProfileExplorer/index.tsx

@@ -116,16 +116,12 @@ const ProfileExplorerApp = ({
   let {
     from_a,
     to_a,
-    profile_name_a,
-    labels_a,
     merge_from_a,
     merge_to_a,
     time_selection_a,
     compare_a,
     from_b,
     to_b,
-    profile_name_b,
-    labels_b,
     merge_from_b,
     merge_to_b,
     time_selection_b,
@@ -140,45 +136,43 @@ const ProfileExplorerApp = ({
   // eslint-disable-next-line @typescript-eslint/naming-convention
   const expression_b = getExpressionAsAString(queryParams.expression_b);
 
+  // eslint-disable-next-line @typescript-eslint/naming-convention
+  const selection_a = getExpressionAsAString(queryParams.selection_a);
+
+  // eslint-disable-next-line @typescript-eslint/naming-convention
+  const selection_b = getExpressionAsAString(queryParams.selection_b);
+
   /* eslint-enable @typescript-eslint/naming-convention */
   const [profileA, setProfileA] = useState<ProfileSelection | null>(null);
   const [profileB, setProfileB] = useState<ProfileSelection | null>(null);
 
   useEffect(() => {
     const mergeFrom = merge_from_a ?? undefined;
     const mergeTo = merge_to_a ?? undefined;
-    const labels = typeof labels_a === 'string' ? [labels_a] : (labels_a as string[]) ?? [''];
     const profileA = ProfileSelectionFromParams(
-      expression_a,
-      from_a as string,
-      to_a as string,
       mergeFrom as string,
       mergeTo as string,
-      labels,
+      selection_a,
       filter_by_function as string
     );
 
     setProfileA(profileA);
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [expression_a, from_a, to_a, merge_from_a, merge_to_a, labels_a, filter_by_function]);
+  }, [merge_from_a, merge_to_a, selection_a, filter_by_function]);
 
   useEffect(() => {
     const mergeFrom = merge_from_b ?? undefined;
     const mergeTo = merge_to_b ?? undefined;
-    const labels = typeof labels_b === 'string' ? [labels_b] : (labels_b as string[]) ?? [''];
     const profileB = ProfileSelectionFromParams(
-      expression_b,
-      from_b as string,
-      to_b as string,
       mergeFrom as string,
       mergeTo as string,
-      labels,
+      selection_b,
       filter_by_function as string
     );
 
     setProfileB(profileB);
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [expression_b, from_b, to_b, merge_from_b, merge_to_b, labels_b, filter_by_function]);
+  }, [merge_from_b, merge_to_b, selection_b, filter_by_function]);
 
   if (profileTypesLoading) {
     return <>{loader}</>;
@@ -206,7 +200,9 @@ const ProfileExplorerApp = ({
 
   const selectProfile = (p: ProfileSelection, suffix: string): void => {
     queryParams.expression_a = encodeURIComponent(queryParams.expression_a);
+    queryParams.selection_a = encodeURIComponent(queryParams.selection_a);
     queryParams.expression_b = encodeURIComponent(queryParams.expression_b);
+    queryParams.selection_b = encodeURIComponent(queryParams.selection_b);
     return navigateTo('/', {
       ...queryParams,
       ...SuffixParams(p.HistoryParams(), suffix),
@@ -227,15 +223,18 @@ const ProfileExplorerApp = ({
     from: parseInt(from_a as string),
     to: parseInt(to_a as string),
     timeSelection: time_selection_a as string,
-    profile_name: profile_name_a as string,
   };
 
   // Show the SingleProfileExplorer when not comparing
   if (compare_a !== 'true' && compare_b !== 'true') {
     const selectQuery = (q: QuerySelection): void => {
       const mergeParams =
         q.mergeFrom !== undefined && q.mergeTo !== undefined
-          ? {merge_from_a: q.mergeFrom, merge_to_a: q.mergeTo}
+          ? {
+              merge_from_a: q.mergeFrom,
+              merge_to_a: q.mergeTo,
+              selection_a: encodeURIComponent(q.expression),
+            }
           : {};
       return navigateTo(
         '/',
@@ -271,14 +270,12 @@ const ProfileExplorerApp = ({
         from_a: queryA.from.toString(),
         to_a: queryA.to.toString(),
         time_selection_a: queryA.timeSelection,
-        profile_name_a: queryA.profile_name,
 
         compare_b: 'true',
         expression_b: encodeURIComponent(queryA.expression),
         from_b: queryA.from.toString(),
         to_b: queryA.to.toString(),
         time_selection_b: queryA.timeSelection,
-        profile_name_b: queryA.profile_name,
       };
 
       if (profileA != null) {
@@ -313,13 +310,16 @@ const ProfileExplorerApp = ({
     from: parseInt(from_b as string),
     to: parseInt(to_b as string),
     timeSelection: time_selection_b as string,
-    profile_name: profile_name_b as string,
   };
 
   const selectQueryA = (q: QuerySelection): void => {
     const mergeParams =
       q.mergeFrom !== undefined && q.mergeTo !== undefined
-        ? {merge_from_a: q.mergeFrom, merge_to_a: q.mergeTo}
+        ? {
+            merge_from_a: q.mergeFrom,
+            merge_to_a: q.mergeTo,
+            selection_a: encodeURIComponent(q.expression),
+          }
         : {};
     return navigateTo(
       '/',
@@ -331,6 +331,7 @@ const ProfileExplorerApp = ({
           compare_a: 'true',
           expression_a: encodeURIComponent(q.expression),
           expression_b: encodeURIComponent(expression_b),
+          selection_b: encodeURIComponent(selection_b),
           from_a: q.from.toString(),
           to_a: q.to.toString(),
           time_selection_a: q.timeSelection,
@@ -345,7 +346,11 @@ const ProfileExplorerApp = ({
   const selectQueryB = (q: QuerySelection): void => {
     const mergeParams =
       q.mergeFrom !== undefined && q.mergeTo !== undefined
-        ? {merge_from_b: q.mergeFrom, merge_to_b: q.mergeTo}
+        ? {
+            merge_from_b: q.mergeFrom,
+            merge_to_b: q.mergeTo,
+            selection_b: encodeURIComponent(q.expression),
+          }
         : {};
     return navigateTo(
       '/',
@@ -357,6 +362,7 @@ const ProfileExplorerApp = ({
           compare_b: 'true',
           expression_b: encodeURIComponent(q.expression),
           expression_a: encodeURIComponent(expression_a),
+          selection_a: encodeURIComponent(selection_a),
           from_b: q.from.toString(),
           to_b: q.to.toString(),
           time_selection_b: q.timeSelection,

ui/packages/shared/profile/src/ProfileSource.tsx

@@ -20,7 +20,7 @@ import {
   QueryRequest_ReportType,
   Timestamp,
 } from '@parca/client';
-import {Matcher, ProfileType, Query} from '@parca/parser';
+import {Matcher, NewParser, ProfileType, Query} from '@parca/parser';
 import {formatDate} from '@parca/utilities';
 
 export interface ProfileSource {
@@ -58,45 +58,29 @@ export function SuffixParams(params: {[key: string]: any}, suffix: string): {[ke
   );
 }
 
-export function ParseLabels(labels: string[]): Label[] {
-  return labels
-    .filter(str => str !== '')
-    .map(function (labelString): Label {
-      const parts = labelString.split('=', 2);
-      return {name: parts[0], value: parts[1]};
-    });
-}
-
 export function ProfileSelectionFromParams(
-  expression: string | undefined,
-  from: string | undefined,
-  to: string | undefined,
   mergeFrom: string | undefined,
   mergeTo: string | undefined,
-  labels: string[],
+  selection: string | undefined,
   filterQuery?: string
 ): ProfileSelection | null {
   if (
-    from !== undefined &&
-    to !== undefined &&
     mergeFrom !== undefined &&
     mergeTo !== undefined &&
-    expression !== undefined
+    selection !== undefined &&
+    selection !== ''
   ) {
-    // TODO: Refactor parsing the query and adding matchers
-    let query = Query.parse(expression);
-    if (labels !== undefined) {
-      ParseLabels(labels ?? ['']).forEach(l => {
-        const hasLabels = labels.length > 0 && labels.filter(val => val !== '').length > 0;
-        if (hasLabels) {
-          const [newQuery, changed] = query.setMatcher(l.name, l.value);
-          if (changed) {
-            query = newQuery;
-          }
-        }
-      });
+    const p = NewParser();
+    p.save();
+    const {successfulParse} = Query.tryParse(p, selection);
+
+    if (!successfulParse) {
+      console.log('Failed to parse selected query.');
+      console.log(selection);
+      return null;
     }
 
+    let query = Query.parse(selection);
     return new MergedProfileSelection(parseInt(mergeFrom), parseInt(mergeTo), query, filterQuery);
   }
 
@@ -131,9 +115,7 @@ export class MergedProfileSelection implements ProfileSelection {
     return {
       merge_from: this.mergeFrom.toString(),
       merge_to: this.mergeTo.toString(),
-      query: this.query,
-      profile_name: this.ProfileName(),
-      labels: this.query.matchers.map(m => `${m.key}=${encodeURIComponent(m.value)}`),
+      selection: encodeURIComponent(this.query.toString()),
     };
   }
 

ui/packages/shared/profile/src/__tests__/suffix_params.test.ts

@@ -11,14 +11,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import {ParseLabels, SuffixParams} from '../ProfileSource';
+import {SuffixParams} from '../ProfileSource';
 
 test('prefixes keys', () => {
   const input = {key: 'value'};
   expect(SuffixParams(input, '_a')).toMatchObject({key_a: 'value'});
 });
-
-test('parses labels', () => {
-  const input = ['key=value'];
-  expect(ParseLabels(input)).toMatchObject([{name: 'key', value: 'value'}]);
-});