-
Notifications
You must be signed in to change notification settings - Fork 11
chore: suggested-log-tab-changes #935
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
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,41 +1,38 @@ | ||
import { ChangeDetectionStrategy, Component } from '@angular/core'; | ||
import { SubscriptionLifecycle } from '@hypertrace/common'; | ||
|
||
import { Dashboard } from '@hypertrace/hyperdash'; | ||
import { Observable } from 'rxjs'; | ||
import { traceDetailDashboard } from '../trace-detail.dashboard'; | ||
import { TraceDetails, TraceDetailService } from './../trace-detail.service'; | ||
import { map } from 'rxjs/operators'; | ||
import { TraceDetailService } from './../trace-detail.service'; | ||
import { traceSequenceDashboard } from './trace-sequence.dashboard'; | ||
|
||
@Component({ | ||
changeDetection: ChangeDetectionStrategy.OnPush, | ||
template: ` | ||
<ht-navigable-dashboard | ||
*htLoadAsync="this.traceVariables$ as traceVariables" | ||
class="scrollable-container" | ||
[padding]="0" | ||
navLocation="${traceDetailDashboard.location}" | ||
(dashboardReady)="this.onDashboardReady($event)" | ||
[variables]="traceVariables" | ||
navLocation="${traceSequenceDashboard.location}" | ||
> | ||
</ht-navigable-dashboard> | ||
` | ||
}) | ||
export class TraceSequenceComponent { | ||
public readonly traceDetails$: Observable<TraceDetails>; | ||
|
||
public constructor( | ||
private readonly subscriptionLifecycle: SubscriptionLifecycle, | ||
private readonly traceDetailService: TraceDetailService | ||
) { | ||
this.traceDetails$ = this.traceDetailService.fetchTraceDetails(); | ||
} | ||
public readonly traceVariables$: Observable<TraceDetailVariables>; | ||
|
||
public onDashboardReady(dashboard: Dashboard): void { | ||
this.subscriptionLifecycle.add( | ||
this.traceDetails$.subscribe(traceDetails => { | ||
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 is just a really ugly pattern that we have a better alternative for now, although we don't use it enough |
||
dashboard.setVariable('traceId', traceDetails.id); | ||
dashboard.setVariable('spanId', traceDetails.entrySpanId); | ||
dashboard.setVariable('startTime', traceDetails.startTime); | ||
dashboard.refresh(); | ||
}) | ||
public constructor(private readonly traceDetailService: TraceDetailService) { | ||
this.traceVariables$ = this.traceDetailService.fetchTraceDetails().pipe( | ||
map(details => ({ | ||
traceId: details.id, | ||
startTime: details.startTime, | ||
spanId: details.entrySpanId | ||
})) | ||
); | ||
} | ||
} | ||
|
||
interface TraceDetailVariables { | ||
traceId: string; | ||
startTime?: string | number; | ||
spanId?: string; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
import { DashboardDefaultConfiguration } from '../../shared/dashboard/dashboard-wrapper/navigable-dashboard.module'; | ||
import { DashboardDefaultConfiguration } from '../../../shared/dashboard/dashboard-wrapper/navigable-dashboard.module'; | ||
|
||
export const traceDetailDashboard: DashboardDefaultConfiguration = { | ||
location: 'TRACE_DETAIL', | ||
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 page was moved but the dashboard wasn't - fixed |
||
export const traceSequenceDashboard: DashboardDefaultConfiguration = { | ||
location: 'TRACE_SEQUENCE', | ||
json: { | ||
type: 'container-widget', | ||
layout: { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,60 +1,35 @@ | ||
import { ChangeDetectionStrategy, Component } from '@angular/core'; | ||
import { SubscriptionLifecycle } from '@hypertrace/common'; | ||
|
||
import { Dashboard, ModelJson } from '@hypertrace/hyperdash'; | ||
import { Observable } from 'rxjs'; | ||
import { ApiTraceDetails, ApiTraceDetailService } from './../api-trace-detail.service'; | ||
import { map } from 'rxjs/operators'; | ||
import { ApiTraceDetailService } from './../api-trace-detail.service'; | ||
import { apiTraceSequenceDashboard } from './api-trace-sequence.dashboard'; | ||
|
||
@Component({ | ||
changeDetection: ChangeDetectionStrategy.OnPush, | ||
template: ` | ||
<ht-application-aware-dashboard | ||
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 is its own page with a url, so should be navigable dashboard |
||
[json]="this.defaultJson" | ||
<ht-navigable-dashboard | ||
*htLoadAsync="this.traceVariables$ as traceVariables" | ||
navLocation="${apiTraceSequenceDashboard.location}" | ||
[padding]="0" | ||
(dashboardReady)="this.onDashboardReady($event)" | ||
[variables]="traceVariables" | ||
> | ||
</ht-application-aware-dashboard> | ||
</ht-navigable-dashboard> | ||
` | ||
}) | ||
export class ApiTraceSequenceComponent { | ||
public readonly traceDetails$: Observable<ApiTraceDetails>; | ||
|
||
public readonly defaultJson: ModelJson = { | ||
type: 'container-widget', | ||
layout: { | ||
type: 'auto-container-layout', | ||
'enable-style': false | ||
}, | ||
children: [ | ||
{ | ||
type: 'waterfall-widget', | ||
title: 'Sequence Diagram', | ||
data: { | ||
type: 'api-trace-waterfall-data-source', | ||
// tslint:disable-next-line: no-invalid-template-strings | ||
'trace-id': '${traceId}', | ||
// tslint:disable-next-line: no-invalid-template-strings | ||
'start-time': '${startTime}' | ||
} | ||
} | ||
] | ||
}; | ||
|
||
public constructor( | ||
private readonly subscriptionLifecycle: SubscriptionLifecycle, | ||
private readonly apiTraceDetailService: ApiTraceDetailService | ||
) { | ||
this.traceDetails$ = this.apiTraceDetailService.fetchTraceDetails(); | ||
} | ||
public readonly traceVariables$: Observable<ApiTraceDetailVariables>; | ||
|
||
public onDashboardReady(dashboard: Dashboard): void { | ||
this.subscriptionLifecycle.add( | ||
this.traceDetails$.subscribe(traceDetails => { | ||
dashboard.setVariable('traceId', traceDetails.id); | ||
dashboard.setVariable('traceType', traceDetails.type); | ||
dashboard.setVariable('startTime', traceDetails.startTime); | ||
dashboard.refresh(); | ||
}) | ||
public constructor(private readonly apiTraceDetailService: ApiTraceDetailService) { | ||
this.traceVariables$ = this.apiTraceDetailService.fetchTraceDetails().pipe( | ||
map(details => ({ | ||
traceId: details.id, | ||
startTime: details.startTime | ||
})) | ||
); | ||
} | ||
} | ||
|
||
interface ApiTraceDetailVariables { | ||
traceId: string; | ||
startTime?: string | number; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import { DashboardDefaultConfiguration } from '@hypertrace/distributed-tracing'; | ||
|
||
export const apiTraceSequenceDashboard: DashboardDefaultConfiguration = { | ||
location: 'API_TRACE_SEQUENCE', | ||
json: { | ||
type: 'container-widget', | ||
layout: { | ||
type: 'auto-container-layout', | ||
'enable-style': false | ||
}, | ||
children: [ | ||
{ | ||
type: 'waterfall-widget', | ||
title: 'Sequence Diagram', | ||
data: { | ||
type: 'api-trace-waterfall-data-source', | ||
// tslint:disable-next-line: no-invalid-template-strings | ||
'trace-id': '${traceId}', | ||
// tslint:disable-next-line: no-invalid-template-strings | ||
'start-time': '${startTime}' | ||
} | ||
} | ||
] | ||
} | ||
}; |
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.
this was one problem - subscription lifecycle should be provided in the component whenever used, since it's meant to be be tied to the component lifecycle. With out an explicit provider here, we're not cleaning this up which is why it gets undefined in it when we navigate away.