Merge lp:~alan-griffiths/mir/frontend-ConnectorReport-enablement into lp:mir

[Alan Griffiths]

frontend, logging: Provide reporting from frontend::Connector

frontend, logging: Provide reporting from frontend::Connector

NB adding data members to DefaultServerConfiguration breaks ABI

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1097
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/51/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2169
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2054
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/51
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/51/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-team-mir-development-branch-ci/51/rebuild

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

I think the coupling created by this style of reporting is excessive, and bad design. But we've discussed that already.

review: Abstain

[Alexandros Frantzis (afrantzis)]

183 +// else if (opt == lttng_opt_value)
184 +// {
185 +// return std::make_shared<mir::lttng::ConnectorReport>();
186 +// }

We can add this back when we implement an LTTng connector report.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1098
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/54/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2179
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2064
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/54/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-team-mir-development-branch-ci/54/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1099
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/56/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2181
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2066
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/56
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/56/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-team-mir-development-branch-ci/56/rebuild

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

422 + ss << "Starting " << count << ") ended.";

ss << "Starting " << count << " threads.";

457 +ss << "thread (" << std::this_thread::get_id() << ") Error: " << boost::diagnostic_information(error) << std::endl;

Do we need the endl?

review: Needs Fixing

[Robert Carr (robertcarr)]

LGTM.

>>
+ ss << "thread (" << std::this_thread::get_id() << ") started.";
>>

These might all be easier to read in actual output if instances of "Thread" were capitalized

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1100
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/59/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2184
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/2069
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/59
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-saucy-amd64-ci/59/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-team-mir-development-branch-ci/59/rebuild

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

Looks good.

review: Approve

=== modified file 'include/server/mir/default_server_configuration.h'
--- include/server/mir/default_server_configuration.h	2013-09-24 11:43:27 +0000
+++ include/server/mir/default_server_configuration.h	2013-09-30 14:16:13 +0000
@@ -42,6 +42,7 @@
 {
 class Shell;
 class Connector;
+class ConnectorReport;
 class ProtobufIpcFactory;
 class SessionCreator;
 class SessionMediatorReport;
@@ -173,6 +174,7 @@
      * internal dependencies of frontend
      *  @{ */
     virtual std::shared_ptr<frontend::SessionCreator>         the_session_creator();
+    virtual std::shared_ptr<frontend::ConnectorReport>        the_connector_report();
     /** @} */
     /** @} */
 
@@ -267,6 +269,7 @@
     CachedPtr<graphics::GraphicBufferAllocator> buffer_allocator;
     CachedPtr<graphics::Display>      display;
 
+    CachedPtr<frontend::ConnectorReport>   connector_report;
     CachedPtr<frontend::ProtobufIpcFactory>  ipc_factory;
     CachedPtr<frontend::SessionMediatorReport> session_mediator_report;
     CachedPtr<frontend::MessageProcessorReport> message_processor_report;

=== modified file 'include/server/mir/frontend/connector_report.h'
--- include/server/mir/frontend/connector_report.h	2013-09-24 16:20:17 +0000
+++ include/server/mir/frontend/connector_report.h	2013-09-30 14:16:13 +0000
@@ -20,6 +20,7 @@
 #define MIR_FRONTEND_CONNECTOR_REPORT_H_
 
 #include <stdexcept>
+#include <string>
 
 namespace mir
 {
@@ -30,6 +31,16 @@
 {
 public:
 
+    virtual void thread_start() = 0;
+    virtual void thread_end() = 0;
+    virtual void starting_threads(int count) = 0;
+    virtual void stopping_threads(int count) = 0;
+
+    virtual void creating_session_for(int socket_handle) = 0;
+    virtual void creating_socket_pair(int server_handle, int client_handle) = 0;
+
+    virtual void listening_on(std::string const& endpoint) = 0;
+
     virtual void error(std::exception const& error) = 0;
 
 protected:
@@ -42,8 +53,17 @@
 class NullConnectorReport : public ConnectorReport
 {
 public:
-
-    void error(std::exception const& error);
+    void thread_start() override;
+    void thread_end() override;
+    void starting_threads(int count) override;
+    void stopping_threads(int count) override;
+
+    void creating_session_for(int socket_handle) override;
+    void creating_socket_pair(int server_handle, int client_handle) override;
+
+    void listening_on(std::string const& endpoint) override;
+
+    void error(std::exception const& error) override;
 };
 }
 }

=== added file 'include/server/mir/logging/connector_report.h'
--- include/server/mir/logging/connector_report.h	1970-01-01 00:00:00 +0000
+++ include/server/mir/logging/connector_report.h	2013-09-30 14:16:13 +0000
@@ -0,0 +1,56 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Alan Griffiths <[e-mail]>
+ */
+
+
+#ifndef MIR_LOGGING_CONNECTOR_REPORT_H_
+#define MIR_LOGGING_CONNECTOR_REPORT_H_
+
+#include "mir/frontend/connector_report.h"
+
+#include <memory>
+
+namespace mir
+{
+namespace logging
+{
+class Logger;
+
+class ConnectorReport : public frontend::ConnectorReport
+{
+public:
+    ConnectorReport(std::shared_ptr<Logger> const& log);
+
+    void thread_start() override;
+    void thread_end() override;
+    void starting_threads(int count) override;
+    void stopping_threads(int count) override;
+
+    void creating_session_for(int socket_handle) override;
+    void creating_socket_pair(int server_handle, int client_handle) override;
+
+    void listening_on(std::string const& endpoint) override;
+
+    void error(std::exception const& error) override;
+
+private:
+    std::shared_ptr<Logger> const logger;
+};
+}
+}
+
+#endif /* MIR_LOGGING_CONNECTOR_REPORT_H_ */

=== modified file 'src/server/default_server_configuration.cpp'
--- src/server/default_server_configuration.cpp	2013-09-24 17:25:47 +0000
+++ src/server/default_server_configuration.cpp	2013-09-30 14:16:13 +0000
@@ -68,6 +68,7 @@
 #include "mir/input/vt_filter.h"
 #include "mir/input/android/default_android_input_configuration.h"
 #include "mir/input/input_manager.h"
+#include "mir/logging/connector_report.h"
 #include "mir/logging/logger.h"
 #include "mir/logging/input_report.h"
 #include "mir/logging/dumb_console_logger.h"
@@ -176,6 +177,7 @@
 char const* const msg_processor_report_opt    = "msg-processor-report";
 char const* const display_report_opt          = "display-report";
 char const* const legacy_input_report_opt     = "legacy-input-report";
+char const* const connector_report_opt        = "connector-report";
 char const* const input_report_opt            = "input-report";
 char const* const host_socket_opt             = "host-socket";
 char const* const standalone_opt              = "standalone";
@@ -261,6 +263,8 @@
             "Library to use for platform graphics support [default=libmirplatformgraphics.so]")
         ("enable-input,i", po::value<bool>(),
             "Enable input. [bool:default=true]")
+        (connector_report_opt, po::value<std::string>(),
+            "How to handle the Connector report. [{log,off}:default=off]")
         (display_report_opt, po::value<std::string>(),
             "How to handle the Display report. [{log,off}:default=off]")
         (input_report_opt, po::value<std::string>(),
@@ -1019,6 +1023,30 @@
         });
 }
 
