refactor: use new group limit in explorer (#698)

* refactor: use new group limit in explorer

* test: fix up explorer query handler test

* test: more cleanup

Author: aaron-steinfeld

projects/observability/src/pages/explorer/explorer-dashboard-builder.test.ts

@@ -87,6 +87,7 @@ describe('Explorer dashboard builder', () => {
       const mockRequest: ExploreVisualizationRequest = {
         context: ObservabilityTraceType.Api,
         series: [series],
+        resultLimit: 1000,
         exploreQuery$: EMPTY,
         resultsQuery$: of({
           requestType: TRACES_GQL_REQUEST,

projects/observability/src/pages/explorer/explorer.component.test.ts

@@ -138,7 +138,7 @@ describe('Explorer component', () => {
       expect.objectContaining({
         requestType: EXPLORE_GQL_REQUEST,
         context: ObservabilityTraceType.Api,
-        limit: 500,
+        limit: 1000,
         interval: new TimeDuration(15, TimeUnit.Second)
       }),
       expect.objectContaining({})
@@ -186,7 +186,7 @@ describe('Explorer component', () => {
         requestType: EXPLORE_GQL_REQUEST,
         context: ObservabilityTraceType.Api,
         filters: [new GraphQlFieldFilter('first', GraphQlOperatorType.Equals, 'foo')],
-        limit: 500,
+        limit: 1000,
         interval: new TimeDuration(15, TimeUnit.Second)
       }),
       expect.objectContaining({})
@@ -222,7 +222,7 @@ describe('Explorer component', () => {
       expect.objectContaining({
         requestType: EXPLORE_GQL_REQUEST,
         context: SPAN_SCOPE,
-        limit: 500,
+        limit: 1000,
         interval: new TimeDuration(15, TimeUnit.Second)
       }),
       expect.objectContaining({})
@@ -274,7 +274,7 @@ describe('Explorer component', () => {
       expect.objectContaining({
         requestType: EXPLORE_GQL_REQUEST,
         context: SPAN_SCOPE,
-        limit: 500,
+        limit: 1000,
         interval: new TimeDuration(15, TimeUnit.Second),
         filters: [new GraphQlFieldFilter('first', GraphQlOperatorType.Equals, 'foo')]
       }),

projects/observability/src/shared/components/explore-query-editor/explore-query-editor.component.test.ts

@@ -174,9 +174,9 @@ describe('Explore query editor', () => {
     expect(onRequestChange).toHaveBeenLastCalledWith(
       expect.objectContaining({
         series: [defaultSeries],
-        groupByLimit: 5, // Default group by limit
         groupBy: {
-          keys: ['first groupable']
+          keys: ['first groupable'],
+          limit: 5 // Default group by limit
         }
       })
     );
@@ -213,9 +213,9 @@ describe('Explore query editor', () => {
     expect(onQueryChange).toHaveBeenLastCalledWith(
       expect.objectContaining({
         series: [defaultSeries],
-        groupByLimit: 6,
         groupBy: {
-          keys: ['first groupable']
+          keys: ['first groupable'],
+          limit: 6
         }
       })
     );

projects/observability/src/shared/components/explore-query-editor/explore-query-editor.component.ts

@@ -44,19 +44,20 @@ import {
         ></ht-explore-query-group-by-editor>
 
         <ht-explore-query-limit-editor
+          *ngIf="currentVisualization.groupBy"
           class="limit"
-          [limit]="currentVisualization.groupByLimit"
-          (limitChange)="this.setLimit($event)"
+          [limit]="currentVisualization.groupBy?.limit"
+          (limitChange)="this.updateGroupByLimit(currentVisualization.groupBy!, $event)"
           [includeRest]="currentVisualization.groupBy?.includeRest"
           (includeRestChange)="this.updateGroupByIncludeRest(currentVisualization.groupBy!, $event)"
-          [disabled]="!currentVisualization.groupBy"
         >
         </ht-explore-query-limit-editor>
       </div>
     </div>
   `
 })
 export class ExploreQueryEditorComponent implements OnChanges, OnInit {
+  private static readonly DEFAULT_GROUP_LIMIT: number = 5;
   @Input()
   public filters?: Filter[];
 
@@ -94,16 +95,18 @@ export class ExploreQueryEditorComponent implements OnChanges, OnInit {
     if (key === undefined) {
       this.visualizationBuilder.groupBy();
     } else {
-      this.visualizationBuilder.groupBy(groupBy ? { ...groupBy, keys: [key] } : { keys: [key] });
+      this.visualizationBuilder.groupBy(
+        groupBy ? { ...groupBy, keys: [key] } : { keys: [key], limit: ExploreQueryEditorComponent.DEFAULT_GROUP_LIMIT }
+      );
     }
   }
 
   public updateGroupByIncludeRest(groupBy: GraphQlGroupBy, includeRest: boolean): void {
     this.visualizationBuilder.groupBy({ ...groupBy, includeRest: includeRest });
   }
 
-  public setLimit(limit: number): void {
-    this.visualizationBuilder.groupByLimit(limit);
+  public updateGroupByLimit(groupBy: GraphQlGroupBy, limit: number): void {
+    this.visualizationBuilder.groupBy({ ...groupBy, limit: limit });
   }
 
   public setInterval(interval: IntervalValue): void {

projects/observability/src/shared/components/explore-query-editor/explore-visualization-builder.test.ts

@@ -83,30 +83,23 @@ describe('Explore visualization builder', () => {
       spectator.service
         .setSeries([buildSeries('test1')])
         .groupBy({
-          keys: ['testGroupBy']
+          keys: ['testGroupBy'],
+          limit: 15
         })
-        .groupByLimit(15)
         .setSeries([buildSeries('test2')]);
 
-      expectObservable(recordedRequests).toBe('(abcde)', {
+      expectObservable(recordedRequests).toBe('(abcd)', {
         a: expectedQuery(),
         b: expectedQuery({
           series: [matchSeriesWithName('test1')]
         }),
         c: expectedQuery({
           series: [matchSeriesWithName('test1')],
-          groupBy: { keys: ['testGroupBy'] },
-          groupByLimit: 5
+          groupBy: { keys: ['testGroupBy'], limit: 15 }
         }),
         d: expectedQuery({
-          series: [matchSeriesWithName('test1')],
-          groupBy: { keys: ['testGroupBy'] },
-          groupByLimit: 15
-        }),
-        e: expectedQuery({
           series: [matchSeriesWithName('test2')],
-          groupBy: { keys: ['testGroupBy'] },
-          groupByLimit: 15
+          groupBy: { keys: ['testGroupBy'], limit: 15 }
         })
       });
     });

projects/observability/src/shared/components/explore-query-editor/explore-visualization-builder.ts

@@ -30,8 +30,7 @@ import { CartesianSeriesVisualizationType } from '../cartesian/chart';
 
 @Injectable()
 export class ExploreVisualizationBuilder implements OnDestroy {
-  private static readonly DEFAULT_GROUP_LIMIT: number = 5;
-  private static readonly DEFAULT_UNGROUPED_LIMIT: number = 500;
+  private static readonly DEFAULT_LIMIT: number = 1000;
 
   public readonly visualizationRequest$: Observable<ExploreVisualizationRequest>;
   private readonly destroyed$: Subject<void> = new Subject();
@@ -80,20 +79,8 @@ export class ExploreVisualizationBuilder implements OnDestroy {
   }
 
   public groupBy(groupBy?: GraphQlGroupBy): this {
-    const groupByLimit =
-      this.currentState().groupByLimit !== undefined
-        ? this.currentState().groupByLimit
-        : ExploreVisualizationBuilder.DEFAULT_GROUP_LIMIT;
-
-    return this.updateState({
-      groupBy: groupBy,
-      groupByLimit: groupBy && groupByLimit
-    });
-  }
-
-  public groupByLimit(limit: number): this {
     return this.updateState({
-      groupByLimit: limit
+      groupBy: groupBy
     });
   }
 
@@ -128,11 +115,11 @@ export class ExploreVisualizationBuilder implements OnDestroy {
   private buildRequest(state: ExploreRequestState): ExploreVisualizationRequest {
     return {
       context: state.context,
+      resultLimit: state.resultLimit,
       series: [...state.series],
       filters: state.filters && [...state.filters],
       interval: this.resolveInterval(state.interval),
       groupBy: state.groupBy && { ...state.groupBy },
-      groupByLimit: state.groupByLimit,
       exploreQuery$: this.mapStateToExploreQuery(state),
       resultsQuery$: this.mapStateToResultsQuery(state)
     };
@@ -146,7 +133,7 @@ export class ExploreVisualizationBuilder implements OnDestroy {
       interval: this.resolveInterval(state.interval),
       filters: state.filters && this.graphQlFilterBuilderService.buildGraphQlFilters(state.filters),
       groupBy: state.groupBy,
-      limit: state.groupByLimit === undefined ? ExploreVisualizationBuilder.DEFAULT_UNGROUPED_LIMIT : state.groupByLimit
+      limit: state.resultLimit
     });
   }
 
@@ -215,6 +202,7 @@ export class ExploreVisualizationBuilder implements OnDestroy {
     return {
       context: context,
       interval: 'AUTO',
+      resultLimit: ExploreVisualizationBuilder.DEFAULT_LIMIT,
       series: [this.buildDefaultSeries(context)]
     };
   }
@@ -242,8 +230,8 @@ export interface ExploreRequestState {
   interval?: TimeDuration | 'AUTO';
   filters?: Filter[];
   groupBy?: GraphQlGroupBy;
-  groupByLimit?: number;
   useGroupName?: boolean;
+  resultLimit: number;
 }
 
 export type ExploreRequestContext = TraceType | 'SPAN' | 'DOMAIN_EVENT';

projects/observability/src/shared/components/explore-query-editor/limit/explore-query-limit-editor.component.scss

@@ -13,7 +13,6 @@
 
     .limit-label {
       @include body-1-medium($gray-9);
-      @include disableable;
       height: 32px;
       line-height: 32px;
       margin-bottom: 12px;

projects/observability/src/shared/components/explore-query-editor/limit/explore-query-limit-editor.component.test.ts

@@ -26,25 +26,6 @@ describe('Explore Query Limit Editor component', () => {
     expect(spectator.query(InputComponent)!.value).toBe(12);
   });
 
-  test('sets limit as disabled and display to Auto when disabled', () => {
-    const spectator = hostBuilder(
-      `
-    <ht-explore-query-limit-editor [limit]="limit" [disabled]="disabled">
-    </ht-explore-query-limit-editor>`,
-      {
-        hostProps: {
-          limit: 12,
-          disabled: true
-        }
-      }
-    );
-
-    expect(spectator.query(InputComponent)!.value).toBe('Auto');
-    expect(spectator.query(InputComponent)!.type).toBe('text');
-    expect(spectator.query(InputComponent)!.disabled).toBe(true);
-    expect(spectator.query('.limit-label')).toHaveClass('disabled');
-  });
-
   test('updates when the provided limit changes', () => {
     const spectator = hostBuilder(
       `

projects/observability/src/shared/components/explore-query-editor/limit/explore-query-limit-editor.component.ts

@@ -8,27 +8,21 @@ import { InputAppearance } from '@hypertrace/components';
   template: `
     <div class="limit-container">
       <div class="limit-number-input-container">
-        <span class="limit-label" [ngClass]="{ disabled: this.disabled }"> Max Results </span>
+        <span class="limit-label"> Max Groups </span>
         <div class="limit-input-wrapper">
           <ht-input
-            [type]="this.getInputType()"
+            type="number"
             class="limit-input"
             appearance="${InputAppearance.Border}"
-            [disabled]="this.disabled"
-            [value]="this.getLimitValue()"
+            [value]="this.limit"
             (valueChange)="this.limitChange.emit($event)"
           >
           </ht-input>
         </div>
       </div>
-      <div class="limit-include-rest-container" *ngIf="!this.disabled">
+      <div class="limit-include-rest-container">
         <span class="limit-include-rest-label"> Show Other: </span>
-        <ht-checkbox
-          [disabled]="this.disabled"
-          [checked]="this.includeRest"
-          (checkedChange)="this.includeRestChange.emit($event)"
-        >
-        </ht-checkbox>
+        <ht-checkbox [checked]="this.includeRest" (checkedChange)="this.includeRestChange.emit($event)"> </ht-checkbox>
       </div>
     </div>
   `
@@ -40,20 +34,9 @@ export class ExploreQueryLimitEditorComponent {
   @Input()
   public includeRest: boolean = false;
 
-  @Input()
-  public disabled: boolean = false;
-
   @Output()
   public readonly limitChange: EventEmitter<number> = new EventEmitter();
 
   @Output()
   public readonly includeRestChange: EventEmitter<boolean> = new EventEmitter();
-
-  public getInputType(): string {
-    return this.disabled ? 'text' : 'number';
-  }
-
-  public getLimitValue(): string | number {
-    return this.disabled ? 'Auto' : this.limit;
-  }
 }

projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts

@@ -150,7 +150,8 @@ describe('Explore cartesian data source model', () => {
 
     model.groupBy = {
       keys: ['baz'],
-      includeRest: true
+      includeRest: true,
+      limit: 5
     };
 
     runFakeRxjs(({ expectObservable }) => {
@@ -212,7 +213,8 @@ describe('Explore cartesian data source model', () => {
     ];
 
     model.groupBy = {
-      keys: ['baz']
+      keys: ['baz'],
+      limit: 5
     };
 
     runFakeRxjs(({ expectObservable }) => {
@@ -316,7 +318,7 @@ export class TestExploreCartesianDataSourceModel extends ExploreCartesianDataSou
   public series?: ExploreSeries[];
   public context?: string = 'context_scope';
   public groupBy?: GraphQlGroupBy;
-  public groupByLimit?: number;
+  public resultLimit: number = 100;
 
   protected buildRequestState(interval?: TimeDuration | 'AUTO'): ExploreRequestState | undefined {
     if ((this.series ?? [])?.length === 0 || this.context === undefined) {
@@ -328,7 +330,7 @@ export class TestExploreCartesianDataSourceModel extends ExploreCartesianDataSou
       context: this.context,
       interval: interval,
       groupBy: this.groupBy,
-      groupByLimit: this.groupByLimit
+      resultLimit: this.resultLimit
     };
   }
 }