We want this for the telemetry battor_agent work. So I upgrade the priority to P1.

For easier reads, I copy-paste the discussion here:

Yes. I think we need to separate StopTracing and collection of results. For battor, this is important because collecting the results takes a long time.

This means, we will need to update the StopTracing API to 2 steps. Something close to Alex's systrace refactor CL is preferred. So later when we unify all tracing tools, it will be easier. https://codereview.chromium.org/1776013005/diff/200001/systrace/systrace/tracing_agents/__init__.py

I'm not opposed to this, but I'm also not sure I understand why it's necessary. Why is it not okay for a StopTracing() call to take a long time?

Its to lessen the extra time other traces are running while others are being stopped.

I think it is better to clarify a bit. The model is like following:

||------- Telemetry side ------------------------|||------------ BattOr side --||
TracingController -> BattorTracingAgent <-> [stdin/stdout] <-> battor_agent_bin

StopTracing() and GetResults() are APIs on the Telemetry side. battor_agent_bin's API is not really relevant to this.

Here is one possible way of working:

1. Telemetry: StopTracing(), send a message to BattOr to stop tracing and move on
2. BattOr: stop battor and start to dump trace to a log file
3. Telemetry: GetResults(), block on message from BattOr
4. BattOr: finish dumping traces, send a message to Telemetry: I am done, here is the log path
5. Telemetry: receive message in GetResults(), read trace from log path (or do whatever)

You see that battor_agent_bin does not need to have GetResults(). GetResults() is Telemetry side's API.

So I think this is a modification on telemetry side, not on the battor side. Reassign this to Randy.

Correct me if I'm wrong, but the battor_agent_bin when calling StopTracing does not return 'Done.' until it pulls the results off of the battor, correct?

I dont like not having any confirmation that tracing has stopped before moving on. This will also cause issues since the stdout of the subprocess talking to the battor binary will have a hanging 'Done.' left around from the StopTracing call that is hard to account for cleanly since it wont be issued until the results are pulled from the battor.

So above in #6 is assuming battor_agent_bin dump to a file. Even if it is dumping to stdout, it is similar.

1. Telemetry: StopTracing(), send a message to BattOr to stop tracing and move on
2. BattOr: stop battor and start to dump trace to stdout (with an end message at the end)
3. Telemetry: GetResults(), reading traces from stdin until the end message (then do whatever)

This applies to atrace (and battor if we dump to stdout).

Re#8, I don't think we are able to force every agent to ACK StopTracing. GetResults() is the blocking call waiting for the agent to return the result. If anything goes wrong, GetResults() will time out.

Ok, I think I know what to do here then. I will just have GetResults look for the 'Done.' issued by the binary StopTracing call, and add some checks to make sure that nothing is called in the interim.