From 3b1bb34ba1b391880006fc355b74272d8b000dc0 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Thu, 3 Jun 2021 18:55:26 +0530 Subject: [PATCH 01/10] feat: log marker indicator in the waterfall view --- projects/components/src/popover/popover.ts | 3 +- projects/components/src/public-api.ts | 2 +- .../renderer/sequence-bar-renderer.service.ts | 79 +++++++++++++- .../sequence/sequence-chart.component.scss | 4 + .../sequence/sequence-chart.component.test.ts | 67 ++++++++++-- .../src/sequence/sequence-chart.component.ts | 17 ++- .../src/sequence/sequence-chart.service.ts | 3 + projects/components/src/sequence/sequence.ts | 14 +++ .../src/tabs/content/tab-group.component.ts | 55 +++++++++- .../components/span-detail/span-detail-tab.ts | 7 ++ .../span-detail/span-detail.component.ts | 16 ++- .../waterfall-widget-renderer.component.ts | 15 ++- .../marker-tooltip.component.scss | 60 ++++++++++ .../marker-tooltip.component.test.ts | 68 ++++++++++++ .../marker-tooltip.component.ts | 56 ++++++++++ .../marker-tooltip/marker-tooltip.module.ts | 12 ++ .../waterfall/waterfall-chart.component.ts | 103 +++++++++++++++++- .../waterfall/waterfall-chart.module.ts | 4 +- .../waterfall/waterfall-chart.service.ts | 16 ++- 19 files changed, 560 insertions(+), 41 deletions(-) create mode 100644 projects/distributed-tracing/src/shared/components/span-detail/span-detail-tab.ts create mode 100644 projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.scss create mode 100644 projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.test.ts create mode 100644 projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.ts create mode 100644 projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.module.ts diff --git a/projects/components/src/popover/popover.ts b/projects/components/src/popover/popover.ts index 4c6c4e944..755f7a3de 100644 --- a/projects/components/src/popover/popover.ts +++ b/projects/components/src/popover/popover.ts @@ -1,4 +1,5 @@ import { ElementRef, InjectionToken, Injector, TemplateRef, Type } from '@angular/core'; +import { Point } from '@hypertrace/common'; export interface PopoverOptions { componentOrTemplate: Type | TemplateRef; @@ -27,7 +28,7 @@ export interface PopoverHiddenPosition { export interface PopoverRelativePosition { type: PopoverPositionType.Relative; - origin: ElementRef; + origin: ElementRef | Element | Point; locationPreferences: PopoverRelativePositionLocation[]; } diff --git a/projects/components/src/public-api.ts b/projects/components/src/public-api.ts index 280fcb179..38e153b9c 100644 --- a/projects/components/src/public-api.ts +++ b/projects/components/src/public-api.ts @@ -238,7 +238,7 @@ export * from './select/select.component'; export * from './select/select.module'; // Sequence -export { SequenceSegment } from './sequence/sequence'; +export { Marker, MarkerDatum, SequenceSegment } from './sequence/sequence'; export * from './sequence/sequence-chart.component'; export * from './sequence/sequence-chart.module'; diff --git a/projects/components/src/sequence/renderer/sequence-bar-renderer.service.ts b/projects/components/src/sequence/renderer/sequence-bar-renderer.service.ts index 44b75d2be..937398fe5 100644 --- a/projects/components/src/sequence/renderer/sequence-bar-renderer.service.ts +++ b/projects/components/src/sequence/renderer/sequence-bar-renderer.service.ts @@ -2,7 +2,7 @@ import { Injectable } from '@angular/core'; import { ScaleLinear } from 'd3-scale'; import { BaseType, select, Selection } from 'd3-selection'; import { SequenceChartAxisService } from '../axis/sequence-chart-axis.service'; -import { SequenceLayoutStyleClass, SequenceOptions, SequenceSegment, SequenceSVGSelection } from '../sequence'; +import { Marker, SequenceLayoutStyleClass, SequenceOptions, SequenceSegment, SequenceSVGSelection } from '../sequence'; import { SequenceObject } from '../sequence-object'; @Injectable() @@ -10,10 +10,14 @@ export class SequenceBarRendererService { private static readonly DATA_ROW_CLASS: string = 'data-row'; private static readonly HOVERED_ROW_CLASS: string = 'hovered-row'; private static readonly SELECTED_ROW_CLASS: string = 'selected-row'; + private static readonly MARKER_CLASS: string = 'marker'; + private static readonly MARKERS_CLASS: string = 'markers'; private static readonly BACKDROP_CLASS: string = 'backdrop'; private static readonly BACKDROP_BORDER_TOP_CLASS: string = 'backdrop-border-top'; private static readonly BACKDROP_BORDER_BOTTOM_CLASS: string = 'backdrop-border-bottom'; + private readonly markerWidth: number = 2; + public constructor(private readonly sequenceChartAxisService: SequenceChartAxisService) {} public drawBars(chartSelection: SequenceSVGSelection, options: SequenceOptions): void { @@ -39,9 +43,11 @@ export class SequenceBarRendererService { this.drawBackdropRect(transformedBars, options, plotWidth); this.drawBarValueRect(transformedBars, xScale, options); + this.drawBarMarkers(plotSelection, xScale, options); this.drawBarValueText(transformedBars, xScale, options); this.setupHoverListener(transformedBars, options); this.setupClickListener(transformedBars, options); + this.setupMarkerHoverListener(plotSelection, options); this.updateDataRowHover(chartSelection, options); this.updateDataRowSelection(chartSelection, options); } @@ -108,6 +114,34 @@ export class SequenceBarRendererService { .classed('bar-value', true); } + private drawBarMarkers( + plotSelection: SequenceSVGSelection, + xScale: ScaleLinear, + options: SequenceOptions + ): void { + let index = 0; + for (const segment of options.data) { + plotSelection + .selectAll(`g.${SequenceBarRendererService.MARKERS_CLASS}`) + .data(this.getGroupedMarkers(segment, xScale)) + .enter() + .append('g') + .classed(`${SequenceBarRendererService.MARKER_CLASS}`, true) + .append('rect') + .attr( + 'transform', + dataRow => + `translate(${dataRow.markerTime},${ + (options.rowHeight - options.barHeight) / 2 + options.rowHeight * index + })` + ) + .attr('width', this.markerWidth) + .attr('height', 12) + .style('fill', 'white'); + index++; + } + } + private drawBarValueText( transformedBars: TransformedBarSelection, xScale: ScaleLinear, @@ -175,6 +209,49 @@ export class SequenceBarRendererService { options.selected = options.selected === dataRow ? undefined : dataRow; options.onSegmentSelected(dataRow); } + + private setupMarkerHoverListener(plotSelection: SequenceSVGSelection, options: SequenceOptions): void { + plotSelection + .selectAll(`g.${SequenceBarRendererService.MARKER_CLASS}`) + .on('mouseenter', (dataRow, index, nodes) => { + options.onMarkerHovered({ marker: dataRow, origin: nodes[index].getBoundingClientRect() }); + }); + } + + private getGroupedMarkers(segment: SequenceSegment, xScale: ScaleLinear): Marker[] { + const scaledStart: number = Math.floor(xScale(segment.start)!); + const scaledEnd: number = Math.floor(xScale(segment.end)!); + const pixelScaledMarkers: Marker[] = segment.markers.map((marker: Marker) => ({ + ...marker, + markerTime: Math.floor(xScale(marker.markerTime)!) + })); + const scaledNormalizedMarkers: Marker[] = []; + let markerTime = -1 * Infinity; + let index = -1; + pixelScaledMarkers.forEach((marker: Marker) => { + // For 1px gap + if (marker.markerTime >= markerTime + this.markerWidth + 1) { + index++; + scaledNormalizedMarkers.push({ + ...marker, + markerTime: + marker.markerTime <= scaledStart + this.markerWidth // Grouping - closest to start + ? scaledStart + this.markerWidth + 1 + : marker.markerTime >= scaledEnd - this.markerWidth // Grouping - closest to end + ? scaledEnd - this.markerWidth - 2 + : marker.markerTime + }); + markerTime = scaledNormalizedMarkers[index].markerTime; + } else { + scaledNormalizedMarkers[index] = { + ...scaledNormalizedMarkers[index], + timestamps: [...scaledNormalizedMarkers[index].timestamps, ...marker.timestamps] + }; + } + }); + + return scaledNormalizedMarkers; + } } type TransformedBarSelection = Selection; diff --git a/projects/components/src/sequence/sequence-chart.component.scss b/projects/components/src/sequence/sequence-chart.component.scss index 17248925c..3a18d69aa 100644 --- a/projects/components/src/sequence/sequence-chart.component.scss +++ b/projects/components/src/sequence/sequence-chart.component.scss @@ -54,6 +54,10 @@ } } + .marker { + cursor: pointer; + } + .hovered-row { .backdrop { fill-opacity: 100; diff --git a/projects/components/src/sequence/sequence-chart.component.test.ts b/projects/components/src/sequence/sequence-chart.component.test.ts index 9665b7370..1d2c35fc5 100644 --- a/projects/components/src/sequence/sequence-chart.component.test.ts +++ b/projects/components/src/sequence/sequence-chart.component.test.ts @@ -24,28 +24,32 @@ describe('Sequence Chart component', () => { start: 1569357460346, end: 1569357465346, label: 'Segment 1', - color: 'blue' + color: 'blue', + markers: [] }, { id: '2', start: 1569357461346, end: 1569357465346, label: 'Segment 2', - color: 'green' + color: 'green', + markers: [] }, { id: '3', start: 1569357462346, end: 1569357465346, label: 'Segment 3', - color: 'green' + color: 'green', + markers: [] }, { id: '4', start: 1569357463346, end: 1569357465346, label: 'Segment 4', - color: 'green' + color: 'green', + markers: [] } ]; const chart = createHost(``, { @@ -91,7 +95,8 @@ describe('Sequence Chart component', () => { start: 1569357460346, end: 1569357465346, label: 'Segment 1', - color: 'blue' + color: 'blue', + markers: [] } ]; const chart = createHost(``, { @@ -117,7 +122,8 @@ describe('Sequence Chart component', () => { start: 1569357460346, end: 1569357465346, label: 'Segment 1', - color: 'blue' + color: 'blue', + markers: [] } ]; const chart = createHost(``, { @@ -132,6 +138,37 @@ describe('Sequence Chart component', () => { expect(dataRow!.getAttribute('height')).toEqual('50'); }); + test('should render marker with correct width and height', () => { + const segmentsData = [ + { + id: '1', + start: 1569357460346, + end: 1569357465346, + label: 'Segment 1', + color: 'blue', + markers: [ + { + nodeId: '1', + markerTime: 1569357460347, + timestamps: ['1569357460347'] + } + ] + } + ]; + const chart = createHost(``, { + hostProps: { + data: segmentsData, + rowHeight: 50 + } + }); + + const markers = chart.queryAll('.marker', { root: true }); + expect(markers.length).toBe(1); + const markerRect = select(markers[0].querySelector('rect')); + expect(markerRect.attr('width')).toBe('2'); + expect(markerRect.attr('height')).toBe('12'); + }); + test('should render with correct bar height', () => { const segmentsData = [ { @@ -139,7 +176,8 @@ describe('Sequence Chart component', () => { start: 1569357460346, end: 1569357465346, label: 'Segment 1', - color: 'blue' + color: 'blue', + markers: [] } ]; const chart = createHost(``, { @@ -164,7 +202,8 @@ describe('Sequence Chart component', () => { start: 1569357460346, end: 1569357465346, label: 'Segment 1', - color: 'blue' + color: 'blue', + markers: [] } ]; const chart = createHost(``, { @@ -186,14 +225,16 @@ describe('Sequence Chart component', () => { start: 1569357460346, end: 1569357465346, label: 'Segment 1', - color: 'blue' + color: 'blue', + markers: [] }, { id: '2', start: 1569357460346, end: 1569357465346, label: 'Segment 2', - color: 'green' + color: 'green', + markers: [] } ]; const chart = createHost(``, { @@ -230,14 +271,16 @@ describe('Sequence Chart component', () => { start: 1569357460346, end: 1569357465346, label: 'Segment 1', - color: 'blue' + color: 'blue', + markers: [] }, { id: '2', start: 1569357460346, end: 1569357465346, label: 'Segment 2', - color: 'green' + color: 'green', + markers: [] } ]; const chart = createHost(``, { diff --git a/projects/components/src/sequence/sequence-chart.component.ts b/projects/components/src/sequence/sequence-chart.component.ts index aefda7e57..4b0e4a071 100644 --- a/projects/components/src/sequence/sequence-chart.component.ts +++ b/projects/components/src/sequence/sequence-chart.component.ts @@ -10,7 +10,7 @@ import { } from '@angular/core'; import { RecursivePartial, TypedSimpleChanges } from '@hypertrace/common'; import { minBy } from 'lodash-es'; -import { SequenceOptions, SequenceSegment } from './sequence'; +import { Marker, MarkerDatum, SequenceOptions, SequenceSegment } from './sequence'; import { SequenceChartService } from './sequence-chart.service'; import { SequenceObject } from './sequence-object'; @@ -52,6 +52,9 @@ export class SequenceChartComponent implements OnChanges { SequenceSegment | undefined >(); + @Output() + public readonly markerHoveredChange: EventEmitter = new EventEmitter(); + @ViewChild('chartContainer', { static: true }) private readonly chartContainer!: ElementRef; @@ -100,7 +103,8 @@ export class SequenceChartComponent implements OnChanges { barHeight: this.barHeight, unit: this.unit, onSegmentSelected: (segment?: SequenceSegment) => this.onSegmentSelected(segment), - onSegmentHovered: (segment?: SequenceSegment) => this.onSegmentHovered(segment) + onSegmentHovered: (segment?: SequenceSegment) => this.onSegmentHovered(segment), + onMarkerHovered: (datum?: MarkerDatum) => this.onMarkerHovered(datum) }; } @@ -115,7 +119,10 @@ export class SequenceChartComponent implements OnChanges { id: segment.id, start: segment.start - minStart, end: segment.end - minStart, - color: segment.color + color: segment.color, + markers: segment.markers + .map((marker: Marker) => ({ ...marker, markerTime: marker.markerTime - minStart })) + .sort((marker1, marker2) => marker1.markerTime - marker2.markerTime) })); } @@ -128,4 +135,8 @@ export class SequenceChartComponent implements OnChanges { this.hovered = segment; this.hoveredChange.emit(segment); } + + private onMarkerHovered(datum?: MarkerDatum): void { + this.markerHoveredChange.emit(datum); + } } diff --git a/projects/components/src/sequence/sequence-chart.service.ts b/projects/components/src/sequence/sequence-chart.service.ts index e69b6cca0..9a5360106 100644 --- a/projects/components/src/sequence/sequence-chart.service.ts +++ b/projects/components/src/sequence/sequence-chart.service.ts @@ -101,6 +101,9 @@ export class SequenceChartService { }, onSegmentHovered: () => { /** NOOP */ + }, + onMarkerHovered: () => { + /** NOOP */ } }; } diff --git a/projects/components/src/sequence/sequence.ts b/projects/components/src/sequence/sequence.ts index 3495d58d3..db0e9ba1c 100644 --- a/projects/components/src/sequence/sequence.ts +++ b/projects/components/src/sequence/sequence.ts @@ -1,3 +1,4 @@ +import { Point } from '@hypertrace/common'; import { Selection } from 'd3-selection'; import { SequenceObject } from './sequence-object'; @@ -6,6 +7,18 @@ export interface SequenceSegment { start: number; end: number; color: string; + markers: Marker[]; +} + +export interface Marker { + nodeId: string; + markerTime: number; + timestamps: string[]; +} + +export interface MarkerDatum { + marker: Marker; + origin: Point; } /* Internal Types */ @@ -20,6 +33,7 @@ export interface SequenceOptions { unit: string | undefined; onSegmentSelected(row?: SequenceSegment): void; onSegmentHovered(row?: SequenceSegment): void; + onMarkerHovered(datum?: MarkerDatum): void; } export interface MarginOptions { diff --git a/projects/components/src/tabs/content/tab-group.component.ts b/projects/components/src/tabs/content/tab-group.component.ts index 20aa560aa..41a5f8106 100644 --- a/projects/components/src/tabs/content/tab-group.component.ts +++ b/projects/components/src/tabs/content/tab-group.component.ts @@ -1,4 +1,15 @@ -import { ChangeDetectionStrategy, Component, ContentChildren, QueryList } from '@angular/core'; +import { + AfterViewInit, + ChangeDetectionStrategy, + Component, + ContentChildren, + EventEmitter, + Input, + OnChanges, + Output, + QueryList +} from '@angular/core'; +import { isEmpty } from 'lodash-es'; import { TabComponent } from './tab/tab.component'; @Component({ @@ -7,16 +18,21 @@ import { TabComponent } from './tab/tab.component'; changeDetection: ChangeDetectionStrategy.OnPush, template: `
- +
{{ tab.label }} -
+
{{ tab.badge }}
-
+
@@ -26,9 +42,38 @@ import { TabComponent } from './tab/tab.component';
` }) -export class TabGroupComponent { +export class TabGroupComponent implements OnChanges, AfterViewInit { @ContentChildren(TabComponent) public tabs!: QueryList; + @Input() + public activeLabel?: string; + + @Output() + public readonly activeLabelChange: EventEmitter = new EventEmitter(); + public activeTabIndex: number = 0; + + public ngOnChanges(): void { + this.setActiveTabIndexBasedOnLabel(); + } + + public ngAfterViewInit(): void { + this.setActiveTabIndexBasedOnLabel(); + } + + public onSelectedTabChange(index: number): void { + this.activeTabIndex = index; + this.activeLabelChange.emit(this.tabs?.get(index)?.label); + } + + private setActiveTabIndexBasedOnLabel(): void { + if (!isEmpty(this.tabs) && !isEmpty(this.activeLabel)) { + this.tabs.forEach((tab: TabComponent, index: number) => { + if (tab.label === this.activeLabel) { + this.activeTabIndex = index; + } + }); + } + } } diff --git a/projects/distributed-tracing/src/shared/components/span-detail/span-detail-tab.ts b/projects/distributed-tracing/src/shared/components/span-detail/span-detail-tab.ts new file mode 100644 index 000000000..0a0bb6b83 --- /dev/null +++ b/projects/distributed-tracing/src/shared/components/span-detail/span-detail-tab.ts @@ -0,0 +1,7 @@ +export enum SpanDetailTab { + Request = 'Request', + Response = 'Response', + Attributes = 'Attributes', + ExitCalls = 'Exit Calls', + Logs = 'Logs' +} diff --git a/projects/distributed-tracing/src/shared/components/span-detail/span-detail.component.ts b/projects/distributed-tracing/src/shared/components/span-detail/span-detail.component.ts index 9f54a2901..aa01afe40 100644 --- a/projects/distributed-tracing/src/shared/components/span-detail/span-detail.component.ts +++ b/projects/distributed-tracing/src/shared/components/span-detail/span-detail.component.ts @@ -3,6 +3,7 @@ import { TypedSimpleChanges } from '@hypertrace/common'; import { isEmpty } from 'lodash-es'; import { SpanData } from './span-data'; import { SpanDetailLayoutStyle } from './span-detail-layout-style'; +import { SpanDetailTab } from './span-detail-tab'; @Component({ selector: 'ht-span-detail', @@ -23,8 +24,8 @@ import { SpanDetailLayoutStyle } from './span-detail-layout-style';
- - + + - + - + - + - + = new EventEmitter(); diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall-widget-renderer.component.ts b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall-widget-renderer.component.ts index 3b6a27395..746aa9f7a 100644 --- a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall-widget-renderer.component.ts +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall-widget-renderer.component.ts @@ -12,11 +12,13 @@ import { ButtonStyle, OverlayService, SheetRef, SheetSize } from '@hypertrace/co import { WidgetRenderer } from '@hypertrace/dashboards'; import { Renderer } from '@hypertrace/hyperdash'; import { RendererApi, RENDERER_API } from '@hypertrace/hyperdash-angular'; +import { isEmpty } from 'lodash-es'; import { Observable } from 'rxjs'; import { SpanDetailLayoutStyle } from '../../../components/span-detail/span-detail-layout-style'; +import { SpanDetailTab } from '../../../components/span-detail/span-detail-tab'; import { WaterfallWidgetModel } from './waterfall-widget.model'; import { WaterfallData } from './waterfall/waterfall-chart'; -import { WaterfallChartComponent } from './waterfall/waterfall-chart.component'; +import { MarkerSelection, WaterfallChartComponent } from './waterfall/waterfall-chart.component'; @Renderer({ modelClass: WaterfallWidgetModel }) @Component({ @@ -50,6 +52,7 @@ import { WaterfallChartComponent } from './waterfall/waterfall-chart.component'; class="waterfall-widget" [data]="data" (selectionChange)="this.onTableRowSelection($event)" + (markerSelection)="this.onMarkerSelection($event)" > @@ -61,6 +64,7 @@ import { WaterfallChartComponent } from './waterfall/waterfall-chart.component'; [spanData]="this.selectedData" [showTitleHeader]="true" layout="${SpanDetailLayoutStyle.Vertical}" + [activeLabel]="this.activeLabel" (closed)="this.closeSheet()" > , @@ -95,13 +100,19 @@ export class WaterfallWidgetRendererComponent } public onTableRowSelection(selectedData: WaterfallData): void { - if (selectedData === this.selectedData) { + this.activeLabel = undefined; + if (selectedData === this.selectedData || isEmpty(selectedData)) { this.closeSheet(); } else { this.openSheet(selectedData); } } + public onMarkerSelection(selectedMarker: MarkerSelection): void { + this.activeLabel = SpanDetailTab.Logs; + this.openSheet(selectedMarker.selectedData!); + } + public onExpandAll(): void { this.waterfallChart.onExpandAll(); } diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.scss b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.scss new file mode 100644 index 000000000..887ffa5d7 --- /dev/null +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.scss @@ -0,0 +1,60 @@ +@import 'color-palette'; +@import 'font'; + +.marker-tooltip-container { + @include body-small(white); + background-color: $gray-9; + box-shadow: 0px 4px 5px rgba(0, 0, 0, 0.2), 0px 3px 14px rgba(0, 0, 0, 0.12), 0px 8px 10px rgba(0, 0, 0, 0.14); + border-radius: 4px; + padding: 8px 12px; + margin: 8px; + width: 320px; + max-height: 172px; + + .tooltip-header { + display: flex; + align-items: center; + justify-content: space-between; + + .log-count { + display: flex; + align-items: center; + + .count { + width: 18px; + height: 18px; + display: flex; + align-items: center; + justify-content: center; + background-color: $gray-5; + margin-left: 4px; + border-radius: 4px; + } + } + } + + .divider { + margin: 4px 0; + border: 1px solid $gray-6; + } + + .attribute { + display: flex; + align-items: center; + @include ellipsis-overflow(); + + .key { + font-weight: 600; + margin-right: 4px; + } + } + + .view-all-text { + cursor: pointer; + width: max-content; + + &:hover { + color: $gray-2; + } + } +} diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.test.ts b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.test.ts new file mode 100644 index 000000000..b216e29f1 --- /dev/null +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.test.ts @@ -0,0 +1,68 @@ +import { RouterTestingModule } from '@angular/router/testing'; +import { createHostFactory, Spectator } from '@ngneat/spectator/jest'; +import { Observable, of } from 'rxjs'; +import { MarkerTooltipComponent, MarkerTooltipData } from './marker-tooltip.component'; +import { MarkerTooltipModule } from './marker-tooltip.module'; + +describe('Marker Tooltip Component', () => { + let spectator: Spectator; + + const createHost = createHostFactory({ + component: MarkerTooltipComponent, + imports: [MarkerTooltipModule, RouterTestingModule], + shallow: true + }); + + test('should have log attributes without view all', () => { + const data: Observable = of({ + relativeTimes: [2], + attributes: [ + ['id', 'test-id'], + ['error', 'test-error'] + ] + }); + spectator = createHost(``, { + hostProps: { + data: data + } + }); + + expect(spectator.query('.marker-tooltip-container')).toExist(); + expect(spectator.query('.count')).toHaveText('1'); + expect(spectator.query('.time-range')).toHaveText('2ms'); + expect(spectator.query('.divider')).toExist(); + expect(spectator.queryAll('.key')[0]).toHaveText('id'); + expect(spectator.queryAll('.key')[1]).toHaveText('error'); + expect(spectator.queryAll('.value')[0]).toHaveText('test-id'); + expect(spectator.queryAll('.value')[1]).toHaveText('test-error'); + expect(spectator.query('.view-all')).not.toExist(); + }); + + test('should have view all', () => { + const data: Observable = of({ + relativeTimes: [2, 3], + attributes: [ + ['id', 'test-id'], + ['error', 'test-error'], + ['name', 'test-name'], + ['class', 'test-class'], + ['reason', 'test-reason'], + ['summary', 'test-summary'] + ] + }); + spectator = createHost(``, { + hostProps: { + data: data, + viewAll: { + emit: jest.fn() + } + } + }); + + expect(spectator.query('.time-range')).toHaveText('2ms - 3ms'); + expect(spectator.query('.view-all')).toExist(); + const viewAllTextElement = spectator.query('.view-all-text'); + expect(viewAllTextElement).toExist(); + spectator.click(viewAllTextElement!); + }); +}); diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.ts b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.ts new file mode 100644 index 000000000..4eb7c6c1e --- /dev/null +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.ts @@ -0,0 +1,56 @@ +import { ChangeDetectionStrategy, Component, EventEmitter, Input, Output } from '@angular/core'; +import { Observable } from 'rxjs'; + +@Component({ + selector: 'ht-marker-tooltip', + styleUrls: ['./marker-tooltip.component.scss'], + changeDetection: ChangeDetectionStrategy.OnPush, + template: ` +
+
+
+ Logs +
{{ data.relativeTimes.length }}
+
+
+ {{ data.relativeTimes[0] }}ms + {{ data.relativeTimes[0] }}ms - {{ data.relativeTimes[data.relativeTimes.length - 1] }}ms +
+
+
+
+
{{ attribute[0] }}:
+
{{ attribute[1] }}
+
+
+
...
+
View all >
+
+
+ ` +}) +export class MarkerTooltipComponent { + public static readonly MAX_ATTRIBUTES_TO_DISPLAY: number = 5; + + @Input() + public data?: Observable; + + @Output() + public readonly viewAll: EventEmitter = new EventEmitter(); +} + +export interface MarkerTooltipData { + relativeTimes: number[]; + attributes: [string, unknown][]; +} diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.module.ts b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.module.ts new file mode 100644 index 000000000..a30ad098c --- /dev/null +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.module.ts @@ -0,0 +1,12 @@ +import { CommonModule } from '@angular/common'; +import { NgModule } from '@angular/core'; +import { FormattingModule } from '@hypertrace/common'; +import { LabelModule, PopoverModule } from '@hypertrace/components'; +import { MarkerTooltipComponent } from './marker-tooltip.component'; + +@NgModule({ + declarations: [MarkerTooltipComponent], + exports: [MarkerTooltipComponent], + imports: [PopoverModule, CommonModule, FormattingModule, LabelModule] +}) +export class MarkerTooltipModule {} diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.component.ts b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.component.ts index ffe8ad722..ff7f5ebbe 100644 --- a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.component.ts +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.component.ts @@ -1,6 +1,23 @@ -import { ChangeDetectionStrategy, Component, EventEmitter, Input, OnChanges, Output, ViewChild } from '@angular/core'; +import { + ChangeDetectionStrategy, + Component, + EventEmitter, + Input, + OnChanges, + Output, + TemplateRef, + ViewChild +} from '@angular/core'; +import { DateCoercer, TypedSimpleChanges } from '@hypertrace/common'; import { CoreTableCellRendererType, + Marker, + MarkerDatum, + PopoverBackdrop, + PopoverPositionType, + PopoverRef, + PopoverRelativePositionLocation, + PopoverService, SequenceSegment, StatefulTableRow, TableColumnConfig, @@ -9,11 +26,10 @@ import { TableMode, TableStyle } from '@hypertrace/components'; -import { of } from 'rxjs'; - -import { TypedSimpleChanges } from '@hypertrace/common'; +import { Observable, of } from 'rxjs'; +import { MarkerTooltipData } from './marker-tooltip/marker-tooltip.component'; import { WaterfallTableCellType } from './span-name/span-name-cell-type'; -import { WaterfallData, WaterfallDataNode } from './waterfall-chart'; +import { LogEvent, WaterfallData, WaterfallDataNode } from './waterfall-chart'; import { WaterfallChartService } from './waterfall-chart.service'; @Component({ @@ -49,9 +65,13 @@ import { WaterfallChartService } from './waterfall-chart.service'; (selectionChange)="this.onSelection($event ? [$event] : [])" [hovered]="this.hoveredNode" (hoveredChange)="this.onHover($event)" + (markerHoveredChange)="this.onMarkerHover($event)" > + + + ` }) @@ -62,9 +82,15 @@ export class WaterfallChartComponent implements OnChanges { @Output() public readonly selectionChange: EventEmitter = new EventEmitter(); + @Output() + public readonly markerSelection: EventEmitter = new EventEmitter(); + @ViewChild('table') private readonly table!: TableComponent; + @ViewChild('markerTooltipTemplate') + private readonly markerTooltipTemplate!: TemplateRef; + public datasource?: TableDataSource; public readonly columnDefs: TableColumnConfig[] = [ { @@ -91,7 +117,15 @@ export class WaterfallChartComponent implements OnChanges { public selectedNode?: WaterfallDataNode; public hoveredNode?: WaterfallDataNode; - public constructor(private readonly waterfallChartService: WaterfallChartService) {} + public markerTooltipData?: Observable; + private marker?: Marker; + private popover?: PopoverRef; + private readonly dateCoercer: DateCoercer = new DateCoercer(); + + public constructor( + private readonly waterfallChartService: WaterfallChartService, + private readonly popoverService: PopoverService + ) {} public ngOnChanges(changes: TypedSimpleChanges): void { if (changes.data && this.data) { @@ -131,6 +165,58 @@ export class WaterfallChartComponent implements OnChanges { this.hoveredNode = datum?.id !== undefined ? this.dataNodeMap.get(datum.id) : undefined; } + public viewAll(): void { + this.markerSelection.emit({ + selectedData: this.marker?.nodeId !== undefined ? this.dataNodeMap.get(this.marker?.nodeId) : undefined, + timestamps: this.marker?.timestamps ?? [] + }); + this.closePopover(); + } + + public onMarkerHover(datum?: MarkerDatum): void { + if (datum && datum.marker && datum.origin) { + this.closePopover(); + this.marker = datum.marker; + this.markerTooltipData = this.buildMarkerDataSource(datum.marker); + this.popover = this.popoverService.drawPopover({ + componentOrTemplate: this.markerTooltipTemplate, + data: this.markerTooltipTemplate, + position: { + type: PopoverPositionType.Relative, + origin: datum.origin, + locationPreferences: [PopoverRelativePositionLocation.AboveRightAligned] + }, + backdrop: PopoverBackdrop.Transparent + }); + this.popover.closeOnBackdropClick(); + } + } + + private closePopover(): void { + this.popover?.close(); + this.popover = undefined; + } + + private buildMarkerDataSource(marker: Marker): Observable { + const spanWaterfallData: WaterfallDataNode = this.dataNodeMap.get(marker.nodeId)!; + + let markerData: MarkerTooltipData = { + relativeTimes: [], + attributes: [] + }; + spanWaterfallData.logEvents.forEach((logEvent: LogEvent) => { + if (marker.timestamps.includes(logEvent.timestamp)) { + const logEventTime = this.dateCoercer.coerce(logEvent.timestamp)!.getTime(); + markerData = { + relativeTimes: [...markerData.relativeTimes, logEventTime - spanWaterfallData.startTime], + attributes: [...markerData.attributes, ...Object.entries(logEvent.attributes)] + }; + } + }); + + return of(markerData); + } + private buildDatasource(sequenceData: WaterfallData[]): TableDataSource { const rootLevelRows = this.buildAndCollectSequenceRootNodes(sequenceData); @@ -194,3 +280,8 @@ export class WaterfallChartComponent implements OnChanges { return dataNode.$$state.children.every(child => child.$$state.expanded); } } + +export interface MarkerSelection { + selectedData: WaterfallDataNode | undefined; + timestamps: string[]; +} diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.module.ts b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.module.ts index aff1f69e6..d4134a661 100644 --- a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.module.ts +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.module.ts @@ -2,6 +2,7 @@ import { CommonModule } from '@angular/common'; import { NgModule } from '@angular/core'; import { FormattingModule } from '@hypertrace/common'; import { IconModule, SequenceChartModule, TableModule, TooltipModule } from '@hypertrace/components'; +import { MarkerTooltipModule } from './marker-tooltip/marker-tooltip.module'; 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'; @@ -15,7 +16,8 @@ import { WaterfallChartComponent } from './waterfall-chart.component'; TableModule.withCellRenderers([SpanNameTableCellRendererComponent]), TooltipModule, IconModule, - FormattingModule + FormattingModule, + MarkerTooltipModule ], exports: [WaterfallChartComponent] }) diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.service.ts b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.service.ts index e98e18f20..33561f580 100644 --- a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.service.ts +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.service.ts @@ -1,16 +1,17 @@ import { Injectable } from '@angular/core'; -import { Color, ColorService } from '@hypertrace/common'; +import { Color, ColorService, DateCoercer } from '@hypertrace/common'; import { SequenceSegment } from '@hypertrace/components'; import { isNil, sortBy } from 'lodash-es'; import { of } from 'rxjs'; import { TracingIconLookupService } from '../../../../services/icon-lookup/tracing-icon-lookup.service'; -import { WaterfallData, WaterfallDataNode } from './waterfall-chart'; +import { LogEvent, WaterfallData, WaterfallDataNode } from './waterfall-chart'; @Injectable({ providedIn: 'root' }) export class WaterfallChartService { private static readonly SEQUENCE_COLORS: symbol = Symbol('Sequence colors'); + private readonly dateCoercer: DateCoercer = new DateCoercer(); public constructor( private readonly colorService: ColorService, private readonly iconLookupService: TracingIconLookupService @@ -89,7 +90,16 @@ export class WaterfallChartService { id: node.id, start: node.startTime, end: node.endTime, - color: node.color! + color: node.color!, + markers: node.logEvents.map((logEvent: LogEvent) => { + const timeInMs = this.dateCoercer.coerce(logEvent.timestamp)!.getTime(); + + return { + nodeId: node.id, + markerTime: timeInMs, + timestamps: [logEvent.timestamp] + }; + }) }); sequenceNodes.unshift(...node.$$state.children); From 1c9d3a11ae6a147077676f45fdd94aa066029360 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Mon, 7 Jun 2021 19:31:41 +0530 Subject: [PATCH 02/10] fix: addressing review comments --- .../sequence/renderer/sequence-bar-renderer.service.ts | 6 ++---- .../components/src/tabs/content/tab-group.component.ts | 10 +++++----- .../components/span-detail/span-detail.component.ts | 10 +++++----- .../waterfall/waterfall-widget-renderer.component.ts | 8 ++++---- .../marker-tooltip/marker-tooltip.component.ts | 4 ++-- 5 files changed, 18 insertions(+), 20 deletions(-) diff --git a/projects/components/src/sequence/renderer/sequence-bar-renderer.service.ts b/projects/components/src/sequence/renderer/sequence-bar-renderer.service.ts index 937398fe5..a9633f5e1 100644 --- a/projects/components/src/sequence/renderer/sequence-bar-renderer.service.ts +++ b/projects/components/src/sequence/renderer/sequence-bar-renderer.service.ts @@ -119,8 +119,7 @@ export class SequenceBarRendererService { xScale: ScaleLinear, options: SequenceOptions ): void { - let index = 0; - for (const segment of options.data) { + options.data.forEach((segment, index) => { plotSelection .selectAll(`g.${SequenceBarRendererService.MARKERS_CLASS}`) .data(this.getGroupedMarkers(segment, xScale)) @@ -138,8 +137,7 @@ export class SequenceBarRendererService { .attr('width', this.markerWidth) .attr('height', 12) .style('fill', 'white'); - index++; - } + }); } private drawBarValueText( diff --git a/projects/components/src/tabs/content/tab-group.component.ts b/projects/components/src/tabs/content/tab-group.component.ts index 41a5f8106..ab0743b28 100644 --- a/projects/components/src/tabs/content/tab-group.component.ts +++ b/projects/components/src/tabs/content/tab-group.component.ts @@ -47,10 +47,10 @@ export class TabGroupComponent implements OnChanges, AfterViewInit { public tabs!: QueryList; @Input() - public activeLabel?: string; + public activeTabLabel?: string; @Output() - public readonly activeLabelChange: EventEmitter = new EventEmitter(); + public readonly activeTabLabelChange: EventEmitter = new EventEmitter(); public activeTabIndex: number = 0; @@ -64,13 +64,13 @@ export class TabGroupComponent implements OnChanges, AfterViewInit { public onSelectedTabChange(index: number): void { this.activeTabIndex = index; - this.activeLabelChange.emit(this.tabs?.get(index)?.label); + this.activeTabLabelChange.emit(this.tabs?.get(index)?.label); } private setActiveTabIndexBasedOnLabel(): void { - if (!isEmpty(this.tabs) && !isEmpty(this.activeLabel)) { + if (!isEmpty(this.tabs) && !isEmpty(this.activeTabLabel)) { this.tabs.forEach((tab: TabComponent, index: number) => { - if (tab.label === this.activeLabel) { + if (tab.label === this.activeTabLabel) { this.activeTabIndex = index; } }); diff --git a/projects/distributed-tracing/src/shared/components/span-detail/span-detail.component.ts b/projects/distributed-tracing/src/shared/components/span-detail/span-detail.component.ts index aa01afe40..d6666b47e 100644 --- a/projects/distributed-tracing/src/shared/components/span-detail/span-detail.component.ts +++ b/projects/distributed-tracing/src/shared/components/span-detail/span-detail.component.ts @@ -24,7 +24,7 @@ import { SpanDetailTab } from './span-detail-tab'; - + - + = new EventEmitter(); @@ -76,7 +76,7 @@ export class SpanDetailComponent implements OnChanges { public showRequestTab?: boolean; public showResponseTab?: boolean; public showExitCallsTab?: boolean; - public showLogEventstab?: boolean; + public showLogEventsTab?: boolean; public totalLogEvents?: number; public ngOnChanges(changes: TypedSimpleChanges): void { @@ -84,7 +84,7 @@ export class SpanDetailComponent implements OnChanges { this.showRequestTab = !isEmpty(this.spanData?.requestHeaders) || !isEmpty(this.spanData?.requestBody); this.showResponseTab = !isEmpty(this.spanData?.responseHeaders) || !isEmpty(this.spanData?.responseBody); this.showExitCallsTab = !isEmpty(this.spanData?.exitCallsBreakup); - this.showLogEventstab = !isEmpty(this.spanData?.logEvents); + this.showLogEventsTab = !isEmpty(this.spanData?.logEvents); this.totalLogEvents = (this.spanData?.logEvents ?? []).length; } } diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall-widget-renderer.component.ts b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall-widget-renderer.component.ts index 746aa9f7a..f8f7ed219 100644 --- a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall-widget-renderer.component.ts +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall-widget-renderer.component.ts @@ -64,7 +64,7 @@ import { MarkerSelection, WaterfallChartComponent } from './waterfall/waterfall- [spanData]="this.selectedData" [showTitleHeader]="true" layout="${SpanDetailLayoutStyle.Vertical}" - [activeLabel]="this.activeLabel" + [activeTabLabel]="this.activeTabLabel" (closed)="this.closeSheet()" > , @@ -100,7 +100,7 @@ export class WaterfallWidgetRendererComponent } public onTableRowSelection(selectedData: WaterfallData): void { - this.activeLabel = undefined; + this.activeTabLabel = undefined; if (selectedData === this.selectedData || isEmpty(selectedData)) { this.closeSheet(); } else { @@ -109,7 +109,7 @@ export class WaterfallWidgetRendererComponent } public onMarkerSelection(selectedMarker: MarkerSelection): void { - this.activeLabel = SpanDetailTab.Logs; + this.activeTabLabel = SpanDetailTab.Logs; this.openSheet(selectedMarker.selectedData!); } diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.ts b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.ts index 4eb7c6c1e..e7cbd6edf 100644 --- a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.ts +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.ts @@ -35,7 +35,7 @@ import { Observable } from 'rxjs'; *ngIf="!!data.attributes && data.attributes.length > ${MarkerTooltipComponent.MAX_ATTRIBUTES_TO_DISPLAY}" >
...
-
View all >
+
View all >
` @@ -47,7 +47,7 @@ export class MarkerTooltipComponent { public data?: Observable; @Output() - public readonly viewAll: EventEmitter = new EventEmitter(); + public readonly viewAll: EventEmitter = new EventEmitter(); } export interface MarkerTooltipData { From b1f9976650ddda73e39190b131a1d3f40bf8da2e Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Mon, 7 Jun 2021 21:22:01 +0530 Subject: [PATCH 03/10] fix: addressing review comments --- .../marker-tooltip/marker-tooltip.component.scss | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.scss b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.scss index 887ffa5d7..fa38d8248 100644 --- a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.scss +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.scss @@ -39,13 +39,17 @@ } .attribute { - display: flex; - align-items: center; - @include ellipsis-overflow(); + display: grid; + gap: 4px; + grid-template-columns: 20% 80%; + + .key, + .value { + @include ellipsis-overflow(); + } .key { font-weight: 600; - margin-right: 4px; } } From c2f99a62a468be25709fc39aba42df342351d4c4 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Tue, 8 Jun 2021 22:27:27 +0530 Subject: [PATCH 04/10] fix: errors after merge --- .../src/shared/components/span-detail/span-detail.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/distributed-tracing/src/shared/components/span-detail/span-detail.component.ts b/projects/distributed-tracing/src/shared/components/span-detail/span-detail.component.ts index 13e1ff3b8..13f988aa9 100644 --- a/projects/distributed-tracing/src/shared/components/span-detail/span-detail.component.ts +++ b/projects/distributed-tracing/src/shared/components/span-detail/span-detail.component.ts @@ -47,7 +47,7 @@ import { SpanDetailTab } from './span-detail-tab'; - + Date: Wed, 9 Jun 2021 18:51:49 +0530 Subject: [PATCH 05/10] fix: lint errors --- projects/components/src/tabs/content/tab-group.component.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/components/src/tabs/content/tab-group.component.ts b/projects/components/src/tabs/content/tab-group.component.ts index 72f10b6d2..31b50f99b 100644 --- a/projects/components/src/tabs/content/tab-group.component.ts +++ b/projects/components/src/tabs/content/tab-group.component.ts @@ -9,8 +9,8 @@ import { Output, QueryList } from '@angular/core'; -import { isEmpty } from 'lodash-es'; import { Color } from '@hypertrace/common'; +import { isEmpty } from 'lodash-es'; import { TabComponent } from './tab/tab.component'; @Component({ From 091bf28581e3c0bf236f9022a19c3a15ca37a6a3 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Wed, 9 Jun 2021 18:56:11 +0530 Subject: [PATCH 06/10] fix: addressed review comments --- .../src/sequence/sequence-chart.component.test.ts | 2 +- projects/components/src/sequence/sequence.ts | 2 +- .../waterfall/waterfall-chart.component.ts | 4 ++-- .../waterfall/waterfall/waterfall-chart.service.ts | 14 +++++--------- 4 files changed, 9 insertions(+), 13 deletions(-) diff --git a/projects/components/src/sequence/sequence-chart.component.test.ts b/projects/components/src/sequence/sequence-chart.component.test.ts index 1d2c35fc5..85203fce5 100644 --- a/projects/components/src/sequence/sequence-chart.component.test.ts +++ b/projects/components/src/sequence/sequence-chart.component.test.ts @@ -148,7 +148,7 @@ describe('Sequence Chart component', () => { color: 'blue', markers: [ { - nodeId: '1', + id: '1', markerTime: 1569357460347, timestamps: ['1569357460347'] } diff --git a/projects/components/src/sequence/sequence.ts b/projects/components/src/sequence/sequence.ts index db0e9ba1c..d5d3874b9 100644 --- a/projects/components/src/sequence/sequence.ts +++ b/projects/components/src/sequence/sequence.ts @@ -11,7 +11,7 @@ export interface SequenceSegment { } export interface Marker { - nodeId: string; + id: string; markerTime: number; timestamps: string[]; } diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.component.ts b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.component.ts index ff7f5ebbe..342a2f2eb 100644 --- a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.component.ts +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.component.ts @@ -167,7 +167,7 @@ export class WaterfallChartComponent implements OnChanges { public viewAll(): void { this.markerSelection.emit({ - selectedData: this.marker?.nodeId !== undefined ? this.dataNodeMap.get(this.marker?.nodeId) : undefined, + selectedData: this.marker?.id !== undefined ? this.dataNodeMap.get(this.marker?.id) : undefined, timestamps: this.marker?.timestamps ?? [] }); this.closePopover(); @@ -198,7 +198,7 @@ export class WaterfallChartComponent implements OnChanges { } private buildMarkerDataSource(marker: Marker): Observable { - const spanWaterfallData: WaterfallDataNode = this.dataNodeMap.get(marker.nodeId)!; + const spanWaterfallData: WaterfallDataNode = this.dataNodeMap.get(marker.id)!; let markerData: MarkerTooltipData = { relativeTimes: [], diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.service.ts b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.service.ts index 33561f580..8c67928b1 100644 --- a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.service.ts +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.service.ts @@ -91,15 +91,11 @@ export class WaterfallChartService { start: node.startTime, end: node.endTime, color: node.color!, - markers: node.logEvents.map((logEvent: LogEvent) => { - const timeInMs = this.dateCoercer.coerce(logEvent.timestamp)!.getTime(); - - return { - nodeId: node.id, - markerTime: timeInMs, - timestamps: [logEvent.timestamp] - }; - }) + markers: node.logEvents.map((logEvent: LogEvent) => ({ + id: node.id, + markerTime: this.dateCoercer.coerce(logEvent.timestamp)!.getTime(), + timestamps: [logEvent.timestamp] + })) }); sequenceNodes.unshift(...node.$$state.children); From b927bf453677aab74bec3bb24b2e9fa5cd0f35c3 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Wed, 9 Jun 2021 19:31:31 +0530 Subject: [PATCH 07/10] fix: addressed review comments --- projects/components/src/popover/popover.ts | 3 +-- .../src/sequence/renderer/sequence-bar-renderer.service.ts | 4 ++-- projects/components/src/sequence/sequence.ts | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/projects/components/src/popover/popover.ts b/projects/components/src/popover/popover.ts index 755f7a3de..4c6c4e944 100644 --- a/projects/components/src/popover/popover.ts +++ b/projects/components/src/popover/popover.ts @@ -1,5 +1,4 @@ import { ElementRef, InjectionToken, Injector, TemplateRef, Type } from '@angular/core'; -import { Point } from '@hypertrace/common'; export interface PopoverOptions { componentOrTemplate: Type | TemplateRef; @@ -28,7 +27,7 @@ export interface PopoverHiddenPosition { export interface PopoverRelativePosition { type: PopoverPositionType.Relative; - origin: ElementRef | Element | Point; + origin: ElementRef; locationPreferences: PopoverRelativePositionLocation[]; } diff --git a/projects/components/src/sequence/renderer/sequence-bar-renderer.service.ts b/projects/components/src/sequence/renderer/sequence-bar-renderer.service.ts index a9633f5e1..b274c0d8b 100644 --- a/projects/components/src/sequence/renderer/sequence-bar-renderer.service.ts +++ b/projects/components/src/sequence/renderer/sequence-bar-renderer.service.ts @@ -1,4 +1,4 @@ -import { Injectable } from '@angular/core'; +import { ElementRef, Injectable } from '@angular/core'; import { ScaleLinear } from 'd3-scale'; import { BaseType, select, Selection } from 'd3-selection'; import { SequenceChartAxisService } from '../axis/sequence-chart-axis.service'; @@ -212,7 +212,7 @@ export class SequenceBarRendererService { plotSelection .selectAll(`g.${SequenceBarRendererService.MARKER_CLASS}`) .on('mouseenter', (dataRow, index, nodes) => { - options.onMarkerHovered({ marker: dataRow, origin: nodes[index].getBoundingClientRect() }); + options.onMarkerHovered({ marker: dataRow, origin: new ElementRef(nodes[index]) }); }); } diff --git a/projects/components/src/sequence/sequence.ts b/projects/components/src/sequence/sequence.ts index d5d3874b9..f5f124b87 100644 --- a/projects/components/src/sequence/sequence.ts +++ b/projects/components/src/sequence/sequence.ts @@ -1,4 +1,4 @@ -import { Point } from '@hypertrace/common'; +import { ElementRef } from '@angular/core'; import { Selection } from 'd3-selection'; import { SequenceObject } from './sequence-object'; @@ -18,7 +18,7 @@ export interface Marker { export interface MarkerDatum { marker: Marker; - origin: Point; + origin: ElementRef; } /* Internal Types */ From e8a757b4157034e32198699d3a43d621dd149414 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Wed, 9 Jun 2021 19:53:53 +0530 Subject: [PATCH 08/10] fix: using summary instead of attributes --- .../marker-tooltip.component.scss | 15 ----------- .../marker-tooltip.component.test.ts | 26 +++++-------------- .../marker-tooltip.component.ts | 17 +++--------- .../waterfall/waterfall-chart.component.ts | 6 ++--- 4 files changed, 13 insertions(+), 51 deletions(-) diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.scss b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.scss index fa38d8248..644e2eb89 100644 --- a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.scss +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.scss @@ -38,21 +38,6 @@ border: 1px solid $gray-6; } - .attribute { - display: grid; - gap: 4px; - grid-template-columns: 20% 80%; - - .key, - .value { - @include ellipsis-overflow(); - } - - .key { - font-weight: 600; - } - } - .view-all-text { cursor: pointer; width: max-content; diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.test.ts b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.test.ts index b216e29f1..aa21a8be3 100644 --- a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.test.ts +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.test.ts @@ -13,13 +13,10 @@ describe('Marker Tooltip Component', () => { shallow: true }); - test('should have log attributes without view all', () => { + test('should have single time with summary', () => { const data: Observable = of({ relativeTimes: [2], - attributes: [ - ['id', 'test-id'], - ['error', 'test-error'] - ] + summary: 'test-summary' }); spectator = createHost(``, { hostProps: { @@ -31,24 +28,14 @@ describe('Marker Tooltip Component', () => { expect(spectator.query('.count')).toHaveText('1'); expect(spectator.query('.time-range')).toHaveText('2ms'); expect(spectator.query('.divider')).toExist(); - expect(spectator.queryAll('.key')[0]).toHaveText('id'); - expect(spectator.queryAll('.key')[1]).toHaveText('error'); - expect(spectator.queryAll('.value')[0]).toHaveText('test-id'); - expect(spectator.queryAll('.value')[1]).toHaveText('test-error'); - expect(spectator.query('.view-all')).not.toExist(); + expect(spectator.query('.summary')).toHaveText('test-summary'); + expect(spectator.query('.view-all')).toExist(); }); - test('should have view all', () => { + test('should have time range', () => { const data: Observable = of({ relativeTimes: [2, 3], - attributes: [ - ['id', 'test-id'], - ['error', 'test-error'], - ['name', 'test-name'], - ['class', 'test-class'], - ['reason', 'test-reason'], - ['summary', 'test-summary'] - ] + summary: 'test-summary' }); spectator = createHost(``, { hostProps: { @@ -60,7 +47,6 @@ describe('Marker Tooltip Component', () => { }); expect(spectator.query('.time-range')).toHaveText('2ms - 3ms'); - expect(spectator.query('.view-all')).toExist(); const viewAllTextElement = spectator.query('.view-all-text'); expect(viewAllTextElement).toExist(); spectator.click(viewAllTextElement!); diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.ts b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.ts index e7cbd6edf..455bce9f7 100644 --- a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.ts +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/marker-tooltip/marker-tooltip.component.ts @@ -23,17 +23,10 @@ import { Observable } from 'rxjs';
-
-
{{ attribute[0] }}:
-
{{ attribute[1] }}
+
+ {{ data.summary }}
-
+
...
View all >
@@ -41,8 +34,6 @@ import { Observable } from 'rxjs'; ` }) export class MarkerTooltipComponent { - public static readonly MAX_ATTRIBUTES_TO_DISPLAY: number = 5; - @Input() public data?: Observable; @@ -52,5 +43,5 @@ export class MarkerTooltipComponent { export interface MarkerTooltipData { relativeTimes: number[]; - attributes: [string, unknown][]; + summary: string; } diff --git a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.component.ts b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.component.ts index 342a2f2eb..05a0d1054 100644 --- a/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.component.ts +++ b/projects/distributed-tracing/src/shared/dashboard/widgets/waterfall/waterfall/waterfall-chart.component.ts @@ -202,14 +202,14 @@ export class WaterfallChartComponent implements OnChanges { let markerData: MarkerTooltipData = { relativeTimes: [], - attributes: [] + summary: spanWaterfallData.logEvents[0].summary }; spanWaterfallData.logEvents.forEach((logEvent: LogEvent) => { if (marker.timestamps.includes(logEvent.timestamp)) { const logEventTime = this.dateCoercer.coerce(logEvent.timestamp)!.getTime(); markerData = { - relativeTimes: [...markerData.relativeTimes, logEventTime - spanWaterfallData.startTime], - attributes: [...markerData.attributes, ...Object.entries(logEvent.attributes)] + ...markerData, + relativeTimes: [...markerData.relativeTimes, logEventTime - spanWaterfallData.startTime] }; } }); From 5e1efc991ece6a1accd3fb79218d2e4c1d3103e9 Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Thu, 10 Jun 2021 18:38:34 +0530 Subject: [PATCH 09/10] fix: addressed review comments --- .../renderer/sequence-bar-renderer.service.ts | 41 ++++++++----------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/projects/components/src/sequence/renderer/sequence-bar-renderer.service.ts b/projects/components/src/sequence/renderer/sequence-bar-renderer.service.ts index b274c0d8b..3c9748dad 100644 --- a/projects/components/src/sequence/renderer/sequence-bar-renderer.service.ts +++ b/projects/components/src/sequence/renderer/sequence-bar-renderer.service.ts @@ -43,11 +43,11 @@ export class SequenceBarRendererService { this.drawBackdropRect(transformedBars, options, plotWidth); this.drawBarValueRect(transformedBars, xScale, options); - this.drawBarMarkers(plotSelection, xScale, options); + this.drawBarMarkers(transformedBars, xScale, options); this.drawBarValueText(transformedBars, xScale, options); this.setupHoverListener(transformedBars, options); this.setupClickListener(transformedBars, options); - this.setupMarkerHoverListener(plotSelection, options); + this.setupMarkerHoverListener(transformedBars, options); this.updateDataRowHover(chartSelection, options); this.updateDataRowSelection(chartSelection, options); } @@ -115,29 +115,20 @@ export class SequenceBarRendererService { } private drawBarMarkers( - plotSelection: SequenceSVGSelection, + transformedBars: TransformedBarSelection, xScale: ScaleLinear, options: SequenceOptions ): void { - options.data.forEach((segment, index) => { - plotSelection - .selectAll(`g.${SequenceBarRendererService.MARKERS_CLASS}`) - .data(this.getGroupedMarkers(segment, xScale)) - .enter() - .append('g') - .classed(`${SequenceBarRendererService.MARKER_CLASS}`, true) - .append('rect') - .attr( - 'transform', - dataRow => - `translate(${dataRow.markerTime},${ - (options.rowHeight - options.barHeight) / 2 + options.rowHeight * index - })` - ) - .attr('width', this.markerWidth) - .attr('height', 12) - .style('fill', 'white'); - }); + transformedBars + .selectAll(`g.${SequenceBarRendererService.MARKERS_CLASS}`) + .data(segment => this.getGroupedMarkers(segment, xScale)) + .enter() + .append('rect') + .classed(`${SequenceBarRendererService.MARKER_CLASS}`, true) + .attr('transform', dataRow => `translate(${dataRow.markerTime},${(options.rowHeight - options.barHeight) / 2})`) + .attr('width', this.markerWidth) + .attr('height', 12) + .style('fill', 'white'); } private drawBarValueText( @@ -208,9 +199,9 @@ export class SequenceBarRendererService { options.onSegmentSelected(dataRow); } - private setupMarkerHoverListener(plotSelection: SequenceSVGSelection, options: SequenceOptions): void { - plotSelection - .selectAll(`g.${SequenceBarRendererService.MARKER_CLASS}`) + private setupMarkerHoverListener(transformedBars: TransformedBarSelection, options: SequenceOptions): void { + transformedBars + .selectAll(`rect.${SequenceBarRendererService.MARKER_CLASS}`) .on('mouseenter', (dataRow, index, nodes) => { options.onMarkerHovered({ marker: dataRow, origin: new ElementRef(nodes[index]) }); }); From 2272030ac3ca47317b8ff5711e8a9068d76e7d9f Mon Sep 17 00:00:00 2001 From: Sandeep Kumar Sharma Date: Thu, 10 Jun 2021 18:41:26 +0530 Subject: [PATCH 10/10] fix: test cases --- .../components/src/sequence/sequence-chart.component.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/projects/components/src/sequence/sequence-chart.component.test.ts b/projects/components/src/sequence/sequence-chart.component.test.ts index 85203fce5..b05606408 100644 --- a/projects/components/src/sequence/sequence-chart.component.test.ts +++ b/projects/components/src/sequence/sequence-chart.component.test.ts @@ -164,7 +164,7 @@ describe('Sequence Chart component', () => { const markers = chart.queryAll('.marker', { root: true }); expect(markers.length).toBe(1); - const markerRect = select(markers[0].querySelector('rect')); + const markerRect = select(markers[0]); expect(markerRect.attr('width')).toBe('2'); expect(markerRect.attr('height')).toBe('12'); });