From 5ad6e8515d9537cac87fd5378ec3e9388aced1f6 Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld Date: Mon, 3 Jan 2022 16:36:21 -0500 Subject: [PATCH 1/6] feat: add support for attribute expressions --- .../src/pages/explorer/explorer.component.ts | 26 ++++++++++--- projects/observability/src/public-api.ts | 1 + .../explore-query-editor.component.ts | 13 ++++--- .../explore-cartesian-data-source.model.ts | 21 ++++++---- .../data/graphql/explore/explore-result.ts | 15 +++++-- .../graphql/graphql-data-source.module.ts | 2 - .../specifiers/field-specification.model.ts | 27 ------------- .../explore-table-data-source.model.ts | 2 +- .../donut/trace-donut-data-source.model.ts | 4 +- .../top-n/data/top-n-data-source.model.ts | 2 +- .../model/attribute/attribute-expression.ts | 4 ++ .../filter/field/graphql-field-filter.ts | 25 ++++++++++-- .../model/schema/filter/graphql-filter.ts | 1 - .../model/schema/groupby/graphql-group-by.ts | 4 +- .../schema/sort/graphql-sort-argument.ts | 3 +- .../argument/graphql-argument-builder.ts | 35 +++++++++++++---- .../graphql-observability-argument-builder.ts | 39 +++---------------- .../observability-specification-builder.ts | 2 +- ...nriched-attribute-specification-builder.ts | 2 +- .../entity/entity-specification-builder.ts | 2 +- .../explore/explore-specification-builder.ts | 32 +++++++++++---- .../specification/specification-builder.ts | 22 +---------- .../trace-status-specification-builder.ts | 2 +- .../explore-graphql-query-builder.service.ts | 4 +- 24 files changed, 155 insertions(+), 135 deletions(-) delete mode 100644 projects/observability/src/shared/dashboard/data/graphql/specifiers/field-specification.model.ts create mode 100644 projects/observability/src/shared/graphql/model/attribute/attribute-expression.ts diff --git a/projects/observability/src/pages/explorer/explorer.component.ts b/projects/observability/src/pages/explorer/explorer.component.ts index 185d1a348..9a379decc 100644 --- a/projects/observability/src/pages/explorer/explorer.component.ts +++ b/projects/observability/src/pages/explorer/explorer.component.ts @@ -9,7 +9,7 @@ import { TimeDurationService } from '@hypertrace/common'; import { Filter, ToggleItem } from '@hypertrace/components'; -import { isNil } from 'lodash-es'; +import { isEmpty, isNil } from 'lodash-es'; import { concat, EMPTY, Observable, Subject } from 'rxjs'; import { map, take } from 'rxjs/operators'; import { CartesianSeriesVisualizationType } from '../../shared/components/cartesian/chart'; @@ -19,6 +19,7 @@ import { ExploreVisualizationRequest } from '../../shared/components/explore-query-editor/explore-visualization-builder'; import { IntervalValue } from '../../shared/components/interval-select/interval-select.component'; +import { AttributeExpression } from '../../shared/graphql/model/attribute/attribute-expression'; import { AttributeMetadata } from '../../shared/graphql/model/metadata/attribute-metadata'; import { MetricAggregationType } from '../../shared/graphql/model/metrics/metric-aggregation'; import { GraphQlGroupBy } from '../../shared/graphql/model/schema/groupby/graphql-group-by'; @@ -216,8 +217,8 @@ export class ExplorerComponent { } private getGroupByQueryParams(groupBy?: GraphQlGroupBy): QueryParamObject { - const key = groupBy?.keys[0]; - if (key === undefined) { + const keyExpressions = groupBy?.keyExpressions ?? []; + if (keyExpressions.length === 0) { return { // Clear existing selection [ExplorerQueryParam.Group]: undefined, @@ -227,7 +228,7 @@ export class ExplorerComponent { } return { - [ExplorerQueryParam.Group]: key, + [ExplorerQueryParam.Group]: keyExpressions.map(expression => this.encodeAttributeExpression(expression)), [ExplorerQueryParam.OtherGroup]: groupBy?.includeRest || undefined, // No param needed for false [ExplorerQueryParam.GroupLimit]: groupBy?.limit }; @@ -238,7 +239,9 @@ export class ExplorerComponent { contextToggle: this.getOrDefaultContextItemFromQueryParam(param.get(ExplorerQueryParam.Scope) as ScopeQueryParam), groupBy: param.has(ExplorerQueryParam.Group) ? { - keys: param.getAll(ExplorerQueryParam.Group), + keyExpressions: param + .getAll(ExplorerQueryParam.Group) + .flatMap(expressionString => this.tryDecodeAttributeExpression(expressionString)), includeRest: param.get(ExplorerQueryParam.OtherGroup) === 'true', // tslint:disable-next-line: strict-boolean-expressions limit: parseInt(param.get(ExplorerQueryParam.GroupLimit)!) || 5 @@ -294,6 +297,19 @@ export class ExplorerComponent { } ]; } + + private encodeAttributeExpression(attributeExpression: AttributeExpression): string { + if (isEmpty(attributeExpression.subpath)) { + return attributeExpression.key; + } + + return `${attributeExpression.key}__${attributeExpression.subpath}`; + } + private tryDecodeAttributeExpression(expressionString: string): [AttributeExpression] | [] { + const [key, subpath] = expressionString.split('__'); + + return [{ key: key, ...(isEmpty(subpath) ? { subpath: subpath } : {}) }]; + } } interface ContextToggleItem extends ToggleItem { value: ExplorerContextScope; diff --git a/projects/observability/src/public-api.ts b/projects/observability/src/public-api.ts index 030e8fbe9..634484dbb 100644 --- a/projects/observability/src/public-api.ts +++ b/projects/observability/src/public-api.ts @@ -64,6 +64,7 @@ export * from './shared/services/metadata/metadata.service'; export * from './shared/services/metadata/metadata.service.module'; export * from './shared/graphql/model/metadata/attribute-metadata'; +export * from './shared/graphql/model/attribute/attribute-expression'; export * from './shared/graphql/model/metrics/metric-aggregation'; export * from './shared/graphql/model/metrics/metric-health'; 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 cca51a6fd..de8755e25 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 { @@ -103,8 +104,8 @@ export class ExploreQueryEditorComponent implements OnChanges, OnInit { this.setInterval(this.interval); } - if (changeObject.groupBy && this.groupBy?.keys.length) { - this.updateGroupByKey(this.groupBy, this.groupBy.keys[0]); + if (changeObject.groupBy && this.groupBy?.keyExpressions.length) { + this.updateGroupByKey(this.groupBy, this.groupBy.keyExpressions[0]); } } @@ -112,12 +113,14 @@ export class ExploreQueryEditorComponent implements OnChanges, OnInit { this.visualizationBuilder.setSeries(series); } - public updateGroupByKey(groupBy?: GraphQlGroupBy, key?: string): void { - if (key === undefined) { + public updateGroupByKey(groupBy?: GraphQlGroupBy, keyExpression?: AttributeExpression): void { + if (keyExpression === undefined) { this.visualizationBuilder.groupBy(); } else { this.visualizationBuilder.groupBy( - groupBy ? { ...groupBy, keys: [key] } : { keys: [key], limit: ExploreQueryEditorComponent.DEFAULT_GROUP_LIMIT } + groupBy + ? { ...groupBy, keyExpressions: [keyExpression] } + : { keyExpressions: [keyExpression], limit: ExploreQueryEditorComponent.DEFAULT_GROUP_LIMIT } ); } } diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts index 27fa2e81e..035b13a03 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts @@ -1,6 +1,7 @@ import { ColorService, forkJoinSafeEmpty, RequireBy, TimeDuration } from '@hypertrace/common'; import { ModelInject } from '@hypertrace/hyperdash-angular'; import { isEmpty } from 'lodash-es'; +import { AttributeExpression } from 'projects/observability/src/shared/graphql/model/attribute/attribute-expression'; import { NEVER, Observable, of } from 'rxjs'; import { map, mergeMap } from 'rxjs/operators'; import { Series } from '../../../../components/cartesian/chart'; @@ -107,19 +108,19 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM (selection): selection is RequireBy => selection.aggregation !== undefined ); - const groupByKeys = request.groupBy?.keys ?? []; - const isGroupBy = groupByKeys.length > 0; + const groupByExpressions = request.groupBy?.keyExpressions ?? []; + const isGroupBy = groupByExpressions.length > 0; if (!isGroupBy && request.interval) { return aggregatableSpecs.map(spec => this.buildTimeseriesData(spec, result)); } if (isGroupBy && !request.interval) { - return aggregatableSpecs.map(spec => this.buildGroupedSeriesData(spec, groupByKeys, result)); + return aggregatableSpecs.map(spec => this.buildGroupedSeriesData(spec, groupByExpressions, result)); } if (isGroupBy && request.interval) { - return aggregatableSpecs.map(spec => this.buildGroupedTimeseriesData(spec, groupByKeys, result)).flat(); + return aggregatableSpecs.map(spec => this.buildGroupedTimeseriesData(spec, groupByExpressions, result)).flat(); } return []; @@ -149,10 +150,14 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM }; } - public buildGroupedSeriesData(spec: AggregatableSpec, groupByKeys: string[], result: ExploreResult): SeriesData { + public buildGroupedSeriesData( + spec: AggregatableSpec, + groupByExpressions: AttributeExpression[], + result: ExploreResult + ): SeriesData { return { data: result - .getGroupedSeriesData(groupByKeys, spec.name, spec.aggregation) + .getGroupedSeriesData(groupByExpressions, spec.name, spec.aggregation) .map(({ keys, value }) => [this.buildGroupedSeriesName(keys), value]), spec: spec }; @@ -160,10 +165,10 @@ export abstract class ExploreCartesianDataSourceModel extends GraphQlDataSourceM public buildGroupedTimeseriesData( spec: AggregatableSpec, - groupByKeys: string[], + groupByExpressions: AttributeExpression[], result: ExploreResult ): SeriesData[] { - return Array.from(result.getGroupedTimeSeriesData(groupByKeys, spec.name, spec.aggregation).entries()).map( + return Array.from(result.getGroupedTimeSeriesData(groupByExpressions, spec.name, spec.aggregation).entries()).map( ([groupNames, data]) => ({ data: data, groupName: this.buildGroupedSeriesName(groupNames), diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-result.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-result.ts index 005a22a47..8a8d923d1 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-result.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-result.ts @@ -1,5 +1,6 @@ import { TimeDuration } from '@hypertrace/common'; import { groupBy } from 'lodash-es'; +import { AttributeExpression } from 'projects/observability/src/shared/graphql/model/attribute/attribute-expression'; import { MetricTimeseriesInterval } from '../../../../graphql/model/metric/metric-timeseries'; import { MetricAggregationType } from '../../../../graphql/model/metrics/metric-aggregation'; import { ExploreSpecification } from '../../../../graphql/model/schema/specifications/explore-specification'; @@ -27,19 +28,25 @@ export class ExploreResult { return this.extractTimeseriesForSpec(this.specBuilder.exploreSpecificationForKey(metricKey, aggregation)); } - public getGroupedSeriesData(groupKeys: string[], metricKey: string, aggregation: MetricAggregationType): GroupData[] { + public getGroupedSeriesData( + groupExpressions: AttributeExpression[], + metricKey: string, + aggregation: MetricAggregationType + ): GroupData[] { return this.extractGroupSeriesForSpec( - groupKeys.map(key => this.specBuilder.exploreSpecificationForKey(key)), + groupExpressions.map(expression => this.specBuilder.exploreSpecificationForAttributeExpression(expression)), this.specBuilder.exploreSpecificationForKey(metricKey, aggregation) ); } public getGroupedTimeSeriesData( - groupKeys: string[], + groupExpressions: AttributeExpression[], metricKey: string, aggregation: MetricAggregationType ): Map { - const groupSpecs = groupKeys.map(key => this.specBuilder.exploreSpecificationForKey(key)); + const groupSpecs = groupExpressions.map(expression => + this.specBuilder.exploreSpecificationForAttributeExpression(expression) + ); const spec = this.specBuilder.exploreSpecificationForKey(metricKey, aggregation); const groupedResults = groupBy(this.response.results, result => this.getGroupNamesFromResult(result, groupSpecs).join(',') diff --git a/projects/observability/src/shared/dashboard/data/graphql/graphql-data-source.module.ts b/projects/observability/src/shared/dashboard/data/graphql/graphql-data-source.module.ts index 0df624830..99f700892 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/graphql-data-source.module.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/graphql-data-source.module.ts @@ -11,7 +11,6 @@ import { SpanDataSourceModel } from './span/span-data-source.model'; import { AttributeSpecificationModel } from './specifiers/attribute-specification.model'; import { CompositeSpecificationModel } from './specifiers/composite-specification.model'; import { EnrichedAttributeSpecificationModel } from './specifiers/enriched-attribute-specification.model'; -import { FieldSpecificationModel } from './specifiers/field-specification.model'; import { MappedAttributeSpecificationModel } from './specifiers/mapped-attribute-specification.model'; import { TraceStatusSpecificationModel } from './specifiers/trace-status-specification.model'; import { SpansTableDataSourceModel } from './table/spans/spans-table-data-source.model'; @@ -38,7 +37,6 @@ import { TraceWaterfallDataSourceModel } from './waterfall/trace-waterfall-data- TracesDataSourceModel, CompositeSpecificationModel, AttributeSpecificationModel, - FieldSpecificationModel, TraceStatusSpecificationModel, EnrichedAttributeSpecificationModel, MappedAttributeSpecificationModel diff --git a/projects/observability/src/shared/dashboard/data/graphql/specifiers/field-specification.model.ts b/projects/observability/src/shared/dashboard/data/graphql/specifiers/field-specification.model.ts deleted file mode 100644 index bdf175e99..000000000 --- a/projects/observability/src/shared/dashboard/data/graphql/specifiers/field-specification.model.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { Dictionary } from '@hypertrace/common'; -import { Model, ModelProperty, STRING_PROPERTY } from '@hypertrace/hyperdash'; -import { Specification } from '../../../../graphql/model/schema/specifier/specification'; -import { SpecificationBuilder } from '../../../../graphql/request/builders/specification/specification-builder'; -import { SpecificationModel } from './specification.model'; - -@Model({ - type: 'field-specification', - displayName: 'Field' -}) -export class FieldSpecificationModel extends SpecificationModel { - @ModelProperty({ - key: 'field', - displayName: 'Field', - type: STRING_PROPERTY.type, - required: true - }) - public field!: string; - - protected buildInnerSpec(): Specification { - return new SpecificationBuilder().fieldSpecificationForKey(this.field); - } - - public extractFromServerData(resultContainer: Dictionary): unknown { - return this.innerSpec.extractFromServerData(resultContainer); - } -} diff --git a/projects/observability/src/shared/dashboard/data/graphql/table/explore/explore-table-data-source.model.ts b/projects/observability/src/shared/dashboard/data/graphql/table/explore/explore-table-data-source.model.ts index 808fd0fe1..240548e67 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/table/explore/explore-table-data-source.model.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/table/explore/explore-table-data-source.model.ts @@ -87,7 +87,7 @@ export class ExploreTableDataSourceModel extends TableDataSourceModel { filters: [...filters, ...this.toGraphQlFilters(request.filters)], timeRange: this.getTimeRangeOrThrow(), groupBy: { - keys: this.groupBy, + keyExpressions: this.groupBy.map(key => ({ key: key })), includeRest: this.groupByIncludeRest, limit: this.groupLimit } diff --git a/projects/observability/src/shared/dashboard/data/graphql/trace/donut/trace-donut-data-source.model.ts b/projects/observability/src/shared/dashboard/data/graphql/trace/donut/trace-donut-data-source.model.ts index c272b9c68..0bcfc1005 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/trace/donut/trace-donut-data-source.model.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/trace/donut/trace-donut-data-source.model.ts @@ -65,7 +65,7 @@ export class TraceDonutDataSourceModel extends GraphQlDataSourceModel this.buildDonutResults(exploreResponse, this.metric))); @@ -78,7 +78,7 @@ export class TraceDonutDataSourceModel extends GraphQlDataSourceModel { total = total + seriesTuple.value; diff --git a/projects/observability/src/shared/dashboard/widgets/top-n/data/top-n-data-source.model.ts b/projects/observability/src/shared/dashboard/widgets/top-n/data/top-n-data-source.model.ts index 27d408307..45bfbccd2 100644 --- a/projects/observability/src/shared/dashboard/widgets/top-n/data/top-n-data-source.model.ts +++ b/projects/observability/src/shared/dashboard/widgets/top-n/data/top-n-data-source.model.ts @@ -65,7 +65,7 @@ export class TopNDataSourceModel extends GraphQlDataSourceModel; type: GraphQlEnumArgument; diff --git a/projects/observability/src/shared/graphql/model/schema/filter/graphql-filter.ts b/projects/observability/src/shared/graphql/model/schema/filter/graphql-filter.ts index 7688a5939..c19cff0c7 100644 --- a/projects/observability/src/shared/graphql/model/schema/filter/graphql-filter.ts +++ b/projects/observability/src/shared/graphql/model/schema/filter/graphql-filter.ts @@ -2,7 +2,6 @@ import { GraphQlArgumentObject } from '@hypertrace/graphql-client'; export interface GraphQlFilter { asArgumentObjects(): GraphQlArgumentObject[]; - key: string; } export interface GraphQlFilterable { diff --git a/projects/observability/src/shared/graphql/model/schema/groupby/graphql-group-by.ts b/projects/observability/src/shared/graphql/model/schema/groupby/graphql-group-by.ts index a209a7368..342bc60ed 100644 --- a/projects/observability/src/shared/graphql/model/schema/groupby/graphql-group-by.ts +++ b/projects/observability/src/shared/graphql/model/schema/groupby/graphql-group-by.ts @@ -1,5 +1,7 @@ +import { AttributeExpression } from '../../attribute/attribute-expression'; + export interface GraphQlGroupBy { - keys: string[]; + keyExpressions: AttributeExpression[]; includeRest?: boolean; limit: number; } diff --git a/projects/observability/src/shared/graphql/model/schema/sort/graphql-sort-argument.ts b/projects/observability/src/shared/graphql/model/schema/sort/graphql-sort-argument.ts index a944605c8..ad8c8bef6 100644 --- a/projects/observability/src/shared/graphql/model/schema/sort/graphql-sort-argument.ts +++ b/projects/observability/src/shared/graphql/model/schema/sort/graphql-sort-argument.ts @@ -1,7 +1,8 @@ import { GraphQlEnumArgument } from '@hypertrace/graphql-client'; +import { AttributeExpression } from '../../attribute/attribute-expression'; import { GraphQlSortDirection } from './graphql-sort-direction'; export interface GraphQlSortArgument { direction: GraphQlEnumArgument; - key: string; + expression: AttributeExpression; } diff --git a/projects/observability/src/shared/graphql/request/builders/argument/graphql-argument-builder.ts b/projects/observability/src/shared/graphql/request/builders/argument/graphql-argument-builder.ts index a55a06143..7d4276dde 100644 --- a/projects/observability/src/shared/graphql/request/builders/argument/graphql-argument-builder.ts +++ b/projects/observability/src/shared/graphql/request/builders/argument/graphql-argument-builder.ts @@ -1,4 +1,6 @@ -import { GraphQlArgument, GraphQlEnumArgument } from '@hypertrace/graphql-client'; +import { GraphQlArgument, GraphQlArgumentObject, GraphQlEnumArgument } from '@hypertrace/graphql-client'; +import { isEmpty } from 'lodash-es'; +import { AttributeExpression } from '../../../model/attribute/attribute-expression'; import { GraphQlFilter } from '../../../model/schema/filter/graphql-filter'; import { GraphQlSortBySpecification } from '../../../model/schema/sort/graphql-sort-by-specification'; import { GraphQlTimeRange } from '../../../model/schema/timerange/graphql-time-range'; @@ -10,9 +12,13 @@ export class GraphQlArgumentBuilder { } public forAttributeKey(key: string): GraphQlArgument { + return this.forAttributeExpression({ key: key }); + } + + public forAttributeExpression(attributeExpression: AttributeExpression): GraphQlArgument { return { - name: 'key', - value: key + name: 'expression', + value: this.buildAttributeExpression(attributeExpression) }; } @@ -32,10 +38,7 @@ export class GraphQlArgumentBuilder { return [ { name: 'orderBy', - value: orderBys.map(orderBy => ({ - ...orderBy.key.asGraphQlOrderByFragment(), - direction: new GraphQlEnumArgument(orderBy.direction) - })) + value: orderBys.map(orderBy => this.buildOrderByArgumentValue(orderBy)) } ]; } @@ -77,4 +80,22 @@ export class GraphQlArgumentBuilder { public forScope(scope: string): GraphQlArgument { return { name: 'scope', value: scope }; } + + protected buildAttributeExpression( + attributeExpression: AttributeExpression + ): AttributeExpression & GraphQlArgumentObject { + return { + key: attributeExpression.key, + ...(!isEmpty(attributeExpression.subpath) ? { subpath: attributeExpression.subpath } : {}) + }; + } + + protected buildOrderByArgumentValue(orderBy: GraphQlSortBySpecification): GraphQlArgumentObject { + const orderByFragment = orderBy.key.asGraphQlOrderByFragment(); + + return { + direction: new GraphQlEnumArgument(orderBy.direction), + keyExpression: this.buildAttributeExpression(orderByFragment.expression) + }; + } } diff --git a/projects/observability/src/shared/graphql/request/builders/argument/graphql-observability-argument-builder.ts b/projects/observability/src/shared/graphql/request/builders/argument/graphql-observability-argument-builder.ts index 4aebf06d1..1f0801c3b 100644 --- a/projects/observability/src/shared/graphql/request/builders/argument/graphql-observability-argument-builder.ts +++ b/projects/observability/src/shared/graphql/request/builders/argument/graphql-observability-argument-builder.ts @@ -1,11 +1,11 @@ import { TimeDuration } from '@hypertrace/common'; import { GraphQlArgument, GraphQlEnumArgument } from '@hypertrace/graphql-client'; +import { isNil } from 'lodash-es'; import { MetricAggregationType } from '../../../model/metrics/metric-aggregation'; import { EntityType } from '../../../model/schema/entity'; import { GraphQlGroupBy } from '../../../model/schema/groupby/graphql-group-by'; import { GraphQlIntervalUnit } from '../../../model/schema/interval/graphql-interval-unit'; import { convertToGraphQlMetricAggregationType } from '../../../model/schema/metrics/graphql-metric-aggregation-type'; -import { GraphQlSortBySpecification } from '../../../model/schema/sort/graphql-sort-by-specification'; import { TraceType } from '../../../model/schema/trace'; import { convertToGraphQlIntervalUnit } from '../specification/metric/metric-interval-unit-converter'; import { GraphQlArgumentBuilder } from './graphql-argument-builder'; @@ -101,26 +101,6 @@ export class GraphQlObservabilityArgumentBuilder extends GraphQlArgumentBuilder : [{ name: 'aggregation', value: new GraphQlEnumArgument(convertToGraphQlMetricAggregationType(aggregation)) }]; } - public forOrderBy(orderBy?: GraphQlSortBySpecification): GraphQlArgument[] { - return this.forOrderBys(orderBy && [orderBy]); - } - - public forOrderBys(orderBys: GraphQlSortBySpecification[] = []): GraphQlArgument[] { - if (orderBys.length === 0) { - return []; - } - - return [ - { - name: 'orderBy', - value: orderBys.map(orderBy => ({ - ...orderBy.key.asGraphQlOrderByFragment(), - direction: new GraphQlEnumArgument(orderBy.direction) - })) - } - ]; - } - public forGroupBy(groupBy?: GraphQlGroupBy): GraphQlArgument[] { if (!groupBy) { return []; @@ -129,18 +109,11 @@ export class GraphQlObservabilityArgumentBuilder extends GraphQlArgumentBuilder return [ { name: 'groupBy', - value: - // Remove includeRest key if undefined - groupBy.includeRest === undefined - ? { - keys: groupBy.keys, - groupLimit: groupBy.limit - } - : { - keys: groupBy.keys, - groupLimit: groupBy.limit, - includeRest: groupBy.includeRest - } + value: { + expressions: groupBy.keyExpressions.map(expression => this.buildAttributeExpression(expression)), + groupLimit: groupBy.limit, + ...(isNil(groupBy.includeRest) ? {} : { includeRest: groupBy.includeRest }) + } } ]; } diff --git a/projects/observability/src/shared/graphql/request/builders/selections/observability-specification-builder.ts b/projects/observability/src/shared/graphql/request/builders/selections/observability-specification-builder.ts index 47183bf2d..86b43b1f0 100644 --- a/projects/observability/src/shared/graphql/request/builders/selections/observability-specification-builder.ts +++ b/projects/observability/src/shared/graphql/request/builders/selections/observability-specification-builder.ts @@ -297,7 +297,7 @@ export class ObservabilitySpecificationBuilder extends SpecificationBuilder { name: metric, aggregation: aggregation, asGraphQlOrderByFragment: () => ({ - key: metric, + expression: { key: metric }, aggregation: this.aggregationAsEnum(aggregation) }) }; diff --git a/projects/observability/src/shared/graphql/request/builders/specification/attribute/enriched-attribute-specification-builder.ts b/projects/observability/src/shared/graphql/request/builders/specification/attribute/enriched-attribute-specification-builder.ts index 3dcaefcf0..aafcb5834 100644 --- a/projects/observability/src/shared/graphql/request/builders/specification/attribute/enriched-attribute-specification-builder.ts +++ b/projects/observability/src/shared/graphql/request/builders/specification/attribute/enriched-attribute-specification-builder.ts @@ -13,7 +13,7 @@ export class EnrichedAttributeSpecificationBuilder { asGraphQlSelections: () => this.buildGraphQlSelections(attributeKey), extractFromServerData: serverData => this.extractFromServerData(attributeKey, units, serverData), asGraphQlOrderByFragment: () => ({ - key: attributeKey + expression: { key: attributeKey } }) }; } diff --git a/projects/observability/src/shared/graphql/request/builders/specification/entity/entity-specification-builder.ts b/projects/observability/src/shared/graphql/request/builders/specification/entity/entity-specification-builder.ts index 0f46ffad4..0c38ab0bf 100644 --- a/projects/observability/src/shared/graphql/request/builders/specification/entity/entity-specification-builder.ts +++ b/projects/observability/src/shared/graphql/request/builders/specification/entity/entity-specification-builder.ts @@ -44,7 +44,7 @@ export class EntitySpecificationBuilder { additionalSpecifications ), asGraphQlOrderByFragment: () => ({ - key: nameKey + expression: { key: nameKey } }) }; } 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 627e3a41d..e1742a94d 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 @@ -1,5 +1,7 @@ import { DateCoercer, Dictionary } from '@hypertrace/common'; import { GraphQlEnumArgument } from '@hypertrace/graphql-client'; +import { isEmpty } from 'lodash-es'; +import { AttributeExpression } from '../../../../model/attribute/attribute-expression'; import { AttributeMetadataType } from '../../../../model/metadata/attribute-metadata'; import { MetricAggregationType } from '../../../../model/metrics/metric-aggregation'; import { INTERVAL_START_QUERY_KEY } from '../../../../model/schema/explore'; @@ -25,23 +27,31 @@ export class ExploreSpecificationBuilder { type: AttributeMetadataType.Timestamp }), asGraphQlOrderByFragment: () => ({ - key: 'intervalStart' + expression: { key: 'intervalStart' } }) }; } public exploreSpecificationForKey(key: string, aggregation?: MetricAggregationType): ExploreSpecification { - const queryAlias = aggregation === undefined ? key : `${aggregation}_${key}`; + return this.exploreSpecificationForAttributeExpression({ key: key }, aggregation); + } + + public exploreSpecificationForAttributeExpression( + expression: AttributeExpression, + aggregation?: MetricAggregationType + ): ExploreSpecification { + const expressionString = this.attributeExpressionAsString(expression); + const queryAlias = aggregation === undefined ? expressionString : `${aggregation}_${expressionString}`; return { - resultAlias: () => this.buildResultAlias(key, aggregation), - name: key, + resultAlias: () => this.buildResultAlias(expression, aggregation), + name: expressionString, aggregation: aggregation, asGraphQlSelections: () => ({ path: 'selection', alias: queryAlias, arguments: [ - this.argBuilder.forAttributeKey(key), + this.argBuilder.forAttributeExpression(expression), ...this.argBuilder.forAggregation(aggregation), ...this.argBuilder.forAggregationArgs(aggregation) ], @@ -50,7 +60,7 @@ export class ExploreSpecificationBuilder { extractFromServerData: serverData => serverData[queryAlias], asGraphQlOrderByFragment: () => { const fragment: GraphQlSortWithoutDirection & Dictionary = { - key: key + expression: expression }; if (aggregation !== undefined) { @@ -62,8 +72,14 @@ export class ExploreSpecificationBuilder { }; } - protected buildResultAlias(key: string, aggregation?: MetricAggregationType): string { - return aggregation === undefined ? key : `${aggregation}(${key})`; + protected attributeExpressionAsString(expression: AttributeExpression): string { + return isEmpty(expression.subpath) ? expression.key : `${expression.key}.${expression.subpath}`; + } + + protected buildResultAlias(expression: AttributeExpression, aggregation?: MetricAggregationType): string { + const expressionString = this.attributeExpressionAsString(expression); + + return aggregation === undefined ? expressionString : `${aggregation}(${expressionString})`; } protected aggregationAsEnum(aggregation: MetricAggregationType): GraphQlEnumArgument { diff --git a/projects/observability/src/shared/graphql/request/builders/specification/specification-builder.ts b/projects/observability/src/shared/graphql/request/builders/specification/specification-builder.ts index 924080935..e1519addf 100644 --- a/projects/observability/src/shared/graphql/request/builders/specification/specification-builder.ts +++ b/projects/observability/src/shared/graphql/request/builders/specification/specification-builder.ts @@ -20,7 +20,7 @@ export class SpecificationBuilder { extractFromServerData: serverData => specifications.map(specification => specification.extractFromServerData(serverData)), asGraphQlOrderByFragment: () => ({ - key: orderByKey + expression: { key: orderByKey } }) }; } @@ -44,25 +44,7 @@ export class SpecificationBuilder { return serverValue === 'null' ? undefined : serverValue; }, asGraphQlOrderByFragment: () => ({ - key: attributeKey - }) - }; - } - - public fieldSpecificationForKey(field: string): Specification { - return { - resultAlias: () => field, - name: field, - asGraphQlSelections: () => ({ - path: field - }), - extractFromServerData: serverData => { - const serverValue = serverData[field]; - - return serverValue === 'null' ? undefined : serverValue; - }, - asGraphQlOrderByFragment: () => ({ - key: field + expression: { key: attributeKey } }) }; } diff --git a/projects/observability/src/shared/graphql/request/builders/specification/trace/trace-status/trace-status-specification-builder.ts b/projects/observability/src/shared/graphql/request/builders/specification/trace/trace-status/trace-status-specification-builder.ts index 025d557f0..e57e83f40 100644 --- a/projects/observability/src/shared/graphql/request/builders/specification/trace/trace-status/trace-status-specification-builder.ts +++ b/projects/observability/src/shared/graphql/request/builders/specification/trace/trace-status/trace-status-specification-builder.ts @@ -17,7 +17,7 @@ export class TraceStatusSpecificationBuilder { name: TraceStatusSpecificationBuilder.STATUS_CODE_FIELD, asGraphQlSelections: () => this.buildGraphQlSelections(), extractFromServerData: serverData => this.extractFromServerData(serverData), - asGraphQlOrderByFragment: () => ({ key: TraceStatusSpecificationBuilder.STATUS_CODE_FIELD }) + asGraphQlOrderByFragment: () => ({ expression: { key: TraceStatusSpecificationBuilder.STATUS_CODE_FIELD } }) }; } diff --git a/projects/observability/src/shared/graphql/request/handlers/explore/explore-graphql-query-builder.service.ts b/projects/observability/src/shared/graphql/request/handlers/explore/explore-graphql-query-builder.service.ts index 8337583d2..eb00e23d8 100644 --- a/projects/observability/src/shared/graphql/request/handlers/explore/explore-graphql-query-builder.service.ts +++ b/projects/observability/src/shared/graphql/request/handlers/explore/explore-graphql-query-builder.service.ts @@ -102,6 +102,8 @@ export class ExploreGraphqlQueryBuilderService { } private groupByAsSpecifications(groupBy?: GraphQlGroupBy): ExploreSpecification[] { - return (groupBy?.keys ?? []).map(key => this.specBuilder.exploreSpecificationForKey(key)); + return (groupBy?.keyExpressions ?? []).map(expression => + this.specBuilder.exploreSpecificationForAttributeExpression(expression) + ); } } From 775d4aa56bfb49e67884ea6cbb253ea0c887c78c Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld Date: Wed, 12 Jan 2022 01:38:51 -0500 Subject: [PATCH 2/6] test: update tests --- .../pages/explorer/explorer.component.test.ts | 4 +- .../explore-query-editor.component.test.ts | 4 +- .../explore-query-editor.component.ts | 13 ++-- .../explore-visualization-builder.test.ts | 4 +- .../navigable-dashboard.component.test.ts | 2 +- ...xplore-cartesian-data-source.model.test.ts | 4 +- .../explore-cartesian-data-source.model.ts | 2 +- .../graphql/explore/explore-result.test.ts | 6 +- .../data/graphql/explore/explore-result.ts | 2 +- ...zation-cartesian-data-source.model.test.ts | 4 +- ...tric-aggregation-data-source.model.test.ts | 2 +- .../explore-table-data-source.model.test.ts | 2 +- .../trace-donut-data-source.model.test.ts | 2 +- .../data/top-n-data-source.model.test.ts | 2 +- .../argument/graphql-argument-builder.test.ts | 21 +++++-- .../argument/graphql-argument-builder.ts | 4 +- ...ed-attribute-specification-builder.test.ts | 4 +- .../entity-specification-builder.test.ts | 60 +++++++++---------- .../specification-builder.test.ts | 14 ----- ...trace-status-specification-builder.test.ts | 12 ++-- ...ties-graphql-query-handler.service.test.ts | 12 ++-- ...ions-graphql-query-handler.service.test.ts | 8 ++- ...logy-graphql-query-handler.service.test.ts | 40 +++++++++---- ...lore-graphql-query-handler.service.test.ts | 30 +++++----- ...span-graphql-query-handler.service.test.ts | 12 ++-- ...pans-graphql-query-handler.service.test.ts | 16 +---- ...race-graphql-query-handler.service.test.ts | 20 +++++-- ...aces-graphql-query-handler.service.test.ts | 7 +-- 28 files changed, 164 insertions(+), 149 deletions(-) diff --git a/projects/observability/src/pages/explorer/explorer.component.test.ts b/projects/observability/src/pages/explorer/explorer.component.test.ts index a77f92535..617e82718 100644 --- a/projects/observability/src/pages/explorer/explorer.component.test.ts +++ b/projects/observability/src/pages/explorer/explorer.component.test.ts @@ -360,7 +360,7 @@ describe('Explorer component', () => { spectator.query(ExploreQueryEditorComponent)!.setInterval(new TimeDuration(30, TimeUnit.Second)); spectator.query(ExploreQueryEditorComponent)!.updateGroupByKey( { - keys: ['apiName'], + keyExpressions: [{ key: 'apiName' }], limit: 6, includeRest: true }, @@ -370,7 +370,7 @@ describe('Explorer component', () => { expect(queryParamChangeSpy).toHaveBeenLastCalledWith({ scope: 'spans', series: ['column:avg(second)'], - group: 'apiName', + group: ['apiName'], limit: 6, other: true, interval: '30s' 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 984ed8188..3b63a3e12 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 @@ -176,7 +176,7 @@ describe('Explore query editor', () => { expect.objectContaining({ series: [defaultSeries], groupBy: { - keys: ['first groupable'], + keyExpressions: [{ key: 'first groupable' }], limit: 5 // Default group by limit } }) @@ -215,7 +215,7 @@ describe('Explore query editor', () => { expect.objectContaining({ series: [defaultSeries], groupBy: { - keys: ['first groupable'], + keyExpressions: [{ key: 'first groupable' }], limit: 6 } }) 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 de8755e25..0a2a1b99d 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,7 +2,6 @@ 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 { @@ -40,7 +39,7 @@ import { @@ -105,7 +104,7 @@ export class ExploreQueryEditorComponent implements OnChanges, OnInit { } if (changeObject.groupBy && this.groupBy?.keyExpressions.length) { - this.updateGroupByKey(this.groupBy, this.groupBy.keyExpressions[0]); + this.updateGroupByKey(this.groupBy, this.groupBy.keyExpressions[0]?.key); } } @@ -113,14 +112,14 @@ export class ExploreQueryEditorComponent implements OnChanges, OnInit { this.visualizationBuilder.setSeries(series); } - public updateGroupByKey(groupBy?: GraphQlGroupBy, keyExpression?: AttributeExpression): void { - if (keyExpression === undefined) { + public updateGroupByKey(groupBy?: GraphQlGroupBy, key?: string): void { + if (key === undefined) { this.visualizationBuilder.groupBy(); } else { this.visualizationBuilder.groupBy( groupBy - ? { ...groupBy, keyExpressions: [keyExpression] } - : { keyExpressions: [keyExpression], limit: ExploreQueryEditorComponent.DEFAULT_GROUP_LIMIT } + ? { ...groupBy, keyExpressions: [{ key: key }] } + : { keyExpressions: [{ key: key }], limit: ExploreQueryEditorComponent.DEFAULT_GROUP_LIMIT } ); } } diff --git a/projects/observability/src/shared/components/explore-query-editor/explore-visualization-builder.test.ts b/projects/observability/src/shared/components/explore-query-editor/explore-visualization-builder.test.ts index f71afa668..d46c5bd84 100644 --- a/projects/observability/src/shared/components/explore-query-editor/explore-visualization-builder.test.ts +++ b/projects/observability/src/shared/components/explore-query-editor/explore-visualization-builder.test.ts @@ -84,7 +84,7 @@ describe('Explore visualization builder', () => { spectator.service .setSeries([buildSeries('test1')]) .groupBy({ - keys: ['testGroupBy'], + keyExpressions: [{ key: 'testGroupBy' }], limit: 15 }) .setSeries([buildSeries('test2')]); @@ -92,7 +92,7 @@ describe('Explore visualization builder', () => { expectObservable(recordedRequests).toBe('10ms x', { x: expectedQuery({ series: [matchSeriesWithName('test2')], - groupBy: { keys: ['testGroupBy'], limit: 15 } + groupBy: { keyExpressions: [{ key: 'testGroupBy' }], limit: 15 } }) }); }); diff --git a/projects/observability/src/shared/dashboard/dashboard-wrapper/navigable-dashboard.component.test.ts b/projects/observability/src/shared/dashboard/dashboard-wrapper/navigable-dashboard.component.test.ts index beb17daaa..a80fd0f27 100644 --- a/projects/observability/src/shared/dashboard/dashboard-wrapper/navigable-dashboard.component.test.ts +++ b/projects/observability/src/shared/dashboard/dashboard-wrapper/navigable-dashboard.component.test.ts @@ -144,7 +144,7 @@ describe('Navigable dashboard component', () => { }; spectator.query(FilterBarComponent)?.filtersChange.next([explicitFilter]); expect(mockDataSource.addFilters).toHaveBeenCalledWith( - expect.objectContaining({ key: 'foo', operator: GraphQlOperatorType.Equals, value: 'bar' }) + expect.objectContaining({ keyOrExpression: 'foo', operator: GraphQlOperatorType.Equals, value: 'bar' }) ); }); diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts index b901b1ba7..55d4195be 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.test.ts @@ -155,7 +155,7 @@ describe('Explore cartesian data source model', () => { ]; model.groupBy = { - keys: ['baz'], + keyExpressions: [{ key: 'baz' }], includeRest: true, limit: 5 }; @@ -220,7 +220,7 @@ describe('Explore cartesian data source model', () => { ]; model.groupBy = { - keys: ['baz'], + keyExpressions: [{ key: 'baz' }], limit: 5 }; diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts index 035b13a03..092bcd828 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-cartesian-data-source.model.ts @@ -1,11 +1,11 @@ import { ColorService, forkJoinSafeEmpty, RequireBy, TimeDuration } from '@hypertrace/common'; import { ModelInject } from '@hypertrace/hyperdash-angular'; import { isEmpty } from 'lodash-es'; -import { AttributeExpression } from 'projects/observability/src/shared/graphql/model/attribute/attribute-expression'; import { NEVER, Observable, of } from 'rxjs'; import { map, mergeMap } from 'rxjs/operators'; import { Series } from '../../../../components/cartesian/chart'; import { ExploreRequestState } from '../../../../components/explore-query-editor/explore-visualization-builder'; +import { AttributeExpression } from '../../../../graphql/model/attribute/attribute-expression'; import { MetricTimeseriesInterval } from '../../../../graphql/model/metric/metric-timeseries'; import { GraphQlFilter } from '../../../../graphql/model/schema/filter/graphql-filter'; import { ExploreSpecification } from '../../../../graphql/model/schema/specifications/explore-specification'; diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-result.test.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-result.test.ts index 87afdff75..c562600e9 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-result.test.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-result.test.ts @@ -83,7 +83,7 @@ describe('Explore result', () => { ] }); - expect(result.getGroupedSeriesData(['group'], 'foo', MetricAggregationType.Sum)).toEqual([ + expect(result.getGroupedSeriesData([{ key: 'group' }], 'foo', MetricAggregationType.Sum)).toEqual([ { keys: ['first'], value: 10 }, { keys: ['second'], value: 15 }, { keys: ['third'], value: 20 } @@ -116,7 +116,7 @@ describe('Explore result', () => { ] }); - expect(result.getGroupedSeriesData(['group'], 'foo', MetricAggregationType.Sum)).toEqual([ + expect(result.getGroupedSeriesData([{ key: 'group' }], 'foo', MetricAggregationType.Sum)).toEqual([ { keys: ['first'], value: 10 }, { keys: ['Others'], value: 15 } ]); @@ -172,7 +172,7 @@ describe('Explore result', () => { ] }); - expect(result.getGroupedTimeSeriesData(['group'], 'foo', MetricAggregationType.Sum)).toEqual( + expect(result.getGroupedTimeSeriesData([{ key: 'group' }], 'foo', MetricAggregationType.Sum)).toEqual( new Map([ [ ['first'], diff --git a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-result.ts b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-result.ts index 8a8d923d1..732df3187 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explore/explore-result.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explore/explore-result.ts @@ -1,6 +1,6 @@ import { TimeDuration } from '@hypertrace/common'; import { groupBy } from 'lodash-es'; -import { AttributeExpression } from 'projects/observability/src/shared/graphql/model/attribute/attribute-expression'; +import { AttributeExpression } from '../../../../graphql/model/attribute/attribute-expression'; import { MetricTimeseriesInterval } from '../../../../graphql/model/metric/metric-timeseries'; import { MetricAggregationType } from '../../../../graphql/model/metrics/metric-aggregation'; import { ExploreSpecification } from '../../../../graphql/model/schema/specifications/explore-specification'; diff --git a/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts b/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts index 836deda98..621fad703 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model.test.ts @@ -167,7 +167,7 @@ describe('Explorer Visualization cartesian data source model', () => { model.request = buildVisualizationRequest({ interval: undefined, groupBy: { - keys: ['baz'], + keyExpressions: [{ key: 'baz' }], limit: 10 }, series: [ @@ -233,7 +233,7 @@ describe('Explorer Visualization cartesian data source model', () => { model.request = buildVisualizationRequest({ interval: new TimeDuration(5, TimeUnit.Minute), groupBy: { - keys: ['baz'], + keyExpressions: [{ key: 'baz' }], limit: 5 }, series: [ diff --git a/projects/observability/src/shared/dashboard/data/graphql/metric-aggregation/metric-aggregation-data-source.model.test.ts b/projects/observability/src/shared/dashboard/data/graphql/metric-aggregation/metric-aggregation-data-source.model.test.ts index 9ea16c3ef..46c38fb26 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/metric-aggregation/metric-aggregation-data-source.model.test.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/metric-aggregation/metric-aggregation-data-source.model.test.ts @@ -76,7 +76,7 @@ describe('Metric aggregation data source model', () => { }), filters: [ expect.objectContaining({ - key: 'duration', + keyOrExpression: 'duration', operator: GraphQlOperatorType.GreaterThan, value: 500 }) diff --git a/projects/observability/src/shared/dashboard/data/graphql/table/explore/explore-table-data-source.model.test.ts b/projects/observability/src/shared/dashboard/data/graphql/table/explore/explore-table-data-source.model.test.ts index 689c07f08..b75ae910a 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/table/explore/explore-table-data-source.model.test.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/table/explore/explore-table-data-source.model.test.ts @@ -187,7 +187,7 @@ describe('Explore table data source model', () => { limit: 100, offset: 0, groupBy: expect.objectContaining({ - keys: ['name'], + keyExpressions: [{ key: 'name' }], includeRest: false }), orderBy: [ diff --git a/projects/observability/src/shared/dashboard/data/graphql/trace/donut/trace-donut-data-source.model.test.ts b/projects/observability/src/shared/dashboard/data/graphql/trace/donut/trace-donut-data-source.model.test.ts index c74c1b8e4..cc6c05381 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/trace/donut/trace-donut-data-source.model.test.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/trace/donut/trace-donut-data-source.model.test.ts @@ -63,7 +63,7 @@ describe('Donut data source model', () => { timeRange: GraphQlTimeRange.fromTimeRange(mockTimeRange), filters: [], groupBy: { - keys: ['bar'], + keyExpressions: [{ key: 'bar' }], limit: 3 }, limit: 3, diff --git a/projects/observability/src/shared/dashboard/widgets/top-n/data/top-n-data-source.model.test.ts b/projects/observability/src/shared/dashboard/widgets/top-n/data/top-n-data-source.model.test.ts index f242c5ca6..d95da0557 100644 --- a/projects/observability/src/shared/dashboard/widgets/top-n/data/top-n-data-source.model.test.ts +++ b/projects/observability/src/shared/dashboard/widgets/top-n/data/top-n-data-source.model.test.ts @@ -61,7 +61,7 @@ describe('Top N Data Source Model', () => { ]), filters: [], groupBy: expect.objectContaining({ - keys: ['nameKey', 'idKey'] + keyExpressions: [{ key: 'nameKey' }, { key: 'idKey' }] }) }) ); diff --git a/projects/observability/src/shared/graphql/request/builders/argument/graphql-argument-builder.test.ts b/projects/observability/src/shared/graphql/request/builders/argument/graphql-argument-builder.test.ts index c3773912d..a48b0faed 100644 --- a/projects/observability/src/shared/graphql/request/builders/argument/graphql-argument-builder.test.ts +++ b/projects/observability/src/shared/graphql/request/builders/argument/graphql-argument-builder.test.ts @@ -15,7 +15,9 @@ describe('Graphql Argument Builder', () => { }); test('construct arguments for attribute key correctly', () => { - expect(argBuilder.forAttributeKey('test-key')).toEqual(expect.objectContaining({ name: 'key', value: 'test-key' })); + expect(argBuilder.forAttributeKey('test-key')).toEqual( + expect.objectContaining({ name: 'expression', value: { key: 'test-key' } }) + ); }); test('construct arguments for limit correctly', () => { @@ -26,7 +28,11 @@ describe('Graphql Argument Builder', () => { expect(argBuilder.forOrderBy(undefined)).toEqual(expect.arrayContaining([])); expect( argBuilder.forOrderBy({ direction: 'ASC', key: specBuilder.attributeSpecificationForKey('test-key') }) - ).toEqual(expect.arrayContaining([{ name: 'orderBy', value: [{ direction: { value: 'ASC' }, key: 'test-key' }] }])); + ).toEqual( + expect.arrayContaining([ + { name: 'orderBy', value: [{ direction: { value: 'ASC' }, keyExpression: { key: 'test-key' } }] } + ]) + ); }); test('construct arguments for orderBys correctly', () => { @@ -41,8 +47,8 @@ describe('Graphql Argument Builder', () => { { name: 'orderBy', value: [ - { direction: { value: 'ASC' }, key: 'test-key' }, - { direction: { value: 'ASC' }, key: 'test-key2' } + { direction: { value: 'ASC' }, keyExpression: { key: 'test-key' } }, + { direction: { value: 'ASC' }, keyExpression: { key: 'test-key2' } } ] } ]) @@ -82,7 +88,12 @@ describe('Graphql Argument Builder', () => { { name: 'filterBy', value: [ - { key: 'filter-key', operator: { value: 'EQUALS' }, type: { value: 'ATTRIBUTE' }, value: 'filter-value' } + { + keyExpression: { key: 'filter-key' }, + operator: { value: 'EQUALS' }, + type: { value: 'ATTRIBUTE' }, + value: 'filter-value' + } ] } ]) diff --git a/projects/observability/src/shared/graphql/request/builders/argument/graphql-argument-builder.ts b/projects/observability/src/shared/graphql/request/builders/argument/graphql-argument-builder.ts index 7d4276dde..abc21b9d4 100644 --- a/projects/observability/src/shared/graphql/request/builders/argument/graphql-argument-builder.ts +++ b/projects/observability/src/shared/graphql/request/builders/argument/graphql-argument-builder.ts @@ -1,5 +1,5 @@ import { GraphQlArgument, GraphQlArgumentObject, GraphQlEnumArgument } from '@hypertrace/graphql-client'; -import { isEmpty } from 'lodash-es'; +import { isEmpty, omit } from 'lodash-es'; import { AttributeExpression } from '../../../model/attribute/attribute-expression'; import { GraphQlFilter } from '../../../model/schema/filter/graphql-filter'; import { GraphQlSortBySpecification } from '../../../model/schema/sort/graphql-sort-by-specification'; @@ -92,8 +92,10 @@ export class GraphQlArgumentBuilder { protected buildOrderByArgumentValue(orderBy: GraphQlSortBySpecification): GraphQlArgumentObject { const orderByFragment = orderBy.key.asGraphQlOrderByFragment(); + const unknownFields = omit(orderByFragment, 'direction', 'expression'); return { + ...unknownFields, direction: new GraphQlEnumArgument(orderBy.direction), keyExpression: this.buildAttributeExpression(orderByFragment.expression) }; diff --git a/projects/observability/src/shared/graphql/request/builders/specification/attribute/enriched-attribute-specification-builder.test.ts b/projects/observability/src/shared/graphql/request/builders/specification/attribute/enriched-attribute-specification-builder.test.ts index 161465c74..ad85f6c30 100644 --- a/projects/observability/src/shared/graphql/request/builders/specification/attribute/enriched-attribute-specification-builder.test.ts +++ b/projects/observability/src/shared/graphql/request/builders/specification/attribute/enriched-attribute-specification-builder.test.ts @@ -11,8 +11,8 @@ describe('EnrichedAttributeSpecificationBuilder', () => { alias: 'duration', arguments: [ { - name: 'key', - value: 'duration' + name: 'expression', + value: { key: 'duration' } } ] }); diff --git a/projects/observability/src/shared/graphql/request/builders/specification/entity/entity-specification-builder.test.ts b/projects/observability/src/shared/graphql/request/builders/specification/entity/entity-specification-builder.test.ts index b4faf9711..16bbc89b8 100644 --- a/projects/observability/src/shared/graphql/request/builders/specification/entity/entity-specification-builder.test.ts +++ b/projects/observability/src/shared/graphql/request/builders/specification/entity/entity-specification-builder.test.ts @@ -18,8 +18,8 @@ describe('Entity Specification Builder', () => { alias: 'id', arguments: [ { - name: 'key', - value: 'id' + name: 'expression', + value: { key: 'id' } } ] }, @@ -28,8 +28,8 @@ describe('Entity Specification Builder', () => { alias: 'name', arguments: [ { - name: 'key', - value: 'name' + name: 'expression', + value: { key: 'name' } } ] }, @@ -38,8 +38,8 @@ describe('Entity Specification Builder', () => { alias: 'attribute1', arguments: [ { - name: 'key', - value: 'attribute1' + name: 'expression', + value: { key: 'attribute1' } } ] }, @@ -48,8 +48,8 @@ describe('Entity Specification Builder', () => { alias: 'attribute2', arguments: [ { - name: 'key', - value: 'attribute2' + name: 'expression', + value: { key: 'attribute2' } } ] } @@ -89,8 +89,8 @@ describe('Entity Specification Builder', () => { alias: 'id', arguments: [ { - name: 'key', - value: 'id' + name: 'expression', + value: { key: 'id' } } ] }, @@ -99,8 +99,8 @@ describe('Entity Specification Builder', () => { alias: 'name', arguments: [ { - name: 'key', - value: 'name' + name: 'expression', + value: { key: 'name' } } ] }, @@ -109,8 +109,8 @@ describe('Entity Specification Builder', () => { alias: 'attribute1', arguments: [ { - name: 'key', - value: 'attribute1' + name: 'expression', + value: { key: 'attribute1' } } ] }, @@ -119,8 +119,8 @@ describe('Entity Specification Builder', () => { alias: 'attribute2', arguments: [ { - name: 'key', - value: 'attribute2' + name: 'expression', + value: { key: 'attribute2' } } ] }, @@ -129,8 +129,8 @@ describe('Entity Specification Builder', () => { alias: 'status', arguments: [ { - name: 'key', - value: 'status' + name: 'expression', + value: { key: 'status' } } ] }, @@ -139,8 +139,8 @@ describe('Entity Specification Builder', () => { alias: 'statusCode', arguments: [ { - name: 'key', - value: 'statusCode' + name: 'expression', + value: { key: 'statusCode' } } ] }, @@ -149,8 +149,8 @@ describe('Entity Specification Builder', () => { alias: 'statusMessage', arguments: [ { - name: 'key', - value: 'statusMessage' + name: 'expression', + value: { key: 'statusMessage' } } ] } @@ -189,8 +189,8 @@ describe('Entity Specification Builder', () => { alias: 'id', arguments: [ { - name: 'key', - value: 'id' + name: 'expression', + value: { key: 'id' } } ] }, @@ -199,8 +199,8 @@ describe('Entity Specification Builder', () => { alias: 'name', arguments: [ { - name: 'key', - value: 'name' + name: 'expression', + value: { key: 'name' } } ] }, @@ -231,8 +231,8 @@ describe('Entity Specification Builder', () => { alias: 'id', arguments: [ { - name: 'key', - value: 'id' + name: 'expression', + value: { key: 'id' } } ] }, @@ -241,8 +241,8 @@ describe('Entity Specification Builder', () => { alias: 'name', arguments: [ { - name: 'key', - value: 'name' + name: 'expression', + value: { key: 'name' } } ] }, diff --git a/projects/observability/src/shared/graphql/request/builders/specification/specification-builder.test.ts b/projects/observability/src/shared/graphql/request/builders/specification/specification-builder.test.ts index eb0859c0f..a60d4faf9 100644 --- a/projects/observability/src/shared/graphql/request/builders/specification/specification-builder.test.ts +++ b/projects/observability/src/shared/graphql/request/builders/specification/specification-builder.test.ts @@ -9,18 +9,4 @@ describe('Specification Builder', () => { }) ).toBeUndefined(); }); - - test('field specs should build correct selection and response', () => { - const fieldSpec = specBuilder.fieldSpecificationForKey('test'); - - expect(fieldSpec.resultAlias()).toEqual('test'); - expect(fieldSpec.name).toEqual('test'); - - expect(fieldSpec.asGraphQlSelections()).toEqual({ path: 'test' }); - expect(fieldSpec.extractFromServerData({ test: 'null' })).toBeUndefined(); - - expect(fieldSpec.extractFromServerData({ test: 'test value' })).toEqual('test value'); - - expect(fieldSpec.asGraphQlOrderByFragment()).toEqual({ key: 'test' }); - }); }); diff --git a/projects/observability/src/shared/graphql/request/builders/specification/trace/trace-status/trace-status-specification-builder.test.ts b/projects/observability/src/shared/graphql/request/builders/specification/trace/trace-status/trace-status-specification-builder.test.ts index b9eb6f6d5..d52f31832 100644 --- a/projects/observability/src/shared/graphql/request/builders/specification/trace/trace-status/trace-status-specification-builder.test.ts +++ b/projects/observability/src/shared/graphql/request/builders/specification/trace/trace-status/trace-status-specification-builder.test.ts @@ -13,8 +13,8 @@ describe('TraceStatusSpecificationBuilder', () => { alias: 'status', arguments: [ { - name: 'key', - value: 'status' + name: 'expression', + value: { key: 'status' } } ] }, @@ -23,8 +23,8 @@ describe('TraceStatusSpecificationBuilder', () => { alias: 'statusCode', arguments: [ { - name: 'key', - value: 'statusCode' + name: 'expression', + value: { key: 'statusCode' } } ] }, @@ -33,8 +33,8 @@ describe('TraceStatusSpecificationBuilder', () => { alias: 'statusMessage', arguments: [ { - name: 'key', - value: 'statusMessage' + name: 'expression', + value: { key: 'statusMessage' } } ] } diff --git a/projects/observability/src/shared/graphql/request/handlers/entities/query/entities-graphql-query-handler.service.test.ts b/projects/observability/src/shared/graphql/request/handlers/entities/query/entities-graphql-query-handler.service.test.ts index 1ad28b8aa..a3a5d1c95 100644 --- a/projects/observability/src/shared/graphql/request/handlers/entities/query/entities-graphql-query-handler.service.test.ts +++ b/projects/observability/src/shared/graphql/request/handlers/entities/query/entities-graphql-query-handler.service.test.ts @@ -94,7 +94,7 @@ describe('Entities graphql query handler', () => { { path: 'attribute', alias: 'name', - arguments: [{ name: 'key', value: 'name' }] + arguments: [{ name: 'expression', value: { key: 'name' } }] } ] }, @@ -217,7 +217,7 @@ describe('Entities graphql query handler', () => { { path: 'metric', alias: 'some_metric', - arguments: [{ name: 'key', value: 'some_metric' }], + arguments: [{ name: 'expression', value: { key: 'some_metric' } }], children: [ { alias: 'avgrate_min', @@ -234,7 +234,7 @@ describe('Entities graphql query handler', () => { { path: 'metric', alias: 'some_metric', - arguments: [{ name: 'key', value: 'some_metric' }], + arguments: [{ name: 'expression', value: { key: 'some_metric' } }], children: [ { alias: 'avgrate_sec', @@ -343,7 +343,7 @@ describe('Entities graphql query handler', () => { { path: 'metric', alias: 'a_metric', - arguments: [{ name: 'key', value: 'a_metric' }], + arguments: [{ name: 'expression', value: { key: 'a_metric' } }], children: [ { alias: 'min_series_1m', @@ -362,7 +362,7 @@ describe('Entities graphql query handler', () => { { path: 'metric', alias: 'a_metric', - arguments: [{ name: 'key', value: 'a_metric' }], + arguments: [{ name: 'expression', value: { key: 'a_metric' } }], children: [ { alias: 'max_series_1m', @@ -382,7 +382,7 @@ describe('Entities graphql query handler', () => { { path: 'metric', alias: 'a_metric', - arguments: [{ name: 'key', value: 'a_metric' }], + arguments: [{ name: 'expression', value: { key: 'a_metric' } }], children: [ { alias: 'min_series_5m', diff --git a/projects/observability/src/shared/graphql/request/handlers/entities/query/interactions/interactions-graphql-query-handler.service.test.ts b/projects/observability/src/shared/graphql/request/handlers/entities/query/interactions/interactions-graphql-query-handler.service.test.ts index 9daeb73ed..b6b5d8433 100644 --- a/projects/observability/src/shared/graphql/request/handlers/entities/query/interactions/interactions-graphql-query-handler.service.test.ts +++ b/projects/observability/src/shared/graphql/request/handlers/entities/query/interactions/interactions-graphql-query-handler.service.test.ts @@ -110,7 +110,7 @@ describe('Interactions graphql query handler', () => { { path: 'metric', alias: 'duration', - arguments: [{ name: 'key', value: 'duration' }], + arguments: [{ name: 'expression', value: { key: 'duration' } }], children: [ { path: 'avg', @@ -124,7 +124,11 @@ describe('Interactions graphql query handler', () => { path: 'neighbor', children: [ { path: 'id' }, - { path: 'attribute', alias: 'name', arguments: [{ name: 'key', value: 'name' }] } + { + path: 'attribute', + alias: 'name', + arguments: [{ name: 'expression', value: { key: 'name' } }] + } ] } ] diff --git a/projects/observability/src/shared/graphql/request/handlers/entities/query/topology/entity-topology-graphql-query-handler.service.test.ts b/projects/observability/src/shared/graphql/request/handlers/entities/query/topology/entity-topology-graphql-query-handler.service.test.ts index b61a9f3f4..19d3139b8 100644 --- a/projects/observability/src/shared/graphql/request/handlers/entities/query/topology/entity-topology-graphql-query-handler.service.test.ts +++ b/projects/observability/src/shared/graphql/request/handlers/entities/query/topology/entity-topology-graphql-query-handler.service.test.ts @@ -259,11 +259,11 @@ describe('Entity topology graphql query handler', () => { path: 'results', children: [ { path: 'id' }, - { path: 'attribute', alias: 'name', arguments: [{ name: 'key', value: 'name' }] }, + { path: 'attribute', alias: 'name', arguments: [{ name: 'expression', value: { key: 'name' } }] }, { path: 'metric', alias: 'duration', - arguments: [{ name: 'key', value: 'duration' }], + arguments: [{ name: 'expression', value: { key: 'duration' } }], children: [ { path: 'avg', @@ -284,7 +284,7 @@ describe('Entity topology graphql query handler', () => { { path: 'metric', alias: 'duration', - arguments: [{ name: 'key', value: 'duration' }], + arguments: [{ name: 'expression', value: { key: 'duration' } }], children: [ { path: 'avg', @@ -298,12 +298,20 @@ describe('Entity topology graphql query handler', () => { path: 'neighbor', children: [ { path: 'id' }, - { path: 'attribute', alias: 'name', arguments: [{ name: 'key', value: 'name' }] }, - { path: 'attribute', alias: 'type', arguments: [{ name: 'key', value: 'type' }] }, + { + path: 'attribute', + alias: 'name', + arguments: [{ name: 'expression', value: { key: 'name' } }] + }, + { + path: 'attribute', + alias: 'type', + arguments: [{ name: 'expression', value: { key: 'type' } }] + }, { path: 'metric', alias: 'numCalls', - arguments: [{ name: 'key', value: 'numCalls' }], + arguments: [{ name: 'expression', value: { key: 'numCalls' } }], children: [ { path: 'avgrate', @@ -333,7 +341,7 @@ describe('Entity topology graphql query handler', () => { { path: 'metric', alias: 'duration', - arguments: [{ name: 'key', value: 'duration' }], + arguments: [{ name: 'expression', value: { key: 'duration' } }], children: [ { path: 'avg', @@ -347,11 +355,15 @@ describe('Entity topology graphql query handler', () => { path: 'neighbor', children: [ { path: 'id' }, - { path: 'attribute', alias: 'name', arguments: [{ name: 'key', value: 'name' }] }, + { + path: 'attribute', + alias: 'name', + arguments: [{ name: 'expression', value: { key: 'name' } }] + }, { path: 'metric', alias: 'duration', - arguments: [{ name: 'key', value: 'duration' }], + arguments: [{ name: 'expression', value: { key: 'duration' } }], children: [ { path: 'avg', @@ -378,7 +390,7 @@ describe('Entity topology graphql query handler', () => { { path: 'metric', alias: 'duration', - arguments: [{ name: 'key', value: 'duration' }], + arguments: [{ name: 'expression', value: { key: 'duration' } }], children: [ { path: 'avg', @@ -392,11 +404,15 @@ describe('Entity topology graphql query handler', () => { path: 'neighbor', children: [ { path: 'id' }, - { path: 'attribute', alias: 'name', arguments: [{ name: 'key', value: 'name' }] }, + { + path: 'attribute', + alias: 'name', + arguments: [{ name: 'expression', value: { key: 'name' } }] + }, { path: 'metric', alias: 'duration', - arguments: [{ name: 'key', value: 'duration' }], + arguments: [{ name: 'expression', value: { key: 'duration' } }], children: [ { path: 'avg', diff --git a/projects/observability/src/shared/graphql/request/handlers/explore/explore-graphql-query-handler.service.test.ts b/projects/observability/src/shared/graphql/request/handlers/explore/explore-graphql-query-handler.service.test.ts index 8b51596b6..4a240fc89 100644 --- a/projects/observability/src/shared/graphql/request/handlers/explore/explore-graphql-query-handler.service.test.ts +++ b/projects/observability/src/shared/graphql/request/handlers/explore/explore-graphql-query-handler.service.test.ts @@ -57,7 +57,7 @@ describe('Explore graphql query handler', () => { groupBy: { limit: 2, includeRest: true, - keys: ['serviceName'] + keyExpressions: [{ key: 'serviceName' }] }, orderBy: [ { @@ -98,13 +98,13 @@ describe('Explore graphql query handler', () => { name: 'filterBy', value: [ { - key: 'duration', + keyExpression: { key: 'duration' }, operator: new GraphQlEnumArgument(GraphQlOperatorType.GreaterThan), value: 0, type: new GraphQlEnumArgument(GraphQlFilterType.Attribute) }, { - key: 'duration', + keyExpression: { key: 'duration' }, operator: new GraphQlEnumArgument(GraphQlOperatorType.LessThan), value: 100, type: new GraphQlEnumArgument(GraphQlFilterType.Attribute) @@ -115,7 +115,7 @@ describe('Explore graphql query handler', () => { name: 'groupBy', value: { includeRest: true, - keys: ['serviceName'], + expressions: [{ key: 'serviceName' }], groupLimit: 2 } }, @@ -124,7 +124,7 @@ describe('Explore graphql query handler', () => { value: [ { direction: new GraphQlEnumArgument('ASC'), - key: 'duration', + keyExpression: { key: 'duration' }, aggregation: new GraphQlEnumArgument(GraphQlMetricAggregationType.Average) } ] @@ -138,14 +138,14 @@ describe('Explore graphql query handler', () => { { path: 'selection', alias: 'serviceName', - arguments: [{ name: 'key', value: 'serviceName' }], + arguments: [{ name: 'expression', value: { key: 'serviceName' } }], children: [{ path: 'value' }, { path: 'type' }] }, { path: 'selection', alias: 'avg_duration', arguments: [ - { name: 'key', value: 'duration' }, + { name: 'expression', value: { key: 'duration' } }, { name: 'aggregation', value: new GraphQlEnumArgument(GraphQlMetricAggregationType.Average) } ], children: [{ path: 'value' }, { path: 'type' }] @@ -154,7 +154,7 @@ describe('Explore graphql query handler', () => { path: 'selection', alias: 'avgrate_min_duration', arguments: [ - { name: 'key', value: 'duration' }, + { name: 'expression', value: { key: 'duration' } }, { name: 'aggregation', value: new GraphQlEnumArgument(GraphQlMetricAggregationType.Avgrate) }, { name: 'units', value: new GraphQlEnumArgument(GraphQlIntervalUnit.Minutes) }, { name: 'size', value: 1 } @@ -193,13 +193,13 @@ describe('Explore graphql query handler', () => { name: 'filterBy', value: [ { - key: 'duration', + keyExpression: { key: 'duration' }, operator: new GraphQlEnumArgument(GraphQlOperatorType.GreaterThan), value: 0, type: new GraphQlEnumArgument(GraphQlFilterType.Attribute) }, { - key: 'duration', + keyExpression: { key: 'duration' }, operator: new GraphQlEnumArgument(GraphQlOperatorType.LessThan), value: 100, type: new GraphQlEnumArgument(GraphQlFilterType.Attribute) @@ -210,7 +210,7 @@ describe('Explore graphql query handler', () => { name: 'groupBy', value: { includeRest: true, - keys: ['serviceName'], + expressions: [{ key: 'serviceName' }], groupLimit: 2 } }, @@ -219,7 +219,7 @@ describe('Explore graphql query handler', () => { value: [ { direction: new GraphQlEnumArgument('ASC'), - key: 'duration', + keyExpression: { key: 'duration' }, aggregation: new GraphQlEnumArgument(GraphQlMetricAggregationType.Average) } ] @@ -233,14 +233,14 @@ describe('Explore graphql query handler', () => { { path: 'selection', alias: 'serviceName', - arguments: [{ name: 'key', value: 'serviceName' }], + arguments: [{ name: 'expression', value: { key: 'serviceName' } }], children: [{ path: 'value' }, { path: 'type' }] }, { path: 'selection', alias: 'avg_duration', arguments: [ - { name: 'key', value: 'duration' }, + { name: 'expression', value: { key: 'duration' } }, { name: 'aggregation', value: new GraphQlEnumArgument(GraphQlMetricAggregationType.Average) } ], children: [{ path: 'value' }, { path: 'type' }] @@ -249,7 +249,7 @@ describe('Explore graphql query handler', () => { path: 'selection', alias: 'avgrate_min_duration', arguments: [ - { name: 'key', value: 'duration' }, + { name: 'expression', value: { key: 'duration' } }, { name: 'aggregation', value: new GraphQlEnumArgument(GraphQlMetricAggregationType.Avgrate) }, { name: 'units', value: new GraphQlEnumArgument(GraphQlIntervalUnit.Minutes) }, { name: 'size', value: 1 } diff --git a/projects/observability/src/shared/graphql/request/handlers/spans/span-graphql-query-handler.service.test.ts b/projects/observability/src/shared/graphql/request/handlers/spans/span-graphql-query-handler.service.test.ts index a8216410d..60049401d 100644 --- a/projects/observability/src/shared/graphql/request/handlers/spans/span-graphql-query-handler.service.test.ts +++ b/projects/observability/src/shared/graphql/request/handlers/spans/span-graphql-query-handler.service.test.ts @@ -101,7 +101,7 @@ describe('SpanGraphQlQueryHandlerService', () => { operator: new GraphQlEnumArgument('EQUALS'), value: 'test-id', type: new GraphQlEnumArgument(GraphQlFilterType.Attribute), - key: 'id' + keyExpression: { key: 'id' } } ] } @@ -111,8 +111,8 @@ describe('SpanGraphQlQueryHandlerService', () => { path: 'results', children: [ { path: 'id' }, - { path: 'attribute', alias: 'apiName', arguments: [{ name: 'key', value: 'apiName' }] }, - { path: 'attribute', alias: 'duration', arguments: [{ name: 'key', value: 'duration' }] } + { path: 'attribute', alias: 'apiName', arguments: [{ name: 'expression', value: { key: 'apiName' } }] }, + { path: 'attribute', alias: 'duration', arguments: [{ name: 'expression', value: { key: 'duration' } }] } ] }, { path: 'total' } @@ -145,7 +145,7 @@ describe('SpanGraphQlQueryHandlerService', () => { operator: new GraphQlEnumArgument('EQUALS'), value: 'test-id', type: new GraphQlEnumArgument(GraphQlFilterType.Attribute), - key: 'id' + keyExpression: { key: 'id' } } ] } @@ -155,8 +155,8 @@ describe('SpanGraphQlQueryHandlerService', () => { path: 'results', children: [ { path: 'id' }, - { path: 'attribute', alias: 'apiName', arguments: [{ name: 'key', value: 'apiName' }] }, - { path: 'attribute', alias: 'duration', arguments: [{ name: 'key', value: 'duration' }] } + { path: 'attribute', alias: 'apiName', arguments: [{ name: 'expression', value: { key: 'apiName' } }] }, + { path: 'attribute', alias: 'duration', arguments: [{ name: 'expression', value: { key: 'duration' } }] } ] }, { path: 'total' } diff --git a/projects/observability/src/shared/graphql/request/handlers/spans/spans-graphql-query-handler.service.test.ts b/projects/observability/src/shared/graphql/request/handlers/spans/spans-graphql-query-handler.service.test.ts index f91e6a988..dbb0a0cb1 100644 --- a/projects/observability/src/shared/graphql/request/handlers/spans/spans-graphql-query-handler.service.test.ts +++ b/projects/observability/src/shared/graphql/request/handlers/spans/spans-graphql-query-handler.service.test.ts @@ -97,7 +97,7 @@ describe('SpansGraphQlQueryHandlerService', () => { direction: { value: 'ASC' }, - key: 'apiName' + keyExpression: { key: 'apiName' } } ] } @@ -109,22 +109,12 @@ describe('SpansGraphQlQueryHandlerService', () => { { path: 'id' }, { alias: 'apiName', - arguments: [ - { - name: 'key', - value: 'apiName' - } - ], + arguments: [{ name: 'expression', value: { key: 'apiName' } }], path: 'attribute' }, { alias: 'duration', - arguments: [ - { - name: 'key', - value: 'duration' - } - ], + arguments: [{ name: 'expression', value: { key: 'duration' } }], path: 'attribute' } ] diff --git a/projects/observability/src/shared/graphql/request/handlers/traces/trace-graphql-query-handler.service.test.ts b/projects/observability/src/shared/graphql/request/handlers/traces/trace-graphql-query-handler.service.test.ts index 4112f5846..a0a076ffd 100644 --- a/projects/observability/src/shared/graphql/request/handlers/traces/trace-graphql-query-handler.service.test.ts +++ b/projects/observability/src/shared/graphql/request/handlers/traces/trace-graphql-query-handler.service.test.ts @@ -79,7 +79,10 @@ describe('TraceGraphQlQueryHandlerService', () => { children: [ { path: 'results', - children: [{ path: 'id' }, { path: 'attribute', alias: 'name', arguments: [{ name: 'key', value: 'name' }] }] + children: [ + { path: 'id' }, + { path: 'attribute', alias: 'name', arguments: [{ name: 'expression', value: { key: 'name' } }] } + ] } ] }); @@ -113,7 +116,10 @@ describe('TraceGraphQlQueryHandlerService', () => { children: [ { path: 'results', - children: [{ path: 'id' }, { path: 'attribute', alias: 'name', arguments: [{ name: 'key', value: 'name' }] }] + children: [ + { path: 'id' }, + { path: 'attribute', alias: 'name', arguments: [{ name: 'expression', value: { key: 'name' } }] } + ] } ] }); @@ -156,7 +162,10 @@ describe('TraceGraphQlQueryHandlerService', () => { children: [ { path: 'results', - children: [{ path: 'id' }, { path: 'attribute', alias: 'name', arguments: [{ name: 'key', value: 'name' }] }] + children: [ + { path: 'id' }, + { path: 'attribute', alias: 'name', arguments: [{ name: 'expression', value: { key: 'name' } }] } + ] } ] }); @@ -190,7 +199,10 @@ describe('TraceGraphQlQueryHandlerService', () => { children: [ { path: 'results', - children: [{ path: 'id' }, { path: 'attribute', alias: 'name', arguments: [{ name: 'key', value: 'name' }] }] + children: [ + { path: 'id' }, + { path: 'attribute', alias: 'name', arguments: [{ name: 'expression', value: { key: 'name' } }] } + ] } ] }); diff --git a/projects/observability/src/shared/graphql/request/handlers/traces/traces-graphql-query-handler.service.test.ts b/projects/observability/src/shared/graphql/request/handlers/traces/traces-graphql-query-handler.service.test.ts index a35a92502..a16752d56 100644 --- a/projects/observability/src/shared/graphql/request/handlers/traces/traces-graphql-query-handler.service.test.ts +++ b/projects/observability/src/shared/graphql/request/handlers/traces/traces-graphql-query-handler.service.test.ts @@ -74,12 +74,7 @@ describe('TracesGraphQlQueryHandlerService', () => { { alias: 'name', path: 'attribute', - arguments: [ - { - name: 'key', - value: 'name' - } - ] + arguments: [{ name: 'expression', value: { key: 'name' } }] } ] }, From a7efe69cecbc10565806edab2f220f12742e605f Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld Date: Thu, 6 Jan 2022 12:13:51 -0500 Subject: [PATCH 3/6] feat: add support for filtering with attribute expressions --- projects/common/src/utilities/types/types.ts | 2 + .../filter-chip/filter-chip.component.ts | 3 +- .../filter-chip/filter-chip.service.ts | 183 +++++++++--------- .../builder/types/abstract-filter-builder.ts | 74 ++++++- .../builder/types/boolean-filter-builder.ts | 6 +- .../builder/types/number-filter-builder.ts | 6 +- .../builder/types/string-filter-builder.ts | 6 +- .../types/string-map-filter-builder.ts | 46 +---- .../src/filtering/filter/filter-delimiters.ts | 1 - .../src/filtering/filter/filter-operators.ts | 14 +- .../filtering/filter/filter-url.service.ts | 28 +-- .../components/src/filtering/filter/filter.ts | 1 + .../parser/filter-parser-lookup.service.ts | 1 - .../filtering/filter/parser/parsed-filter.ts | 73 ++++++- .../parser/types/abstract-filter-parser.ts | 80 +------- .../parser/types/comparison-filter-parser.ts | 19 +- .../parser/types/contains-filter-parser.ts | 84 +------- .../filter/parser/types/in-filter-parser.ts | 10 +- .../src/pages/explorer/explorer-service.ts | 2 +- .../tags/span-tags-detail.component.ts | 2 +- .../filter/graphql-id-scope-filter.model.ts | 4 +- .../filter/graphql-key-value-filter.model.ts | 3 +- .../model/schema/filter/graphql-filter.ts | 3 +- .../graphql-filter-builder.service.ts | 8 +- 24 files changed, 310 insertions(+), 349 deletions(-) diff --git a/projects/common/src/utilities/types/types.ts b/projects/common/src/utilities/types/types.ts index 74a8cb7b6..de105c912 100644 --- a/projects/common/src/utilities/types/types.ts +++ b/projects/common/src/utilities/types/types.ts @@ -7,6 +7,8 @@ export type DistributiveOmit> = T extends any ? Omit = Omit & Partial>; export type RequireBy = T & Required>; +export type KeysWithType = { [K in keyof T]-?: T[K] extends V ? K : never }[keyof T]; + export interface Dictionary { [key: string]: T; } diff --git a/projects/components/src/filtering/filter-bar/filter-chip/filter-chip.component.ts b/projects/components/src/filtering/filter-bar/filter-chip/filter-chip.component.ts index 2937fdc3b..b5294f54d 100644 --- a/projects/components/src/filtering/filter-bar/filter-chip/filter-chip.component.ts +++ b/projects/components/src/filtering/filter-bar/filter-chip/filter-chip.component.ts @@ -120,8 +120,7 @@ export class FilterChipComponent implements OnInit, OnChanges { private mapToComboBoxOption(filter: IncompleteFilter): ComboBoxOption { return { text: filter.userString, - value: filter, - tooltip: `${filter.userString} (${filter.field})` + value: filter }; } } diff --git a/projects/components/src/filtering/filter-bar/filter-chip/filter-chip.service.ts b/projects/components/src/filtering/filter-bar/filter-chip/filter-chip.service.ts index 5aa5416d3..c72761724 100644 --- a/projects/components/src/filtering/filter-bar/filter-chip/filter-chip.service.ts +++ b/projects/components/src/filtering/filter-bar/filter-chip/filter-chip.service.ts @@ -1,12 +1,18 @@ import { Injectable } from '@angular/core'; +import { isEmpty } from 'lodash-es'; import { FilterBuilderLookupService } from '../../filter/builder/filter-builder-lookup.service'; import { AbstractFilterBuilder } from '../../filter/builder/types/abstract-filter-builder'; import { IncompleteFilter } from '../../filter/filter'; import { FilterAttribute } from '../../filter/filter-attribute'; import { FilterOperator } from '../../filter/filter-operators'; import { FilterParserLookupService } from '../../filter/parser/filter-parser-lookup.service'; -import { SplitFilter } from '../../filter/parser/parsed-filter'; -import { AbstractFilterParser, splitFilterStringByOperator } from '../../filter/parser/types/abstract-filter-parser'; +import { + FilterAttributeExpression, + SplitFilter, + splitFilterStringByOperator, + tryParseStringForAttribute +} from '../../filter/parser/parsed-filter'; +import { AbstractFilterParser } from '../../filter/parser/types/abstract-filter-parser'; @Injectable({ providedIn: 'root' @@ -20,118 +26,119 @@ export class FilterChipService { public autocompleteFilters(attributes: FilterAttribute[], text: string = ''): IncompleteFilter[] { return attributes .filter(attribute => this.filterBuilderLookupService.isBuildableType(attribute.type)) - .flatMap(attribute => this.toIncompleteFilters(attribute, text)) - .filter(incompleteFilter => this.filterMatchesUserText(text, incompleteFilter)); + .flatMap(attribute => this.buildIncompleteFilters(attribute, text)); } - private filterMatchesUserText(text: string, incompleteFilter: IncompleteFilter): boolean { - const isStringMatch = incompleteFilter.userString.toLowerCase().includes(text.trim().toLowerCase()); - - if (isStringMatch || incompleteFilter.operator === undefined) { - return isStringMatch; - } - - /* - * In most cases, the isStringMatch should be the only check that is needed, however this check fails for - * STRING_MAP types with an operator since the LHS includes a user entered key between the attribute name - * and the operator. We fix this by sending the full string through our parsing logic to see if we can pull - * the key value off the LHS and build a filter out of it. If so, its a valid match and we want to include - * it as an autocomplete option. - */ - - return ( - this.filterParserLookupService - .lookup(incompleteFilter.operator) - .parseFilterString(incompleteFilter.metadata, incompleteFilter.userString) !== undefined - ); - } - - private toIncompleteFilters(attribute: FilterAttribute, text: string): IncompleteFilter[] { + private buildIncompleteFilters(attribute: FilterAttribute, text: string): IncompleteFilter[] { const filterBuilder = this.filterBuilderLookupService.lookup(attribute.type); - - // Check for operator - - const splitFilter = splitFilterStringByOperator(filterBuilder.supportedOperators(), text, true); - - if (splitFilter === undefined) { - // Unable to find operator - return this.toIncompleteFiltersWithoutOperator(filterBuilder, attribute, text); + const splitFilter = splitFilterStringByOperator(attribute, filterBuilder.allSupportedOperators(), text); + // If we've got a split filter we've got both an attribute and operator + if (splitFilter) { + return [ + this.buildIncompleteFilterForAttributeAndOperator( + filterBuilder, + this.filterParserLookupService.lookup(splitFilter.operator), + splitFilter, + text + ) + ]; } - // Operator found + // Next, look to see if this string starts with the attribute. If it does, continue on to see which operators also match the string. + const attributeExpression = tryParseStringForAttribute(attribute, text); + if (attributeExpression) { + return this.buildIncompleteFiltersForAttribute(text, filterBuilder, attributeExpression); + } - const filterParser = this.filterParserLookupService.lookup(splitFilter.operator); + // We can't figure out the attribute. If the partial string it could later match, present this attribute + if (this.isPartialAttributeMatch(text, attribute)) { + return [this.buildIncompleteFilterForPartialAttributeMatch(filterBuilder, attribute)]; + } - return this.toIncompleteFiltersWithOperator(filterBuilder, filterParser, splitFilter, attribute, text); + // Not even a partial match, present no options + return []; } - private toIncompleteFiltersWithoutOperator( + private buildIncompleteFiltersForAttribute( + text: string, filterBuilder: AbstractFilterBuilder, - attribute: FilterAttribute, - text: string + attributeExpression: FilterAttributeExpression ): IncompleteFilter[] { - if (text.toLowerCase().includes(attribute.displayName.toLowerCase()) && text.endsWith(' ')) { - // Attribute found, but no operator or value so let's provide all operator options for autocomplete - - return filterBuilder.supportedOperators().map(operator => { - const filterParser = this.filterParserLookupService.lookup(operator); - - const splitFilter = splitFilterStringByOperator([operator], `${text} ${operator} `); - const value = splitFilter === undefined ? undefined : filterParser.parseValueString(attribute, splitFilter); - - return { - metadata: attribute, - field: attribute.name, - operator: operator, - userString: filterBuilder.buildUserFilterString(attribute, operator, value) - }; - }); - } - - // Nothing matching yet, so just provide the attribute for autocomplete - - return [ - { - metadata: attribute, - field: attribute.name, - userString: filterBuilder.buildUserFilterString(attribute) - } - ]; + const topLevelOperatorFilters = filterBuilder.supportedTopLevelOperators().map(operator => ({ + metadata: attributeExpression.attribute, + field: attributeExpression.attribute.name, + operator: operator, + userString: filterBuilder.buildUserStringWithMatchingWhitespace( + text, + { attribute: attributeExpression.attribute }, + operator + ) + })); + + // Subpath operators should add a subpath placeholder to the user string + const subpathOperatorFilters = filterBuilder.supportedSubpathOperators().map(operator => ({ + metadata: attributeExpression.attribute, + field: attributeExpression.attribute.name, + subpath: attributeExpression.subpath, + operator: operator, + userString: filterBuilder.buildUserStringWithMatchingWhitespace( + text, + { + attribute: attributeExpression.attribute, + subpath: isEmpty(attributeExpression.subpath) ? 'example' : attributeExpression.subpath + }, + operator + ) + })); + + return [...topLevelOperatorFilters, ...subpathOperatorFilters]; } - private toIncompleteFiltersWithOperator( + private buildIncompleteFilterForAttributeAndOperator( filterBuilder: AbstractFilterBuilder, filterParser: AbstractFilterParser, splitFilter: SplitFilter, - attribute: FilterAttribute, text: string - ): IncompleteFilter[] { + ): IncompleteFilter { // Check for complete filter - const parsedFilter = filterParser.parseFilterString(attribute, text); + const parsedFilter = filterParser.parseSplitFilter(splitFilter); if (parsedFilter !== undefined) { // Found complete filter - - return [filterBuilder.buildFilter(attribute, parsedFilter.operator, parsedFilter.value)]; + return { + ...filterBuilder.buildFilter( + splitFilter.attribute, + parsedFilter.operator, + parsedFilter.value, + parsedFilter.subpath + ), + userString: text // Use the actual text provided by user, so it matches their input + }; } - // Not a complete filter, but we know we have an operator. Let's check if we also have an attribute - - if (splitFilter.lhs.trim().toLowerCase() === attribute.displayName.toLowerCase()) { - // Attribute found, and we have the operator but we do not have a value (else it would have been complete) - return [ - { - metadata: attribute, - field: attribute.name, - operator: splitFilter.operator, - userString: filterBuilder.buildUserFilterString(attribute, splitFilter.operator) - } - ]; - } + // Not a complete filter, but we know we have attribute and operator - + return { + metadata: splitFilter.attribute, + field: splitFilter.attribute.name, + operator: splitFilter.operator, + userString: text // Use the actual text provided by user, so it matches their input + }; + } - // This attribute not found in text + private isPartialAttributeMatch(text: string, attribute: FilterAttribute): boolean { + return attribute.displayName.toLowerCase().startsWith(text.toLowerCase()); + } - return []; + private buildIncompleteFilterForPartialAttributeMatch( + filterBuilder: AbstractFilterBuilder, + attribute: FilterAttribute + ): IncompleteFilter { + return { + metadata: attribute, + field: attribute.name, + userString: filterBuilder.buildUserFilterString(attribute) + }; } } diff --git a/projects/components/src/filtering/filter/builder/types/abstract-filter-builder.ts b/projects/components/src/filtering/filter/builder/types/abstract-filter-builder.ts index 7a1572a44..b981692d4 100644 --- a/projects/components/src/filtering/filter/builder/types/abstract-filter-builder.ts +++ b/projects/components/src/filtering/filter/builder/types/abstract-filter-builder.ts @@ -1,41 +1,95 @@ import { collapseWhitespace } from '@hypertrace/common'; +import { isEmpty } from 'lodash-es'; import { Filter } from '../../filter'; import { FilterAttribute } from '../../filter-attribute'; import { FilterAttributeType } from '../../filter-attribute-type'; +import { MAP_LHS_DELIMITER } from '../../filter-delimiters'; import { FilterOperator, toUrlFilterOperator } from '../../filter-operators'; +import { FilterAttributeExpression } from '../../parser/parsed-filter'; export abstract class AbstractFilterBuilder { public abstract supportedAttributeType(): FilterAttributeType; - public abstract supportedOperators(): FilterOperator[]; + + public abstract supportedSubpathOperators(): FilterOperator[]; + public abstract supportedTopLevelOperators(): FilterOperator[]; protected abstract buildValueString(value: TValue): string; + public allSupportedOperators(): FilterOperator[] { + return [...this.supportedTopLevelOperators(), ...this.supportedSubpathOperators()]; + } + public buildFiltersForSupportedOperators(attribute: FilterAttribute, value: TValue): Filter[] { - return this.supportedOperators().map(operator => this.buildFilter(attribute, operator, value)); + return this.supportedTopLevelOperators().map(operator => this.buildFilter(attribute, operator, value)); } - public buildFilter(attribute: FilterAttribute, operator: FilterOperator, value: TValue): Filter { - if (!this.supportedOperators().includes(operator)) { + public buildFilter( + attribute: FilterAttribute, + operator: FilterOperator, + value: TValue, + subpath?: string + ): Filter { + if ( + (isEmpty(subpath) && !this.supportedTopLevelOperators().includes(operator)) || + (!isEmpty(subpath) && !this.supportedSubpathOperators().includes(operator)) + ) { throw Error(`Operator '${operator}' not supported for filter attribute type '${attribute.type}'`); } return { metadata: attribute, field: attribute.name, + subpath: subpath, operator: operator, value: value, - userString: this.buildUserFilterString(attribute, operator, value), - urlString: this.buildUrlFilterString(attribute, operator, value) + userString: this.buildUserFilterString(attribute, subpath, operator, value), + urlString: this.buildUrlFilterString(attribute, subpath, operator, value) }; } - public buildUserFilterString(attribute: FilterAttribute, operator?: FilterOperator, value?: TValue): string { + public buildUserFilterString( + attribute: FilterAttribute, + subpath?: string, + operator?: FilterOperator, + value?: TValue + ): string { + const attributeString = this.buildAttributeExpressionString(attribute.displayName, subpath); + return collapseWhitespace( - `${attribute.displayName} ${operator ?? ''} ${value !== undefined ? this.buildValueString(value) : ''}` + `${attributeString} ${operator ?? ''} ${value !== undefined ? this.buildValueString(value) : ''}` ).trim(); } - protected buildUrlFilterString(attribute: FilterAttribute, operator: FilterOperator, value: TValue): string { - return encodeURIComponent(`${attribute.name}${toUrlFilterOperator(operator)}${this.buildValueString(value)}`); + public buildUserStringWithMatchingWhitespace( + userEnteredString: string, + attributeExpression: FilterAttributeExpression, + operator: FilterOperator + ): string { + const attributeString = this.buildAttributeExpressionString( + attributeExpression.attribute.displayName, + attributeExpression.subpath + ); + const userStringAfterAttributeExpression = userEnteredString.slice(attributeString.length); + const whitespace = + userStringAfterAttributeExpression.length === 0 + ? ' ' + : userStringAfterAttributeExpression.match(/(\s*)/)?.[0] ?? ''; + + return `${attributeString}${whitespace}${operator}`; + } + + protected buildUrlFilterString( + attribute: FilterAttribute, + subpath: string | undefined, + operator: FilterOperator, + value: TValue + ): string { + const attributeString = this.buildAttributeExpressionString(attribute.name, subpath); + + return encodeURIComponent(`${attributeString}${toUrlFilterOperator(operator)}${this.buildValueString(value)}`); + } + + private buildAttributeExpressionString(attributeString: string, subpath?: string): string { + return isEmpty(subpath) ? attributeString : `${attributeString}${MAP_LHS_DELIMITER}${subpath}`; } } diff --git a/projects/components/src/filtering/filter/builder/types/boolean-filter-builder.ts b/projects/components/src/filtering/filter/builder/types/boolean-filter-builder.ts index f0ec4d719..bf9015b67 100644 --- a/projects/components/src/filtering/filter/builder/types/boolean-filter-builder.ts +++ b/projects/components/src/filtering/filter/builder/types/boolean-filter-builder.ts @@ -7,10 +7,14 @@ export class BooleanFilterBuilder extends AbstractFilterBuilder { return FilterAttributeType.Boolean; } - public supportedOperators(): FilterOperator[] { + public supportedTopLevelOperators(): FilterOperator[] { return [FilterOperator.Equals, FilterOperator.NotEquals]; } + public supportedSubpathOperators(): FilterOperator[] { + return []; + } + protected buildValueString(value: boolean): string { return String(value); } diff --git a/projects/components/src/filtering/filter/builder/types/number-filter-builder.ts b/projects/components/src/filtering/filter/builder/types/number-filter-builder.ts index 6b2c24db4..b742d06a8 100644 --- a/projects/components/src/filtering/filter/builder/types/number-filter-builder.ts +++ b/projects/components/src/filtering/filter/builder/types/number-filter-builder.ts @@ -8,7 +8,7 @@ export class NumberFilterBuilder extends AbstractFilterBuilder 0 ? value[0] : '') : value; - - return `${attribute.displayName}${MAP_LHS_DELIMITER}${displayValue}`; - } - - private buildUserFilterStringRhs(operator?: FilterOperator, value?: string | string[]): string { - return operator === FilterOperator.ContainsKey - ? Array.isArray(value) - ? value[0] ?? '' - : value ?? '' - : Array.isArray(value) - ? value[1] ?? '' - : value ?? ''; + protected buildValueString(value: string): string { + return String(value); } } diff --git a/projects/components/src/filtering/filter/filter-delimiters.ts b/projects/components/src/filtering/filter/filter-delimiters.ts index 008a4b4fa..c26dac8c9 100644 --- a/projects/components/src/filtering/filter/filter-delimiters.ts +++ b/projects/components/src/filtering/filter/filter-delimiters.ts @@ -1,3 +1,2 @@ export const ARRAY_DELIMITER = ','; export const MAP_LHS_DELIMITER = '.'; -export const MAP_RHS_DELIMITER = ':'; diff --git a/projects/components/src/filtering/filter/filter-operators.ts b/projects/components/src/filtering/filter/filter-operators.ts index 69f6029a4..36c92ecb1 100644 --- a/projects/components/src/filtering/filter/filter-operators.ts +++ b/projects/components/src/filtering/filter/filter-operators.ts @@ -9,8 +9,7 @@ export const enum FilterOperator { GreaterThanOrEqualTo = '>=', Like = '~', In = 'IN', - ContainsKey = 'CONTAINS_KEY', - ContainsKeyValue = 'CONTAINS_KEY_VALUE' + ContainsKey = 'CONTAINS_KEY' } export const enum UrlFilterOperator { @@ -22,8 +21,7 @@ export const enum UrlFilterOperator { GreaterThanOrEqualTo = '_gte_', Like = '_lk_', In = '_in_', - ContainsKey = '_ck_', - ContainsKeyValue = '_ckv_' + ContainsKey = '_ck_' } export const toUrlFilterOperator = (operator: FilterOperator): UrlFilterOperator => { @@ -46,8 +44,6 @@ export const toUrlFilterOperator = (operator: FilterOperator): UrlFilterOperator return UrlFilterOperator.In; case FilterOperator.ContainsKey: return UrlFilterOperator.ContainsKey; - case FilterOperator.ContainsKeyValue: - return UrlFilterOperator.ContainsKeyValue; default: return assertUnreachable(operator); } @@ -73,8 +69,6 @@ export const fromUrlFilterOperator = (operator: UrlFilterOperator): FilterOperat return FilterOperator.In; case UrlFilterOperator.ContainsKey: return FilterOperator.ContainsKey; - case UrlFilterOperator.ContainsKeyValue: - return FilterOperator.ContainsKeyValue; default: return assertUnreachable(operator); } @@ -93,11 +87,9 @@ export const incompatibleOperators = (operator: FilterOperator): FilterOperator[ FilterOperator.GreaterThan, FilterOperator.GreaterThanOrEqualTo, FilterOperator.Like, - FilterOperator.ContainsKey, - FilterOperator.ContainsKeyValue + FilterOperator.ContainsKey ]; case FilterOperator.ContainsKey: - case FilterOperator.ContainsKeyValue: case FilterOperator.NotEquals: case FilterOperator.Like: return [FilterOperator.In, FilterOperator.Equals]; diff --git a/projects/components/src/filtering/filter/filter-url.service.ts b/projects/components/src/filtering/filter/filter-url.service.ts index 261d5c396..54373b267 100644 --- a/projects/components/src/filtering/filter/filter-url.service.ts +++ b/projects/components/src/filtering/filter/filter-url.service.ts @@ -7,7 +7,7 @@ import { areCompatibleFilters, Filter, IncompleteFilter } from './filter'; import { FilterAttribute } from './filter-attribute'; import { fromUrlFilterOperator, toUrlFilterOperator } from './filter-operators'; import { FilterParserLookupService } from './parser/filter-parser-lookup.service'; -import { splitFilterStringByOperator } from './parser/types/abstract-filter-parser'; +import { splitFilterStringByOperator } from './parser/parsed-filter'; @Injectable({ providedIn: 'root' @@ -62,25 +62,29 @@ export class FilterUrlService { .filter(attribute => this.filterBuilderLookupService.isBuildableType(attribute.type)) .flatMap(attribute => { const filterBuilder = this.filterBuilderLookupService.lookup(attribute.type); - const supportedUrlOperators = filterBuilder.supportedOperators().map(toUrlFilterOperator); + const supportedUrlOperators = filterBuilder.allSupportedOperators().map(toUrlFilterOperator); - const splitUrlFilter = splitFilterStringByOperator(supportedUrlOperators, filterString, false); + const splitUrlFilter = splitFilterStringByOperator( + attribute, + supportedUrlOperators, + decodeURIComponent(filterString) + ); if (splitUrlFilter === undefined) { return undefined; } - const filterParser = this.filterParserLookupService.lookup(fromUrlFilterOperator(splitUrlFilter.operator)); + const convertedOperator = fromUrlFilterOperator(splitUrlFilter.operator); - const parsedFilter = filterParser.parseUrlFilterString(attribute, filterString); + const parsedFilter = this.filterParserLookupService.lookup(convertedOperator).parseSplitFilter({ + ...splitUrlFilter, + operator: convertedOperator + }); - if (parsedFilter === undefined) { - return undefined; - } - - return splitUrlFilter.lhs === parsedFilter.field - ? filterBuilder.buildFilter(attribute, parsedFilter.operator, parsedFilter.value) - : undefined; + return ( + parsedFilter && + filterBuilder.buildFilter(attribute, parsedFilter.operator, parsedFilter.value, parsedFilter.subpath) + ); }) .find(splitFilter => splitFilter !== undefined); } diff --git a/projects/components/src/filtering/filter/filter.ts b/projects/components/src/filtering/filter/filter.ts index 5e3ca5cd7..a7bf36ab2 100644 --- a/projects/components/src/filtering/filter/filter.ts +++ b/projects/components/src/filtering/filter/filter.ts @@ -14,6 +14,7 @@ export interface IncompleteFilter extends FieldFilter export interface FieldFilter { field: string; + subpath?: string; operator?: FilterOperator; value?: TValue; } diff --git a/projects/components/src/filtering/filter/parser/filter-parser-lookup.service.ts b/projects/components/src/filtering/filter/parser/filter-parser-lookup.service.ts index df0c15505..14c41cd15 100644 --- a/projects/components/src/filtering/filter/parser/filter-parser-lookup.service.ts +++ b/projects/components/src/filtering/filter/parser/filter-parser-lookup.service.ts @@ -24,7 +24,6 @@ export class FilterParserLookupService { case FilterOperator.In: return new InFilterParser(); case FilterOperator.ContainsKey: - case FilterOperator.ContainsKeyValue: return new ContainsFilterParser(); default: return assertUnreachable(operator); diff --git a/projects/components/src/filtering/filter/parser/parsed-filter.ts b/projects/components/src/filtering/filter/parser/parsed-filter.ts index 0b2ba9bea..b63616b3d 100644 --- a/projects/components/src/filtering/filter/parser/parsed-filter.ts +++ b/projects/components/src/filtering/filter/parser/parsed-filter.ts @@ -1,13 +1,82 @@ +import { KeysWithType } from '@hypertrace/common'; +import { FilterAttribute } from '../filter-attribute'; +import { FilterAttributeType } from '../filter-attribute-type'; +import { MAP_LHS_DELIMITER } from '../filter-delimiters'; import { FilterOperator } from '../filter-operators'; export interface ParsedFilter { field: string; + subpath?: string; operator: FilterOperator; value: TValue; } -export interface SplitFilter { - lhs: string; +export interface FilterAttributeExpression { + attribute: FilterAttribute; + subpath?: string; +} + +export interface SplitFilter extends FilterAttributeExpression { operator: TOperator; rhs: string; } + +export const splitFilterStringByOperator = ( + attribute: FilterAttribute, + possibleOperators: TOperator[], + filterString: string +): SplitFilter | undefined => { + const matchingOperator = possibleOperators + .sort((a, b) => b.length - a.length) // Prefer longer matches + .find(op => filterString.includes(op)); + + if (!matchingOperator) { + return undefined; + } + if ( + filterString.endsWith(matchingOperator) && + possibleOperators.filter(operator => operator.startsWith(matchingOperator)).length > 1 + ) { + // If our string ends with the start of multiple operators (e.g. "attr <"), it could continue to a different operator so abort to avoid ambiguity + return undefined; + } + const [lhs, rhs] = filterString.split(matchingOperator).map(str => str.trim()); + const attributeExpression = tryParseStringForAttribute(attribute, lhs, ['displayName', 'name']); + + return attributeExpression && { ...attributeExpression, operator: matchingOperator, rhs: rhs }; +}; + +export const tryParseStringForAttribute = ( + attributeToTest: FilterAttribute, + text: string, + nameFields: KeysWithType[] = ['displayName'] +): FilterAttributeExpression | undefined => { + const [stringContainingFullAttribute] = text.trim().split(MAP_LHS_DELIMITER, 1); + // The string to the left of any delimeter must start with the attribute otherwise no match + const matchingNameField = nameFields.find(nameField => + stringContainingFullAttribute.toLowerCase().startsWith(attributeToTest[nameField].toLowerCase()) + ); + if (!matchingNameField) { + return undefined; + } + // Now, we know that it does match. Remove the attribute name from the beginning and try to determine the subpath next. + const stringAfterAttributeName = text.slice(attributeToTest[matchingNameField].length).trim(); + + if (stringAfterAttributeName.startsWith(MAP_LHS_DELIMITER)) { + if (attributeToTest.type !== FilterAttributeType.StringMap) { + // Can't have a subpath if not a map + return undefined; + } + const potentialSubpath = stringAfterAttributeName.slice(MAP_LHS_DELIMITER.length); + // Subpaths support alphanumeric, -, - and . characters + const firstNonSubpathCharacterIndex = potentialSubpath.search(/[^\w\-\.]/); + const subpath = + firstNonSubpathCharacterIndex === -1 + ? potentialSubpath + : potentialSubpath.slice(0, firstNonSubpathCharacterIndex); + + return { attribute: attributeToTest, subpath: subpath }; + } + + return { attribute: attributeToTest }; +}; diff --git a/projects/components/src/filtering/filter/parser/types/abstract-filter-parser.ts b/projects/components/src/filtering/filter/parser/types/abstract-filter-parser.ts index 8dbbf594f..fb6139430 100644 --- a/projects/components/src/filtering/filter/parser/types/abstract-filter-parser.ts +++ b/projects/components/src/filtering/filter/parser/types/abstract-filter-parser.ts @@ -1,94 +1,26 @@ -import { FilterAttribute } from '../../filter-attribute'; import { FilterAttributeType } from '../../filter-attribute-type'; -import { FilterOperator, fromUrlFilterOperator, toUrlFilterOperator, UrlFilterOperator } from '../../filter-operators'; +import { FilterOperator } from '../../filter-operators'; import { ParsedFilter, SplitFilter } from '../parsed-filter'; export abstract class AbstractFilterParser { public abstract supportedAttributeTypes(): FilterAttributeType[]; public abstract supportedOperators(): FilterOperator[]; - public parseNameString(attribute: FilterAttribute, splitFilter: SplitFilter): string | undefined { - return attribute.displayName.toLowerCase() !== splitFilter.lhs.toLowerCase() ? undefined : attribute.name; - } - - public abstract parseValueString( - attribute: FilterAttribute, - splitFilter: SplitFilter - ): TValue | undefined; - - public parseFilterString(attribute: FilterAttribute, filterString: string): ParsedFilter | undefined { - const splitFilter: SplitFilter | undefined = splitFilterStringByOperator( - this.supportedOperators(), - filterString, - true - ); - - return splitFilter === undefined ? undefined : this.parseSplitFilter(attribute, splitFilter); - } - - public parseUrlFilterString(attribute: FilterAttribute, urlFilterString: string): ParsedFilter | undefined { - const splitUrlFilter: SplitFilter | undefined = splitFilterStringByOperator( - this.supportedOperators().map(toUrlFilterOperator), - decodeURIComponent(urlFilterString), - false - ); - - return splitUrlFilter !== undefined - ? this.parseSplitFilter(attribute, { - lhs: attribute.displayName, - operator: fromUrlFilterOperator(splitUrlFilter.operator), - rhs: splitUrlFilter.rhs - }) - : undefined; - } - - private parseSplitFilter( - attribute: FilterAttribute, - splitFilter: SplitFilter - ): ParsedFilter | undefined { - const parsedName = this.parseNameString(attribute, splitFilter); + public abstract parseValueString(splitFilter: SplitFilter): TValue | undefined; - if (parsedName === undefined) { - // Unable to parse attribute from lhs - return undefined; - } - - const parsedValue = this.parseValueString(attribute, splitFilter); + public parseSplitFilter(splitFilter: SplitFilter): ParsedFilter | undefined { + const parsedValue = this.parseValueString(splitFilter); if (parsedValue === undefined) { // Unable to parse value from rhs return undefined; } - // Successfully parsed URL filter - return { - field: attribute.name, + field: splitFilter.attribute.name, + subpath: splitFilter.subpath, operator: splitFilter.operator, value: parsedValue }; } } - -export const splitFilterStringByOperator = ( - possibleOperators: TOperator[], - filterString: string, - expectSpaceAroundOperator: boolean = true -): SplitFilter | undefined => { - const matchingOperator = possibleOperators - .sort((o1: string, o2: string) => o2.length - o1.length) // Sort by length to check multichar ops first - .map(op => (expectSpaceAroundOperator ? ` ${op as string} ` : op)) - .find(op => filterString.includes(op)); - - if (matchingOperator === undefined) { - return undefined; - } - - const parts = filterString.split(matchingOperator).map(str => str.trim()); - - return { - lhs: parts[0], - operator: matchingOperator.trim() as TOperator, - rhs: parts[1] - }; -}; diff --git a/projects/components/src/filtering/filter/parser/types/comparison-filter-parser.ts b/projects/components/src/filtering/filter/parser/types/comparison-filter-parser.ts index eb5a841e5..fb9b7e773 100644 --- a/projects/components/src/filtering/filter/parser/types/comparison-filter-parser.ts +++ b/projects/components/src/filtering/filter/parser/types/comparison-filter-parser.ts @@ -1,5 +1,4 @@ import { assertUnreachable } from '@hypertrace/common'; -import { FilterAttribute } from '../../filter-attribute'; import { FilterAttributeType } from '../../filter-attribute-type'; import { FilterOperator } from '../../filter-operators'; import { SplitFilter } from '../parsed-filter'; @@ -7,7 +6,12 @@ import { AbstractFilterParser } from './abstract-filter-parser'; export class ComparisonFilterParser extends AbstractFilterParser { public supportedAttributeTypes(): FilterAttributeType[] { - return [FilterAttributeType.Boolean, FilterAttributeType.Number, FilterAttributeType.String]; + return [ + FilterAttributeType.Boolean, + FilterAttributeType.Number, + FilterAttributeType.String, + FilterAttributeType.StringMap + ]; } public supportedOperators(): FilterOperator[] { @@ -22,23 +26,20 @@ export class ComparisonFilterParser extends AbstractFilterParser - ): PossibleValuesTypes | undefined { - switch (attribute.type) { + public parseValueString(splitFilter: SplitFilter): PossibleValuesTypes | undefined { + switch (splitFilter.attribute.type) { case FilterAttributeType.Boolean: return this.parseBooleanValue(splitFilter.rhs); case FilterAttributeType.Number: return this.parseNumberValue(splitFilter.rhs); case FilterAttributeType.String: + case FilterAttributeType.StringMap: return this.parseStringValue(splitFilter.rhs); case FilterAttributeType.StringArray: // Unsupported - case FilterAttributeType.StringMap: // Unsupported case FilterAttributeType.Timestamp: // Unsupported return undefined; default: - assertUnreachable(attribute.type); + assertUnreachable(splitFilter.attribute.type); } } diff --git a/projects/components/src/filtering/filter/parser/types/contains-filter-parser.ts b/projects/components/src/filtering/filter/parser/types/contains-filter-parser.ts index a7a88ddf9..afe37b05e 100644 --- a/projects/components/src/filtering/filter/parser/types/contains-filter-parser.ts +++ b/projects/components/src/filtering/filter/parser/types/contains-filter-parser.ts @@ -1,35 +1,22 @@ -import { assertUnreachable, isNonEmptyString } from '@hypertrace/common'; -import { FilterAttribute } from '../../filter-attribute'; +import { assertUnreachable } from '@hypertrace/common'; import { FilterAttributeType } from '../../filter-attribute-type'; -import { MAP_LHS_DELIMITER, MAP_RHS_DELIMITER } from '../../filter-delimiters'; import { FilterOperator } from '../../filter-operators'; import { SplitFilter } from '../parsed-filter'; import { AbstractFilterParser } from './abstract-filter-parser'; -export class ContainsFilterParser extends AbstractFilterParser { +export class ContainsFilterParser extends AbstractFilterParser { public supportedAttributeTypes(): FilterAttributeType[] { return [FilterAttributeType.StringMap]; } public supportedOperators(): FilterOperator[] { - return [FilterOperator.ContainsKey, FilterOperator.ContainsKeyValue]; + return [FilterOperator.ContainsKey]; } - public parseNameString(attribute: FilterAttribute, splitFilter: SplitFilter): string | undefined { - const splitLhs = this.splitLhs(attribute, splitFilter); - - return splitLhs === undefined - ? undefined - : super.parseNameString(attribute, { ...splitFilter, lhs: splitLhs.displayName }); - } - - public parseValueString( - attribute: FilterAttribute, - splitFilter: SplitFilter - ): PossibleValuesTypes | undefined { - switch (attribute.type) { + public parseValueString(splitFilter: SplitFilter): string | undefined { + switch (splitFilter.attribute.type) { case FilterAttributeType.StringMap: - return this.parseStringMapValue(attribute, splitFilter); + return String(splitFilter.rhs); case FilterAttributeType.StringArray: // Unsupported case FilterAttributeType.Number: // Unsupported case FilterAttributeType.Boolean: // Unsupported @@ -37,64 +24,7 @@ export class ContainsFilterParser extends AbstractFilterParser - ): string[] | undefined { - if (splitFilter.lhs === attribute.displayName) { - switch (splitFilter.operator) { - case FilterOperator.ContainsKey: - return [splitFilter.rhs]; - case FilterOperator.ContainsKeyValue: - return splitFirstOccurrenceOmitEmpty(splitFilter.rhs, MAP_RHS_DELIMITER); - case FilterOperator.Equals: - case FilterOperator.NotEquals: - case FilterOperator.LessThan: - case FilterOperator.LessThanOrEqualTo: - case FilterOperator.GreaterThan: - case FilterOperator.GreaterThanOrEqualTo: - case FilterOperator.Like: - case FilterOperator.In: - return undefined; - default: - assertUnreachable(splitFilter.operator); - } - } - - const splitLhs = this.splitLhs(attribute, splitFilter); - - return splitLhs === undefined || splitLhs.key === undefined ? undefined : [splitLhs.key, splitFilter.rhs]; - } - - private splitLhs(attribute: FilterAttribute, splitFilter: SplitFilter): SplitLhs | undefined { - if (splitFilter.lhs === attribute.displayName) { - return { displayName: attribute.displayName }; + return assertUnreachable(splitFilter.attribute.type); } - - const parts = splitFilter.lhs.split(MAP_LHS_DELIMITER); - - return parts.length < 2 - ? undefined - : { - displayName: attribute.displayName, - key: parts.slice(1).join(MAP_LHS_DELIMITER) - }; } } - -type PossibleValuesTypes = string[]; - -interface SplitLhs { - displayName: string; - key?: string; -} - -const splitFirstOccurrenceOmitEmpty = (str: string, delimiter: string): string[] => { - const firstIndex = str.indexOf(delimiter); - - return [str.substr(0, firstIndex), str.substr(firstIndex + 1)].filter(isNonEmptyString); -}; diff --git a/projects/components/src/filtering/filter/parser/types/in-filter-parser.ts b/projects/components/src/filtering/filter/parser/types/in-filter-parser.ts index 62e052958..b540143ec 100644 --- a/projects/components/src/filtering/filter/parser/types/in-filter-parser.ts +++ b/projects/components/src/filtering/filter/parser/types/in-filter-parser.ts @@ -1,5 +1,4 @@ import { assertUnreachable } from '@hypertrace/common'; -import { FilterAttribute } from '../../filter-attribute'; import { FilterAttributeType } from '../../filter-attribute-type'; import { ARRAY_DELIMITER } from '../../filter-delimiters'; import { FilterOperator } from '../../filter-operators'; @@ -15,11 +14,8 @@ export class InFilterParser extends AbstractFilterParser { return [FilterOperator.In]; } - public parseValueString( - attribute: FilterAttribute, - splitFilter: SplitFilter - ): PossibleValuesTypes | undefined { - switch (attribute.type) { + public parseValueString(splitFilter: SplitFilter): PossibleValuesTypes | undefined { + switch (splitFilter.attribute.type) { case FilterAttributeType.String: return this.parseStringArrayValue(splitFilter.rhs); case FilterAttributeType.Number: @@ -30,7 +26,7 @@ export class InFilterParser extends AbstractFilterParser { case FilterAttributeType.Timestamp: // Unsupported return undefined; default: - assertUnreachable(attribute.type); + assertUnreachable(splitFilter.attribute.type); } } diff --git a/projects/observability/src/pages/explorer/explorer-service.ts b/projects/observability/src/pages/explorer/explorer-service.ts index 004444d05..be24a5982 100644 --- a/projects/observability/src/pages/explorer/explorer-service.ts +++ b/projects/observability/src/pages/explorer/explorer-service.ts @@ -31,7 +31,7 @@ export class ExplorerService { filterAttribute => this.filterBuilderLookupService .lookup(filterAttribute.type) - .buildFilter(filterAttribute, filter.operator, filter.value).urlString + .buildFilter(filterAttribute, filter.operator, filter.value, filter.subpath).urlString ) ) ); diff --git a/projects/observability/src/shared/components/span-detail/tags/span-tags-detail.component.ts b/projects/observability/src/shared/components/span-detail/tags/span-tags-detail.component.ts index fdf77a6c0..5d574940a 100644 --- a/projects/observability/src/shared/components/span-detail/tags/span-tags-detail.component.ts +++ b/projects/observability/src/shared/components/span-detail/tags/span-tags-detail.component.ts @@ -44,7 +44,7 @@ export class SpanTagsDetailComponent implements OnChanges { public getExploreNavigationParams = (tag: ListViewRecord): Observable => this.explorerService.buildNavParamsWithFilters(ScopeQueryParam.EndpointTraces, [ - { field: 'tags', operator: FilterOperator.ContainsKeyValue, value: [tag.key, tag.value] } + { field: 'tags', subpath: tag.key, operator: FilterOperator.Equals, value: tag.value } ]); private buildTagRecords(): void { diff --git a/projects/observability/src/shared/dashboard/data/graphql/filter/graphql-id-scope-filter.model.ts b/projects/observability/src/shared/dashboard/data/graphql/filter/graphql-id-scope-filter.model.ts index 74facbe06..65254749a 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/filter/graphql-id-scope-filter.model.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/filter/graphql-id-scope-filter.model.ts @@ -33,9 +33,7 @@ export class GraphqlIdScopeFilterModel implements GraphQlFilter { GraphQlOperatorType.GreaterThanOrEqualTo, GraphQlOperatorType.LessThan, GraphQlOperatorType.LessThanOrEqualTo, - GraphQlOperatorType.Like, - GraphQlOperatorType.ContainsKey, - GraphQlOperatorType.ContainsKeyValue + GraphQlOperatorType.Like ] } as EnumPropertyTypeInstance }) diff --git a/projects/observability/src/shared/dashboard/data/graphql/filter/graphql-key-value-filter.model.ts b/projects/observability/src/shared/dashboard/data/graphql/filter/graphql-key-value-filter.model.ts index 45964c99f..664d72999 100644 --- a/projects/observability/src/shared/dashboard/data/graphql/filter/graphql-key-value-filter.model.ts +++ b/projects/observability/src/shared/dashboard/data/graphql/filter/graphql-key-value-filter.model.ts @@ -31,8 +31,7 @@ export class GraphQlKeyValueFilterModel implements GraphQlFilter { GraphQlOperatorType.LessThanOrEqualTo, GraphQlOperatorType.Like, GraphQlOperatorType.NotIn, - GraphQlOperatorType.ContainsKey, - GraphQlOperatorType.ContainsKeyValue + GraphQlOperatorType.ContainsKey ] } as EnumPropertyTypeInstance }) diff --git a/projects/observability/src/shared/graphql/model/schema/filter/graphql-filter.ts b/projects/observability/src/shared/graphql/model/schema/filter/graphql-filter.ts index c19cff0c7..899bf40af 100644 --- a/projects/observability/src/shared/graphql/model/schema/filter/graphql-filter.ts +++ b/projects/observability/src/shared/graphql/model/schema/filter/graphql-filter.ts @@ -23,6 +23,5 @@ export const enum GraphQlOperatorType { Like = 'LIKE', In = 'IN', NotIn = 'NOT_IN', - ContainsKey = 'CONTAINS_KEY', - ContainsKeyValue = 'CONTAINS_KEY_VALUE' + ContainsKey = 'CONTAINS_KEY' } diff --git a/projects/observability/src/shared/services/filter-builder/graphql-filter-builder.service.ts b/projects/observability/src/shared/services/filter-builder/graphql-filter-builder.service.ts index 608c248d9..cc5d0bf47 100644 --- a/projects/observability/src/shared/services/filter-builder/graphql-filter-builder.service.ts +++ b/projects/observability/src/shared/services/filter-builder/graphql-filter-builder.service.ts @@ -10,7 +10,11 @@ export class GraphQlFilterBuilderService { public buildGraphQlFilters(filters: TableFilter[]): GraphQlFilter[] { return filters.map( filter => - new GraphQlFieldFilter(filter.field, toGraphQlOperator(filter.operator), filter.value as GraphQlArgumentValue) + new GraphQlFieldFilter( + { key: filter.field, subpath: filter.subpath }, + toGraphQlOperator(filter.operator), + filter.value as GraphQlArgumentValue + ) ); } } @@ -35,8 +39,6 @@ export const toGraphQlOperator = (operator: FilterOperator): GraphQlOperatorType return GraphQlOperatorType.In; case FilterOperator.ContainsKey: return GraphQlOperatorType.ContainsKey; - case FilterOperator.ContainsKeyValue: - return GraphQlOperatorType.ContainsKeyValue; default: return assertUnreachable(operator); } From bb7ba1a536fff05960e0b06bfb24b6b2be01f554 Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld Date: Wed, 12 Jan 2022 13:45:15 -0500 Subject: [PATCH 4/6] fix: caught a couple bugs while updating tests --- .../filter-bar/filter-bar.service.test.ts | 45 +++- .../filter-bar/filter-bar.service.ts | 9 +- .../filter-chip/filter-chip.service.test.ts | 69 ++++++- .../filter-chip/filter-chip.service.ts | 2 +- .../filter-builder-lookup.service.test.ts | 14 +- .../types/string-map-filter-builder.ts | 2 +- .../filter/filter-url.service.test.ts | 49 +++-- .../components/src/filtering/filter/filter.ts | 9 +- .../filter-parser-lookup.service.test.ts | 195 +++++------------- .../parser/filter-parser-lookup.service.ts | 3 + .../types/abstract-filter-parser.test.ts | 92 --------- .../filter/parser/types/in-filter-parser.ts | 4 +- .../pages/explorer/explorer.component.test.ts | 8 +- .../navigable-dashboard.component.test.ts | 2 +- .../graphql-filter-builder.service.test.ts | 12 +- 15 files changed, 211 insertions(+), 304 deletions(-) delete mode 100644 projects/components/src/filtering/filter/parser/types/abstract-filter-parser.test.ts diff --git a/projects/components/src/filtering/filter-bar/filter-bar.service.test.ts b/projects/components/src/filtering/filter-bar/filter-bar.service.test.ts index 6348afc89..e3d07c9dd 100644 --- a/projects/components/src/filtering/filter-bar/filter-bar.service.test.ts +++ b/projects/components/src/filtering/filter-bar/filter-bar.service.test.ts @@ -35,8 +35,9 @@ describe('Filter Bar service', () => { ), new StringMapFilterBuilder().buildFilter( getTestFilterAttribute(FilterAttributeType.StringMap), - FilterOperator.ContainsKeyValue, - ['myKey', 'myValue'] + FilterOperator.Equals, + 'myValue', + 'myKey' ) ]; @@ -184,18 +185,33 @@ describe('Filter Bar service', () => { expect(testFilters).toEqual([stringFilter, inNumberFilter]); /* - * Add a StringMap CONTAINS_KEY_VALUE that should not replace any existing filters + * Add a StringMap EQUALS should not replace any existing filters */ - const ckvStringMapFilter = new StringMapFilterBuilder().buildFilter( + const firstStringMapFilter = new StringMapFilterBuilder().buildFilter( getTestFilterAttribute(FilterAttributeType.StringMap), - FilterOperator.ContainsKeyValue, - ['myKey', 'myValue'] + FilterOperator.Equals, + 'myValue', + 'myKey' ); - testFilters = spectator.service.addFilter(testFilters, ckvStringMapFilter); + testFilters = spectator.service.addFilter(testFilters, firstStringMapFilter); + + expect(testFilters).toEqual([stringFilter, inNumberFilter, firstStringMapFilter]); + /* + * Add a second StringMap EQUALS filters with a different key should not replace any existing filters + */ - expect(testFilters).toEqual([stringFilter, inNumberFilter, ckvStringMapFilter]); + const secondStringMapFilter = new StringMapFilterBuilder().buildFilter( + getTestFilterAttribute(FilterAttributeType.StringMap), + FilterOperator.Equals, + 'myValue', + 'mySecondKey' + ); + + testFilters = spectator.service.addFilter(testFilters, secondStringMapFilter); + + expect(testFilters).toEqual([stringFilter, inNumberFilter, firstStringMapFilter, secondStringMapFilter]); /* * Add a StringMap CONTAINS_KEY that should not replace any existing filters @@ -209,7 +225,13 @@ describe('Filter Bar service', () => { testFilters = spectator.service.addFilter(testFilters, ckStringMapFilter); - expect(testFilters).toEqual([stringFilter, inNumberFilter, ckvStringMapFilter, ckStringMapFilter]); + expect(testFilters).toEqual([ + stringFilter, + inNumberFilter, + firstStringMapFilter, + secondStringMapFilter, + ckStringMapFilter + ]); }); test('correctly updates filters', () => { @@ -221,8 +243,9 @@ describe('Filter Bar service', () => { const testStringMapFilter = new StringMapFilterBuilder().buildFilter( getTestFilterAttribute(FilterAttributeType.StringMap), - FilterOperator.ContainsKeyValue, - ['myTestKey', 'myTestValue'] + FilterOperator.Equals, + 'myTestValue', + 'myTestKey' ); const testNumberFilter = new NumberFilterBuilder().buildFilter( diff --git a/projects/components/src/filtering/filter-bar/filter-bar.service.ts b/projects/components/src/filtering/filter-bar/filter-bar.service.ts index 1aabc74c4..e9a5a7e10 100644 --- a/projects/components/src/filtering/filter-bar/filter-bar.service.ts +++ b/projects/components/src/filtering/filter-bar/filter-bar.service.ts @@ -1,5 +1,6 @@ import { Injectable } from '@angular/core'; -import { areCompatibleFilters, areEqualFilters, Filter } from '../filter/filter'; +import { isEqual } from 'lodash-es'; +import { areCompatibleFilters, Filter } from '../filter/filter'; @Injectable({ providedIn: 'root' @@ -14,7 +15,7 @@ export class FilterBarService { public updateFilter(filters: Filter[], oldFilter: Filter, newFilter: Filter): Filter[] { const clonedFilters = [...filters]; - const index = filters.findIndex(f => areEqualFilters(f, oldFilter)); + const index = filters.findIndex(f => isEqual(f, oldFilter)); if (index < 0) { throw new Error(`Unable to update filter. Filter for '${oldFilter.field}' not found.`); @@ -22,10 +23,10 @@ export class FilterBarService { clonedFilters.splice(index, 1, newFilter); - return clonedFilters.filter(f => areEqualFilters(f, newFilter) || areCompatibleFilters(f, newFilter)); + return clonedFilters.filter(f => isEqual(f, newFilter) || areCompatibleFilters(f, newFilter)); } public deleteFilter(filters: Filter[], filter: Filter): Filter[] { - return filters.filter(f => !areEqualFilters(f, filter)); + return filters.filter(f => !isEqual(f, filter)); } } diff --git a/projects/components/src/filtering/filter-bar/filter-chip/filter-chip.service.test.ts b/projects/components/src/filtering/filter-bar/filter-chip/filter-chip.service.test.ts index b926e36da..899fdb321 100644 --- a/projects/components/src/filtering/filter-bar/filter-chip/filter-chip.service.test.ts +++ b/projects/components/src/filtering/filter-bar/filter-chip/filter-chip.service.test.ts @@ -62,7 +62,6 @@ describe('Filter Chip service', () => { case FilterOperator.In: return new InFilterParser(); case FilterOperator.ContainsKey: - case FilterOperator.ContainsKeyValue: return new ContainsFilterParser(); default: assertUnreachable(operator); @@ -246,21 +245,79 @@ describe('Filter Chip service', () => { ]); }); - test('correctly autocompletes operator filters for string map attribute once space key is entered', () => { + test('correctly autocompletes operator filters for string map attribute', () => { const attribute = getTestFilterAttribute(FilterAttributeType.StringMap); - expect(spectator.service.autocompleteFilters(attributes, 'String Map Attribute.testKey ')).toEqual([ + // Contains key or subpath operators if no subpath specified but could be + expect(spectator.service.autocompleteFilters(attributes, 'String Map Attribute')).toEqual([ { metadata: attribute, field: attribute.name, operator: FilterOperator.ContainsKey, - userString: `${attribute.displayName} ${FilterOperator.ContainsKey} testKey` + userString: `${attribute.displayName} ${FilterOperator.ContainsKey}` }, { metadata: attribute, field: attribute.name, - operator: FilterOperator.ContainsKeyValue, - userString: `${attribute.displayName}.testKey ${FilterOperator.ContainsKeyValue}` + operator: FilterOperator.Equals, + userString: `${attribute.displayName}.example ${FilterOperator.Equals}` + }, + { + metadata: attribute, + field: attribute.name, + operator: FilterOperator.NotEquals, + userString: `${attribute.displayName}.example ${FilterOperator.NotEquals}` + }, + { + metadata: attribute, + field: attribute.name, + operator: FilterOperator.In, + userString: `${attribute.displayName}.example ${FilterOperator.In}` + }, + { + metadata: attribute, + field: attribute.name, + operator: FilterOperator.Like, + userString: `${attribute.displayName}.example ${FilterOperator.Like}` + } + ]); + + // Regular operators only once subpath included + expect(spectator.service.autocompleteFilters(attributes, 'String Map Attribute.testKey')).toEqual([ + expect.objectContaining({ + metadata: attribute, + field: attribute.name, + operator: FilterOperator.ContainsKey, + // This operator isn't actually eligible but filtering operators is done by the chip/combobox, so just make sure the string doesn't match + userString: expect.not.stringMatching(`${attribute.displayName}.testKey`) + }), + { + metadata: attribute, + field: attribute.name, + subpath: 'testKey', + operator: FilterOperator.Equals, + userString: `${attribute.displayName}.testKey ${FilterOperator.Equals}` + }, + { + metadata: attribute, + field: attribute.name, + subpath: 'testKey', + operator: FilterOperator.NotEquals, + userString: `${attribute.displayName}.testKey ${FilterOperator.NotEquals}` + }, + { + metadata: attribute, + field: attribute.name, + subpath: 'testKey', + operator: FilterOperator.In, + userString: `${attribute.displayName}.testKey ${FilterOperator.In}` + }, + { + metadata: attribute, + field: attribute.name, + subpath: 'testKey', + operator: FilterOperator.Like, + userString: `${attribute.displayName}.testKey ${FilterOperator.Like}` } ]); }); diff --git a/projects/components/src/filtering/filter-bar/filter-chip/filter-chip.service.ts b/projects/components/src/filtering/filter-bar/filter-chip/filter-chip.service.ts index c72761724..66ae2dd20 100644 --- a/projects/components/src/filtering/filter-bar/filter-chip/filter-chip.service.ts +++ b/projects/components/src/filtering/filter-bar/filter-chip/filter-chip.service.ts @@ -128,7 +128,7 @@ export class FilterChipService { } private isPartialAttributeMatch(text: string, attribute: FilterAttribute): boolean { - return attribute.displayName.toLowerCase().startsWith(text.toLowerCase()); + return attribute.displayName.toLowerCase().includes(text.toLowerCase()); } private buildIncompleteFilterForPartialAttributeMatch( diff --git a/projects/components/src/filtering/filter/builder/filter-builder-lookup.service.test.ts b/projects/components/src/filtering/filter/builder/filter-builder-lookup.service.test.ts index 50a8051b8..579b8e929 100644 --- a/projects/components/src/filtering/filter/builder/filter-builder-lookup.service.test.ts +++ b/projects/components/src/filtering/filter/builder/filter-builder-lookup.service.test.ts @@ -177,17 +177,15 @@ describe('Filter Builder Lookup service', () => { expect( spectator.service .lookup(FilterAttributeType.StringMap) - .buildFilter(getTestFilterAttribute(FilterAttributeType.StringMap), FilterOperator.ContainsKeyValue, [ - 'myKey', - 'myValue' - ]) + .buildFilter(getTestFilterAttribute(FilterAttributeType.StringMap), FilterOperator.Equals, 'myValue', 'myKey') ).toEqual({ metadata: getTestFilterAttribute(FilterAttributeType.StringMap), field: getTestFilterAttribute(FilterAttributeType.StringMap).name, - operator: FilterOperator.ContainsKeyValue, - value: ['myKey', 'myValue'], - userString: 'String Map Attribute.myKey CONTAINS_KEY_VALUE myValue', - urlString: 'stringMapAttribute_ckv_myKey%3AmyValue' + subpath: 'myKey', + operator: FilterOperator.Equals, + value: 'myValue', + userString: 'String Map Attribute.myKey = myValue', + urlString: 'stringMapAttribute.myKey_eq_myValue' }); }); }); diff --git a/projects/components/src/filtering/filter/builder/types/string-map-filter-builder.ts b/projects/components/src/filtering/filter/builder/types/string-map-filter-builder.ts index 64c68d040..79a90c511 100644 --- a/projects/components/src/filtering/filter/builder/types/string-map-filter-builder.ts +++ b/projects/components/src/filtering/filter/builder/types/string-map-filter-builder.ts @@ -2,7 +2,7 @@ import { FilterAttributeType } from '../../filter-attribute-type'; import { FilterOperator } from '../../filter-operators'; import { AbstractFilterBuilder } from './abstract-filter-builder'; -export class StringMapFilterBuilder extends AbstractFilterBuilder { +export class StringMapFilterBuilder extends AbstractFilterBuilder { public supportedAttributeType(): FilterAttributeType { return FilterAttributeType.StringMap; } diff --git a/projects/components/src/filtering/filter/filter-url.service.test.ts b/projects/components/src/filtering/filter/filter-url.service.test.ts index cb7396c50..5de0a1b8e 100644 --- a/projects/components/src/filtering/filter/filter-url.service.test.ts +++ b/projects/components/src/filtering/filter/filter-url.service.test.ts @@ -39,8 +39,9 @@ describe('Filter URL service', () => { ), new StringMapFilterBuilder().buildFilter( getTestFilterAttribute(FilterAttributeType.StringMap), - FilterOperator.ContainsKeyValue, - ['myKey', 'myValue'] + FilterOperator.Equals, + 'myValue', + 'myKey' ) ]; @@ -50,7 +51,7 @@ describe('Filter URL service', () => { 'numberAttribute_neq_415', 'numberAttribute_lte_707', 'stringAttribute_eq_test', - 'stringMapAttribute_ckv_myKey%3AmyValue' + 'stringMapAttribute.myKey_eq_myValue' ] }; @@ -232,19 +233,41 @@ describe('Filter URL service', () => { }); /* - * Add a StringMap CONTAINS_KEY_VALUE that should not replace any existing filters + * Add a StringMap EQUALS that should not replace any existing filters */ spectator.service.addUrlFilter( attributes, new StringMapFilterBuilder().buildFilter( getTestFilterAttribute(FilterAttributeType.StringMap), - FilterOperator.ContainsKeyValue, - ['myKey', 'myValue'] + FilterOperator.Equals, + 'myValue', + 'myKey' ) ); expect(testQueryParamObject).toEqual({ - filter: ['stringAttribute_neq_test', 'numberAttribute_in_1984', 'stringMapAttribute_ckv_myKey%3AmyValue'] + filter: ['stringAttribute_neq_test', 'numberAttribute_in_1984', 'stringMapAttribute.myKey_eq_myValue'] + }); + + /* + * Add a second StringMap EQUALS that should not replace any existing filters + */ + spectator.service.addUrlFilter( + attributes, + new StringMapFilterBuilder().buildFilter( + getTestFilterAttribute(FilterAttributeType.StringMap), + FilterOperator.Equals, + 'myValue', + 'mySecondKey' + ) + ); + expect(testQueryParamObject).toEqual({ + filter: [ + 'stringAttribute_neq_test', + 'numberAttribute_in_1984', + 'stringMapAttribute.myKey_eq_myValue', + 'stringMapAttribute.mySecondKey_eq_myValue' + ] }); /* @@ -263,7 +286,8 @@ describe('Filter URL service', () => { filter: [ 'stringAttribute_neq_test', 'numberAttribute_in_1984', - 'stringMapAttribute_ckv_myKey%3AmyValue', + 'stringMapAttribute.myKey_eq_myValue', + 'stringMapAttribute.mySecondKey_eq_myValue', 'stringMapAttribute_ck_myTestKey' ] }); @@ -283,7 +307,7 @@ describe('Filter URL service', () => { spectator.service.removeUrlFilter(attributes, testFilter); expect(testQueryParamObject).toEqual({ - filter: ['stringAttribute_eq_test', 'stringMapAttribute_ckv_myKey%3AmyValue'] + filter: ['stringAttribute_eq_test', 'stringMapAttribute.myKey_eq_myValue'] }); }); @@ -300,7 +324,7 @@ describe('Filter URL service', () => { spectator.service.removeUrlFilter(attributes, testFilter); expect(testQueryParamObject).toEqual({ - filter: ['numberAttribute_lte_707', 'stringAttribute_eq_test', 'stringMapAttribute_ckv_myKey%3AmyValue'] + filter: ['numberAttribute_lte_707', 'stringAttribute_eq_test', 'stringMapAttribute.myKey_eq_myValue'] }); }); @@ -326,14 +350,15 @@ describe('Filter URL service', () => { 'numberAttribute_neq_217', 'numberAttribute_lte_707', 'stringAttribute_eq_test', - 'stringMapAttribute_ckv_myKey%3AmyValue' + + 'stringMapAttribute.myKey_eq_myValue' ] }); spectator.service.removeUrlFilter(attributes, test2Filter); expect(testQueryParamObject).toEqual({ - filter: ['numberAttribute_neq_217', 'stringAttribute_eq_test', 'stringMapAttribute_ckv_myKey%3AmyValue'] + filter: ['numberAttribute_neq_217', 'stringAttribute_eq_test', 'stringMapAttribute.myKey_eq_myValue'] }); }); }); diff --git a/projects/components/src/filtering/filter/filter.ts b/projects/components/src/filtering/filter/filter.ts index a7bf36ab2..1f9cafaa8 100644 --- a/projects/components/src/filtering/filter/filter.ts +++ b/projects/components/src/filtering/filter/filter.ts @@ -19,12 +19,5 @@ export interface FieldFilter { value?: TValue; } -export const areEqualFilters = (f1: IncompleteFilter, f2: IncompleteFilter) => - (f1.field === f2.field && f1.operator === undefined) || - f2.operator === undefined || - (f1.operator === f2.operator && f1.value === undefined) || - f2.value === undefined || - f1.value === f2.value; - export const areCompatibleFilters = (f1: Filter, f2: Filter) => - f1.field !== f2.field || (f1.field === f2.field && !incompatibleOperators(f1.operator).includes(f2.operator)); + f1.field !== f2.field || f1.subpath !== f2.subpath || !incompatibleOperators(f1.operator).includes(f2.operator); diff --git a/projects/components/src/filtering/filter/parser/filter-parser-lookup.service.test.ts b/projects/components/src/filtering/filter/parser/filter-parser-lookup.service.test.ts index bd836c75f..d2cf065e9 100644 --- a/projects/components/src/filtering/filter/parser/filter-parser-lookup.service.test.ts +++ b/projects/components/src/filtering/filter/parser/filter-parser-lookup.service.test.ts @@ -28,7 +28,6 @@ describe('Filter Parser Lookup service', () => { expect(spectator.service.lookup(FilterOperator.In)).toEqual(expect.any(InFilterParser)); expect(spectator.service.lookup(FilterOperator.ContainsKey)).toEqual(expect.any(ContainsFilterParser)); - expect(spectator.service.lookup(FilterOperator.ContainsKeyValue)).toEqual(expect.any(ContainsFilterParser)); }); test('correctly identify parsable operators for a type', () => { @@ -42,7 +41,7 @@ describe('Filter Parser Lookup service', () => { true ); expect(spectator.service.isParsableOperatorForType(FilterOperator.Equals, FilterAttributeType.StringMap)).toEqual( - false + true ); expect(spectator.service.isParsableOperatorForType(FilterOperator.Equals, FilterAttributeType.Timestamp)).toEqual( false @@ -59,7 +58,7 @@ describe('Filter Parser Lookup service', () => { ); expect( spectator.service.isParsableOperatorForType(FilterOperator.NotEquals, FilterAttributeType.StringMap) - ).toEqual(false); + ).toEqual(true); expect( spectator.service.isParsableOperatorForType(FilterOperator.NotEquals, FilterAttributeType.Timestamp) ).toEqual(false); @@ -73,9 +72,7 @@ describe('Filter Parser Lookup service', () => { expect(spectator.service.isParsableOperatorForType(FilterOperator.LessThan, FilterAttributeType.String)).toEqual( true ); - expect(spectator.service.isParsableOperatorForType(FilterOperator.LessThan, FilterAttributeType.StringMap)).toEqual( - false - ); + expect(spectator.service.isParsableOperatorForType(FilterOperator.LessThan, FilterAttributeType.Timestamp)).toEqual( false ); @@ -89,9 +86,7 @@ describe('Filter Parser Lookup service', () => { expect( spectator.service.isParsableOperatorForType(FilterOperator.LessThanOrEqualTo, FilterAttributeType.String) ).toEqual(true); - expect( - spectator.service.isParsableOperatorForType(FilterOperator.LessThanOrEqualTo, FilterAttributeType.StringMap) - ).toEqual(false); + expect( spectator.service.isParsableOperatorForType(FilterOperator.LessThanOrEqualTo, FilterAttributeType.Timestamp) ).toEqual(false); @@ -105,9 +100,7 @@ describe('Filter Parser Lookup service', () => { expect(spectator.service.isParsableOperatorForType(FilterOperator.GreaterThan, FilterAttributeType.String)).toEqual( true ); - expect( - spectator.service.isParsableOperatorForType(FilterOperator.GreaterThan, FilterAttributeType.StringMap) - ).toEqual(false); + expect( spectator.service.isParsableOperatorForType(FilterOperator.GreaterThan, FilterAttributeType.Timestamp) ).toEqual(false); @@ -121,9 +114,6 @@ describe('Filter Parser Lookup service', () => { expect( spectator.service.isParsableOperatorForType(FilterOperator.GreaterThanOrEqualTo, FilterAttributeType.String) ).toEqual(true); - expect( - spectator.service.isParsableOperatorForType(FilterOperator.GreaterThanOrEqualTo, FilterAttributeType.StringMap) - ).toEqual(false); expect( spectator.service.isParsableOperatorForType(FilterOperator.GreaterThanOrEqualTo, FilterAttributeType.Timestamp) ).toEqual(false); @@ -131,9 +121,7 @@ describe('Filter Parser Lookup service', () => { expect(spectator.service.isParsableOperatorForType(FilterOperator.In, FilterAttributeType.Boolean)).toEqual(false); expect(spectator.service.isParsableOperatorForType(FilterOperator.In, FilterAttributeType.Number)).toEqual(true); expect(spectator.service.isParsableOperatorForType(FilterOperator.In, FilterAttributeType.String)).toEqual(true); - expect(spectator.service.isParsableOperatorForType(FilterOperator.In, FilterAttributeType.StringMap)).toEqual( - false - ); + expect(spectator.service.isParsableOperatorForType(FilterOperator.In, FilterAttributeType.StringMap)).toEqual(true); expect(spectator.service.isParsableOperatorForType(FilterOperator.In, FilterAttributeType.Timestamp)).toEqual( false ); @@ -153,29 +141,15 @@ describe('Filter Parser Lookup service', () => { expect( spectator.service.isParsableOperatorForType(FilterOperator.ContainsKey, FilterAttributeType.Timestamp) ).toEqual(false); - - expect( - spectator.service.isParsableOperatorForType(FilterOperator.ContainsKeyValue, FilterAttributeType.Boolean) - ).toEqual(false); - expect( - spectator.service.isParsableOperatorForType(FilterOperator.ContainsKeyValue, FilterAttributeType.Number) - ).toEqual(false); - expect( - spectator.service.isParsableOperatorForType(FilterOperator.ContainsKeyValue, FilterAttributeType.String) - ).toEqual(false); - expect( - spectator.service.isParsableOperatorForType(FilterOperator.ContainsKeyValue, FilterAttributeType.StringMap) - ).toEqual(true); - expect( - spectator.service.isParsableOperatorForType(FilterOperator.ContainsKeyValue, FilterAttributeType.Timestamp) - ).toEqual(false); }); - test('can provide a parser to parse user filter strings', () => { + test('can provide a parser to parse split filters', () => { expect( - spectator.service - .lookup(FilterOperator.Equals) - .parseFilterString(getTestFilterAttribute(FilterAttributeType.Boolean), 'Boolean Attribute = false') + spectator.service.lookup(FilterOperator.Equals).parseSplitFilter({ + attribute: getTestFilterAttribute(FilterAttributeType.Boolean), + operator: FilterOperator.Equals, + rhs: 'false' + }) ).toEqual({ field: 'booleanAttribute', operator: FilterOperator.Equals, @@ -183,9 +157,11 @@ describe('Filter Parser Lookup service', () => { }); expect( - spectator.service - .lookup(FilterOperator.LessThanOrEqualTo) - .parseFilterString(getTestFilterAttribute(FilterAttributeType.Number), 'Number Attribute <= 217') + spectator.service.lookup(FilterOperator.LessThanOrEqualTo).parseSplitFilter({ + attribute: getTestFilterAttribute(FilterAttributeType.Number), + operator: FilterOperator.LessThanOrEqualTo, + rhs: '217' + }) ).toEqual({ field: 'numberAttribute', operator: FilterOperator.LessThanOrEqualTo, @@ -193,9 +169,11 @@ describe('Filter Parser Lookup service', () => { }); expect( - spectator.service - .lookup(FilterOperator.NotEquals) - .parseFilterString(getTestFilterAttribute(FilterAttributeType.String), 'String Attribute != myString') + spectator.service.lookup(FilterOperator.NotEquals).parseSplitFilter({ + attribute: getTestFilterAttribute(FilterAttributeType.String), + operator: FilterOperator.NotEquals, + rhs: 'myString' + }) ).toEqual({ field: 'stringAttribute', operator: FilterOperator.NotEquals, @@ -203,9 +181,11 @@ describe('Filter Parser Lookup service', () => { }); expect( - spectator.service - .lookup(FilterOperator.In) - .parseFilterString(getTestFilterAttribute(FilterAttributeType.String), 'String Attribute IN myStr, myString') + spectator.service.lookup(FilterOperator.In).parseSplitFilter({ + attribute: getTestFilterAttribute(FilterAttributeType.String), + operator: FilterOperator.In, + rhs: 'myStr, myString' + }) ).toEqual({ field: 'stringAttribute', operator: FilterOperator.In, @@ -213,119 +193,38 @@ describe('Filter Parser Lookup service', () => { }); expect( - spectator.service - .lookup(FilterOperator.ContainsKey) - .parseFilterString( - getTestFilterAttribute(FilterAttributeType.StringMap), - 'String Map Attribute CONTAINS_KEY myKey' - ) + spectator.service.lookup(FilterOperator.ContainsKey).parseSplitFilter({ + attribute: getTestFilterAttribute(FilterAttributeType.StringMap), + operator: FilterOperator.ContainsKey, + rhs: 'myKey' + }) ).toEqual({ field: 'stringMapAttribute', operator: FilterOperator.ContainsKey, - value: ['myKey'] + value: 'myKey' }); expect( - spectator.service - .lookup(FilterOperator.ContainsKeyValue) - .parseFilterString( - getTestFilterAttribute(FilterAttributeType.StringMap), - 'String Map Attribute CONTAINS_KEY_VALUE myKey:myValue' - ) + spectator.service.lookup(FilterOperator.Equals).parseSplitFilter({ + attribute: getTestFilterAttribute(FilterAttributeType.StringMap), + subpath: 'myKey', + operator: FilterOperator.Equals, + rhs: 'myValue' + }) ).toEqual({ field: 'stringMapAttribute', - operator: FilterOperator.ContainsKeyValue, - value: ['myKey', 'myValue'] - }); - - expect( - spectator.service - .lookup(FilterOperator.Equals) - .parseFilterString(getTestFilterAttribute(FilterAttributeType.Boolean), 'Timestamp Attribute = 1601578015330') - ).toEqual(undefined); - }); - - test('can provide a parser to parse various URL filter strings', () => { - expect( - spectator.service - .lookup(FilterOperator.Equals) - .parseUrlFilterString(getTestFilterAttribute(FilterAttributeType.Boolean), 'booleanAttribute_eq_false') - ).toEqual({ - field: 'booleanAttribute', + subpath: 'myKey', operator: FilterOperator.Equals, - value: false - }); - - expect( - spectator.service - .lookup(FilterOperator.LessThanOrEqualTo) - .parseUrlFilterString(getTestFilterAttribute(FilterAttributeType.Number), 'numberAttribute_lte_217') - ).toEqual({ - field: 'numberAttribute', - operator: FilterOperator.LessThanOrEqualTo, - value: 217 - }); - - expect( - spectator.service - .lookup(FilterOperator.In) - .parseUrlFilterString(getTestFilterAttribute(FilterAttributeType.Number), 'numberAttribute_in_217%2C415%2C707') - ).toEqual({ - field: 'numberAttribute', - operator: FilterOperator.In, - value: [217, 415, 707] - }); - - expect( - spectator.service - .lookup(FilterOperator.NotEquals) - .parseUrlFilterString(getTestFilterAttribute(FilterAttributeType.String), 'stringAttribute_neq_myString') - ).toEqual({ - field: 'stringAttribute', - operator: FilterOperator.NotEquals, - value: 'myString' - }); - - expect( - spectator.service - .lookup(FilterOperator.In) - .parseUrlFilterString(getTestFilterAttribute(FilterAttributeType.String), 'stringAttribute_in_myStr%2CmyString') - ).toEqual({ - field: 'stringAttribute', - operator: FilterOperator.In, - value: ['myStr', 'myString'] - }); - - expect( - spectator.service - .lookup(FilterOperator.ContainsKey) - .parseUrlFilterString(getTestFilterAttribute(FilterAttributeType.StringMap), 'stringMapAttribute_ck_myKey') - ).toEqual({ - field: 'stringMapAttribute', - operator: FilterOperator.ContainsKey, - value: ['myKey'] - }); - - expect( - spectator.service - .lookup(FilterOperator.ContainsKeyValue) - .parseUrlFilterString( - getTestFilterAttribute(FilterAttributeType.StringMap), - 'stringMapAttribute_ckv_myKey%3AmyValue' - ) - ).toEqual({ - field: 'stringMapAttribute', - operator: FilterOperator.ContainsKeyValue, - value: ['myKey', 'myValue'] + value: 'myValue' }); + // Not supported expect( - spectator.service - .lookup(FilterOperator.Equals) - .parseUrlFilterString( - getTestFilterAttribute(FilterAttributeType.Timestamp), - 'timestampAttribute_eq_1601578015330' - ) + spectator.service.lookup(FilterOperator.Equals).parseSplitFilter({ + attribute: getTestFilterAttribute(FilterAttributeType.Boolean), + operator: FilterOperator.Equals, + rhs: '601578015330' + }) ).toEqual(undefined); }); }); diff --git a/projects/components/src/filtering/filter/parser/filter-parser-lookup.service.ts b/projects/components/src/filtering/filter/parser/filter-parser-lookup.service.ts index 14c41cd15..b1b215261 100644 --- a/projects/components/src/filtering/filter/parser/filter-parser-lookup.service.ts +++ b/projects/components/src/filtering/filter/parser/filter-parser-lookup.service.ts @@ -11,6 +11,9 @@ import { InFilterParser } from './types/in-filter-parser'; providedIn: 'root' }) export class FilterParserLookupService { + // TODO remove the separate parsers entirely. + // There's next to no logic left in them, and they duplicate (incorrectly) supported operators, + // Which should be based on attribute type (as defined in filter builders) public lookup(operator: FilterOperator): AbstractFilterParser { switch (operator) { case FilterOperator.Equals: diff --git a/projects/components/src/filtering/filter/parser/types/abstract-filter-parser.test.ts b/projects/components/src/filtering/filter/parser/types/abstract-filter-parser.test.ts deleted file mode 100644 index f0dfa91dd..000000000 --- a/projects/components/src/filtering/filter/parser/types/abstract-filter-parser.test.ts +++ /dev/null @@ -1,92 +0,0 @@ -import { FilterAttribute, FilterAttributeType } from '@hypertrace/components'; -import { getTestFilterAttribute } from '@hypertrace/test-utils'; -import { ParsedFilter } from '../parsed-filter'; -import { ContainsFilterParser } from './contains-filter-parser'; - -describe('Filter Parser', () => { - test('correctly parses CONTAINS_KEY with dot', () => { - const attribute: FilterAttribute = getTestFilterAttribute(FilterAttributeType.StringMap); - const filterString = 'String Map Attribute CONTAINS_KEY http.url'; - - const containsFilterParser: ContainsFilterParser = new ContainsFilterParser(); - const parsedFilter: ParsedFilter | undefined = containsFilterParser.parseFilterString( - attribute, - filterString - ); - - expect(parsedFilter).toEqual({ - field: 'stringMapAttribute', - operator: 'CONTAINS_KEY', - value: ['http.url'] - }); - }); - - test('correctly parses CONTAINS_KEY with colon', () => { - const attribute: FilterAttribute = getTestFilterAttribute(FilterAttributeType.StringMap); - const filterString = 'String Map Attribute CONTAINS_KEY http:url'; - - const containsFilterParser: ContainsFilterParser = new ContainsFilterParser(); - const parsedFilter: ParsedFilter | undefined = containsFilterParser.parseFilterString( - attribute, - filterString - ); - - expect(parsedFilter).toEqual({ - field: 'stringMapAttribute', - operator: 'CONTAINS_KEY', - value: ['http:url'] - }); - }); - - test('correctly parses CONTAINS_KEY_VALUE for tag with dot and URL', () => { - const attribute: FilterAttribute = getTestFilterAttribute(FilterAttributeType.StringMap); - const filterString = - 'String Map Attribute.http.url CONTAINS_KEY_VALUE http://dataservice:9394/userreview?productId=f2620500-8b55-4fab-b7a2-fe8af6f5ae24'; - - const containsFilterParser: ContainsFilterParser = new ContainsFilterParser(); - const parsedFilter: ParsedFilter | undefined = containsFilterParser.parseFilterString( - attribute, - filterString - ); - - expect(parsedFilter).toEqual({ - field: 'stringMapAttribute', - operator: 'CONTAINS_KEY_VALUE', - value: ['http.url', 'http://dataservice:9394/userreview?productId=f2620500-8b55-4fab-b7a2-fe8af6f5ae24'] - }); - }); - - test('correctly parses CONTAINS_KEY_VALUE for tag with colon and URL', () => { - const attribute: FilterAttribute = getTestFilterAttribute(FilterAttributeType.StringMap); - const filterString = - 'String Map Attribute.http:url CONTAINS_KEY_VALUE http://dataservice:9394/userreview?productId=f2620500-8b55-4fab-b7a2-fe8af6f5ae24'; - - const containsFilterParser: ContainsFilterParser = new ContainsFilterParser(); - const parsedFilter: ParsedFilter | undefined = containsFilterParser.parseFilterString( - attribute, - filterString - ); - - expect(parsedFilter).toEqual({ - field: 'stringMapAttribute', - operator: 'CONTAINS_KEY_VALUE', - value: ['http:url', 'http://dataservice:9394/userreview?productId=f2620500-8b55-4fab-b7a2-fe8af6f5ae24'] - }); - }); - - test('correctly parses CONTAINS_KEY_VALUE for tag with colon and empty string', () => { - const attribute: FilterAttribute = getTestFilterAttribute(FilterAttributeType.StringMap); - const filterString = 'String Map Attribute.http:url CONTAINS_KEY_VALUE ""'; - const containsFilterParser: ContainsFilterParser = new ContainsFilterParser(); - const parsedFilter: ParsedFilter | undefined = containsFilterParser.parseFilterString( - attribute, - filterString - ); - - expect(parsedFilter).toEqual({ - field: 'stringMapAttribute', - operator: 'CONTAINS_KEY_VALUE', - value: ['http:url', '""'] - }); - }); -}); diff --git a/projects/components/src/filtering/filter/parser/types/in-filter-parser.ts b/projects/components/src/filtering/filter/parser/types/in-filter-parser.ts index b540143ec..b97575d5c 100644 --- a/projects/components/src/filtering/filter/parser/types/in-filter-parser.ts +++ b/projects/components/src/filtering/filter/parser/types/in-filter-parser.ts @@ -7,7 +7,7 @@ import { AbstractFilterParser } from './abstract-filter-parser'; export class InFilterParser extends AbstractFilterParser { public supportedAttributeTypes(): FilterAttributeType[] { - return [FilterAttributeType.String, FilterAttributeType.Number]; + return [FilterAttributeType.String, FilterAttributeType.Number, FilterAttributeType.StringMap]; } public supportedOperators(): FilterOperator[] { @@ -17,12 +17,12 @@ export class InFilterParser extends AbstractFilterParser { public parseValueString(splitFilter: SplitFilter): PossibleValuesTypes | undefined { switch (splitFilter.attribute.type) { case FilterAttributeType.String: + case FilterAttributeType.StringMap: return this.parseStringArrayValue(splitFilter.rhs); case FilterAttributeType.Number: return this.parseNumberArrayValue(splitFilter.rhs); case FilterAttributeType.Boolean: // Unsupported case FilterAttributeType.StringArray: // Unsupported - case FilterAttributeType.StringMap: // Unsupported case FilterAttributeType.Timestamp: // Unsupported return undefined; default: diff --git a/projects/observability/src/pages/explorer/explorer.component.test.ts b/projects/observability/src/pages/explorer/explorer.component.test.ts index 617e82718..fb1c871e8 100644 --- a/projects/observability/src/pages/explorer/explorer.component.test.ts +++ b/projects/observability/src/pages/explorer/explorer.component.test.ts @@ -203,7 +203,7 @@ describe('Explorer component', () => { expect.objectContaining({ requestType: EXPLORE_GQL_REQUEST, context: ObservabilityTraceType.Api, - filters: [new GraphQlFieldFilter('first', GraphQlOperatorType.Equals, 'foo')], + filters: [new GraphQlFieldFilter({ key: 'first' }, GraphQlOperatorType.Equals, 'foo')], limit: 1000, interval: new TimeDuration(15, TimeUnit.Second) }), @@ -214,7 +214,7 @@ describe('Explorer component', () => { 2, expect.objectContaining({ requestType: TRACES_GQL_REQUEST, - filters: [new GraphQlFieldFilter('first', GraphQlOperatorType.Equals, 'foo')], + filters: [new GraphQlFieldFilter({ key: 'first' }, GraphQlOperatorType.Equals, 'foo')], limit: 100 }), expect.objectContaining({}) @@ -290,7 +290,7 @@ describe('Explorer component', () => { context: SPAN_SCOPE, limit: 1000, interval: new TimeDuration(15, TimeUnit.Second), - filters: [new GraphQlFieldFilter('first', GraphQlOperatorType.Equals, 'foo')] + filters: [new GraphQlFieldFilter({ key: 'first' }, GraphQlOperatorType.Equals, 'foo')] }), expect.objectContaining({}) ); @@ -300,7 +300,7 @@ describe('Explorer component', () => { expect.objectContaining({ requestType: SPANS_GQL_REQUEST, limit: 100, - filters: [new GraphQlFieldFilter('first', GraphQlOperatorType.Equals, 'foo')] + filters: [new GraphQlFieldFilter({ key: 'first' }, GraphQlOperatorType.Equals, 'foo')] }), expect.objectContaining({}) ); diff --git a/projects/observability/src/shared/dashboard/dashboard-wrapper/navigable-dashboard.component.test.ts b/projects/observability/src/shared/dashboard/dashboard-wrapper/navigable-dashboard.component.test.ts index a80fd0f27..c4d9c4d73 100644 --- a/projects/observability/src/shared/dashboard/dashboard-wrapper/navigable-dashboard.component.test.ts +++ b/projects/observability/src/shared/dashboard/dashboard-wrapper/navigable-dashboard.component.test.ts @@ -144,7 +144,7 @@ describe('Navigable dashboard component', () => { }; spectator.query(FilterBarComponent)?.filtersChange.next([explicitFilter]); expect(mockDataSource.addFilters).toHaveBeenCalledWith( - expect.objectContaining({ keyOrExpression: 'foo', operator: GraphQlOperatorType.Equals, value: 'bar' }) + expect.objectContaining({ keyOrExpression: { key: 'foo' }, operator: GraphQlOperatorType.Equals, value: 'bar' }) ); }); diff --git a/projects/observability/src/shared/services/filter-builder/graphql-filter-builder.service.test.ts b/projects/observability/src/shared/services/filter-builder/graphql-filter-builder.service.test.ts index 5af3c9b65..213376efe 100644 --- a/projects/observability/src/shared/services/filter-builder/graphql-filter-builder.service.test.ts +++ b/projects/observability/src/shared/services/filter-builder/graphql-filter-builder.service.test.ts @@ -41,12 +41,12 @@ describe('Graphql filter builder service', () => { buildFilter(attribute1, FilterOperator.LessThanOrEqualTo, 20) ]) ).toEqual([ - new GraphQlFieldFilter(attribute2.name, GraphQlOperatorType.Equals, 'foo'), - new GraphQlFieldFilter(attribute2.name, GraphQlOperatorType.NotEquals, 'bar'), - new GraphQlFieldFilter(attribute1.name, GraphQlOperatorType.GreaterThan, 5), - new GraphQlFieldFilter(attribute1.name, GraphQlOperatorType.GreaterThanOrEqualTo, 10), - new GraphQlFieldFilter(attribute1.name, GraphQlOperatorType.LessThan, 15), - new GraphQlFieldFilter(attribute1.name, GraphQlOperatorType.LessThanOrEqualTo, 20) + new GraphQlFieldFilter({ key: attribute2.name }, GraphQlOperatorType.Equals, 'foo'), + new GraphQlFieldFilter({ key: attribute2.name }, GraphQlOperatorType.NotEquals, 'bar'), + new GraphQlFieldFilter({ key: attribute1.name }, GraphQlOperatorType.GreaterThan, 5), + new GraphQlFieldFilter({ key: attribute1.name }, GraphQlOperatorType.GreaterThanOrEqualTo, 10), + new GraphQlFieldFilter({ key: attribute1.name }, GraphQlOperatorType.LessThan, 15), + new GraphQlFieldFilter({ key: attribute1.name }, GraphQlOperatorType.LessThanOrEqualTo, 20) ]); }); }); From bd713d7c39f964abb2b7714cabb484f90a4cfd7c Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld Date: Fri, 7 Jan 2022 09:44:09 -0500 Subject: [PATCH 5/6] feat: explorer group by subpath support --- .../src/pages/explorer/explorer.component.ts | 2 +- .../explore-query-editor.component.ts | 10 +- .../explore-query-editor.module.ts | 14 +- ...plore-query-group-by-editor.component.scss | 29 +++- ...explore-query-group-by-editor.component.ts | 134 +++++++++++++----- .../explore/explore-specification-builder.ts | 2 +- .../services/metadata/metadata.service.ts | 12 +- 7 files changed, 146 insertions(+), 57 deletions(-) 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.ts b/projects/observability/src/shared/components/explore-query-editor/explore-query-editor.component.ts index 0a2a1b99d..2dabbb218 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 @@ -39,8 +39,8 @@ import { - 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) + ) + .subscribe(this.groupByExpressionChange); } public ngOnChanges(changeObject: TypedSimpleChanges): void { @@ -58,24 +92,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 +130,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 +138,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) + ) ); } From d82bb0870600811a1e40ded41d72efd2c99f6a36 Mon Sep 17 00:00:00 2001 From: Aaron Steinfeld Date: Wed, 12 Jan 2022 16:07:49 -0500 Subject: [PATCH 6/6] test: update tests --- .../pages/explorer/explorer.component.test.ts | 6 +- .../explore-query-editor.component.test.ts | 6 +- .../explore-query-editor.component.ts | 5 +- ...re-query-group-by-editor.component.test.ts | 132 +++++++++++++++--- ...explore-query-group-by-editor.component.ts | 5 +- 5 files changed, 123 insertions(+), 31 deletions(-) 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/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 2dabbb218..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 { @@ -118,8 +119,8 @@ export class ExploreQueryEditorComponent implements OnChanges, OnInit { } 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/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 index 0c7544fbc..fdcd02141 100644 --- 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(` - ` - ); + `); 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 c842b2d18..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,7 +1,7 @@ import { ChangeDetectionStrategy, Component, EventEmitter, Input, OnChanges, Output } from '@angular/core'; import { TypedSimpleChanges } from '@hypertrace/common'; import { InputAppearance, SelectOption } from '@hypertrace/components'; -import { isEmpty, isNil } from 'lodash-es'; +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'; @@ -82,7 +82,8 @@ export class ExploreQueryGroupByEditorComponent implements OnChanges { this.groupByExpressionsToEmit .pipe( filter(expression => this.isValidExpressionToEmit(expression)), - debounceTime(500) + debounceTime(500), + map(expression => omit(expression, 'metadata')) ) .subscribe(this.groupByExpressionChange); }