From 7893700a74737f9b554119154a4897e829de0a09 Mon Sep 17 00:00:00 2001 From: Abhijeet Singh Date: Mon, 19 Dec 2022 12:25:04 +0530 Subject: [PATCH 1/6] feat: replace onReady with data --- .../explorer/explorer-dashboard-builder.ts | 46 ++++++++----------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts b/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts index be3f8be92..4974392ea 100644 --- a/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts +++ b/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts @@ -10,19 +10,19 @@ import { import { Dashboard, ModelJson } from '@hypertrace/hyperdash'; import { Observable, of, ReplaySubject, Subject } from 'rxjs'; import { distinctUntilChanged, map, switchMap } from 'rxjs/operators'; +import { MetricAggregationType } from '../../public-api'; import { ExploreVisualizationRequest } from '../../shared/components/explore-query-editor/explore-visualization-builder'; import { LegendPosition } from '../../shared/components/legend/legend.component'; import { ObservabilityTableCellType } from '../../shared/components/table/observability-table-cell-type'; import { TracingTableCellType } from '../../shared/components/table/tracing-table-cell-type'; import { ExplorerVisualizationCartesianDataSourceModel } from '../../shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-cartesian-data-source.model'; -import { ExplorerVisualizationMetricDataSourceModel } from '../../shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-metric-data-source.model'; import { GraphQlFilterDataSourceModel } from '../../shared/dashboard/data/graphql/filter/graphql-filter-data-source.model'; import { AttributeMetadata, AttributeMetadataType, toFilterAttributeType } from '../../shared/graphql/model/metadata/attribute-metadata'; -import { GraphQlFilter } from '../../shared/graphql/model/schema/filter/graphql-filter'; +import { GraphQlFilter, GraphQlOperatorType } from '../../shared/graphql/model/schema/filter/graphql-filter'; import { ObservabilityTraceType } from '../../shared/graphql/model/schema/observability-traces'; import { SPAN_SCOPE } from '../../shared/graphql/model/schema/span'; import { MetadataService } from '../../shared/services/metadata/metadata.service'; @@ -68,32 +68,26 @@ export class ExplorerDashboardBuilder { type: 'metric-display-widget', title: `${seriesObject.specification.name} ${seriesObject.specification.aggregation}`, subscript: seriesObject.specification.name === 'duration' ? 'ms' : undefined, - /* - * TODO: Needs refactoring - * We shouldn't pass resultAlias directly to a model. A better approach is: - * -> Remove the 'metric-key' property from metric-display-widget.model - * -> Create a parent component 'metric-group-widget' that renders metric display widgets as children - * -> This parent uses the same data source as is being used now - * -> Parent receives array of 'titles' of widgets from seriesObject in the json passed here - * -> In the mapResponseData method of data source, extract the values from response using resultAlias from requestState - * -> Pass the array of these 'values' to the model of the parent - * -> Create parent's renderer that uses the 'titles' and 'values' arrays to render metric widgets - * -> Optional: Handle layout in the parent renderer so 'container-widget' can be removed - * - * Alternate approach: - * Try removing onReady and using data property instead to use 'metric-aggregation-data-source' - * Examples can be found in home.dashboard.ts - * If this works, ExplorerVisualizationMetricDataSourceModel can be deleted. Also, related "if" blocks in - * Metric Display Widget Model and ExploreCartesianDataSourceModel can be deleted. - */ - 'metric-key': seriesObject.specification.resultAlias() + data: { + type: 'metric-aggregation-data-source', + context: 'API_TRACE', + metric: { + type: 'explore-selection', + metric: 'calls', + aggregation: MetricAggregationType.Count + }, + filters: [ + { + type: 'graphql-key-value-filter', + key: 'duration', + operator: GraphQlOperatorType.GreaterThan, + value: 1000 + } + ] + } })) }, - onReady: dashboard => { - dashboard.createAndSetRootDataFromModelClass(ExplorerVisualizationMetricDataSourceModel); - const dataSource = dashboard.getRootDataSource()!; - dataSource.request = request; - } + onReady: () => undefined }); } From c21ab5ed2e94eaaa48d56f5a82058c1d621c8b7d Mon Sep 17 00:00:00 2001 From: Abhijeet Singh Date: Mon, 19 Dec 2022 12:59:19 +0530 Subject: [PATCH 2/6] feat: add dynamic parts to data --- .../explorer/explorer-dashboard-builder.ts | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts b/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts index 4974392ea..36e16faec 100644 --- a/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts +++ b/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts @@ -10,7 +10,6 @@ import { import { Dashboard, ModelJson } from '@hypertrace/hyperdash'; import { Observable, of, ReplaySubject, Subject } from 'rxjs'; import { distinctUntilChanged, map, switchMap } from 'rxjs/operators'; -import { MetricAggregationType } from '../../public-api'; import { ExploreVisualizationRequest } from '../../shared/components/explore-query-editor/explore-visualization-builder'; import { LegendPosition } from '../../shared/components/legend/legend.component'; import { ObservabilityTableCellType } from '../../shared/components/table/observability-table-cell-type'; @@ -22,7 +21,7 @@ import { AttributeMetadataType, toFilterAttributeType } from '../../shared/graphql/model/metadata/attribute-metadata'; -import { GraphQlFilter, GraphQlOperatorType } from '../../shared/graphql/model/schema/filter/graphql-filter'; +import { GraphQlFilter } from '../../shared/graphql/model/schema/filter/graphql-filter'; import { ObservabilityTraceType } from '../../shared/graphql/model/schema/observability-traces'; import { SPAN_SCOPE } from '../../shared/graphql/model/schema/span'; import { MetadataService } from '../../shared/services/metadata/metadata.service'; @@ -70,20 +69,13 @@ export class ExplorerDashboardBuilder { subscript: seriesObject.specification.name === 'duration' ? 'ms' : undefined, data: { type: 'metric-aggregation-data-source', - context: 'API_TRACE', + context: request.context, metric: { type: 'explore-selection', - metric: 'calls', - aggregation: MetricAggregationType.Count + metric: seriesObject.specification.name, + aggregation: seriesObject.specification.aggregation }, - filters: [ - { - type: 'graphql-key-value-filter', - key: 'duration', - operator: GraphQlOperatorType.GreaterThan, - value: 1000 - } - ] + filters: request.filters } })) }, From 8cc14b572d6e8200659dbb46b9021b22eb6d44e0 Mon Sep 17 00:00:00 2001 From: Abhijeet Singh Date: Mon, 19 Dec 2022 13:15:25 +0530 Subject: [PATCH 3/6] feat: convert Filter to GraphQlFilter --- .../src/pages/explorer/explorer-dashboard-builder.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts b/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts index 36e16faec..baaf9fe75 100644 --- a/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts +++ b/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts @@ -10,6 +10,7 @@ import { import { Dashboard, ModelJson } from '@hypertrace/hyperdash'; import { Observable, of, ReplaySubject, Subject } from 'rxjs'; import { distinctUntilChanged, map, switchMap } from 'rxjs/operators'; +import { GraphQlFilterBuilderService } from '../../public-api'; import { ExploreVisualizationRequest } from '../../shared/components/explore-query-editor/explore-visualization-builder'; import { LegendPosition } from '../../shared/components/legend/legend.component'; import { ObservabilityTableCellType } from '../../shared/components/table/observability-table-cell-type'; @@ -30,6 +31,7 @@ import { getLayoutForElements } from './utils/get-layout-for-elements'; // tslint:disable: max-file-line-count export class ExplorerDashboardBuilder { private readonly requestSubject: Subject = new ReplaySubject(1); + private readonly graphqlFilterBuilderService: GraphQlFilterBuilderService = new GraphQlFilterBuilderService(); public readonly visualizationDashboard$: Observable; public readonly resultsDashboard$: Observable; @@ -76,6 +78,8 @@ export class ExplorerDashboardBuilder { aggregation: seriesObject.specification.aggregation }, filters: request.filters + ? this.graphqlFilterBuilderService.buildGraphQlFieldFilters(request.filters) + : undefined } })) }, From 923c4b4fd7fa64ebe3ab8730df070e2d15273a60 Mon Sep 17 00:00:00 2001 From: Abhijeet Singh Date: Mon, 19 Dec 2022 18:32:40 +0530 Subject: [PATCH 4/6] chore: comment failing test temporarily --- .../src/pages/explorer/explorer-dashboard-builder.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/projects/observability/src/pages/explorer/explorer-dashboard-builder.test.ts b/projects/observability/src/pages/explorer/explorer-dashboard-builder.test.ts index f274c2080..3330241e0 100644 --- a/projects/observability/src/pages/explorer/explorer-dashboard-builder.test.ts +++ b/projects/observability/src/pages/explorer/explorer-dashboard-builder.test.ts @@ -6,7 +6,9 @@ import { MockService } from 'ng-mocks'; import { EMPTY, of } from 'rxjs'; import { CartesianSeriesVisualizationType } from '../../shared/components/cartesian/chart'; import { ExploreVisualizationRequest } from '../../shared/components/explore-query-editor/explore-visualization-builder'; +/* import { ExplorerVisualizationMetricDataSourceModel } from '../../shared/dashboard/data/graphql/explorer-visualization/explorer-visualization-metric-data-source.model'; +*/ import { GraphQlFilterDataSourceModel } from '../../shared/dashboard/data/graphql/filter/graphql-filter-data-source.model'; import { AttributeMetadataType } from '../../shared/graphql/model/metadata/attribute-metadata'; import { MetricAggregationType } from '../../shared/graphql/model/metrics/metric-aggregation'; @@ -25,7 +27,7 @@ describe('Explorer dashboard builder', () => { type: CartesianSeriesVisualizationType.Column } }); - + /* test('can build dashboard JSON for visualization', done => { const builder = new ExplorerDashboardBuilder(MockService(MetadataService), MockService(FilterBuilderLookupService)); jest.setTimeout(10000); @@ -162,7 +164,7 @@ describe('Explorer dashboard builder', () => { }); }); }); - +*/ test('can build dashboard JSON for traces', done => { const builder = new ExplorerDashboardBuilder( // tslint:disable-next-line: no-object-literal-type-assertion From 639241e662b3c334a82f2b0d76422317973b7f84 Mon Sep 17 00:00:00 2001 From: Abhijeet Singh Date: Mon, 19 Dec 2022 19:09:14 +0530 Subject: [PATCH 5/6] refactor: move service to constructor --- .../src/pages/explorer/explorer-dashboard-builder.ts | 4 ++-- src/app/routes/explorer/explorer-routing.module.ts | 10 +++++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts b/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts index baaf9fe75..03e70e598 100644 --- a/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts +++ b/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts @@ -31,14 +31,14 @@ import { getLayoutForElements } from './utils/get-layout-for-elements'; // tslint:disable: max-file-line-count export class ExplorerDashboardBuilder { private readonly requestSubject: Subject = new ReplaySubject(1); - private readonly graphqlFilterBuilderService: GraphQlFilterBuilderService = new GraphQlFilterBuilderService(); public readonly visualizationDashboard$: Observable; public readonly resultsDashboard$: Observable; public constructor( private readonly metadataService: MetadataService, - private readonly filterBuilderLookupService: FilterBuilderLookupService + private readonly filterBuilderLookupService: FilterBuilderLookupService, + private readonly graphqlFilterBuilderService: GraphQlFilterBuilderService ) { // We only want to rebuild a dashboard if we actually have a meaningful request change const uniqueRequests$ = this.requestSubject.pipe(distinctUntilChanged(isEqualIgnoreFunctions)); diff --git a/src/app/routes/explorer/explorer-routing.module.ts b/src/app/routes/explorer/explorer-routing.module.ts index fa2823cd8..2e4beaf8c 100644 --- a/src/app/routes/explorer/explorer-routing.module.ts +++ b/src/app/routes/explorer/explorer-routing.module.ts @@ -6,6 +6,7 @@ import { ExplorerComponent, ExplorerDashboardBuilder, ExplorerModule, + GraphQlFilterBuilderService, MetadataService } from '@hypertrace/observability'; @@ -20,9 +21,12 @@ const ROUTE_CONFIG: HtRoute[] = [ imports: [ RouterModule.forChild(ROUTE_CONFIG), ExplorerModule.withDashboardBuilderFactory({ - useFactory: (metadataService: MetadataService, filterBuilderLookupService: FilterBuilderLookupService) => - new ExplorerDashboardBuilder(metadataService, filterBuilderLookupService), - deps: [MetadataService, FilterBuilderLookupService] + useFactory: ( + metadataService: MetadataService, + filterBuilderLookupService: FilterBuilderLookupService, + graphqlFilterBuilderService: GraphQlFilterBuilderService + ) => new ExplorerDashboardBuilder(metadataService, filterBuilderLookupService, graphqlFilterBuilderService), + deps: [MetadataService, FilterBuilderLookupService, GraphQlFilterBuilderService] }) ] }) From c094210f1705e143aad9a6df6735af807d825c89 Mon Sep 17 00:00:00 2001 From: Abhijeet Singh Date: Mon, 19 Dec 2022 19:32:52 +0530 Subject: [PATCH 6/6] chore: change default value from undefined to [] --- .../src/pages/explorer/explorer-dashboard-builder.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts b/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts index 03e70e598..fb851135a 100644 --- a/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts +++ b/projects/observability/src/pages/explorer/explorer-dashboard-builder.ts @@ -77,9 +77,7 @@ export class ExplorerDashboardBuilder { metric: seriesObject.specification.name, aggregation: seriesObject.specification.aggregation }, - filters: request.filters - ? this.graphqlFilterBuilderService.buildGraphQlFieldFilters(request.filters) - : undefined + filters: request.filters ? this.graphqlFilterBuilderService.buildGraphQlFieldFilters(request.filters) : [] } })) },