Skip to content

Commit add7153

Browse files
muditgokhale2copybara-github
authored andcommitted
Add all_hosts information to the session_snapshot and move the device collision logic for trace_viewer to CreateTraceEventsContainer.
PiperOrigin-RevId: 820222547
1 parent 7424479 commit add7153

24 files changed

+504
-194
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: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,17 @@
3232

3333
<div class="item-container">
3434
<div [ngClass]="{'mat-subheading-2': true, 'disabled': !hosts.length}">
35-
Hosts ({{hosts.length}})
35+
Hosts ({{ isMultiHostsEnabled ? selectedHostsInternal.length : hosts.length }})
3636
</div>
3737
<mat-form-field class="full-width" appearance="outline">
38-
<mat-select panelClass="panel-override" [value]="selectedHost" [disabled]="!hosts.length" (selectionChange)="onHostSelectionChange($event.value)">
38+
<!-- Multi-host select -->
39+
<mat-select *ngIf="isMultiHostsEnabled" panelClass="panel-override" [value]="selectedHostsInternal" [disabled]="!hosts.length" (selectionChange)="onHostsSelectionChange($event.value)" multiple>
40+
<mat-option *ngFor="let host of hosts" [value]="host">
41+
{{host}}
42+
</mat-option>
43+
</mat-select>
44+
<!-- Single-host select -->
45+
<mat-select *ngIf="!isMultiHostsEnabled" panelClass="panel-override" [value]="selectedHost" [disabled]="!hosts.length" (selectionChange)="onHostSelectionChange($event.value)">
3946
<mat-option *ngFor="let host of hosts" [value]="host">
4047
{{host}}
4148
</mat-option>

frontend/app/components/sidenav/sidenav.ts

