[trace controller] Update trace controller FIDL

Split the trace controller protocol into Provisioner and Session
protocols. The existing controller will be removed once all clients are
migrated to the new protocol.

Bug: b/42083286

Change-Id: I6cfa09afe58bac00205b956f14f067531df92bab
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1080794
Reviewed-by: Gwen Mittertreiner <[email protected]>
API-Review: Christopher Johnson <[email protected]>
Commit-Queue: Krithika Balan <[email protected]>
Reviewed-by: Christopher Johnson <[email protected]>

sdk/fidl/fuchsia.tracing.controller/trace_controller.fidl

@@ -38,7 +38,41 @@ type SessionState = flexible enum {
     TERMINATING = 7;
 };
 
-/// The controller interface used by the trace tool to initialize/start/stop/terminate tracing.
+/// The provisioner interface used by the trace tool to initialize tracing
+///
+/// The provisioner is required to start a tracing session and bind the control
+/// to the client. It can also perform tasks like getting the list of registered
+/// providers and known categories, which can be performed without a existing
+/// trace session.
+@discoverable
+open protocol Provisioner {
+    /// Requests to initialize tracing with the specified `config`.
+    ///
+    /// A bad request will terminate the connection.
+    ///
+    /// The trace controller emits trace data to `output` as a sequence of
+    /// binary formatted trace records.  Traces obtained from different providers
+    /// are delimited by metadata records within the stream.
+    /// The format of these records can be found here:
+    /// https://fuchsia.dev/fuchsia-src/reference/tracing/trace-format
+    flexible InitializeTracing(resource struct {
+        controller server_end:Session;
+        config TraceConfig;
+        output zx.Handle:SOCKET;
+    });
+
+    /// Return the set of registered providers.
+    flexible GetProviders() -> (struct {
+        providers vector<ProviderInfo>:MAX_NUM_PROVIDERS;
+    });
+
+    // Gets the known categories.
+    flexible GetKnownCategories() -> (struct {
+        categories vector<fuchsia.tracing.KnownCategory>:fuchsia.tracing.MAX_NUM_KNOWN_CATEGORIES;
+    });
+};
+
+/// The controller interface used by the trace tool to start/stop/terminate a tracing session.
 ///
 /// The trace controller may lightly validate the structure of
 /// trace records as it copies them from trace buffers into the output.
