Fix broken trace collection

- Fix order of accesses to TracingConfig class. The CollectTracesComponent used to fetch
configurations from TracingConfig before TraceConfigComponent changes, i.e. CollectTracesComponent
was missing the latest changes to TracingConfig.
- Remove TracingConfig initialization performed in AppComponent. Now TracingConfig takes care of
fetching the globalConfig's MODE (DEV or PROD) and initialize accordingly.
- Remove TracingConfig#isTracingConfigSet(). Now TracingConfig is always fully constructed/initialized.
- Rename various "tracing config" methods to "trace config" to reduce confusion with TracingConfig class

Fix: b/267275104
Test: npm run build:all && npm run test:all
Change-Id: I168a25f69a405422d8d983d5710d6a67773f0b48
This commit is contained in:
Kean Mariotti
2023-01-31 09:42:37 +00:00
parent abf057630e
commit f180c8eb7d
6 changed files with 88 additions and 105 deletions

View File

@@ -35,7 +35,6 @@ import {TraceDataListener} from 'interfaces/trace_data_listener';
import {Timestamp} from 'trace/timestamp';
import {TraceType} from 'trace/trace_type';
import {proxyClient, ProxyState} from 'trace_collection/proxy_client';
import {TracingConfig} from 'trace_collection/tracing_config';
import {ViewerInputMethodComponent} from 'viewers/components/viewer_input_method_component';
import {View, Viewer} from 'viewers/viewer';
import {ViewerProtologComponent} from 'viewers/viewer_protolog/viewer_protolog_component';
@@ -257,8 +256,6 @@ export class AppComponent implements TraceDataListener {
createCustomElement(ViewerWindowManagerComponent, {injector})
);
}
TracingConfig.getInstance().initialize(localStorage);
}
ngAfterViewInit() {

View File

@@ -128,9 +128,7 @@ import {ParserErrorSnackBarComponent} from './parser_error_snack_bar_component';
label="Trace"
[disabled]="connect.isEndTraceState() || connect.isLoadDataState()">
<div class="tabbed-section">
<div
class="trace-section"
*ngIf="tracingConfig.tracingConfigIsSet() && connect.isStartTraceState()">
<div class="trace-section" *ngIf="connect.isStartTraceState()">
<trace-config></trace-config>
<div class="start-btn">
<button color="primary" mat-stroked-button (click)="startTracing()">
@@ -138,11 +136,6 @@ import {ParserErrorSnackBarComponent} from './parser_error_snack_bar_component';
</button>
</div>
</div>
<div
class="loading-info"
*ngIf="!tracingConfig.tracingConfigIsSet() && connect.isStartTraceState()">
<p class="mat-body-1">Loading tracing config...</p>
</div>
<div *ngIf="connect.isEndTraceState()" class="end-tracing">
<div class="progress-desc">
@@ -172,9 +165,7 @@ import {ParserErrorSnackBarComponent} from './parser_error_snack_bar_component';
label="Dump"
[disabled]="connect.isEndTraceState() || connect.isLoadDataState()">
<div class="tabbed-section">
<div
class="dump-section"
*ngIf="tracingConfig.tracingConfigIsSet() && connect.isStartTraceState()">
<div class="dump-section" *ngIf="connect.isStartTraceState()">
<h3 class="mat-subheading-2">Dump targets</h3>
<div class="selection">
<mat-checkbox
@@ -192,10 +183,6 @@ import {ParserErrorSnackBarComponent} from './parser_error_snack_bar_component';
</div>
</div>
<div class="loading-info" *ngIf="!tracingConfig.tracingConfigIsSet()">
<p class="mat-body-1">Loading dumping config...</p>
</div>
<load-progress
*ngIf="connect.isLoadDataState()"
[progressPercentage]="loadProgress"
@@ -476,7 +463,7 @@ export class CollectTracesComponent implements OnInit, OnDestroy {
private requestedTraces() {
const tracesFromCollection: string[] = [];
const tracingConfig = this.tracingConfig.getTracingConfig();
const tracingConfig = this.tracingConfig.getTraceConfig();
const req = Object.keys(tracingConfig).filter((traceKey: string) => {
const traceConfig = tracingConfig[traceKey];
if (traceConfig.isTraceCollection) {
@@ -501,7 +488,7 @@ export class CollectTracesComponent implements OnInit, OnDestroy {
private requestedEnableConfig(): string[] {
const req: string[] = [];
const tracingConfig = this.tracingConfig.getTracingConfig();
const tracingConfig = this.tracingConfig.getTraceConfig();
Object.keys(tracingConfig).forEach((traceKey: string) => {
const trace = tracingConfig[traceKey];
if (!trace.isTraceCollection && trace.run && trace.config && trace.config.enableConfigs) {
@@ -516,7 +503,7 @@ export class CollectTracesComponent implements OnInit, OnDestroy {
}
private requestedSelection(traceType: string): ConfigMap | undefined {
const tracingConfig = this.tracingConfig.getTracingConfig();
const tracingConfig = this.tracingConfig.getTraceConfig();
if (!tracingConfig[traceType].run) {
return undefined;
}

View File

@@ -28,38 +28,51 @@ import {TracingConfig} from 'trace_collection/tracing_config';
<div class="checkboxes">
<mat-checkbox
*ngFor="let traceKey of objectKeys(traces)"
*ngFor="let traceKey of objectKeys(tracingConfig.getTraceConfig())"
color="primary"
class="trace-checkbox"
[checked]="traces[traceKey].run"
[indeterminate]="traces[traceKey].isTraceCollection ? someTraces(traces[traceKey]) : false"
(change)="changeRunTrace($event.checked, traces[traceKey])"
>{{ traces[traceKey].name }}</mat-checkbox
[checked]="tracingConfig.getTraceConfig()[traceKey].run"
[indeterminate]="
tracingConfig.getTraceConfig()[traceKey].isTraceCollection
? someTraces(tracingConfig.getTraceConfig()[traceKey])
: false
"
(change)="changeRunTrace($event.checked, tracingConfig.getTraceConfig()[traceKey])"
>{{ tracingConfig.getTraceConfig()[traceKey].name }}</mat-checkbox
>
</div>
<ng-container *ngFor="let traceKey of advancedConfigTraces()">
<mat-divider></mat-divider>
<h3 class="mat-subheading-2">{{ traces[traceKey].name }} configuration</h3>
<h3 class="mat-subheading-2">
{{ tracingConfig.getTraceConfig()[traceKey].name }} configuration
</h3>
<div *ngIf="traces[traceKey].config?.enableConfigs.length > 0" class="enable-config-opt">
<div
*ngIf="tracingConfig.getTraceConfig()[traceKey].config?.enableConfigs.length > 0"
class="enable-config-opt">
<mat-checkbox
*ngFor="let enableConfig of traceEnableConfigs(traces[traceKey])"
*ngFor="let enableConfig of traceEnableConfigs(tracingConfig.getTraceConfig()[traceKey])"
color="primary"
class="enable-config"
[disabled]="!traces[traceKey].run && !traces[traceKey].isTraceCollection"
[disabled]="
!tracingConfig.getTraceConfig()[traceKey].run &&
!tracingConfig.getTraceConfig()[traceKey].isTraceCollection
"
[(ngModel)]="enableConfig.enabled"
(change)="changeTraceCollectionConfig(traces[traceKey])"
(change)="changeTraceCollectionConfig(tracingConfig.getTraceConfig()[traceKey])"
>{{ enableConfig.name }}</mat-checkbox
>
</div>
<div
*ngIf="traces[traceKey].config?.selectionConfigs.length > 0"
*ngIf="tracingConfig.getTraceConfig()[traceKey].config?.selectionConfigs.length > 0"
class="selection-config-opt">
<mat-form-field
*ngFor="let selectionConfig of traceSelectionConfigs(traces[traceKey])"
*ngFor="
let selectionConfig of traceSelectionConfigs(tracingConfig.getTraceConfig()[traceKey])
"
class="config-selection"
appearance="fill">
<mat-label>{{ selectionConfig.name }}</mat-label>
@@ -67,7 +80,7 @@ import {TracingConfig} from 'trace_collection/tracing_config';
<mat-select
class="selected-value"
[(value)]="selectionConfig.value"
[disabled]="!traces[traceKey].run">
[disabled]="!tracingConfig.getTraceConfig()[traceKey].run">
<mat-option *ngFor="let option of selectionConfig.options" value="{{ option }}">{{
option
}}</mat-option>
@@ -95,12 +108,12 @@ import {TracingConfig} from 'trace_collection/tracing_config';
})
export class TraceConfigComponent {
objectKeys = Object.keys;
traces = TracingConfig.getInstance().getTracingConfig();
tracingConfig = TracingConfig.getInstance();
advancedConfigTraces() {
const advancedConfigs: string[] = [];
Object.keys(this.traces).forEach((traceKey: string) => {
if (this.traces[traceKey].config) {
Object.keys(this.tracingConfig.getTraceConfig()).forEach((traceKey: string) => {
if (this.tracingConfig.getTraceConfig()[traceKey].config) {
advancedConfigs.push(traceKey);
}
});

View File

@@ -46,7 +46,7 @@ describe('TraceConfigComponent', () => {
fixture = TestBed.createComponent(TraceConfigComponent);
component = fixture.componentInstance;
htmlElement = fixture.nativeElement;
component.traces = {
component.tracingConfig.setTraceConfig({
layers_trace: {
name: 'layers_trace',
isTraceCollection: undefined,
@@ -69,7 +69,8 @@ describe('TraceConfigComponent', () => {
],
},
},
};
});
fixture.detectChanges();
});
it('can be created', () => {
@@ -77,7 +78,7 @@ describe('TraceConfigComponent', () => {
});
it('check that trace checkbox ticked on default run', () => {
component.traces['layers_trace'].run = true;
component.tracingConfig.getTraceConfig()['layers_trace'].run = true;
fixture.detectChanges();
const box = htmlElement.querySelector('.trace-checkbox');
expect(box?.innerHTML).toContain('aria-checked="true"');
@@ -85,32 +86,18 @@ describe('TraceConfigComponent', () => {
});
it('check that trace checkbox not ticked on default run', () => {
component.traces['layers_trace'].run = false;
component.tracingConfig.getTraceConfig()['layers_trace'].run = false;
fixture.detectChanges();
const box = htmlElement.querySelector('.trace-checkbox');
expect(box?.innerHTML).toContain('aria-checked="false"');
});
it('check that correct advanced enable config shows', () => {
component.traces['layers_trace'].config!.selectionConfigs = [];
fixture.detectChanges();
it('check that advanced configs show', () => {
const enable_config_opt = htmlElement.querySelector('.enable-config-opt');
expect(enable_config_opt).toBeTruthy();
expect(enable_config_opt?.innerHTML).toContain('trace buffers');
expect(enable_config_opt?.innerHTML).not.toContain('tracing level');
const selection_config_opt = htmlElement.querySelector('.selection-config-opt');
expect(selection_config_opt).toBeFalsy();
});
it('check that correct advanced selection config shows', () => {
component.traces['layers_trace'].config!.enableConfigs = [];
fixture.detectChanges();
const enable_config_opt = htmlElement.querySelector('.enable-config-opt');
expect(enable_config_opt).toBeFalsy();
const selection_config_opt = htmlElement.querySelector('.selection-config-opt');
expect(selection_config_opt).toBeTruthy();
expect(selection_config_opt?.innerHTML).not.toContain('trace buffers');
@@ -118,7 +105,8 @@ describe('TraceConfigComponent', () => {
});
it('check that changing enable config causes box to change', async () => {
component.traces['layers_trace'].config!.enableConfigs[0].enabled = false;
component.tracingConfig.getTraceConfig()['layers_trace'].config!.enableConfigs[0].enabled =
false;
fixture.detectChanges();
await fixture.whenStable();
expect(htmlElement.querySelector('.enable-config')?.innerHTML).toContain(
@@ -129,7 +117,8 @@ describe('TraceConfigComponent', () => {
it('check that changing selected config causes select to change', async () => {
fixture.detectChanges();
expect(htmlElement.querySelector('.config-selection')?.innerHTML).toContain('value="debug"');
component.traces['layers_trace'].config!.selectionConfigs[0].value = 'verbose';
component.tracingConfig.getTraceConfig()['layers_trace'].config!.selectionConfigs[0].value =
'verbose';
fixture.detectChanges();
await fixture.whenStable();
expect(htmlElement.querySelector('.config-selection')?.innerHTML).toContain('value="verbose"');

View File

@@ -182,7 +182,7 @@ export class ProxyConnection implements Connection {
}
if (newState === ProxyState.START_TRACE) {
const isWaylandAvailable = await this.isWaylandAvailable();
TracingConfig.getInstance().setTracingConfigForAvailableTraces(isWaylandAvailable);
TracingConfig.getInstance().setTraceConfigForAvailableTraces(isWaylandAvailable);
}
this.proxyStateChangeCallback(newState);
}

View File

@@ -13,28 +13,64 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import {globalConfig} from 'common/global_config';
import {PersistentStoreProxy} from 'common/persistent_store_proxy';
import {MockStorage} from 'test/unit/mock_storage';
import {TraceConfigurationMap, TRACES} from './trace_collection_utils';
export class TracingConfig {
requestedTraces: string[] = [];
requestedDumps: string[] = [];
private storage: Storage | undefined;
private tracingConfig: TraceConfigurationMap | undefined;
private dumpConfig: TraceConfigurationMap | undefined;
private storage: Storage;
private traceConfig: TraceConfigurationMap;
private dumpConfig: TraceConfigurationMap;
private static instance: TracingConfig | undefined;
static getInstance(): TracingConfig {
return setTracesInstance;
if (!TracingConfig.instance) {
TracingConfig.instance = new TracingConfig();
}
return TracingConfig.instance;
}
initialize(storage: Storage) {
this.storage = storage;
this.tracingConfig = PersistentStoreProxy.new<TraceConfigurationMap>(
setTraceConfigForAvailableTraces(isWaylandAvailable = false) {
const availableTracesConfig = TRACES['default'];
if (isWaylandAvailable) {
Object.assign(availableTracesConfig, TRACES['arc']);
}
this.setTraceConfig(availableTracesConfig);
}
setTraceConfig(traceConfig: TraceConfigurationMap) {
this.traceConfig = PersistentStoreProxy.new<TraceConfigurationMap>(
'TraceConfiguration',
traceConfig,
this.storage
);
}
getTraceConfig(): TraceConfigurationMap {
return this.traceConfig;
}
getDumpConfig(): TraceConfigurationMap {
if (this.dumpConfig === undefined) {
throw Error('Dump config not initialized yet');
}
return this.dumpConfig;
}
private constructor() {
this.storage = globalConfig.MODE === 'PROD' ? localStorage : new MockStorage();
this.traceConfig = PersistentStoreProxy.new<TraceConfigurationMap>(
'TracingSettings',
TRACES['default'],
this.storage
);
this.dumpConfig = PersistentStoreProxy.new<TraceConfigurationMap>(
'DumpSettings',
{
@@ -54,43 +90,4 @@ export class TracingConfig {
this.storage
);
}
setTracingConfigForAvailableTraces(isWaylandAvailable = false) {
const availableTracesConfig = TRACES['default'];
if (isWaylandAvailable) {
Object.assign(availableTracesConfig, TRACES['arc']);
}
this.setTracingConfig(availableTracesConfig);
}
tracingConfigIsSet(): boolean {
return this.tracingConfig !== undefined;
}
getTracingConfig(): TraceConfigurationMap {
if (this.tracingConfig === undefined) {
throw Error('Tracing config not initialized yet');
}
return this.tracingConfig;
}
private setTracingConfig(traceConfig: TraceConfigurationMap) {
if (this.storage === undefined) {
throw Error('not initialized');
}
this.tracingConfig = PersistentStoreProxy.new<TraceConfigurationMap>(
'TraceConfiguration',
traceConfig,
this.storage
);
}
getDumpConfig(): TraceConfigurationMap {
if (this.dumpConfig === undefined) {
throw Error('Dump config not initialized yet');
}
return this.dumpConfig;
}
}
const setTracesInstance = new TracingConfig();