From 59416a13e7299bef25821c6464788200a2067c8a Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 1 May 2015 13:20:30 -0700 Subject: [PATCH] Add "request time" event and viewer and context data to Multimeter Summary: Ref T6930. Only notable thing here is that I prevented non-admins from slicing down by viewing user, since it feels a little creepy to go see what pages you looked at, even though we only show which controllers you invoked. However, it feels important enough to be able to see users destorying the server with crazy requests to let admins see this data. Test Plan: {F389718} Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T6930 Differential Revision: https://secure.phabricator.com/D12630 --- .../AphrontApplicationConfiguration.php | 19 +++++- .../controller/MultimeterSampleController.php | 60 ++++++++++++++++--- .../multimeter/data/MultimeterControl.php | 4 ++ .../multimeter/storage/MultimeterEvent.php | 5 ++ 4 files changed, 77 insertions(+), 11 deletions(-) diff --git a/src/aphront/configuration/AphrontApplicationConfiguration.php b/src/aphront/configuration/AphrontApplicationConfiguration.php index f5a34ae3d6..3e7abe6a9d 100644 --- a/src/aphront/configuration/AphrontApplicationConfiguration.php +++ b/src/aphront/configuration/AphrontApplicationConfiguration.php @@ -125,7 +125,11 @@ abstract class AphrontApplicationConfiguration { $processing_exception = null; try { - $response = $application->processRequest($request, $access_log, $sink); + $response = $application->processRequest( + $request, + $access_log, + $sink, + $multimeter); $response_code = $response->getHTTPResponseCode(); } catch (Exception $ex) { $processing_exception = $ex; @@ -140,6 +144,11 @@ abstract class AphrontApplicationConfiguration { 'T' => PhabricatorStartup::getMicrosecondsSinceStart(), )); + $multimeter->newEvent( + MultimeterEvent::TYPE_REQUEST_TIME, + $multimeter->getEventContext(), + PhabricatorStartup::getMicrosecondsSinceStart()); + $access_log->write(); $multimeter->saveEvents(); @@ -171,16 +180,19 @@ abstract class AphrontApplicationConfiguration { public function processRequest( AphrontRequest $request, PhutilDeferredLog $access_log, - AphrontHTTPSink $sink) { + AphrontHTTPSink $sink, + MultimeterControl $multimeter) { $this->setRequest($request); list($controller, $uri_data) = $this->buildController(); + $controller_class = get_class($controller); $access_log->setData( array( - 'C' => get_class($controller), + 'C' => $controller_class, )); + $multimeter->setEventContext('web.'.$controller_class); $request->setURIMap($uri_data); $controller->setRequest($request); @@ -198,6 +210,7 @@ abstract class AphrontApplicationConfiguration { 'u' => $request->getUser()->getUserName(), 'P' => $request->getUser()->getPHID(), )); + $multimeter->setEventViewer('user.'.$request->getUser()->getPHID()); } if (!$response) { diff --git a/src/applications/multimeter/controller/MultimeterSampleController.php b/src/applications/multimeter/controller/MultimeterSampleController.php index 1b60022eec..62a459f75b 100644 --- a/src/applications/multimeter/controller/MultimeterSampleController.php +++ b/src/applications/multimeter/controller/MultimeterSampleController.php @@ -33,6 +33,15 @@ final class MultimeterSampleController extends MultimeterController { $with = array(); foreach ($group_map as $key => $column) { + + // Don't let non-admins filter by viewers, this feels a little too + // invasive of privacy. + if ($key == 'viewer') { + if (!$viewer->getIsAdmin()) { + continue; + } + } + $with[$key] = $request->getStrList($key); if ($with[$key]) { $where[] = qsprintf( @@ -58,6 +67,16 @@ final class MultimeterSampleController extends MultimeterController { implode(', ', array_select_keys($group_map, $group))); $this->loadDimensions($data); + $phids = array(); + foreach ($data as $row) { + $viewer_name = $this->getViewerDimension($row['eventViewerID']) + ->getName(); + $viewer_phid = $this->getEventViewerPHID($viewer_name); + if ($viewer_phid) { + $phids[] = $viewer_phid; + } + } + $handles = $viewer->loadHandles($phids); $rows = array(); foreach ($data as $row) { @@ -75,13 +94,23 @@ final class MultimeterSampleController extends MultimeterController { } if (isset($group['viewer'])) { - $viewer_col = $this->getViewerDimension($row['eventViewerID']) - ->getName(); - if (!$with['viewer']) { - $viewer_col = $this->renderSelectionLink( - 'viewer', - $row['eventViewerID'], - $viewer_col); + if ($viewer->getIsAdmin()) { + $viewer_col = $this->getViewerDimension($row['eventViewerID']) + ->getName(); + + $viewer_phid = $this->getEventViewerPHID($viewer_col); + if ($viewer_phid) { + $viewer_col = $handles[$viewer_phid]->getName(); + } + + if (!$with['viewer']) { + $viewer_col = $this->renderSelectionLink( + 'viewer', + $row['eventViewerID'], + $viewer_col); + } + } else { + $viewer_col = phutil_tag('em', array(), pht('(Masked)')); } } else { $viewer_col = $this->renderGroupingLink($group, 'viewer'); @@ -145,11 +174,17 @@ final class MultimeterSampleController extends MultimeterController { $host_col, $type_col, $label_col, + MultimeterEvent::formatResourceCost( + $viewer, + $row['eventType'], + $row['totalCost'] / $row['N']), MultimeterEvent::formatResourceCost( $viewer, $row['eventType'], $row['totalCost']), - $row['sampleRate'], + ($row['N'] == 1) + ? $row['sampleRate'] + : '-', phabricator_datetime($row['epoch'], $viewer), ); } @@ -164,6 +199,7 @@ final class MultimeterSampleController extends MultimeterController { pht('Host'), pht('Type'), pht('Label'), + pht('Avg'), pht('Cost'), pht('Rate'), pht('Epoch'), @@ -179,6 +215,7 @@ final class MultimeterSampleController extends MultimeterController { 'wide', 'n', 'n', + 'n', null, )); @@ -281,4 +318,11 @@ final class MultimeterSampleController extends MultimeterController { ); } + private function getEventViewerPHID($viewer_name) { + if (!strncmp($viewer_name, 'user.', 5)) { + return substr($viewer_name, 5); + } + return null; + } + } diff --git a/src/applications/multimeter/data/MultimeterControl.php b/src/applications/multimeter/data/MultimeterControl.php index a7838ca859..9d948609a6 100644 --- a/src/applications/multimeter/data/MultimeterControl.php +++ b/src/applications/multimeter/data/MultimeterControl.php @@ -149,6 +149,10 @@ final class MultimeterControl { return $this; } + public function getEventContext() { + return $this->eventContext; + } + public function setEventViewer($viewer) { $this->eventViewer = $viewer; return $this; diff --git a/src/applications/multimeter/storage/MultimeterEvent.php b/src/applications/multimeter/storage/MultimeterEvent.php index 1f7c27cbaa..23263a6e8d 100644 --- a/src/applications/multimeter/storage/MultimeterEvent.php +++ b/src/applications/multimeter/storage/MultimeterEvent.php @@ -3,6 +3,7 @@ final class MultimeterEvent extends MultimeterDAO { const TYPE_STATIC_RESOURCE = 0; + const TYPE_REQUEST_TIME = 1; protected $eventType; protected $eventLabelID; @@ -29,6 +30,8 @@ final class MultimeterEvent extends MultimeterDAO { switch ($type) { case self::TYPE_STATIC_RESOURCE: return pht('Static Resource'); + case self::TYPE_REQUEST_TIME: + return pht('Web Request'); } return pht('Unknown ("%s")', $type); @@ -42,6 +45,8 @@ final class MultimeterEvent extends MultimeterDAO { switch ($type) { case self::TYPE_STATIC_RESOURCE: return pht('%s Req', new PhutilNumber($cost)); + case self::TYPE_REQUEST_TIME: + return pht('%s us', new PhutilNumber($cost)); } return pht('%s Unit(s)', new PhutilNumber($cost));