@@ -49,6 +83,46 @@ type SessionState = flexible enum {
 /// themselves.  For example, it does not try to check argument lengths in
 /// events.  This ensures that the trace format can be extended without needing
 /// to modify the trace controller.
+open protocol Session {
+    /// Requests to start tracing with the specified `options`.
+    ///
+    /// If tracing has already started then the request is ignored,
+    /// except to send back an error code.
+    ///
+    /// The trace controller acknowledges the request when all
+    /// registered providers have been started or after
+    /// `TraceConfig.start_timeout_milliseconds` milliseconds.
+    /// One useful reason for the has-started acknowledgement is that the
+    /// trace program can start a program to trace knowing that all the
+    /// providers are started.
+    flexible StartTracing(struct {
+        options StartOptions;
+    }) -> () error StartErrorCode;
+
+    /// Requests to stop tracing.
+    ///
+    /// If tracing has already stopped then this does nothing.
+    /// Returning a result lets callers know when it's ok to, for example,
+    /// start tracing again.
+    flexible StopTracing(struct {
+        options StopOptions;
+    }) -> (struct {
+        result TerminateResult;
+    }) error StopErrorCode;
+
+    /// Returns the next alert when it arrives.
+    flexible WatchAlert() -> (struct {
+        alert_name AlertName;
+    });
+
+    /// Event sent when the session state changes.
+    flexible -> OnSessionStateChange(struct {
+        state SessionState;
+    });
+};
+
+// Copy of the existing controller to keep things working until all references
+// are to it are removed
 @discoverable
 protocol Controller {
     /// Requests to initialize tracing with the specified `config`.
@@ -185,6 +259,16 @@ type StartErrorCode = flexible enum {
     TERMINATING = 4;
 };
 
+/// Error codes from Stop operations.
+type StopErrorCode = flexible enum {
+    /// Trace controller is unavailable. Nothing to stop.
+    NOT_INITIALIZED = 1;
+    /// Tracing has not started or is currently stoppped.
+    NOT_STARTED = 2;
+    /// Tracing aborted due to error.
+    ABORTED = 3;
+};
+
 /// Additional options to control trace data collection.
 type StartOptions = table {
     /// Whether and how to clear the buffer when starting data collection.

src/performance/trace_manager/app.cc

@@ -12,20 +12,37 @@ namespace tracing {
 
 TraceManagerApp::TraceManagerApp(std::unique_ptr<sys::ComponentContext> context, Config config,
                                  async::Executor& executor)
-    : context_(std::move(context)), trace_manager_(this, std::move(config), executor) {
+    : context_(std::move(context)),
+      trace_manager_(this, std::move(config), executor),
+      old_trace_manager_(this, &trace_manager_, executor) {
   [[maybe_unused]] zx_status_t status;
 
   status = context_->outgoing()->AddPublicService(
       provider_registry_bindings_.GetHandler(&trace_manager_));
   FX_DCHECK(status == ZX_OK);
 
-  status = context_->outgoing()->AddPublicService(controller_bindings_.GetHandler(&trace_manager_));
+  status = context_->outgoing()->AddPublicService(
+      old_controller_bindings_.GetHandler(&old_trace_manager_));
+  FX_DCHECK(status == ZX_OK);
+  old_controller_bindings_.set_empty_set_handler(
+      [this]() { old_trace_manager_.OnEmptyControllerSet(); });
+
+  status =
+      context_->outgoing()->AddPublicService(provisioner_bindings_.GetHandler(&trace_manager_));
   FX_DCHECK(status == ZX_OK);
-  controller_bindings_.set_empty_set_handler([this]() { trace_manager_.OnEmptyControllerSet(); });
 
   FX_LOGS(DEBUG) << "TraceManager services registered";
 }
 
+void TraceManagerApp::AddSessionBinding(
+    std::shared_ptr<controller::Session> trace_session,
+    fidl::InterfaceRequest<controller::Session> session_controller) {
+  session_bindings_.AddBinding(trace_session, std::move(session_controller));
+  session_bindings_.set_empty_set_handler([this]() { trace_manager_.OnEmptyControllerSet(); });
+
+  FX_LOGS(DEBUG) << "TraceController registered";
+}
+
 TraceManagerApp::~TraceManagerApp() = default;
 
 }  // namespace tracing

src/performance/trace_manager/app.h

@@ -22,21 +22,42 @@ class TraceManagerApp {
                   async::Executor& executor);
   ~TraceManagerApp();
 
+  void AddSessionBinding(std::shared_ptr<controller::Session> trace_session,
+                         fidl::InterfaceRequest<controller::Session> session_controller);
+
+  void CloseSessionBindings() { session_bindings_.CloseAll(); }
+
   // For testing.
   sys::ComponentContext* context() const { return context_.get(); }
   const TraceManager* trace_manager() const { return &trace_manager_; }
+  const OldTraceManager* old_trace_manager() const { return &old_trace_manager_; }
 
   const fidl::BindingSet<controller::Controller>& controller_bindings() const {
-    return controller_bindings_;
+    return old_controller_bindings_;
+  }
+
+  const fidl::BindingSet<controller::Session, std::shared_ptr<controller::Session>>&
+  session_bindings() const {
+    return session_bindings_;
   }
 
  private:
   std::unique_ptr<sys::ComponentContext> context_;
 
   TraceManager trace_manager_;
 
+  // TODO(b/42083286): Remove after the old trace Controller protocol is removed
+  // This is present to maintain backwards comparitibility during the process of
+  // migrating to use the new trace protocols. This will be removed once all tracing
+  // clients are migrated.
+  OldTraceManager old_trace_manager_;
+
   fidl::BindingSet<fuchsia::tracing::provider::Registry> provider_registry_bindings_;
-  fidl::BindingSet<fuchsia::tracing::controller::Controller> controller_bindings_;
+  fidl::BindingSet<fuchsia::tracing::controller::Controller> old_controller_bindings_;
+  fidl::BindingSet<fuchsia::tracing::controller::Provisioner> provisioner_bindings_;
+  fidl::BindingSet<fuchsia::tracing::controller::Session,
+                   std::shared_ptr<fuchsia::tracing::controller::Session>>
+      session_bindings_;
 
   TraceManagerApp(const TraceManagerApp&) = delete;
   TraceManagerApp(TraceManagerApp&&) = delete;

src/performance/trace_manager/tests/init_to_fini_tests.cc

@@ -131,7 +131,8 @@ TEST_F(TraceManagerTest, InitToFiniWithNoStop) {
   VerifyCounts(1, 0);
 
   ASSERT_TRUE(TerminateSession());
-  VerifyCounts(1, 0);
+  // Stop is immplicitly called first before terminating.
+  VerifyCounts(1, 1);
 }
 
 static constexpr char kAlertName[] = "alert-name";

src/performance/trace_manager/tests/trace_manager_test.cc

@@ -317,6 +317,15 @@ bool TraceManagerTest::FinishTerminateSession(controller::TerminateResult* resul
   if (fake_provider_bindings().size() > 0) {
     FX_LOGS(DEBUG) << "Loop done, expecting session terminating";
     SessionState state = GetSessionState();
+    if (state == SessionState::kStopping) {
+      // Terminate issued without stop will implicitly call stop first
+      // Loop will exit for the transition to kStopping. Mark providers
+      // as stopped and run the loop twice to transition to kStopped,
+      // then kTerminating
+      MarkAllProvidersStopped();
+      RunLoopUntilIdle();
+    }
+    state = GetSessionState();
     EXPECT_EQ(state, SessionState::kTerminating);
     if (state != SessionState::kTerminating) {
       return false;