-
Notifications
You must be signed in to change notification settings - Fork 11
feat: custom metrics support for topology widget (WIP) #965
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
796acd3
6d88cbc
a9027e8
02b86fa
78dd8fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,8 @@ | ||
| import { Dictionary } from '@hypertrace/common'; | ||
| import { ArrayPropertyTypeInstance, EnumPropertyTypeInstance, ENUM_TYPE } from '@hypertrace/dashboards'; | ||
| import { GraphQlDataSourceModel, SpecificationBuilder } from '@hypertrace/distributed-tracing'; | ||
| import { GraphQlRequestCacheability, GraphQlRequestOptions } from '@hypertrace/graphql-client'; | ||
| import { ARRAY_PROPERTY, Model, ModelProperty, ModelPropertyType } from '@hypertrace/hyperdash'; | ||
| import { ARRAY_PROPERTY, Model, ModelProperty, PLAIN_OBJECT_PROPERTY } from '@hypertrace/hyperdash'; | ||
| import { uniq } from 'lodash-es'; | ||
| import { Observable } from 'rxjs'; | ||
| import { map } from 'rxjs/operators'; | ||
|
|
@@ -14,7 +15,7 @@ import { | |
| TopologyEdgeSpecification, | ||
| TopologyNodeSpecification | ||
| } from '../../../../graphql/request/handlers/entities/query/topology/entity-topology-graphql-query-handler.service'; | ||
| import { EntityMetricAggregationDataSourceModel } from '../entity/aggregation/entity-metric-aggregation-data-source.model'; | ||
| import { MetricData } from '../../../widgets/topology/metric/metric'; | ||
|
|
||
| @Model({ | ||
| type: 'topology-data-source' | ||
|
|
@@ -59,29 +60,15 @@ export class TopologyDataSourceModel extends GraphQlDataSourceModel<TopologyData | |
|
|
||
| @ModelProperty({ | ||
| key: 'node-metrics', | ||
| // tslint:disable-next-line: no-object-literal-type-assertion | ||
| type: { | ||
| key: ARRAY_PROPERTY.type, | ||
| subtype: { | ||
| key: ModelPropertyType.TYPE, | ||
| defaultModelClass: EntityMetricAggregationDataSourceModel | ||
| } | ||
| } as ArrayPropertyTypeInstance | ||
| type: PLAIN_OBJECT_PROPERTY.type | ||
| }) | ||
| public nodeMetricSpecifications: MetricAggregationSpecification[] = []; | ||
| public nodeMetrics!: MetricData; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we give it a better name? We should convert this into a model object too. may be
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can change the name,
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be minimal.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's rename this and convert it into a model |
||
|
|
||
| @ModelProperty({ | ||
| key: 'edge-metrics', | ||
| // tslint:disable-next-line: no-object-literal-type-assertion | ||
| type: { | ||
| key: ARRAY_PROPERTY.type, | ||
| subtype: { | ||
| key: ModelPropertyType.TYPE, | ||
| defaultModelClass: EntityMetricAggregationDataSourceModel | ||
| } | ||
| } as ArrayPropertyTypeInstance | ||
| type: PLAIN_OBJECT_PROPERTY.type | ||
| }) | ||
| public edgeMetricSpecifications: MetricAggregationSpecification[] = []; | ||
| public edgeMetrics!: MetricData; | ||
|
|
||
| private readonly specBuilder: SpecificationBuilder = new SpecificationBuilder(); | ||
| private readonly requestOptions: GraphQlRequestOptions = { | ||
|
|
@@ -91,7 +78,7 @@ export class TopologyDataSourceModel extends GraphQlDataSourceModel<TopologyData | |
| public getData(): Observable<TopologyData> { | ||
| const rootEntitySpec = this.buildEntitySpec(); | ||
| const edgeSpec = { | ||
| metricSpecifications: this.edgeMetricSpecifications | ||
| metricSpecifications: this.getAllMetricSpecifications(this.edgeMetrics) | ||
| }; | ||
|
|
||
| return this.query<EntityTopologyGraphQlQueryHandlerService>( | ||
|
|
@@ -116,7 +103,11 @@ export class TopologyDataSourceModel extends GraphQlDataSourceModel<TopologyData | |
| this.entityType, | ||
| ...this.defaultedEntityTypeArray(this.upstreamEntityTypes), | ||
| ...this.defaultedEntityTypeArray(this.downstreamEntityTypes) | ||
| ]) | ||
| ]), | ||
| modelProperties: { | ||
| nodeMetrics: this.nodeMetrics, | ||
| edgeMetrics: this.edgeMetrics | ||
| } | ||
| })) | ||
| ); | ||
| } | ||
|
|
@@ -134,7 +125,7 @@ export class TopologyDataSourceModel extends GraphQlDataSourceModel<TopologyData | |
| private buildEntitySpec(): TopologyNodeSpecification { | ||
| // TODO support different specs for different node types | ||
| return { | ||
| metricSpecifications: this.nodeMetricSpecifications, | ||
| metricSpecifications: this.getAllMetricSpecifications(this.nodeMetrics), | ||
| titleSpecification: this.specBuilder.attributeSpecificationForKey('name') | ||
| }; | ||
| } | ||
|
|
@@ -144,11 +135,20 @@ export class TopologyDataSourceModel extends GraphQlDataSourceModel<TopologyData | |
| ): ObservabilityEntityType[] { | ||
| return typeArray; | ||
| } | ||
|
|
||
| private getAllMetricSpecifications(metrics: MetricData): MetricAggregationSpecification[] { | ||
| return [ | ||
| metrics.primary.specification, | ||
| ...(metrics.secondary ? [metrics.secondary.specification] : []), | ||
| ...(metrics.others ? metrics.others.map(_ => _.specification) : []) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: |
||
| ]; | ||
| } | ||
| } | ||
|
|
||
| export interface TopologyData { | ||
| nodes: EntityNode[]; | ||
| nodeTypes: ObservabilityEntityType[]; | ||
| nodeSpecification: TopologyNodeSpecification; | ||
| edgeSpecification: TopologyEdgeSpecification; | ||
| modelProperties: Dictionary<unknown>; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what does this represent? why do we need it in data source model? |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ | |
| stroke-width: 2; | ||
| stroke-linecap: round; | ||
| stroke-linejoin: round; | ||
| stroke: currentColor; | ||
| stroke: $gray-3; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be changing?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This I can look into, I think it was needed, since currentColor was not working.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please take a look |
||
| fill: none; | ||
|
|
||
| @include chart-small-regular; | ||
|
|
@@ -34,7 +34,6 @@ | |
| .edge-path { | ||
| stroke-dasharray: 2, 2; | ||
| stroke-width: 1px; | ||
| stroke: $gray-3; | ||
| } | ||
|
|
||
| &.background { | ||
|
|
@@ -50,7 +49,6 @@ | |
| &.focused { | ||
| .entity-edge-metric-bubble { | ||
| @include show; | ||
| fill: currentColor; | ||
| } | ||
|
|
||
| .entity-edge-metric-value { | ||
|
|
@@ -65,71 +63,13 @@ | |
| .edge-path { | ||
| stroke-dasharray: none; | ||
| stroke-width: 1.5px; | ||
| stroke: currentColor; | ||
| } | ||
|
|
||
| &.less-than-20-category { | ||
| color: $blue-gray-1; | ||
| } | ||
|
|
||
| &.from-20-to-100-category { | ||
| color: $blue-gray-2; | ||
| } | ||
|
|
||
| &.from-100-to-500-category { | ||
| color: $blue-gray-3; | ||
| } | ||
|
|
||
| &.from-500-to-1000-category { | ||
| color: $blue-gray-4; | ||
| } | ||
|
|
||
| &.greater-than-or-equal-to-1000-category { | ||
| color: $blue-gray-5; | ||
| } | ||
|
|
||
| &.not-specified-category { | ||
| color: lightgray; | ||
| } | ||
|
|
||
| &.greater-than-or-equal-to-5-error-category { | ||
| color: $red-5; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| .entity-edge-arrow { | ||
| stroke-linecap: round; | ||
| stroke-linejoin: round; | ||
| fill: currentColor; | ||
|
|
||
| &.less-than-20-category { | ||
| color: $blue-gray-1; | ||
| } | ||
|
|
||
| &.from-20-to-100-category { | ||
| color: $blue-gray-2; | ||
| } | ||
|
|
||
| &.from-100-to-500-category { | ||
| color: $blue-gray-3; | ||
| } | ||
|
|
||
| &.from-500-to-1000-category { | ||
| color: $blue-gray-4; | ||
| } | ||
|
|
||
| &.greater-than-or-equal-to-1000-category { | ||
| color: $blue-gray-5; | ||
| } | ||
|
|
||
| &.not-specified-category { | ||
| color: lightgray; | ||
| } | ||
|
|
||
| &.greater-than-or-equal-to-5-error-category { | ||
| color: $red-5; | ||
| } | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice