From 01e91dc260a737376701a4908af04b67ff2791cf Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 1 Jul 2020 06:23:23 -0700
Subject: [PATCH] Clean up some service profiler behavior in Conduit futures

Summary:
Correct two minor Conduit future issues:

  - FutureAgent shows up in the trace log as "<mystery>". Since it isn't doing anything useful, solve the mystery and drop it from the log.
  - Simply the ConduitFuture code for interacting with the service profiler now that a more structured integration is available.

Test Plan: Ran "arc branches --trace", saw conduit calls and no more "<mystery>" clutter.

Differential Revision: https://secure.phabricator.com/D21388
---
 src/conduit/ConduitFuture.php | 29 +++++++++--------------------
 src/conduit/FutureAgent.php   |  7 +++++++
 src/future/Future.php         |  4 ++++
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/src/conduit/ConduitFuture.php b/src/conduit/ConduitFuture.php
index 20c6906a..351d2d11 100644
--- a/src/conduit/ConduitFuture.php
+++ b/src/conduit/ConduitFuture.php
@@ -5,7 +5,6 @@ final class ConduitFuture extends FutureProxy {
   private $client;
   private $engine;
   private $conduitMethod;
-  private $profilerCallID;
 
   public function setClient(ConduitClient $client, $method) {
     $this->client = $client;
@@ -13,29 +12,19 @@ final class ConduitFuture extends FutureProxy {
     return $this;
   }
 
-  public function isReady() {
-    if ($this->profilerCallID === null) {
-      $profiler = PhutilServiceProfiler::getInstance();
+  protected function getServiceProfilerStartParameters() {
+    return array(
+      'type' => 'conduit',
+      'method' => $this->conduitMethod,
+      'size' => $this->getProxiedFuture()->getHTTPRequestByteLength(),
+    );
+  }
 
-      $this->profilerCallID = $profiler->beginServiceCall(
-        array(
-          'type'    => 'conduit',
-          'method'  => $this->conduitMethod,
-          'size'    => $this->getProxiedFuture()->getHTTPRequestByteLength(),
-        ));
-    }
-
-    return parent::isReady();
+  protected function getServiceProfilerResultParameters() {
+    return array();
   }
 
   protected function didReceiveResult($result) {
-    if ($this->profilerCallID !== null) {
-      $profiler = PhutilServiceProfiler::getInstance();
-      $profiler->endServiceCall(
-        $this->profilerCallID,
-        array());
-    }
-
     list($status, $body, $headers) = $result;
     if ($status->isError()) {
       throw $status;
diff --git a/src/conduit/FutureAgent.php b/src/conduit/FutureAgent.php
index 6000c6b2..3b84ab88 100644
--- a/src/conduit/FutureAgent.php
+++ b/src/conduit/FutureAgent.php
@@ -35,4 +35,11 @@ abstract class FutureAgent
     return $sockets;
   }
 
+  protected function getServiceProfilerStartParameters() {
+    // At least today, the agent construct doesn't add anything interesting
+    // to the trace and the underlying futures always show up in the trace
+    // themselves.
+    return null;
+  }
+
 }
diff --git a/src/future/Future.php b/src/future/Future.php
index 1e52c48a..8078aacc 100644
--- a/src/future/Future.php
+++ b/src/future/Future.php
@@ -115,6 +115,10 @@ abstract class Future extends Phobject {
 
     $params = $this->getServiceProfilerStartParameters();
 
+    if ($params === null) {
+      return;
+    }
+
     $profiler = PhutilServiceProfiler::getInstance();
     $call_id = $profiler->beginServiceCall($params);