+auto mir::DefaultServerConfiguration::the_connector_report()
+    -> std::shared_ptr<mf::ConnectorReport>
+{
+    return connector_report([this]
+        () -> std::shared_ptr<mf::ConnectorReport>
+        {
+            auto opt = the_options()->get(connector_report_opt, off_opt_value);
+
+            if (opt == log_opt_value)
+            {
+                return std::make_shared<ml::ConnectorReport>(the_logger());
+            }
+            else if (opt == off_opt_value)
+            {
+                return std::make_shared<mf::NullConnectorReport>();
+            }
+            else
+            {
+                throw AbnormalExit(std::string("Invalid ") + connector_report_opt + " option: " + opt +
+                    " (valid options are: \"" + off_opt_value + "\" and \"" + log_opt_value + "\")");
+            }
+        });
+}
+
 namespace
 {
 mir::SharedLibrary const* load_library(std::string const& libname)

=== modified file 'src/server/frontend/default_configuration.cpp'
--- src/server/frontend/default_configuration.cpp	2013-09-25 14:18:18 +0000
+++ src/server/frontend/default_configuration.cpp	2013-09-30 14:16:13 +0000
@@ -24,7 +24,6 @@
 #include "mir/shell/session_container.h"
 #include "mir/shell/session.h"
 #include "published_socket_connector.h"
-#include "mir/frontend/connector_report.h"
 
 namespace mf = mir::frontend;
 namespace msh = mir::shell;
@@ -67,7 +66,7 @@
                     the_session_creator(),
                     threads,
                     force_requests_to_complete,
-                    std::make_shared<mf::NullConnectorReport>());
+                    the_connector_report());
             }
             else
             {
@@ -76,7 +75,7 @@
                     the_session_creator(),
                     threads,
                     force_requests_to_complete,
-                    std::make_shared<mf::NullConnectorReport>());
+                    the_connector_report());
             }
         });
 }

