Repository: mesos
Updated Branches:
  refs/heads/master 72c5ee225 -> eeafd0425


Added a common HTTP request log helper function.

Consolidated the master and agent HTTP request log helper
functions into common code.

Review: https://reviews.apache.org/r/58185/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/7963c53f
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/7963c53f
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/7963c53f

Branch: refs/heads/master
Commit: 7963c53fa8f89e57dcda71b456b5123645eb0a9d
Parents: 72c5ee2
Author: James Peach <[email protected]>
Authored: Thu Apr 6 13:11:23 2017 -0700
Committer: Anand Mazumdar <[email protected]>
Committed: Thu Apr 6 13:13:08 2017 -0700

----------------------------------------------------------------------
 src/common/http.cpp   | 18 ++++++++++++++++++
 src/common/http.hpp   |  5 +++++
 src/master/http.cpp   | 18 ------------------
 src/master/master.cpp | 47 +++++++++++++++++++++++-----------------------
 src/master/master.hpp |  4 ----
 src/slave/http.cpp    | 18 ------------------
 src/slave/slave.cpp   | 10 +++++-----
 src/slave/slave.hpp   |  4 ----
 8 files changed, 52 insertions(+), 72 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/7963c53f/src/common/http.cpp
----------------------------------------------------------------------
diff --git a/src/common/http.cpp b/src/common/http.cpp
index 89133e0..92f0636 100644
--- a/src/common/http.cpp
+++ b/src/common/http.cpp
@@ -1106,4 +1106,22 @@ Try<Nothing> initializeHttpAuthenticators(
   return Nothing();
 }
 
+
+void logRequest(const process::http::Request& request)
+{
+  Option<string> userAgent = request.headers.get("User-Agent");
+  Option<string> forwardedFor = request.headers.get("X-Forwarded-For");
+
+  LOG(INFO) << "HTTP " << request.method << " for " << request.url
+            << (request.client.isSome()
+                ? " from " + stringify(request.client.get())
+                : "")
+            << (userAgent.isSome()
+                ? " with User-Agent='" + userAgent.get() + "'"
+                : "")
+            << (forwardedFor.isSome()
+                ? " with X-Forwarded-For='" + forwardedFor.get() + "'"
+                : "");
+}
+
 }  // namespace mesos {

http://git-wip-us.apache.org/repos/asf/mesos/blob/7963c53f/src/common/http.hpp
----------------------------------------------------------------------
diff --git a/src/common/http.hpp b/src/common/http.hpp
index b6e61f7..93d6088 100644
--- a/src/common/http.hpp
+++ b/src/common/http.hpp
@@ -223,6 +223,11 @@ Try<Nothing> initializeHttpAuthenticators(
     const Option<Credentials>& credentials = None(),
     const Option<std::string>& secretKey = None());
 
+
+// Logs the request. Route handlers can compose this with the
+// desired request handler to get consistent request logging.
+void logRequest(const process::http::Request& request);
+
 } // namespace mesos {
 
 #endif // __COMMON_HTTP_HPP__

http://git-wip-us.apache.org/repos/asf/mesos/blob/7963c53f/src/master/http.cpp
----------------------------------------------------------------------
diff --git a/src/master/http.cpp b/src/master/http.cpp
index 6cf9d35..0b3c501 100644
--- a/src/master/http.cpp
+++ b/src/master/http.cpp
@@ -432,24 +432,6 @@ static void json(JSON::ObjectWriter* writer, const 
Summary<Framework>& summary)
 }
 
 
