New issue
Advanced search Search tips

Issue 705087 link

Starred by 2 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Introduce a priority among tracing agents to handle multiple stop and start requests

Project Member Reported by ssid@chromium.org, Mar 24 2017

Issue description

Discussion at https://codereview.chromium.org/2466873002/
ssid:
For all cases which can use event name filtering: startup tracing and slow
reports should disable filtering.
For all cases which can use heap profiler filtering: command line flag should
not disable filtering on any SetDisabled.

Now, considering super-position of these cases with tracing.
				filter set by		
				startup	devtols	cmdline	slow reports
Stop		startup		yes	NA	no	NA
tracing		slow reports	no	no	no	yes
called		devtools	NA	yes	no	no
		tracing UI	no	no	no	no

What the table says is if the filters were set by the same agent that started
recording then filters should be disabled.
But, Primiano feels that doing this in TraceLog is magical.
So, the other option I came up with is this:
 https://codereview.chromium.org/2743473002
Which changes the FILTERING_MODE to persistent. So, now for the case of heap
profiling filtering can be done differently.
But, I still feel that we TraceLog should not be handling this magically and
it's better for the agent to specify explicitly to disable filtering.

There are more cases that are not discussed here which includes tracing being
started by multiple agents at the same time. That causes more complicated issues
with filtering. But, this can be addressed as different issue saying TraceLog
supports only one agent at a time.

Oystein:
We *really* need to come up with a better way of handling
multiple involved tracing agents here :/.

Essentially I think we need a priority system for tracing agents. slow reports <
startup tracing < cmdline < devtools < about://tracing. A higher priority
BeginTracing() will first stop the lower priority one. Then the one actual
exception we need, i.e. heap profiler filtering from cmdline/startup tracing
being persistent across about://tracing calls, should be part of the API that
only about://tracing uses and not the default case.
 
My $.02:

I think this bug is going to be a great fit for the servicification of the tracing stuff.
I am still convinced this should NOT happen in base::TraceLog. That should be dumb and just do what it has been told.
I also agree with oysteine@ that the current state is not ideal, because essentially now we are spreading this responsibility into the callers.
I think this sort of stuff should be addressed at a "client-library" level. Right now we don't have this concept, hence why this awkwardness. However, it seems that we are migrating towards that state, so at some point soon we should factor this in. 

The model I have in mind right now is:
 - Nothing except the service (and tests) should directly poke with base::TraceLog
 - TraceLog is dumb and just has simple enable/disable methods (still separating recording and filtering makes sense to me. I think filtering should be completely out of tracelog, but that is what I was addressing in TV1.5 and will eventually get back to that)
 - The service+client library should have the multi-client logic (we are doing precisely the same with memory-infra servicification).

Now, this could be as simple as a "static" priority field in the client, or perhaps needs to be a bit more sophisticated. We'll see once we get there.

In the meantime:
> slow reports < startup tracing < cmdline < devtools < about://tracing
IMHO cmdline should have the top priority here, as that reflects what the developer really wants. I don't think saying "we didn't respect your cmdline because of slow reports"  is right. I think it's more reasonable the other way round "we disabled slow report because you are using custom cmdline arguments".
In my mind:
slow-reports < about://tracing < devtools < cmdline  (startup tracing is really == cmdline, as it gets triggered form there)
with cmdline being the most prioritary one.

Cc: fmea...@chromium.org
+Fadi

Sign in to add a comment