=== modified file 'src/server/frontend/published_socket_connector.cpp'
--- src/server/frontend/published_socket_connector.cpp	2013-09-24 16:20:17 +0000
+++ src/server/frontend/published_socket_connector.cpp	2013-09-30 14:16:13 +0000
@@ -51,6 +51,8 @@
 
 void mf::PublishedSocketConnector::start_accept()
 {
+    report->listening_on(socket_file);
+
     auto socket = std::make_shared<boost::asio::local::stream_protocol::socket>(io_service);
 
     acceptor.async_accept(
@@ -78,9 +80,9 @@
     int threads,
     std::function<void()> const& force_requests_to_complete,
     std::shared_ptr<ConnectorReport> const& report)
-:   io_service_threads(threads),
+:   report(report),
+    io_service_threads(threads),
     force_requests_to_complete(force_requests_to_complete),
-    report(report),
     session_creator{session_creator}
 {
 }
@@ -92,7 +94,9 @@
         while (true)
         try
         {
+            report->thread_start();
             io_service.run();
+            report->thread_end();
             return;
         }
         catch (std::exception const& e)
@@ -101,6 +105,7 @@
         }
     };
 
+    report->starting_threads(io_service_threads.size());
     for (auto& thread : io_service_threads)
     {
         thread = std::thread(run_io_service);
@@ -118,6 +123,8 @@
      */
     force_requests_to_complete();
 
+    report->stopping_threads(io_service_threads.size());
+
     /* Wait for all io processing threads to finish */
     for (auto& thread : io_service_threads)
     {
@@ -133,6 +140,7 @@
 
 void mf::BasicConnector::create_session_for(std::shared_ptr<boost::asio::local::stream_protocol::socket> const& server_socket) const
 {
+    report->creating_session_for(server_socket->native_handle());
     session_creator->create_session_for(server_socket);
 }
 
@@ -151,6 +159,8 @@
     auto const server_socket = std::make_shared<boost::asio::local::stream_protocol::socket>(
         io_service, boost::asio::local::stream_protocol(), socket_fd[server]);
 
+    report->creating_socket_pair(socket_fd[server], socket_fd[client]);
+
     create_session_for(server_socket);
 
     return socket_fd[client];
@@ -161,6 +171,34 @@
     stop();
 }
 
+void mf::NullConnectorReport::thread_start()
+{
+}
+
+void mf::NullConnectorReport::thread_end()
+{
+}
+
+void mf::NullConnectorReport::starting_threads(int /*count*/)
+{
+}
+
+void mf::NullConnectorReport::stopping_threads(int /*count*/)
+{
+}
+
+void mf::NullConnectorReport::creating_session_for(int /*socket_handle*/)
+{
+}
+
+void mf::NullConnectorReport::creating_socket_pair(int /*server_handle*/, int /*client_handle*/)
+{
+}
+
+void mf::NullConnectorReport::listening_on(std::string const& /*endpoint*/)
+{
+}
+
 void mf::NullConnectorReport::error(std::exception const& /*error*/)
 {
 }

=== modified file 'src/server/frontend/published_socket_connector.h'
--- src/server/frontend/published_socket_connector.h	2013-09-25 10:16:44 +0000
+++ src/server/frontend/published_socket_connector.h	2013-09-30 14:16:13 +0000
@@ -60,11 +60,11 @@
 protected:
     void create_session_for(std::shared_ptr<boost::asio::local::stream_protocol::socket> const& server_socket) const;
     boost::asio::io_service mutable io_service;
+    std::shared_ptr<ConnectorReport> const report;
 
 private:
     std::vector<std::thread> io_service_threads;
     std::function<void()> const force_requests_to_complete;
-    std::shared_ptr<ConnectorReport> const report;
     std::shared_ptr<SessionCreator> const session_creator;
 };
 

