Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ export class TraceWaterfallDataSourceModel extends GraphQlDataSourceModel<Waterf
this.specificationBuilder.attributeSpecificationForKey('spanTags'),
this.specificationBuilder.attributeSpecificationForKey('startTime'),
this.specificationBuilder.attributeSpecificationForKey('type'),
this.specificationBuilder.attributeSpecificationForKey('traceId')
this.specificationBuilder.attributeSpecificationForKey('traceId'),
this.specificationBuilder.attributeSpecificationForKey('errorCount')
];

public getData(): Observable<WaterfallData[]> {
Expand Down Expand Up @@ -100,7 +101,8 @@ export class TraceWaterfallDataSourceModel extends GraphQlDataSourceModel<Waterf
serviceName: span.serviceName as string,
protocolName: span.protocolName as string,
spanType: span.type as SpanType,
tags: span.spanTags as Dictionary<unknown>
tags: span.spanTags as Dictionary<unknown>,
errorCount: span.errorCount as number
}));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ export interface SpanNameCellData {
protocolName: string;
name: string;
color?: string;
error?: boolean;
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

.span-title {
display: grid;
grid-template-columns: 3px min-content min-content auto;
grid-template-columns: 3px min-content min-content min-content auto;
grid-template-rows: 1fr;
column-gap: 4px;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ describe('Span name table cell renderer component', () => {
shallow: true
});

test('should render span name without color and build tooltip ', () => {
test('should render span name without color and error icon and build tooltip ', () => {
const spectator = buildComponent();

const tooltip = `${spanNameData.serviceName} ${spanNameData.protocolName} ${spanNameData.name}`;
Expand All @@ -38,12 +38,14 @@ describe('Span name table cell renderer component', () => {
expect(spectator.query('.protocol-name')).toHaveText('test-protocol');
expect(spectator.query('.span-name')).toHaveText('test-span-name');
expect(spectator.query('.color-bar')).not.toExist();
expect(spectator.query('.error-icon')).not.toExist();
});

test('should render span name with color and build tooltip ', () => {
test('should render span name with color and error icon and build tooltip ', () => {
const spanNameDataWithColor = {
...spanNameData,
color: 'blue'
color: 'blue',
error: true
};
const spectator = buildComponent({
providers: [tableCellDataProvider(spanNameDataWithColor)]
Expand All @@ -57,5 +59,6 @@ describe('Span name table cell renderer component', () => {
expect(spectator.query('.protocol-name')).toHaveText('test-protocol');
expect(spectator.query('.span-name')).toHaveText('test-span-name');
expect(spectator.query('.color-bar')).toExist();
expect(spectator.query('.error-icon')).toExist();
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { ChangeDetectionStrategy, Component } from '@angular/core';
import { TableCellAlignmentType, TableCellRenderer, TableCellRendererBase } from '@hypertrace/components';
import { IconType } from '@hypertrace/assets-library';
import { Color } from '@hypertrace/common';
import { IconSize, TableCellAlignmentType, TableCellRenderer, TableCellRendererBase } from '@hypertrace/components';
import { SpanNameCellData } from './span-name-cell-data';
import { WaterfallTableCellType } from './span-name-cell-type';

Expand All @@ -19,6 +21,13 @@ import { WaterfallTableCellType } from './span-name-cell-type';
<div class="span-name">
<span class="text" data-sensitive-pii>{{ this.value.name }}</span>
</div>
<ht-icon
*ngIf="this.value.error"
class="error-icon"
icon="${IconType.Error}"
size="${IconSize.Medium}"
color="${Color.Red5}"
></ht-icon>
</div>
`
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { CommonModule } from '@angular/common';
import { NgModule } from '@angular/core';
import { SequenceChartModule, TableModule, TooltipModule } from '@hypertrace/components';
import { IconModule, SequenceChartModule, TableModule, TooltipModule } from '@hypertrace/components';
import { SpanNameTableCellParser } from './span-name/span-name-table-cell-parser';
import { SpanNameTableCellRendererComponent } from './span-name/span-name-table-cell-renderer.component';
import { WaterfallChartComponent } from './waterfall-chart.component';
Expand All @@ -12,7 +12,8 @@ import { WaterfallChartComponent } from './waterfall-chart.component';
CommonModule,
TableModule.withCellParsers([SpanNameTableCellParser]),
TableModule.withCellRenderers([SpanNameTableCellRendererComponent]),
TooltipModule
TooltipModule,
IconModule
],
exports: [WaterfallChartComponent]
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ export class WaterfallChartService {
$$spanName: {
name: datum.name,
serviceName: datum.serviceName,
protocolName: datum.protocolName
protocolName: datum.protocolName,
error: datum.errorCount > 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: hasError would convey it is boolean and not an error string

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah,
I have made the changes

},
$$iconType: this.iconLookupService.forSpanType(datum.spanType)!,
getChildren: () => of([]),
Expand Down Expand Up @@ -82,7 +83,6 @@ export class WaterfallChartService {
// Do DFS
while (sequenceNodes.length !== 0) {
const node = sequenceNodes.shift()!;

if (node.$$state.expanded) {
segments.push({
id: node.id,
Expand Down Expand Up @@ -123,7 +123,10 @@ export class WaterfallChartService {
const node = nodes.shift()!;
let color;

if (colorMap.has(node.serviceName)) {
if (node.$$spanName.error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the color logic can be isolated to its own method now (since it has gotten bigger).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess, we don't need this as of now.
It is not that big and we also need to handle some local vars(the way code is written currently)

// If span contains an error
color = Color.Red5;
} else if (colorMap.has(node.serviceName)) {
// ServiceName seen before. Use existing service color
color = colorMap.get(node.serviceName)!;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export interface WaterfallData {
responseHeaders?: Dictionary<unknown>;
responseBody?: string;
tags: Dictionary<unknown>;
errorCount: number;
}

export interface WaterfallDataNode extends WaterfallData, Omit<StatefulPrefetchedTreeTableRow, '$$state'> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ export class ApiTraceWaterfallDataSourceModel extends GraphQlDataSourceModel<Wat
'protocolName',
'spanTags',
'startTime',
'type'
'type',
'errorCount'
];
}

Expand Down Expand Up @@ -106,7 +107,8 @@ export class ApiTraceWaterfallDataSourceModel extends GraphQlDataSourceModel<Wat
name: span.displaySpanName as string,
protocolName: span.protocolName as string,
spanType: span.type as SpanType,
tags: span.spanTags as Dictionary<unknown>
tags: span.spanTags as Dictionary<unknown>,
errorCount: span.errorCount as number
};
}
}