Integrate "ServiceProfiler" into the base "Future"

Summary:
Depends on D21036. Ref T11968. Ref T13177. Currently, each Future integrates separately with ServiceProfiler, but much of the code is similar.

Move integration to the base class and lift up most of the implementation details.

Test Plan: Ran `arc diff --trace`, saw sensible output.

Maniphest Tasks: T13177, T11968

Differential Revision: https://secure.phabricator.com/D21038

src/future/Future.php

@@ -13,6 +13,7 @@ abstract class Future extends Phobject {
   private $result;
   private $exception;
   private $futureKey;
+  private $serviceProfilerCallID;
 
   /**
    * Is this future's process complete? Specifically, can this future be
@@ -60,6 +61,7 @@ abstract class Future extends Phobject {
     }
     $this->hasStarted = true;
 
+    $this->startServiceProfiler();
     $this->isReady();
   }
 
@@ -96,8 +98,49 @@ abstract class Future extends Phobject {
           'than once.'));
     }
     $this->hasEnded = true;
+
+    $this->endServiceProfiler();
   }
 
+  private function startServiceProfiler() {
+
+    // NOTE: This is a soft dependency so that we don't need to build the
+    // ServiceProfiler into the Phage agent. Normally, this class is always
+    // available.
+
+    if (!class_exists('PhutilServiceProfiler')) {
+      return;
+    }
+
+    $params = $this->getServiceProfilerStartParameters();
+
+    $profiler = PhutilServiceProfiler::getInstance();
+    $call_id = $profiler->beginServiceCall($params);
+
+    $this->serviceProfilerCallID = $call_id;
+  }
+
+  private function endServiceProfiler() {
+    $call_id = $this->serviceProfilerCallID;
+    if ($call_id === null) {
+      return;
+    }
+
+    $params = $this->getServiceProfilerResultParameters();
+
+    $profiler = PhutilServiceProfiler::getInstance();
+    $profiler->endServiceCall($call_id, $params);
+  }
+
+  protected function getServiceProfilerStartParameters() {
+    return array();
+  }
+
+  protected function getServiceProfilerResultParameters() {
+    return array();
+  }
+
+
   /**
    * Retrieve a list of sockets which we can wait to become readable while
    * a future is resolving. If your future has sockets which can be

src/future/FutureProxy.php

@@ -63,6 +63,14 @@ abstract class FutureProxy extends Future {
     return $this;
   }
 
+  protected function getServiceProfilerStartParameters() {
+    return $this->getProxiedFuture()->getServiceProfilerStartParameters();
+  }
+
+  protected function getServiceProfilerResultParameters() {
+    return $this->getProxiedFuture()->getServiceProfilerResultParameters();
+  }
+
   abstract protected function didReceiveResult($result);
 
 }

src/future/exec/ExecFuture.php

@@ -530,24 +530,13 @@ final class ExecFuture extends PhutilExecutableFuture {
    * @task internal
    */
   public function isReady() {
-    // NOTE: We have soft dependencies on PhutilServiceProfiler and
-    // PhutilErrorTrap here. These dependencies are soft to avoid the need to
-    // build them into the Phage agent. Under normal circumstances, these
-    // classes are always available.
+    // NOTE: We have a soft dependencies on PhutilErrorTrap here, to avoid
+    // the need to build it into the Phage agent. Under normal circumstances,
+    // this class are always available.
 
     if (!$this->pipes) {
       $is_windows = phutil_is_windows();
 
-      // NOTE: See note above about Phage.
-      if (class_exists('PhutilServiceProfiler')) {
-        $profiler = PhutilServiceProfiler::getInstance();
-        $this->profilerCallID = $profiler->beginServiceCall(
-          array(
-            'type'    => 'exec',
-            'command' => phutil_string_cast($this->getCommand()),
-          ));
-      }
-
       if (!$this->start) {
         // We might already have started the timer via initiating resolution.
         $this->start = microtime(true);
@@ -843,23 +832,6 @@ final class ExecFuture extends PhutilExecutableFuture {
 
     unset($this->windowsStdoutTempFile);
     unset($this->windowsStderrTempFile);
-
-    if ($this->profilerCallID !== null) {
-      if ($this->hasResult()) {
-        $result = $this->getResult();
-        $err = idx($result, 0);
-      } else {
-        $err = null;
-      }
-
-      $profiler = PhutilServiceProfiler::getInstance();
-      $profiler->endServiceCall(
-        $this->profilerCallID,
-        array(
-          'err' => $err,
-        ));
-      $this->profilerCallID = null;
-    }
   }
 
 
@@ -958,4 +930,25 @@ final class ExecFuture extends PhutilExecutableFuture {
     }
   }
 
+  protected function getServiceProfilerStartParameters() {
+    return array(
+      'type' => 'exec',
+      'command' => phutil_string_cast($this->getCommand()),
+    );
+  }
+
+  protected function getServiceProfilerResultParameters() {
+    if ($this->hasResult()) {
+      $result = $this->getResult();
+      $err = idx($result, 0);
+    } else {
+      $err = null;
+    }
+
+    return array(
+      'err' => $err,
+    );
+  }
+
+
 }

src/future/exec/PhutilExecPassthru.php

@@ -34,14 +34,6 @@ final class PhutilExecPassthru extends PhutilExecutableFuture {
   public function execute() {
     $command = $this->getCommand();
 
-    $profiler = PhutilServiceProfiler::getInstance();
-    $call_id = $profiler->beginServiceCall(
-      array(
-        'type'    => 'exec',
-        'subtype' => 'passthru',
-        'command' => $command,
-      ));
-
     $spec  = array(STDIN, STDOUT, STDERR);
     $pipes = array();
 
@@ -85,12 +77,6 @@ final class PhutilExecPassthru extends PhutilExecutableFuture {
 
     $err = proc_close($proc);
 
-    $profiler->endServiceCall(
-      $call_id,
-      array(
-        'err' => $err,
-      ));
-
     return $err;
   }
 
@@ -111,4 +97,26 @@ final class PhutilExecPassthru extends PhutilExecutableFuture {
     return true;
   }
 
+
+
+  protected function getServiceProfilerStartParameters() {
+    return array(
+      'type' => 'exec',
+      'subtype' => 'passthru',
+      'command' => phutil_string_cast($this->getCommand()),
+    );
+  }
+
+  protected function getServiceProfilerResultParameters() {
+    if ($this->hasResult()) {
+      $err = $this->getResult();
+    } else {
+      $err = null;
+    }
+
+    return array(
+      'err' => $err,
+    );
+  }
+
 }

src/future/http/HTTPFuture.php

@@ -123,13 +123,6 @@ final class HTTPFuture extends BaseHTTPFuture {
       if (!$this->socket) {
         return $this->stateReady;
       }
-
-      $profiler = PhutilServiceProfiler::getInstance();
-      $this->profilerCallID = $profiler->beginServiceCall(
-        array(
-          'type' => 'http',
-          'uri' => $this->getURI(),
-        ));
     }
 
     if (!$this->stateConnected) {
@@ -225,9 +218,6 @@ final class HTTPFuture extends BaseHTTPFuture {
       $this->setResult($this->parseRawHTTPResponse($this->response));
     }
 
-    $profiler = PhutilServiceProfiler::getInstance();
-    $profiler->endServiceCall($this->profilerCallID, array());
-
     return true;
   }
 
@@ -303,4 +293,11 @@ final class HTTPFuture extends BaseHTTPFuture {
       $data;
   }
 
+  protected function getServiceProfilerStartParameters() {
+    return array(
+      'type' => 'http',
+      'uri' => phutil_string_cast($this->getURI()),
+    );
+  }
+
 }

src/future/http/HTTPSFuture.php

@@ -211,13 +211,9 @@ final class HTTPSFuture extends BaseHTTPFuture {
       $uri_object = new PhutilURI($uri);
       $proxy = PhutilHTTPEngineExtension::buildHTTPProxyURI($uri_object);
 
-      $profiler = PhutilServiceProfiler::getInstance();
-      $this->profilerCallID = $profiler->beginServiceCall(
-        array(
-          'type' => 'http',
-          'uri' => $uri,
-          'proxy' => (string)$proxy,
-        ));
+      // TODO: Currently, the "proxy" is not passed to the ServiceProfiler
+      // because of changes to how ServiceProfiler is integrated. It would
+      // be nice to pass it again.
 
       if (!self::$multi) {
         self::$multi = curl_multi_init();
@@ -526,9 +522,6 @@ final class HTTPSFuture extends BaseHTTPFuture {
       }
     }
 
-    $profiler = PhutilServiceProfiler::getInstance();
-    $profiler->endServiceCall($this->profilerCallID, array());
-
     return true;
   }
 
@@ -821,4 +814,11 @@ final class HTTPSFuture extends BaseHTTPFuture {
    return ($this->downloadPath !== null);
   }
 
+  protected function getServiceProfilerStartParameters() {
+    return array(
+      'type' => 'http',
+      'uri' => phutil_string_cast($this->getURI()),
+    );
+  }
+
 }