=== modified file 'src/server/logging/CMakeLists.txt'
--- src/server/logging/CMakeLists.txt	2013-08-28 03:41:48 +0000
+++ src/server/logging/CMakeLists.txt	2013-09-30 14:16:13 +0000
@@ -1,6 +1,7 @@
 set(
   LOGGING_SOURCES
 
+  connector_report.cpp
   glog_logger.cpp
   session_mediator_report.cpp
   message_processor_report.cpp

=== added file 'src/server/logging/connector_report.cpp'
--- src/server/logging/connector_report.cpp	1970-01-01 00:00:00 +0000
+++ src/server/logging/connector_report.cpp	2013-09-30 14:16:13 +0000
@@ -0,0 +1,98 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Alan Griffiths <[e-mail]>
+ */
+
+#include "mir/logging/connector_report.h"
+
+#include "mir/logging/logger.h"
+
+#include <boost/exception/diagnostic_information.hpp>
+
+#include <sstream>
+#include <thread>
+
+namespace ml = mir::logging;
+
+namespace
+{
+char const* const component = "frontend::SessionMediator";
+}
+
+ml::ConnectorReport::ConnectorReport(std::shared_ptr<Logger> const& log) :
+    logger(log)
+{
+}
+
+void ml::ConnectorReport::thread_start()
+{
+    std::stringstream ss;
+    ss << "thread (" << std::this_thread::get_id() << ") started.";
+    logger->log<Logger::informational>(ss.str(), component);
+}
+
+void ml::ConnectorReport::thread_end()
+{
+    std::stringstream ss;
+    ss << "thread (" << std::this_thread::get_id() << ") ended.";
+    logger->log<Logger::informational>(ss.str(), component);
+}
+
+void ml::ConnectorReport::starting_threads(int count)
+{
+    std::stringstream ss;
+    ss << "Starting " << count << " threads.";
+    logger->log<Logger::informational>(ss.str(), component);
+}
+
+void ml::ConnectorReport::stopping_threads(int count)
+{
+    std::stringstream ss;
+    ss << "Stopping " << count << " threads.";
+    logger->log<Logger::informational>(ss.str(), component);
+}
+
+void ml::ConnectorReport::creating_session_for(int socket_handle)
+{
+    std::stringstream ss;
+    ss << "thread (" << std::this_thread::get_id() << ") Creating session for socket " << socket_handle;
+    logger->log<Logger::informational>(ss.str(), component);
+}
+
+void ml::ConnectorReport::creating_socket_pair(int server_handle, int client_handle)
+{
+    std::stringstream ss;
+    ss << "thread (" << std::this_thread::get_id() << ") Creating socket pair (server=" << server_handle << ", client=" << client_handle << ").";
+    logger->log<Logger::informational>(ss.str(), component);
+}
+
+void ml::ConnectorReport::listening_on(std::string const& endpoint)
+{
+    std::stringstream ss;
+    ss << "Listening on endpoint: " << endpoint;
+    logger->log<Logger::informational>(ss.str(), component);
+}
+
+void ml::ConnectorReport::error(std::exception const& error)
+{
+    std::stringstream ss;
+    ss << "thread (" << std::this_thread::get_id() << ") Error: " << boost::diagnostic_information(error);
+
+    logger->log<ml::Logger::warning>(ss.str(), component);
+}
+
+
+

=== modified file 'tests/unit-tests/frontend/test_published_socket_connector.cpp'
--- tests/unit-tests/frontend/test_published_socket_connector.cpp	2013-09-25 10:16:44 +0000
+++ tests/unit-tests/frontend/test_published_socket_connector.cpp	2013-09-30 14:16:13 +0000
@@ -46,11 +46,21 @@
 
 namespace
 {
-class MockCommunicatorReport : public mf::ConnectorReport
+class MockConnectorReport : public mf::ConnectorReport
 {
 public:
 
-    ~MockCommunicatorReport() noexcept {}
+    ~MockConnectorReport() noexcept {}
+
+    MOCK_METHOD0(thread_start, void ());
+    MOCK_METHOD0(thread_end, void());
+    MOCK_METHOD1(starting_threads, void (int count));
+    MOCK_METHOD1(stopping_threads, void(int count));
+
+    MOCK_METHOD1(creating_session_for, void(int socket_handle));
+    MOCK_METHOD2(creating_socket_pair, void(int server_handle, int client_handle));
+
+    MOCK_METHOD1(listening_on, void(std::string const& endpoint));
 
     MOCK_METHOD1(error, void (std::exception const& error));
 };
@@ -64,7 +74,7 @@
 
     void SetUp()
     {
-        communicator_report = std::make_shared<MockCommunicatorReport>();
+        communicator_report = std::make_shared<MockConnectorReport>();
         stub_server_tool = std::make_shared<mt::StubServerTool>();
         stub_server = std::make_shared<mt::TestProtobufServer>(
             "./test_socket",
@@ -94,13 +104,13 @@
     }
 
     std::shared_ptr<mt::TestProtobufClient> client;
-    static std::shared_ptr<MockCommunicatorReport> communicator_report;
+    static std::shared_ptr<MockConnectorReport> communicator_report;
     static std::shared_ptr<mt::StubServerTool> stub_server_tool;
     static std::shared_ptr<mt::TestProtobufServer> stub_server;
 };
 
 std::shared_ptr<mt::StubServerTool> PublishedSocketConnector::stub_server_tool;
-std::shared_ptr<MockCommunicatorReport> PublishedSocketConnector::communicator_report;
+std::shared_ptr<MockConnectorReport> PublishedSocketConnector::communicator_report;
 std::shared_ptr<mt::TestProtobufServer> PublishedSocketConnector::stub_server;
 
 TEST_F(PublishedSocketConnector, create_surface_results_in_a_callback)