-void Master::Http::log(const Request& request)
-{
-  Option<string> userAgent = request.headers.get("User-Agent");
-  Option<string> forwardedFor = request.headers.get("X-Forwarded-For");
-
-  LOG(INFO) << "HTTP " << request.method << " for " << request.url.path
-            << (request.client.isSome()
-                ? " from " + stringify(request.client.get())
-                : "")
-            << (userAgent.isSome()
-                ? " with User-Agent='" + userAgent.get() + "'"
-                : "")
-            << (forwardedFor.isSome()
-                ? " with X-Forwarded-For='" + forwardedFor.get() + "'"
-                : "");
-}
-
-
 string Master::Http::API_HELP()
 {
   return HELP(

http://git-wip-us.apache.org/repos/asf/mesos/blob/7963c53f/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 6a6a570..e547d2c 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -73,6 +73,7 @@
 #include "authentication/cram_md5/authenticator.hpp"
 
 #include "common/build.hpp"
+#include "common/http.hpp"
 #include "common/protobuf_utils.hpp"
 #include "common/status_utils.hpp"
 
@@ -910,7 +911,7 @@ void Master::initialize()
         Http::API_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.api(request, principal);
         });
   route("/api/v1/scheduler",
@@ -918,7 +919,7 @@ void Master::initialize()
         Http::SCHEDULER_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.scheduler(request, principal);
         });
   route("/create-volumes",
@@ -926,7 +927,7 @@ void Master::initialize()
         Http::CREATE_VOLUMES_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.createVolumes(request, principal);
         });
   route("/destroy-volumes",
@@ -934,7 +935,7 @@ void Master::initialize()
         Http::DESTROY_VOLUMES_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.destroyVolumes(request, principal);
         });
   route("/frameworks",
@@ -942,7 +943,7 @@ void Master::initialize()
         Http::FRAMEWORKS_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.frameworks(request, principal);
         });
   route("/flags",
@@ -950,7 +951,7 @@ void Master::initialize()
         Http::FLAGS_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.flags(request, principal);
         });
   route("/health",
@@ -968,7 +969,7 @@ void Master::initialize()
         Http::RESERVE_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.reserve(request, principal);
         });
   // TODO(ijimenez): Remove this endpoint at the end of the
@@ -978,7 +979,7 @@ void Master::initialize()
         Http::ROLES_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.roles(request, principal);
         });
   route("/roles",
@@ -986,7 +987,7 @@ void Master::initialize()
         Http::ROLES_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.roles(request, principal);
         });
   route("/teardown",
@@ -994,7 +995,7 @@ void Master::initialize()
         Http::TEARDOWN_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.teardown(request, principal);
         });
   route("/slaves",
@@ -1002,7 +1003,7 @@ void Master::initialize()
         Http::SLAVES_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.slaves(request, principal);
         });
   // TODO(ijimenez): Remove this endpoint at the end of the
@@ -1012,7 +1013,7 @@ void Master::initialize()
         Http::STATE_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.state(request, principal);
         });
   route("/state",
@@ -1020,7 +1021,7 @@ void Master::initialize()
         Http::STATE_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.state(request, principal);
         });
   route("/state-summary",
@@ -1028,7 +1029,7 @@ void Master::initialize()
         Http::STATESUMMARY_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.stateSummary(request, principal);
         });
   // TODO(ijimenez): Remove this endpoint at the end of the
@@ -1038,7 +1039,7 @@ void Master::initialize()
         Http::TASKS_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.tasks(request, principal);
         });
   route("/tasks",
@@ -1046,7 +1047,7 @@ void Master::initialize()
         Http::TASKS_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.tasks(request, principal);
         });
   route("/maintenance/schedule",
@@ -1054,7 +1055,7 @@ void Master::initialize()
         Http::MAINTENANCE_SCHEDULE_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.maintenanceSchedule(request, principal);
         });
   route("/maintenance/status",
@@ -1062,7 +1063,7 @@ void Master::initialize()
         Http::MAINTENANCE_STATUS_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.maintenanceStatus(request, principal);
         });
   route("/machine/down",
@@ -1070,7 +1071,7 @@ void Master::initialize()
         Http::MACHINE_DOWN_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.machineDown(request, principal);
         });
   route("/machine/up",
@@ -1078,7 +1079,7 @@ void Master::initialize()
         Http::MACHINE_UP_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.machineUp(request, principal);
         });
   route("/unreserve",
@@ -1086,7 +1087,7 @@ void Master::initialize()
         Http::UNRESERVE_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.unreserve(request, principal);
         });
   route("/quota",
@@ -1094,7 +1095,7 @@ void Master::initialize()
         Http::QUOTA_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.quota(request, principal);
         });
   route("/weights",
@@ -1102,7 +1103,7 @@ void Master::initialize()
         Http::WEIGHTS_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.weights(request, principal);
         });
 

