diff --git a/projects/observability/src/pages/explorer/explorer.component.test.ts b/projects/observability/src/pages/explorer/explorer.component.test.ts index fb1c871e8..03c94343a 100644 --- 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 index 9a379decc..9f9f304a5 100644 --- 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 { 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 index 3b63a3e12..6608935a1 100644 --- 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 index 0a2a1b99d..dbb4607e9 100644 --- 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 { { // 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(` - ` - ); + `); spectator.tick(); expect(spectator.query(SelectComponent)!.selected).toBeUndefined(); @@ -46,38 +48,38 @@ describe('Explore Query Group by Editor component', () => { const spectator = hostBuilder( ` `, { 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( ` `, { 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( ` + `, + { + 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( + ` + `, { 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( + ` + + `, + { + 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( + ` + + `, + { + 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 index df07d1826..d2661b1a7 100644 --- 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 @@ -1,56 +1,91 @@ import { ChangeDetectionStrategy, Component, EventEmitter, Input, OnChanges, Output } from '@angular/core'; import { TypedSimpleChanges } from '@hypertrace/common'; -import { SelectOption } from '@hypertrace/components'; -import { combineLatest, Observable, of, ReplaySubject, Subject } from 'rxjs'; -import { map, switchMap } from 'rxjs/operators'; +import { InputAppearance, SelectOption } from '@hypertrace/components'; +import { isEmpty, isNil, omit } from 'lodash-es'; +import { combineLatest, merge, Observable, of, ReplaySubject, Subject } from 'rxjs'; +import { debounceTime, filter, map, switchMap } from 'rxjs/operators'; +import { AttributeExpression } from '../../../graphql/model/attribute/attribute-expression'; +import { AttributeMetadata, AttributeMetadataType } from '../../../graphql/model/metadata/attribute-metadata'; import { TraceType } from '../../../graphql/model/schema/trace'; import { MetadataService } from '../../../services/metadata/metadata.service'; - @Component({ selector: 'ht-explore-query-group-by-editor', styleUrls: ['./explore-query-group-by-editor.component.scss'], changeDetection: ChangeDetectionStrategy.OnPush, template: ` -
- Group By - - - +
+
+ Group By + + + +
+
+ {{ currentGroupByExpression.metadata.displayName }} Key +
+ + + + +
+
` }) export class ExploreQueryGroupByEditorComponent implements OnChanges { @Input() - public groupByKey?: string; + public groupByExpression?: AttributeExpression; @Input() public context?: TraceType; @Output() - public readonly groupByKeyChange: EventEmitter = new EventEmitter(); + public readonly groupByExpressionChange: EventEmitter = new EventEmitter(); private readonly contextSubject: Subject = new ReplaySubject(1); - private readonly incomingGroupByKeySubject: Subject = new ReplaySubject(1); + private readonly incomingGroupByExpressionSubject: Subject = new ReplaySubject(1); + private readonly groupByExpressionsToEmit: Subject = new Subject(); - public readonly groupByKey$: Observable; - public readonly groupByKeyOptions$: Observable[]>; + public readonly currentGroupByExpression$: Observable; + public readonly groupByAttributeOptions$: Observable[]>; public constructor(private readonly metadataService: MetadataService) { - this.groupByKeyOptions$ = this.contextSubject.pipe(switchMap(context => this.getGroupByOptionsForContext(context))); - - this.groupByKey$ = combineLatest([this.groupByKeyOptions$, this.incomingGroupByKeySubject]).pipe( - map(optionsAndKey => this.resolveKeyFromOptions(...optionsAndKey)) + this.groupByAttributeOptions$ = this.contextSubject.pipe( + switchMap(context => this.getGroupByOptionsForContext(context)) ); + + this.currentGroupByExpression$ = combineLatest([ + this.groupByAttributeOptions$, + merge(this.incomingGroupByExpressionSubject, this.groupByExpressionsToEmit) + ]).pipe(map(optionsAndKey => this.resolveAttributeFromOptions(...optionsAndKey))); + + this.groupByExpressionsToEmit + .pipe( + filter(expression => this.isValidExpressionToEmit(expression)), + debounceTime(500), + map(expression => omit(expression, 'metadata')) + ) + .subscribe(this.groupByExpressionChange); } public ngOnChanges(changeObject: TypedSimpleChanges): void { @@ -58,24 +93,37 @@ export class ExploreQueryGroupByEditorComponent implements OnChanges { this.contextSubject.next(this.context); } - if (changeObject.groupByKey) { - this.incomingGroupByKeySubject.next(this.groupByKey); + if (changeObject.groupByExpression) { + this.incomingGroupByExpressionSubject.next(this.groupByExpression); } } - public onGroupByKeyChange(newKey?: string): void { - this.groupByKeyChange.emit(newKey); + public onGroupByAttributeChange(newAttribute?: AttributeMetadata): void { + this.groupByExpressionsToEmit.next(newAttribute && { key: newAttribute.name, metadata: newAttribute }); } - private resolveKeyFromOptions(options: SelectOption[], key?: string): string | undefined { - if (key !== undefined && options.find(option => option.value === key)) { - return key; + public onGroupBySubpathChange(previousExpression: AttributeExpressionWithMetadata, newPath: string): void { + this.groupByExpressionsToEmit.next({ ...previousExpression, subpath: newPath }); + } + + public supportsSubpath(attribute?: AttributeMetadata): boolean { + return attribute?.type === AttributeMetadataType.StringMap; + } + + private resolveAttributeFromOptions( + options: SelectOption[], + expression?: AttributeExpression + ): AttributeExpressionWithMetadata | undefined { + if (isNil(expression)) { + return undefined; } - return undefined; + const metadata = options.find(option => option.value?.name === expression.key)?.value; + + return metadata && { ...expression, metadata: metadata }; } - private getGroupByOptionsForContext(context?: TraceType): Observable[]> { + private getGroupByOptionsForContext(context?: TraceType): Observable[]> { if (context === undefined) { return of([this.getEmptyGroupByOption()]); } @@ -83,7 +131,7 @@ export class ExploreQueryGroupByEditorComponent implements OnChanges { return this.metadataService.getGroupableAttributes(context).pipe( map(attributes => attributes.map(attribute => ({ - value: attribute.name, + value: attribute, label: this.metadataService.getAttributeDisplayName(attribute) })) ), @@ -91,10 +139,19 @@ export class ExploreQueryGroupByEditorComponent implements OnChanges { ); } - private getEmptyGroupByOption(): SelectOption { + private getEmptyGroupByOption(): SelectOption { return { value: undefined, label: 'None' }; } + + private isValidExpressionToEmit(expressionToTest?: AttributeExpressionWithMetadata): boolean { + // Can't attept to group by a map attribute without a subpath, so we treat that state as invalid and don't emit + return !(this.supportsSubpath(expressionToTest?.metadata) && isEmpty(expressionToTest?.subpath)); + } +} + +interface AttributeExpressionWithMetadata extends AttributeExpression { + metadata: AttributeMetadata; } 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 index e1742a94d..b5b8290da 100644 --- 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 @@ -40,7 +40,7 @@ export class ExploreSpecificationBuilder { expression: AttributeExpression, aggregation?: MetricAggregationType ): ExploreSpecification { - const expressionString = this.attributeExpressionAsString(expression); + const expressionString = this.attributeExpressionAsString(expression).replaceAll('.', '_'); const queryAlias = aggregation === undefined ? expressionString : `${aggregation}_${expressionString}`; return { diff --git a/projects/observability/src/shared/services/metadata/metadata.service.ts b/projects/observability/src/shared/services/metadata/metadata.service.ts index d0937f81d..a96135dee 100644 --- a/projects/observability/src/shared/services/metadata/metadata.service.ts +++ b/projects/observability/src/shared/services/metadata/metadata.service.ts @@ -5,7 +5,11 @@ import { GraphQlRequestService } from '@hypertrace/graphql-client'; import { isEmpty, isNil } from 'lodash-es'; import { Observable, of } from 'rxjs'; import { catchError, defaultIfEmpty, filter, map, shareReplay, tap, throwIfEmpty } from 'rxjs/operators'; -import { AttributeMetadata, toFilterAttributeType } from '../../graphql/model/metadata/attribute-metadata'; +import { + AttributeMetadata, + AttributeMetadataType, + toFilterAttributeType +} from '../../graphql/model/metadata/attribute-metadata'; import { addAggregationToDisplayName, getAggregationDisplayName, @@ -52,8 +56,10 @@ export class MetadataService { public getGroupableAttributes(scope: string): ReplayObservable { return this.getServerDefinedAttributes(scope).pipe( - // Can only group by strings right now - map(attributes => attributes.filter(attribute => attribute.groupable)) + // Can only group by strings or string map subpaths right now + map(attributes => + attributes.filter(attribute => attribute.groupable || attribute.type === AttributeMetadataType.StringMap) + ) ); }