Lines changed: 85 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,10 @@ export class SideNav implements OnInit, OnDestroy {
3232
selectedRunInternal = '';
3333
selectedTagInternal = '';
3434
selectedHostInternal = '';
35+
selectedHostsInternal: string[] = [];
3536
selectedModuleInternal = '';
3637
navigationParams: {[key: string]: string|boolean} = {};
38+
multiHostEnabledTools: string[] = ['trace_viewer', 'trace_viewer@'];
3739

3840
hideCaptureProfileButton = false;
3941

@@ -65,6 +67,11 @@ export class SideNav implements OnInit, OnDestroy {
6567
return HLO_TOOLS.includes(this.selectedTag);
6668
}
6769

70+
get isMultiHostsEnabled() {
71+
const tag = this.selectedTag || '';
72+
return this.multiHostEnabledTools.includes(tag);
73+
}
74+
6875
// Getter for valid run given url router or user selection.
6976
get selectedRun() {
7077
return this.runs.find(validRun => validRun === this.selectedRunInternal) ||
@@ -90,6 +97,10 @@ export class SideNav implements OnInit, OnDestroy {
9097
this.moduleList[0] || '';
9198
}
9299

100+
get selectedHosts() {
101+
return this.selectedHostsInternal;
102+
}
103+
93104
// https://github.com/angular/angular/issues/11023#issuecomment-752228784
94105
mergeRouteParams(): Map<string, string> {
95106
const params = new Map<string, string>();
@@ -119,20 +130,21 @@ export class SideNav implements OnInit, OnDestroy {
119130
const run = params.get('run') || '';
120131
const tag = params.get('tool') || params.get('tag') || '';
121132
const host = params.get('host') || '';
133+
const hostsParam = params.get('hosts');
122134
const opName = params.get('node_name') || params.get('opName') || '';
123135
const moduleName = params.get('module_name') || '';
124136
this.navigationParams['firstLoad'] = true;
125137
if (opName) {
126138
this.navigationParams['opName'] = opName;
127139
}
128-
if (this.selectedRunInternal === run && this.selectedTagInternal === tag &&
129-
this.selectedHostInternal === host) {
130-
return;
131-
}
132140
this.selectedRunInternal = run;
133141
this.selectedTagInternal = tag;
134-
this.selectedHostInternal = host;
135142
this.selectedModuleInternal = moduleName;
143+
144+
if (hostsParam) {
145+
this.selectedHostsInternal = hostsParam.split(',');
146+
}
147+
this.selectedHostInternal = host;
136148
this.update();
137149
}
138150

@@ -153,9 +165,13 @@ export class SideNav implements OnInit, OnDestroy {
153165
const navigationEvent: NavigationEvent = {
154166
run: this.selectedRun,
155167
tag: this.selectedTag,
156-
host: this.selectedHost,
157168
...this.navigationParams,
158169
};
170+
if (this.isMultiHostsEnabled) {
171+
navigationEvent.hosts = this.selectedHosts;
172+
} else {
173+
navigationEvent.host = this.selectedHost;
174+
}
159175
if (this.is_hlo_tool) {
160176
navigationEvent.moduleName = this.selectedModule;
161177
}
@@ -255,15 +271,24 @@ export class SideNav implements OnInit, OnDestroy {
255271
// Keep them under the same update function as initial step of the separation.
256272
async updateHosts() {
257273
this.hosts = await this.getHostsForSelectedTag();
274+
this.selectedHostsInternal = [this.hosts[0]];
275+
this.selectedHostInternal = this.hosts[0];
258276
if (this.is_hlo_tool) {
259277
this.moduleList = await this.getModuleListForSelectedTag();
260278
}
261279

262280
this.afterUpdateHost();
263281
}
264282

265-
onHostSelectionChange(host: string) {
266-
this.selectedHostInternal = host;
283+
onHostSelectionChange(selection: string) {
284+
this.selectedHostInternal = selection;
285+
this.selectedHostsInternal = [];
286+
this.navigateTools();
287+
}
288+
289+
onHostsSelectionChange(selection: string[]) {
290+
this.selectedHostsInternal = selection;
291+
this.selectedHostInternal = ''; // Ensure single-host is empty
267292
this.navigateTools();
268293
}
269294

@@ -276,26 +301,65 @@ export class SideNav implements OnInit, OnDestroy {
276301
this.navigateTools();
277302
}
278303

304+
// Helper function to serialize query parameters
305+
private serializeQueryParams(
306+
params: {[key: string]: string|string[]|boolean|undefined}): string {
307+
const searchParams = new URLSearchParams();
308+
for (const key in params) {
309+
if (params.hasOwnProperty(key)) {
310+
const value = params[key];
311+
// Only include non-null/non-undefined values
312+
if (value !== undefined && value !== null) {
313+
if (Array.isArray(value)) {
314+
// Arrays are handled as comma-separated strings (like 'hosts')
315+
searchParams.set(key, value.join(','));
316+
} else if (typeof value === 'boolean') {
317+
// Only set boolean flags if they are explicitly true
318+
if (value === true) {
319+
searchParams.set(key, 'true');
320+
}
321+
} else {
322+
searchParams.set(key, String(value));
323+
}
324+
}
325+
}
326+
}
327+
const queryString = searchParams.toString();
328+
return queryString ? `?${queryString}` : '';
329+
}
330+
279331
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`;
332+
const navigationEvent = this.getNavigationEvent();
333+
const queryParams: {[key: string]: string|string[]|boolean|
334+
undefined} = {...navigationEvent};
335+
336+
if (this.isMultiHostsEnabled) {
337+
// For Trace Viewer, ensure 'hosts' is a comma-separated string in the URL
338+
if (queryParams['hosts'] && Array.isArray(queryParams['hosts'])) {
339+
queryParams['hosts'] = (queryParams['hosts'] as string[]).join(',');
340+
}
341+
delete queryParams['host']; // Remove single host param
342+
} else {
343+
// For other tools, ensure 'host' is used
344+
delete queryParams['hosts']; // Remove multi-host param
345+
}
346+
347+
// Get current path to avoid changing the base URL
348+
const pathname = window.parent.location.pathname;
349+
350+
// Use the custom serialization helper
351+
const queryString = this.serializeQueryParams(queryParams);
352+
const url = pathname + queryString;
353+
293354
window.parent.history.pushState({}, '', url);
294355
}
295356

296357
navigateTools() {
297358
const navigationEvent = this.getNavigationEvent();
298359
this.communicationService.onNavigateReady(navigationEvent);
360+
361+
// This router.navigate call remains, as it's responsible for Angular
362+
// routing
299363
this.router.navigate(
300364
[
301365
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' +

0 commit comments

Comments
 (0)