Skip to content

Commit c98ee43

Browse files
muditgokhale2copybara-github
authored andcommitted
Add select / de-select all option and add a submit button to the hosts sidenav for trace_viewer
PiperOrigin-RevId: 820313088
1 parent 7424479 commit c98ee43

File tree

16 files changed

+478
-114
lines changed

16 files changed

+478
-114
lines changed

frontend/app/common/interfaces/navigation_event.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ export declare interface NavigationEvent {
55
run?: string;
66
tag?: string;
77
host?: string;
8+
// Added to support multi-host functionality for trace_viewer.
9+
hosts?: string[];
810
// Graph Viewer crosslink params
911
opName?: string;
1012
moduleName?: string;

frontend/app/components/sidenav/sidenav.ng.html

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
</mat-form-field>
3131
</div>
3232

33-
<div class="item-container">
33+
<div class="item-container" *ngIf="!isMultiHostsEnabled">
3434
<div [ngClass]="{'mat-subheading-2': true, 'disabled': !hosts.length}">
3535
Hosts ({{hosts.length}})
3636
</div>
@@ -43,6 +43,35 @@
4343
</mat-form-field>
4444
</div>
4545

46+
<div class="item-container" *ngIf="isMultiHostsEnabled">
47+
<div [ngClass]="{'mat-subheading-2': true, 'disabled': !hosts.length}">
48+
Hosts ({{selectedHostsInternal.length}})
49+
</div>
50+
<mat-form-field class="full-width" appearance="outline">
51+
<mat-select #multiSelectHost="matSelect" panelClass="multi-host-select-panel" [value]="selectedHostsPending" [disabled]="!hosts.length" (selectionChange)="onHostsSelectionChange($event.value)" multiple>
52+
53+
<div class="select-panel-content">
54+
<div class="select-all-button-container" (click)="$event.stopPropagation()">
55+
<button mat-button class="full-width" (click)="toggleAllHosts()">
56+
<span *ngIf="hosts.length === selectedHostsPending.length">Deselect All</span>
57+
<span *ngIf="hosts.length !== selectedHostsPending.length">Select All</span>
58+
</button>
59+
<mat-divider></mat-divider>
60+
</div>
61+
<mat-option *ngFor="let host of hosts" [value]="host">
62+
{{host}}
63+
</mat-option>
64+
65+
<div class="submit-button-container" (click)="$event.stopPropagation()">
66+
<button mat-flat-button color="primary" class="full-width" (click)="onSubmitHosts(); multiSelectHost.close()">
67+
Apply Host Selection
68+
</button>
69+
</div>
70+
</div>
71+
</mat-select>
72+
</mat-form-field>
73+
</div>
74+
4675
<!-- TODO(xprof): Remove module selector from sidenav once it's been moved to the memory viewer control. -->
4776
<div [hidden]="!is_hlo_tool" class="item-container">
4877
<div [ngClass]="{'mat-subheading-2': true, 'disabled': !moduleList.length}">
@@ -66,4 +95,3 @@
6695
</div>
6796

6897
<br>
69-

frontend/app/components/sidenav/sidenav.scss

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,50 @@
1313
.mat-subheading-2 {
1414
margin-bottom: 0;
1515
}
16+
17+
// Target the custom panel class defined in sidenav.ng.html
18+
.multi-host-select-panel {
19+
.select-panel-content {
20+
display: flex;
21+
flex-direction: column;
22+
}
23+
24+
// Container for the Select All/Deselect All button at the top
25+
.select-all-button-container {
26+
position: sticky;
27+
top: 0;
28+
z-index: 10;
29+
background: #fff;
30+
padding: 0 8px;
31+
box-shadow: 0 2px 5px rgba(0, 0, 0, 0.05);
32+
33+
.full-width {
34+
width: 100%;
35+
text-align: left;
36+
padding-left: 0;
37+
}
38+
39+
mat-divider {
40+
margin: 0 -8px; // Extend divider to panel edge
41+
}
42+
}
43+
44+
// Container for the submit button at the bottom of the dropdown
45+
.submit-button-container {
46+
position: sticky;
47+
bottom: 0;
48+
padding: 8px;
49+
50+
background: #fff;
51+
box-shadow: 0 -2px 5px rgba(0, 0, 0, 0.1);
52+
53+
.full-width {
54+
width: 100%;
55+
}
56+
}
57+
58+
// Ensure options remain clickable and are not covered by the sticky button's padding.
59+
mat-option {
60+
flex-shrink: 0;
61+
}
62+
}

frontend/app/components/sidenav/sidenav.ts

Lines changed: 128 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,11 @@ export class SideNav implements OnInit, OnDestroy {
3232
selectedRunInternal = '';
3333
selectedTagInternal = '';
3434
selectedHostInternal = '';
35+
selectedHostsInternal: string[] = [];
36+
selectedHostsPending: string[] = [];
3537
selectedModuleInternal = '';
3638
navigationParams: {[key: string]: string|boolean} = {};
39+
multiHostEnabledTools: string[] = ['trace_viewer', 'trace_viewer@'];
3740

3841
hideCaptureProfileButton = false;
3942

@@ -65,6 +68,11 @@ export class SideNav implements OnInit, OnDestroy {
6568
return HLO_TOOLS.includes(this.selectedTag);
6669
}
6770

71+
get isMultiHostsEnabled() {
72+
const tag = this.selectedTag || '';
73+
return this.multiHostEnabledTools.includes(tag);
74+
}
75+
6876
// Getter for valid run given url router or user selection.
6977
get selectedRun() {
7078
return this.runs.find(validRun => validRun === this.selectedRunInternal) ||
@@ -90,6 +98,10 @@ export class SideNav implements OnInit, OnDestroy {
9098
this.moduleList[0] || '';
9199
}
92100

101+
get selectedHosts() {
102+
return this.selectedHostsInternal;
103+
}
104+
93105
// https://github.com/angular/angular/issues/11023#issuecomment-752228784
94106
mergeRouteParams(): Map<string, string> {
95107
const params = new Map<string, string>();
@@ -119,20 +131,25 @@ export class SideNav implements OnInit, OnDestroy {
119131
const run = params.get('run') || '';
120132
const tag = params.get('tool') || params.get('tag') || '';
121133
const host = params.get('host') || '';
134+
const hostsParam = params.get('hosts');
122135
const opName = params.get('node_name') || params.get('opName') || '';
123136
const moduleName = params.get('module_name') || '';
124137
this.navigationParams['firstLoad'] = true;
125138
if (opName) {
126139
this.navigationParams['opName'] = opName;
127140
}
128-
if (this.selectedRunInternal === run && this.selectedTagInternal === tag &&
129-
this.selectedHostInternal === host) {
130-
return;
131-
}
132141
this.selectedRunInternal = run;
133142
this.selectedTagInternal = tag;
134-
this.selectedHostInternal = host;
135143
this.selectedModuleInternal = moduleName;
144+
145+
if (this.isMultiHostsEnabled) {
146+
if (hostsParam) {
147+
this.selectedHostsInternal = hostsParam.split(',');
148+
}
149+
this.selectedHostsPending = [...this.selectedHostsInternal];
150+
} else {
151+
this.selectedHostInternal = host;
152+
}
136153
this.update();
137154
}
138155

@@ -153,9 +170,13 @@ export class SideNav implements OnInit, OnDestroy {
153170
const navigationEvent: NavigationEvent = {
154171
run: this.selectedRun,
155172
tag: this.selectedTag,
156-
host: this.selectedHost,
157173
...this.navigationParams,
158174
};
175+
if (this.isMultiHostsEnabled) {
176+
navigationEvent.hosts = this.selectedHosts;
177+
} else {
178+
navigationEvent.host = this.selectedHost;
179+
}
159180
if (this.is_hlo_tool) {
160181
navigationEvent.moduleName = this.selectedModule;
161182
}
@@ -242,8 +263,21 @@ export class SideNav implements OnInit, OnDestroy {
242263
this.afterUpdateTag();
243264
}
244265

245-
onTagSelectionChange(tag: string) {
266+
async onTagSelectionChange(tag: string) {
246267
this.selectedTagInternal = tag;
268+
this.selectedHostsInternal = [];
269+
this.selectedHostsPending = [];
270+
this.selectedHostInternal = '';
271+
272+
if (this.isMultiHostsEnabled) {
273+
this.hosts = await this.getHostsForSelectedTag();
274+
if (this.hosts.length > 0) {
275+
this.selectedHostsInternal = [this.hosts[0]];
276+
} else {
277+
this.selectedHostsInternal = [];
278+
}
279+
this.selectedHostsPending = [...this.selectedHostsInternal];
280+
}
247281
this.afterUpdateTag();
248282
}
249283

@@ -255,18 +289,51 @@ export class SideNav implements OnInit, OnDestroy {
255289
// Keep them under the same update function as initial step of the separation.
256290
async updateHosts() {
257291
this.hosts = await this.getHostsForSelectedTag();
292+
if (this.isMultiHostsEnabled) {
293+
if (this.selectedHostsInternal.length === 0 && this.hosts.length > 0) {
294+
this.selectedHostsInternal = [this.hosts[0]];
295+
}
296+
this.selectedHostsPending = [...this.selectedHostsInternal];
297+
} else {
298+
if (!this.selectedHostInternal && this.hosts.length > 0) {
299+
this.selectedHostInternal = this.hosts[0];
300+
}
301+
}
258302
if (this.is_hlo_tool) {
259303
this.moduleList = await this.getModuleListForSelectedTag();
260304
}
261305

262306
this.afterUpdateHost();
263307
}
264308

265-
onHostSelectionChange(host: string) {
266-
this.selectedHostInternal = host;
309+
onHostSelectionChange(selection: string) {
310+
this.selectedHostInternal = selection;
311+
this.navigateTools();
312+
}
313+
314+
onHostsSelectionChange(selection: string[]) {
315+
this.selectedHostsPending =
316+
Array.isArray(selection) ? selection : [selection];
317+
}
318+
319+
onSubmitHosts() {
320+
this.selectedHostsInternal = [...this.selectedHostsPending];
267321
this.navigateTools();
268322
}
269323

324+
toggleAllHosts() {
325+
const allAvailableHosts = this.hosts;
326+
327+
const areAllSelected = allAvailableHosts.length > 0 &&
328+
allAvailableHosts.length === this.selectedHostsPending.length;
329+
330+
if (areAllSelected) {
331+
this.selectedHostsPending = [];
332+
} else {
333+
this.selectedHostsPending = [...allAvailableHosts];
334+
}
335+
}
336+
270337
onModuleSelectionChange(module: string) {
271338
this.selectedModuleInternal = module;
272339
this.navigateTools();
@@ -276,26 +343,65 @@ export class SideNav implements OnInit, OnDestroy {
276343
this.navigateTools();
277344
}
278345

346+
// Helper function to serialize query parameters
347+
private serializeQueryParams(
348+
params: {[key: string]: string|string[]|boolean|undefined}): string {
349+
const searchParams = new URLSearchParams();
350+
for (const key in params) {
351+
if (params.hasOwnProperty(key)) {
352+
const value = params[key];
353+
// Only include non-null/non-undefined values
354+
if (value !== undefined && value !== null) {
355+
if (Array.isArray(value)) {
356+
// Arrays are handled as comma-separated strings (like 'hosts')
357+
searchParams.set(key, value.join(','));
358+
} else if (typeof value === 'boolean') {
359+
// Only set boolean flags if they are explicitly true
360+
if (value === true) {
361+
searchParams.set(key, 'true');
362+
}
363+
} else {
364+
searchParams.set(key, String(value));
365+
}
366+
}
367+
}
368+
}
369+
const queryString = searchParams.toString();
370+
return queryString ? `?${queryString}` : '';
371+
}
372+
279373
updateUrlHistory() {
280-
// TODO(xprof): change to camel case when constructing url
281-
const toolQueryParams = Object.keys(this.navigationParams)
282-
.map(key => {
283-
return `${key}=${this.navigationParams[key]}`;
284-
})
285-
.join('&');
286-
const toolQueryParamsString =
287-
toolQueryParams.length ? `&${toolQueryParams}` : '';
288-
const moduleNameQuery =
289-
this.is_hlo_tool ? `&module_name=${this.selectedModule}` : '';
290-
const url = `${window.parent.location.origin}?tool=${
291-
this.selectedTag}&host=${this.selectedHost}&run=${this.selectedRun}${
292-
toolQueryParamsString}${moduleNameQuery}#profile`;
374+
const navigationEvent = this.getNavigationEvent();
375+
const queryParams: {[key: string]: string|string[]|boolean|
376+
undefined} = {...navigationEvent};
377+
378+
if (this.isMultiHostsEnabled) {
379+
// For multi-host enabled tools, ensure 'hosts' is a comma-separated string in the URL
380+
if (queryParams['hosts'] && Array.isArray(queryParams['hosts'])) {
381+
queryParams['hosts'] = (queryParams['hosts'] as string[]).join(',');
382+
}
383+
delete queryParams['host']; // Remove single host param
384+
} else {
385+
// For other tools, ensure 'host' is used
386+
delete queryParams['hosts']; // Remove multi-host param
387+
}
388+
389+
// Get current path to avoid changing the base URL
390+
const pathname = window.parent.location.pathname;
391+
392+
// Use the custom serialization helper
393+
const queryString = this.serializeQueryParams(queryParams);
394+
const url = pathname + queryString;
395+
293396
window.parent.history.pushState({}, '', url);
294397
}
295398

296399
navigateTools() {
297400
const navigationEvent = this.getNavigationEvent();
298401
this.communicationService.onNavigateReady(navigationEvent);
402+
403+
// This router.navigate call remains, as it's responsible for Angular
404+
// routing
299405
this.router.navigate(
300406
[
301407
this.selectedTag || 'empty',

frontend/app/components/trace_viewer/trace_viewer.ts

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import {PlatformLocation} from '@angular/common';
2-
import {HttpParams} from '@angular/common/http';
32
import {Component, inject, Injector, OnDestroy} from '@angular/core';
43
import {ActivatedRoute} from '@angular/router';
54
import {API_PREFIX, DATA_API, PLUGIN_NAME} from 'org_xprof/frontend/app/common/constants/constants';
@@ -38,11 +37,19 @@ export class TraceViewer implements OnDestroy {
3837

3938
update(event: NavigationEvent) {
4039
const isStreaming = (event.tag === 'trace_viewer@');
41-
const params = new HttpParams()
42-
.set('run', event.run!)
43-
.set('tag', event.tag!)
44-
.set('host', event.host!);
45-
const traceDataUrl = this.pathPrefix + DATA_API + '?' + params.toString();
40+
const run = event.run || '';
41+
const tag = event.tag || '';
42+
43+
let queryString = `run=${run}&tag=${tag}`;
44+
45+
if (event.hosts && typeof event.hosts === 'string') {
46+
// Since event.hosts is a comma-separated string, we can use it directly.
47+
queryString += `&hosts=${event.hosts}`;
48+
} else if (event.host) {
49+
queryString += `&host=${event.host}`;
50+
}
51+
52+
const traceDataUrl = `${this.pathPrefix}${DATA_API}?${queryString}`;
4653
this.url = this.pathPrefix + API_PREFIX + PLUGIN_NAME +
4754
'/trace_viewer_index.html' +
4855
'?is_streaming=' + isStreaming.toString() + '&is_oss=true' +

plugin/xprof/convert/raw_to_tool_data.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,9 @@ def xspace_to_tool_data(
116116
if success:
117117
data = process_raw_trace(raw_data)
118118
elif tool == 'trace_viewer@':
119-
# Streaming trace viewer handles one host at a time.
120-
assert len(xspace_paths) == 1
121119
options = params.get('trace_viewer_options', {})
122120
options['use_saved_result'] = params.get('use_saved_result', True)
121+
options['hosts'] = params.get('hosts', [])
123122
raw_data, success = xspace_wrapper_func(xspace_paths, tool, options)
124123
if success:
125124
data = raw_data

0 commit comments

Comments
 (0)