Explorer groupby subpath support (#1369)

aaron-steinfeld · web-flow · commit a06a9104314f · 2022-01-13T10:52:15.000-08:00
* feat: add support for attribute expressions

* test: update tests

* feat: add support for filtering with attribute expressions

* fix: caught a couple bugs while updating tests

* feat: explorer group by subpath support

* test: update tests
diff --git a/projects/observability/src/pages/explorer/explorer.component.test.ts b/projects/observability/src/pages/explorer/explorer.component.test.ts
@@ -358,13 +358,13 @@ describe('Explorer component', () => {
     spectator.click(spectator.queryAll('ht-toggle-item')[1]);
     spectator.query(ExploreQueryEditorComponent)!.setSeries([buildSeries('second', MetricAggregationType.Average)]);
     spectator.query(ExploreQueryEditorComponent)!.setInterval(new TimeDuration(30, TimeUnit.Second));
-    spectator.query(ExploreQueryEditorComponent)!.updateGroupByKey(
+    spectator.query(ExploreQueryEditorComponent)!.updateGroupByExpression(
       {
         keyExpressions: [{ key: 'apiName' }],
         limit: 6,
         includeRest: true
       },
-      'apiName'
+      { key: 'apiName' }
     );
     detectQueryChange();
     expect(queryParamChangeSpy).toHaveBeenLastCalledWith({
@@ -394,7 +394,7 @@ describe('Explorer component', () => {
       }
     });
     expect(spectator.query(ToggleGroupComponent)?.activeItem?.label).toBe('Spans');
-    expect(spectator.query(ExploreQueryGroupByEditorComponent)?.groupByKey).toBe('apiName');
+    expect(spectator.query(ExploreQueryGroupByEditorComponent)?.groupByExpression).toEqual({ key: 'apiName' });
     expect(spectator.query(ExploreQueryLimitEditorComponent)?.limit).toBe(6);
     expect(spectator.query(ExploreQueryLimitEditorComponent)?.includeRest).toBe(true);
     expect(spectator.query(ExploreQuerySeriesEditorComponent)?.series).toEqual({
diff --git a/projects/observability/src/pages/explorer/explorer.component.ts b/projects/observability/src/pages/explorer/explorer.component.ts
@@ -308,7 +308,7 @@ export class ExplorerComponent {
   private tryDecodeAttributeExpression(expressionString: string): [AttributeExpression] | [] {
     const [key, subpath] = expressionString.split('__');
 
-    return [{ key: key, ...(isEmpty(subpath) ? { subpath: subpath } : {}) }];
+    return [{ key: key, ...(!isEmpty(subpath) ? { subpath: subpath } : {}) }];
   }
 }
 interface ContextToggleItem extends ToggleItem<ExplorerContextScope> {
diff --git a/projects/observability/src/shared/components/explore-query-editor/explore-query-editor.component.test.ts b/projects/observability/src/shared/components/explore-query-editor/explore-query-editor.component.test.ts
@@ -170,7 +170,7 @@ describe('Explore query editor', () => {
     const options = spectator.queryAll('.select-option', { root: true });
     spectator.click(options[1]);
 
-    spectator.tick(10);
+    spectator.tick(510); // Debounced
 
     expect(onRequestChange).toHaveBeenLastCalledWith(
       expect.objectContaining({
@@ -204,7 +204,7 @@ describe('Explore query editor', () => {
     const options = spectator.queryAll('.select-option', { root: true });
     spectator.click(options[1]);
 
-    spectator.tick(10);
+    spectator.tick(510);
 
     const limitInputEl = spectator.query('ht-explore-query-limit-editor input') as HTMLInputElement;
     limitInputEl.value = '6';
@@ -271,7 +271,7 @@ describe('Explore query editor', () => {
     const options = spectator.queryAll('.select-option', { root: true });
     spectator.click(options[1]);
 
-    spectator.tick(10);
+    spectator.tick(510); // Debounced
 
     spectator.click('.limit-include-rest-container input[type="checkbox"]');
 
diff --git a/projects/observability/src/shared/components/explore-query-editor/explore-query-editor.component.ts b/projects/observability/src/shared/components/explore-query-editor/explore-query-editor.component.ts
@@ -2,6 +2,7 @@ import { ChangeDetectionStrategy, Component, EventEmitter, Input, OnChanges, OnI
 import { TypedSimpleChanges } from '@hypertrace/common';
 import { Filter } from '@hypertrace/components';
 import { Observable } from 'rxjs';
+import { AttributeExpression } from '../../graphql/model/attribute/attribute-expression';
 import { GraphQlGroupBy } from '../../graphql/model/schema/groupby/graphql-group-by';
 import { IntervalValue } from '../interval-select/interval-select.component';
 import {
@@ -39,8 +40,8 @@ import {
         <ht-explore-query-group-by-editor
           class="group-by"
           [context]="currentVisualization.context"
-          [groupByKey]="(currentVisualization.groupBy?.keyExpressions)?.[0]?.key"
-          (groupByKeyChange)="this.updateGroupByKey(currentVisualization.groupBy, $event)"
+          [groupByExpression]="(currentVisualization.groupBy?.keyExpressions)[0]"
+          (groupByExpressionChange)="this.updateGroupByExpression(currentVisualization.groupBy, $event)"
         ></ht-explore-query-group-by-editor>
 
         <ht-explore-query-limit-editor
@@ -104,22 +105,22 @@ export class ExploreQueryEditorComponent implements OnChanges, OnInit {
     }
 
     if (changeObject.groupBy && this.groupBy?.keyExpressions.length) {
-      this.updateGroupByKey(this.groupBy, this.groupBy.keyExpressions[0]?.key);
+      this.updateGroupByExpression(this.groupBy, this.groupBy.keyExpressions[0]);
     }
   }
 
   public setSeries(series: ExploreSeries[]): void {
     this.visualizationBuilder.setSeries(series);
   }
 
-  public updateGroupByKey(groupBy?: GraphQlGroupBy, key?: string): void {
-    if (key === undefined) {
+  public updateGroupByExpression(groupBy?: GraphQlGroupBy, keyExpression?: AttributeExpression): void {
+    if (keyExpression === undefined) {
       this.visualizationBuilder.groupBy();
     } else {
       this.visualizationBuilder.groupBy(
         groupBy
-          ? { ...groupBy, keyExpressions: [{ key: key }] }
-          : { keyExpressions: [{ key: key }], limit: ExploreQueryEditorComponent.DEFAULT_GROUP_LIMIT }
+          ? { ...groupBy, keyExpressions: [keyExpression] }
+          : { keyExpressions: [keyExpression], limit: ExploreQueryEditorComponent.DEFAULT_GROUP_LIMIT }
       );
     }
   }
diff --git a/projects/observability/src/shared/components/explore-query-editor/explore-query-editor.module.ts b/projects/observability/src/shared/components/explore-query-editor/explore-query-editor.module.ts
@@ -1,6 +1,14 @@
 import { CommonModule } from '@angular/common';
 import { NgModule } from '@angular/core';
-import { ButtonModule, InputModule, SelectModule, TooltipModule, TraceCheckboxModule } from '@hypertrace/components';
+import {
+  ButtonModule,
+  FormFieldModule,
+  InputModule,
+  LetAsyncModule,
+  SelectModule,
+  TooltipModule,
+  TraceCheckboxModule
+} from '@hypertrace/components';
 import { IntervalSelectModule } from '../interval-select/interval-select.module';
 import { ExploreQueryEditorComponent } from './explore-query-editor.component';
 import { ExploreQueryGroupByEditorComponent } from './group-by/explore-query-group-by-editor.component';
@@ -26,7 +34,9 @@ import { ExploreQuerySeriesGroupEditorComponent } from './series/explore-query-s
     TooltipModule,
     InputModule,
     IntervalSelectModule,
-    TraceCheckboxModule
+    TraceCheckboxModule,
+    LetAsyncModule,
+    FormFieldModule
   ]
 })
 export class ExploreQueryEditorModule {}
diff --git a/projects/observability/src/shared/components/explore-query-editor/group-by/explore-query-group-by-editor.component.scss b/projects/observability/src/shared/components/explore-query-editor/group-by/explore-query-group-by-editor.component.scss
@@ -3,12 +3,29 @@
 
 .group-by-container {
   display: flex;
-  flex-direction: column;
+  flex-direction: row;
+  gap: 24px;
 
-  .group-by-label {
-    @include body-1-medium($gray-9);
-    height: 32px;
-    line-height: 32px;
-    margin-bottom: 12px;
+  .group-by-input-container {
+    display: flex;
+    flex-direction: column;
+
+    .group-by-label {
+      @include body-1-medium($gray-9);
+      height: 32px;
+      line-height: 32px;
+      margin-bottom: 12px;
+    }
+
+    .group-by-path-wrapper {
+      width: 100px;
+
+      .group-by-path-input {
+        @include body-2-regular($gray-9);
+        width: 100%;
+        height: 100%;
+        line-height: 32px;
+      }
+    }
   }
 }
diff --git a/projects/observability/src/shared/components/explore-query-editor/group-by/explore-query-group-by-editor.component.test.ts b/projects/observability/src/shared/components/explore-query-editor/group-by/explore-query-group-by-editor.component.test.ts
@@ -2,20 +2,24 @@ import { HttpClientTestingModule } from '@angular/common/http/testing';
 import { fakeAsync, flush } from '@angular/core/testing';
 import { IconLibraryTestingModule } from '@hypertrace/assets-library';
 import { NavigationService } from '@hypertrace/common';
-import { SelectComponent, SelectModule } from '@hypertrace/components';
+import { InputModule, LetAsyncModule, SelectComponent, SelectModule } from '@hypertrace/components';
 import { byText, createHostFactory, mockProvider } from '@ngneat/spectator/jest';
 import { EMPTY, of } from 'rxjs';
-import { AttributeMetadata } from '../../../graphql/model/metadata/attribute-metadata';
+import { AttributeMetadata, AttributeMetadataType } from '../../../graphql/model/metadata/attribute-metadata';
 import { ObservabilityTraceType } from '../../../graphql/model/schema/observability-traces';
 import { MetadataService } from '../../../services/metadata/metadata.service';
 import { ExploreQueryGroupByEditorComponent } from './explore-query-group-by-editor.component';
 
 describe('Explore Query Group by Editor component', () => {
   // Define metadata at top level so equality checks always get same values
-  const attributeMetadata = [{ name: 'test value' }, { name: 'foo bar' }];
+  const attributeMetadata = [
+    { name: 'test value', type: AttributeMetadataType.String },
+    { name: 'foo bar', type: AttributeMetadataType.String },
+    { name: 'test map', type: AttributeMetadataType.StringMap }
+  ];
   const hostBuilder = createHostFactory({
     component: ExploreQueryGroupByEditorComponent,
-    imports: [SelectModule, HttpClientTestingModule, IconLibraryTestingModule],
+    imports: [SelectModule, HttpClientTestingModule, IconLibraryTestingModule, LetAsyncModule, InputModule],
     providers: [
       mockProvider(MetadataService, {
         getAttributeDisplayName: (attribute: AttributeMetadata) => attribute.name,
@@ -29,13 +33,11 @@ describe('Explore Query Group by Editor component', () => {
   });
 
   test('sets group by to none if undefined provided', fakeAsync(() => {
-    const spectator = hostBuilder(
-      `
+    const spectator = hostBuilder(`
     <ht-explore-query-group-by-editor
-      context="${ObservabilityTraceType.Api}" [groupByKey]="groupBy"
+      context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression"
   ></ht-explore-query-group-by-editor>
-  `
-    );
+  `);
     spectator.tick();
 
     expect(spectator.query(SelectComponent)!.selected).toBeUndefined();
@@ -46,38 +48,38 @@ describe('Explore Query Group by Editor component', () => {
     const spectator = hostBuilder(
       `
     <ht-explore-query-group-by-editor
-      context="${ObservabilityTraceType.Api}" [groupByKey]="groupByKey"
+      context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression"
   ></ht-explore-query-group-by-editor>
   `,
       {
         hostProps: {
-          groupByKey: 'test value'
+          groupByExpression: { key: 'test value' }
         }
       }
     );
 
-    expect(spectator.query(SelectComponent)!.selected).toBe('test value');
+    expect(spectator.query(SelectComponent)!.selected).toEqual(attributeMetadata[0]);
   });
 
   test('updates if new group by is provided', () => {
     const spectator = hostBuilder(
       `
     <ht-explore-query-group-by-editor
-      context="${ObservabilityTraceType.Api}" [groupByKey]="groupByKey"
+      context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression"
   ></ht-explore-query-group-by-editor>
   `,
       {
         hostProps: {
-          groupByKey: 'test value'
+          groupByExpression: { key: 'test value' }
         }
       }
     );
 
-    spectator.setHostInput({ groupByKey: 'foo bar' });
+    spectator.setHostInput({ groupByExpression: { key: 'foo bar' } });
 
-    expect(spectator.query(SelectComponent)!.selected).toBe('foo bar');
+    expect(spectator.query(SelectComponent)!.selected).toEqual(attributeMetadata[1]);
 
-    spectator.setHostInput({ groupByKey: undefined });
+    spectator.setHostInput({ groupByExpression: undefined });
 
     expect(spectator.query(SelectComponent)!.selected).toBeUndefined();
   });
@@ -87,12 +89,42 @@ describe('Explore Query Group by Editor component', () => {
     const spectator = hostBuilder(
       `
     <ht-explore-query-group-by-editor
-      context="${ObservabilityTraceType.Api}" [groupByKey]="groupByKey" (groupByKeyChange)="onChange($event)"
+      context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression" (groupByExpressionChange)="onChange($event)"
+  ></ht-explore-query-group-by-editor>
+  `,
+      {
+        hostProps: {
+          groupByExpression: { key: 'test value' },
+          onChange: onChange
+        }
+      }
+    );
+    spectator.tick(); // Needs to tick to interact with select which is async
+
+    spectator.click(spectator.query(byText('test value'))!);
+
+    const options = spectator.queryAll('.select-option', { root: true });
+    expect(options.length).toBe(4);
+    spectator.click(options[2]);
+
+    spectator.tick(500); // 500ms debounce after group by change
+    expect(onChange).toHaveBeenCalledTimes(1);
+    expect(onChange).toHaveBeenCalledWith({ key: 'foo bar' });
+
+    flush(); // Overlay removes async
+  }));
+
+  test('emits when group by key is changed', fakeAsync(() => {
+    const onChange = jest.fn();
+    const spectator = hostBuilder(
+      `
+    <ht-explore-query-group-by-editor
+      context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression" (groupByExpressionChange)="onChange($event)"
   ></ht-explore-query-group-by-editor>
   `,
       {
         hostProps: {
-          groupByKey: 'test value',
+          groupByExpression: { key: 'test value' },
           onChange: onChange
         }
       }
@@ -102,12 +134,70 @@ describe('Explore Query Group by Editor component', () => {
     spectator.click(spectator.query(byText('test value'))!);
 
     const options = spectator.queryAll('.select-option', { root: true });
-    expect(options.length).toBe(3);
+    expect(options.length).toBe(4);
     spectator.click(options[2]);
 
+    spectator.tick(500); // 500ms debounce after group by change
     expect(onChange).toHaveBeenCalledTimes(1);
-    expect(onChange).toHaveBeenCalledWith('foo bar');
+    expect(onChange).toHaveBeenCalledWith({ key: 'foo bar' });
+
+    flush(); // Overlay removes async
+  }));
+
+  test('shows subpath for map attributes only', fakeAsync(() => {
+    const spectator = hostBuilder(
+      `
+    <ht-explore-query-group-by-editor
+      context="${ObservabilityTraceType.Api}" [groupByExpression]="groupByExpression"
+  ></ht-explore-query-group-by-editor>
+  `,
+      {
+        hostProps: {
+          groupByExpression: { key: 'test value' }
+        }
+      }
+    );
+    spectator.tick(); // Needs to tick to interact with select which is async
+
+    expect(spectator.query('.select')).toHaveText('test value');
+    expect(spectator.query('.group-by-path-input')).not.toExist();
+    spectator.click(spectator.query(byText('test value'))!);
+    const options = spectator.queryAll('.select-option', { root: true });
+    spectator.click(options[3]);
+
+    expect(spectator.query('.group-by-path-input')).toExist();
+
+    flush(); // Overlay removes async
+  }));
 
+  test('outputs map expressions correctly', fakeAsync(() => {
+    const onChange = jest.fn();
+    const spectator = hostBuilder(
+      `
+    <ht-explore-query-group-by-editor
+      context="${ObservabilityTraceType.Api}" (groupByExpressionChange)="onChange($event)"
+  ></ht-explore-query-group-by-editor>
+  `,
+      {
+        hostProps: {
+          onChange: onChange
+        }
+      }
+    );
+    spectator.tick(); // Needs to tick to interact with select which is async
+    expect(onChange).not.toHaveBeenCalled();
+    spectator.click(spectator.query(byText('None'))!);
+    const options = spectator.queryAll('.select-option', { root: true });
+    spectator.click(options[3]);
+    // Wait for debounce
+    spectator.tick(500);
+    // Should not emit on switching to map group by, not eligible without a subpath
+    expect(onChange).not.toHaveBeenCalled();
+    spectator.typeInElement('test.subpath', '.group-by-path-input input');
+    expect(onChange).not.toHaveBeenCalled();
+    // Wait for debounce
+    spectator.tick(500);
+    expect(onChange).toHaveBeenCalledWith({ key: 'test map', subpath: 'test.subpath' });
     flush(); // Overlay removes async
   }));
 });
diff --git a/projects/observability/src/shared/components/explore-query-editor/group-by/explore-query-group-by-editor.component.ts b/projects/observability/src/shared/components/explore-query-editor/group-by/explore-query-group-by-editor.component.ts
diff --git a/projects/observability/src/shared/graphql/request/builders/specification/explore/explore-specification-builder.ts b/projects/observability/src/shared/graphql/request/builders/specification/explore/explore-specification-builder.ts
diff --git a/projects/observability/src/shared/services/metadata/metadata.service.ts b/projects/observability/src/shared/services/metadata/metadata.service.ts

Original file line number	Diff line number	Diff line change
`@@ -308,7 +308,7 @@ export class ExplorerComponent {`
`308`	`308`	`private tryDecodeAttributeExpression(expressionString: string): [AttributeExpression] \| [] {`
`309`	`309`	`const [key, subpath] = expressionString.split('__');`
`310`	`310`
`311`		`- return [{ key: key, ...(isEmpty(subpath) ? { subpath: subpath } : {}) }];`
	`311`	`+ return [{ key: key, ...(!isEmpty(subpath) ? { subpath: subpath } : {}) }];`
`312`	`312`	`}`
`313`	`313`	`}`
`314`	`314`	`interface ContextToggleItem extends ToggleItem<ExplorerContextScope> {`