diff --git a/src/future/Future.php b/src/future/Future.php index f69f14bc..8ce48eb6 100644 --- a/src/future/Future.php +++ b/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 diff --git a/src/future/FutureProxy.php b/src/future/FutureProxy.php index 65cbffa2..77c8c5bb 100644 --- a/src/future/FutureProxy.php +++ b/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); } diff --git a/src/future/exec/ExecFuture.php b/src/future/exec/ExecFuture.php index df8ce772..afb0fd14 100644 --- a/src/future/exec/ExecFuture.php +++ b/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, + ); + } + + } diff --git a/src/future/exec/PhutilExecPassthru.php b/src/future/exec/PhutilExecPassthru.php index 8a761923..44eef97e 100644 --- a/src/future/exec/PhutilExecPassthru.php +++ b/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, + ); + } + } diff --git a/src/future/http/HTTPFuture.php b/src/future/http/HTTPFuture.php index 738cde27..91dd709a 100644 --- a/src/future/http/HTTPFuture.php +++ b/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()), + ); + } + } diff --git a/src/future/http/HTTPSFuture.php b/src/future/http/HTTPSFuture.php index f65af3e8..70804332 100644 --- a/src/future/http/HTTPSFuture.php +++ b/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()), + ); + } + }