Skip to content

fix: waterfall undefined strings #803

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 28, 2021
Merged

fix: waterfall undefined strings #803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 0 additions & 1 deletion projects/components/src/sequence/sequence-chart.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ export class SequenceChartComponent implements OnChanges {
id: segment.id,
start: segment.start - minStart,
end: segment.end - minStart,
label: segment.label,
Copy link
Contributor Author

@jake-bassett jake-bassett Apr 27, 2021

Choose a reason for hiding this comment

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

I couldn't see anywhere this was used, so removed it. cc: @anandtiwary to confirm this is okay.

color: segment.color
}));
}
Expand Down
1 change: 0 additions & 1 deletion projects/components/src/sequence/sequence.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ export interface SequenceSegment {
id: string;
start: number;
end: number;
label: string;
color: string;
}

Expand Down
4 changes: 2 additions & 2 deletions projects/distributed-tracing/src/pages/trace-detail/trace-detail.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,14 @@ export class TraceDetailService implements OnDestroy {
private buildTimeString(trace: Trace, units: string): string {
return `${new DateFormatter({ mode: DateFormatMode.DateAndTimeWithSeconds }).format(
trace.startTime as number
)} for ${trace.duration as string} ${units}`;
)} for ${trace.duration as string} ${units ?? ''}`.trim();
}

private buildTitleString(trace: Trace): string {
if (trace.spans?.length === 1) {
const entrySpan = trace.spans[0];

return `${entrySpan.serviceName as string} ${entrySpan.displaySpanName as string}`;
return `${entrySpan.serviceName as string} ${(entrySpan.displaySpanName as string) ?? ''}`.trim();
Copy link
Contributor

@palbizu palbizu Apr 27, 2021

Choose a reason for hiding this comment

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

Wouldn't it be correct to do the trim() before generating the string?

Copy link
Contributor Author

@jake-bassett jake-bassett Apr 27, 2021

Choose a reason for hiding this comment

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

In this case since we also have a space in the middle of the template, I trim() once the complete string is built to remove any space between (since there might not be a displaySpanName).

}

return '';
Expand Down
2 changes: 1 addition & 1 deletion projects/distributed-tracing/src/shared/components/span-detail/span-title.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ export class SpanTitle {
public constructor(public serviceName?: string, public protocolName?: string, public apiName?: string) {}

public toString(): string {
return `${this.serviceName} ${this.protocolName} ${this.apiName}`;
return `${this.serviceName} ${this.protocolName ?? ''} ${this.apiName ?? ''}`.trim();
}
}
4 changes: 2 additions & 2 deletions ...ing/src/shared/dashboard/data/graphql/waterfall/trace-waterfall-data-source.model.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ describe('Trace Waterfall data source model', () => {
value: 1,
units: 'ms'
},
name: 'Span Name 1',
apiName: 'Span Name 1',
serviceName: 'Service Name 1',
protocolName: undefined,
spanType: SpanType.Entry,
Expand All @@ -231,7 +231,7 @@ describe('Trace Waterfall data source model', () => {
value: 2,
units: 'ms'
},
name: 'Span Name 2',
apiName: 'Span Name 2',
serviceName: 'Service Name 2',
protocolName: undefined,
spanType: SpanType.Exit,
Expand Down
2 changes: 1 addition & 1 deletion ...-tracing/src/shared/dashboard/data/graphql/waterfall/trace-waterfall-data-source.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,9 @@ export class TraceWaterfallDataSourceModel extends GraphQlDataSourceModel<Waterf
value: span.duration as number,
units: duration.units
},
name: span.displaySpanName as string,
serviceName: span.serviceName as string,
protocolName: span.protocolName as string,
apiName: span.displaySpanName as string,
spanType: span.type as SpanType,
tags: span.spanTags as Dictionary<unknown>,
errorCount: span.errorCount as number
Expand Down
4 changes: 2 additions & 2 deletions ...tracing/src/shared/dashboard/widgets/waterfall/waterfall/span-name/span-name-cell-data.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
export interface SpanNameCellData {
serviceName: string;
protocolName: string;
name: string;
protocolName?: string;
apiName?: string;
color?: string;
hasError?: boolean;
}
2 changes: 1 addition & 1 deletion ...src/shared/dashboard/widgets/waterfall/waterfall/span-name/span-name-table-cell-parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export class SpanNameTableCellParser extends TableCellParserBase<SpanNameCellDat
}

public parseTooltip(cellData: SpanNameCellData): string {
return `${cellData.serviceName} ${cellData.protocolName} ${cellData.name}`;
return `${cellData.serviceName} ${cellData.protocolName ?? ''} ${cellData.apiName ?? ''}`.trim();
}

public parseFilterValue(): undefined {
Expand Down
6 changes: 3 additions & 3 deletions ...ard/widgets/waterfall/waterfall/span-name/span-name-table-cell-renderer.component.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ describe('Span name table cell renderer component', () => {
const spanNameData = {
serviceName: 'test-entity',
protocolName: 'test-protocol',
name: 'test-span-name'
apiName: 'test-span-name'
};

const buildComponent = createComponentFactory({
Expand All @@ -30,7 +30,7 @@ describe('Span name table cell renderer component', () => {
test('should render span name without color and error icon and build tooltip ', () => {
const spectator = buildComponent();

const tooltip = `${spanNameData.serviceName} ${spanNameData.protocolName} ${spanNameData.name}`;
const tooltip = `${spanNameData.serviceName} ${spanNameData.protocolName} ${spanNameData.apiName}`;

expect(spectator.component.value).toEqual(spanNameData);
expect(spectator.component.tooltip).toEqual(tooltip);
Expand All @@ -51,7 +51,7 @@ describe('Span name table cell renderer component', () => {
providers: [tableCellDataProvider(spanNameDataWithColor)]
});

const tooltip = `${spanNameData.serviceName} ${spanNameData.protocolName} ${spanNameData.name}`;
const tooltip = `${spanNameData.serviceName} ${spanNameData.protocolName} ${spanNameData.apiName}`;

expect(spectator.component.value).toEqual(spanNameDataWithColor);
expect(spectator.component.tooltip).toEqual(tooltip);
Expand Down
2 changes: 1 addition & 1 deletion ...ashboard/widgets/waterfall/waterfall/span-name/span-name-table-cell-renderer.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { WaterfallTableCellType } from './span-name-cell-type';
<span class="text" data-sensitive-pii>{{ this.value.protocolName }}</span>
</div>
<div class="span-name">
<span class="text" data-sensitive-pii>{{ this.value.name }}</span>
<span class="text" data-sensitive-pii>{{ this.value.apiName }}</span>
</div>
<ht-icon
*ngIf="this.value.hasError"
Expand Down
6 changes: 3 additions & 3 deletions ...racing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.component.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('Waterfall Chart component', () => {
value: 1,
units: 'ms'
},
name: 'Span Name 1',
apiName: 'Span Name 1',
serviceName: 'Service Name 1',
protocolName: 'Protocol Name 1',
spanType: SpanType.Entry,
Expand All @@ -43,7 +43,7 @@ describe('Waterfall Chart component', () => {
value: 2,
units: 'ms'
},
name: 'Span Name 2',
apiName: 'Span Name 2',
serviceName: 'Service Name 2',
protocolName: 'Protocol Name 2',
spanType: SpanType.Exit,
Expand All @@ -64,7 +64,7 @@ describe('Waterfall Chart component', () => {
value: 2,
units: 'ms'
},
name: 'Span Name 3',
apiName: 'Span Name 3',
serviceName: 'Service Name 1',
protocolName: 'Protocol Name 3',
spanType: SpanType.Exit,
Expand Down
4 changes: 3 additions & 1 deletion ...ibuted-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.module.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { CommonModule } from '@angular/common';
import { NgModule } from '@angular/core';
import { FormattingModule } from '@hypertrace/common';
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';
Expand All @@ -13,7 +14,8 @@ import { WaterfallChartComponent } from './waterfall-chart.component';
TableModule.withCellParsers([SpanNameTableCellParser]),
TableModule.withCellRenderers([SpanNameTableCellRendererComponent]),
TooltipModule,
IconModule
IconModule,
FormattingModule
],
exports: [WaterfallChartComponent]
})
Expand Down
3 changes: 1 addition & 2 deletions ...buted-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export class WaterfallChartService {
children: []
},
$$spanName: {
name: datum.name,
apiName: datum.apiName,
serviceName: datum.serviceName,
protocolName: datum.protocolName,
hasError: datum.errorCount > 0
Expand Down Expand Up @@ -88,7 +88,6 @@ export class WaterfallChartService {
id: node.id,
start: node.startTime,
end: node.endTime,
label: node.name,
color: node.color!
});

Expand Down
4 changes: 2 additions & 2 deletions ...s/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ export interface WaterfallData {
value: number;
units?: string;
};
name: string;
serviceName: string;
protocolName: string;
protocolName?: string;
apiName?: string;
spanType: SpanType;
requestHeaders?: Dictionary<unknown>;
requestBody?: string;
Expand Down
4 changes: 2 additions & 2 deletions ...src/shared/dashboard/data/graphql/waterfall/api-trace-waterfall-data-source.model.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ describe('Api Trace Waterfall data source model', () => {
units: 'ms'
},
serviceName: 'Entity Name 1',
name: 'Span Name 1',
apiName: 'Span Name 1',
protocolName: 'Protocol Name 1',
spanType: SpanType.Entry,
tags: {}
Expand All @@ -240,7 +240,7 @@ describe('Api Trace Waterfall data source model', () => {
units: 'ms'
},
serviceName: 'Entity Name 2',
name: 'Span Name 2',
apiName: 'Span Name 2',
protocolName: 'Protocol Name 2',
spanType: SpanType.Exit,
tags: {}
Expand Down
2 changes: 1 addition & 1 deletion ...lity/src/shared/dashboard/data/graphql/waterfall/api-trace-waterfall-data-source.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ export class ApiTraceWaterfallDataSourceModel extends GraphQlDataSourceModel<Wat
units: duration.units
},
serviceName: span.displayEntityName as string,
name: span.displaySpanName as string,
protocolName: span.protocolName as string,
apiName: span.displaySpanName as string,
spanType: span.type as SpanType,
tags: span.spanTags as Dictionary<unknown>,
errorCount: span.errorCount as number
Expand Down