http://git-wip-us.apache.org/repos/asf/mesos/blob/7963c53f/src/master/master.hpp
----------------------------------------------------------------------
diff --git a/src/master/master.hpp b/src/master/master.hpp
index 1b07742..d537933 100644
--- a/src/master/master.hpp
+++ b/src/master/master.hpp
@@ -1164,10 +1164,6 @@ private:
                                      quotaHandler(_master),
                                      weightsHandler(_master) {}
 
-    // Logs the request, route handlers can compose this with the
-    // desired request handler to get consistent request logging.
-    static void log(const process::http::Request& request);
-
     // /api/v1
     process::Future<process::http::Response> api(
         const process::http::Request& request,

http://git-wip-us.apache.org/repos/asf/mesos/blob/7963c53f/src/slave/http.cpp
----------------------------------------------------------------------
diff --git a/src/slave/http.cpp b/src/slave/http.cpp
index e253ce9..b07ce7c 100644
--- a/src/slave/http.cpp
+++ b/src/slave/http.cpp
@@ -323,24 +323,6 @@ struct FrameworkWriter
 };
 
 
-void Slave::Http::log(const Request& request)
-{
-  Option<string> userAgent = request.headers.get("User-Agent");
-  Option<string> forwardedFor = request.headers.get("X-Forwarded-For");
-
-  LOG(INFO) << "HTTP " << request.method << " for " << request.url.path
-            << (request.client.isSome()
-                ? " from " + stringify(request.client.get())
-                : "")
-            << (userAgent.isSome()
-                ? " with User-Agent='" + userAgent.get() + "'"
-                : "")
-            << (forwardedFor.isSome()
-                ? " with X-Forwarded-For='" + forwardedFor.get() + "'"
-                : "");
-}
-
-
 string Slave::Http::API_HELP()
 {
   return HELP(

http://git-wip-us.apache.org/repos/asf/mesos/blob/7963c53f/src/slave/slave.cpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.cpp b/src/slave/slave.cpp
index 65e4a67..f484513 100644
--- a/src/slave/slave.cpp
+++ b/src/slave/slave.cpp
@@ -620,7 +620,7 @@ void Slave::initialize()
         Http::API_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.api(request, principal);
         },
         options);
@@ -630,7 +630,7 @@ void Slave::initialize()
         Http::EXECUTOR_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.executor(request, principal);
         });
 
@@ -641,7 +641,7 @@ void Slave::initialize()
         Http::STATE_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.state(request, principal);
         });
   route("/state",
@@ -649,7 +649,7 @@ void Slave::initialize()
         Http::STATE_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.state(request, principal);
         });
   route("/flags",
@@ -657,7 +657,7 @@ void Slave::initialize()
         Http::FLAGS_HELP(),
         [this](const process::http::Request& request,
                const Option<Principal>& principal) {
-          Http::log(request);
+          logRequest(request);
           return http.flags(request, principal);
         });
   route("/health",

http://git-wip-us.apache.org/repos/asf/mesos/blob/7963c53f/src/slave/slave.hpp
----------------------------------------------------------------------
diff --git a/src/slave/slave.hpp b/src/slave/slave.hpp
index e4f46d4..b26bdf8 100644
--- a/src/slave/slave.hpp
+++ b/src/slave/slave.hpp
@@ -505,10 +505,6 @@ private:
       : slave(_slave),
         statisticsLimiter(new process::RateLimiter(2, Seconds(1))) {}
 
-    // Logs the request, route handlers can compose this with the
-    // desired request handler to get consistent request logging.
-    static void log(const process::http::Request& request);
-
     // /api/v1
     process::Future<process::http::Response> api(
         const process::http::Request& request,

Reply via email to