Mir

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

Proposed by Alan Griffiths
Status: Merged
Approved by: Alexandros Frantzis
Approved revision: 1100
Merged at revision: 1096
Proposed branch: lp:~alan-griffiths/mir/frontend-ConnectorReport-enablement
Merge into: lp:mir
Diff against target: 516 lines (+266/-13)
10 files modified
include/server/mir/default_server_configuration.h (+3/-0)
include/server/mir/frontend/connector_report.h (+22/-2)
include/server/mir/logging/connector_report.h (+56/-0)
src/server/default_server_configuration.cpp (+28/-0)
src/server/frontend/default_configuration.cpp (+2/-3)
src/server/frontend/published_socket_connector.cpp (+40/-2)
src/server/frontend/published_socket_connector.h (+1/-1)
src/server/logging/CMakeLists.txt (+1/-0)
src/server/logging/connector_report.cpp (+98/-0)
tests/unit-tests/frontend/test_published_socket_connector.cpp (+15/-5)
To merge this branch: bzr merge lp:~alan-griffiths/mir/frontend-ConnectorReport-enablement
Reviewer Review Type Date Requested Status
Alexandros Frantzis (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Robert Carr (community) Approve
Daniel van Vugt Abstain
Review via email: mp+188108@code.launchpad.net

Commit message

frontend, logging: Provide reporting from frontend::Connector

Description of the change

frontend, logging: Provide reporting from frontend::Connector

NB adding data members to DefaultServerConfiguration breaks ABI

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

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

review: Abstain
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

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
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

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
Revision history for this message
Robert Carr (robertcarr) wrote :

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
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alexandros Frantzis (afrantzis) wrote :

Looks good.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/server/mir/default_server_configuration.h'
2--- include/server/mir/default_server_configuration.h 2013-09-24 11:43:27 +0000
3+++ include/server/mir/default_server_configuration.h 2013-09-30 14:16:13 +0000
4@@ -42,6 +42,7 @@
5 {
6 class Shell;
7 class Connector;
8+class ConnectorReport;
9 class ProtobufIpcFactory;
10 class SessionCreator;
11 class SessionMediatorReport;
12@@ -173,6 +174,7 @@
13 * internal dependencies of frontend
14 * @{ */
15 virtual std::shared_ptr<frontend::SessionCreator> the_session_creator();
16+ virtual std::shared_ptr<frontend::ConnectorReport> the_connector_report();
17 /** @} */
18 /** @} */
19
20@@ -267,6 +269,7 @@
21 CachedPtr<graphics::GraphicBufferAllocator> buffer_allocator;
22 CachedPtr<graphics::Display> display;
23
24+ CachedPtr<frontend::ConnectorReport> connector_report;
25 CachedPtr<frontend::ProtobufIpcFactory> ipc_factory;
26 CachedPtr<frontend::SessionMediatorReport> session_mediator_report;
27 CachedPtr<frontend::MessageProcessorReport> message_processor_report;
28
29=== modified file 'include/server/mir/frontend/connector_report.h'
30--- include/server/mir/frontend/connector_report.h 2013-09-24 16:20:17 +0000
31+++ include/server/mir/frontend/connector_report.h 2013-09-30 14:16:13 +0000
32@@ -20,6 +20,7 @@
33 #define MIR_FRONTEND_CONNECTOR_REPORT_H_
34
35 #include <stdexcept>
36+#include <string>
37
38 namespace mir
39 {
40@@ -30,6 +31,16 @@
41 {
42 public:
43
44+ virtual void thread_start() = 0;
45+ virtual void thread_end() = 0;
46+ virtual void starting_threads(int count) = 0;
47+ virtual void stopping_threads(int count) = 0;
48+
49+ virtual void creating_session_for(int socket_handle) = 0;
50+ virtual void creating_socket_pair(int server_handle, int client_handle) = 0;
51+
52+ virtual void listening_on(std::string const& endpoint) = 0;
53+
54 virtual void error(std::exception const& error) = 0;
55
56 protected:
57@@ -42,8 +53,17 @@
58 class NullConnectorReport : public ConnectorReport
59 {
60 public:
61-
62- void error(std::exception const& error);
63+ void thread_start() override;
64+ void thread_end() override;
65+ void starting_threads(int count) override;
66+ void stopping_threads(int count) override;
67+
68+ void creating_session_for(int socket_handle) override;
69+ void creating_socket_pair(int server_handle, int client_handle) override;
70+
71+ void listening_on(std::string const& endpoint) override;
72+
73+ void error(std::exception const& error) override;
74 };
75 }
76 }
77
78=== added file 'include/server/mir/logging/connector_report.h'
79--- include/server/mir/logging/connector_report.h 1970-01-01 00:00:00 +0000
80+++ include/server/mir/logging/connector_report.h 2013-09-30 14:16:13 +0000
81@@ -0,0 +1,56 @@
82+/*
83+ * Copyright © 2013 Canonical Ltd.
84+ *
85+ * This program is free software: you can redistribute it and/or modify it
86+ * under the terms of the GNU General Public License version 3,
87+ * as published by the Free Software Foundation.
88+ *
89+ * This program is distributed in the hope that it will be useful,
90+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
91+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
92+ * GNU General Public License for more details.
93+ *
94+ * You should have received a copy of the GNU General Public License
95+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
96+ *
97+ * Authored by: Alan Griffiths <alan@octopull.co.uk>
98+ */
99+
100+
101+#ifndef MIR_LOGGING_CONNECTOR_REPORT_H_
102+#define MIR_LOGGING_CONNECTOR_REPORT_H_
103+
104+#include "mir/frontend/connector_report.h"
105+
106+#include <memory>
107+
108+namespace mir
109+{
110+namespace logging
111+{
112+class Logger;
113+
114+class ConnectorReport : public frontend::ConnectorReport
115+{
116+public:
117+ ConnectorReport(std::shared_ptr<Logger> const& log);
118+
119+ void thread_start() override;
120+ void thread_end() override;
121+ void starting_threads(int count) override;
122+ void stopping_threads(int count) override;
123+
124+ void creating_session_for(int socket_handle) override;
125+ void creating_socket_pair(int server_handle, int client_handle) override;
126+
127+ void listening_on(std::string const& endpoint) override;
128+
129+ void error(std::exception const& error) override;
130+
131+private:
132+ std::shared_ptr<Logger> const logger;
133+};
134+}
135+}
136+
137+#endif /* MIR_LOGGING_CONNECTOR_REPORT_H_ */
138
139=== modified file 'src/server/default_server_configuration.cpp'
140--- src/server/default_server_configuration.cpp 2013-09-24 17:25:47 +0000
141+++ src/server/default_server_configuration.cpp 2013-09-30 14:16:13 +0000
142@@ -68,6 +68,7 @@
143 #include "mir/input/vt_filter.h"
144 #include "mir/input/android/default_android_input_configuration.h"
145 #include "mir/input/input_manager.h"
146+#include "mir/logging/connector_report.h"
147 #include "mir/logging/logger.h"
148 #include "mir/logging/input_report.h"
149 #include "mir/logging/dumb_console_logger.h"
150@@ -176,6 +177,7 @@
151 char const* const msg_processor_report_opt = "msg-processor-report";
152 char const* const display_report_opt = "display-report";
153 char const* const legacy_input_report_opt = "legacy-input-report";
154+char const* const connector_report_opt = "connector-report";
155 char const* const input_report_opt = "input-report";
156 char const* const host_socket_opt = "host-socket";
157 char const* const standalone_opt = "standalone";
158@@ -261,6 +263,8 @@
159 "Library to use for platform graphics support [default=libmirplatformgraphics.so]")
160 ("enable-input,i", po::value<bool>(),
161 "Enable input. [bool:default=true]")
162+ (connector_report_opt, po::value<std::string>(),
163+ "How to handle the Connector report. [{log,off}:default=off]")
164 (display_report_opt, po::value<std::string>(),
165 "How to handle the Display report. [{log,off}:default=off]")
166 (input_report_opt, po::value<std::string>(),
167@@ -1019,6 +1023,30 @@
168 });
169 }
170
171+auto mir::DefaultServerConfiguration::the_connector_report()
172+ -> std::shared_ptr<mf::ConnectorReport>
173+{
174+ return connector_report([this]
175+ () -> std::shared_ptr<mf::ConnectorReport>
176+ {
177+ auto opt = the_options()->get(connector_report_opt, off_opt_value);
178+
179+ if (opt == log_opt_value)
180+ {
181+ return std::make_shared<ml::ConnectorReport>(the_logger());
182+ }
183+ else if (opt == off_opt_value)
184+ {
185+ return std::make_shared<mf::NullConnectorReport>();
186+ }
187+ else
188+ {
189+ throw AbnormalExit(std::string("Invalid ") + connector_report_opt + " option: " + opt +
190+ " (valid options are: \"" + off_opt_value + "\" and \"" + log_opt_value + "\")");
191+ }
192+ });
193+}
194+
195 namespace
196 {
197 mir::SharedLibrary const* load_library(std::string const& libname)
198
199=== modified file 'src/server/frontend/default_configuration.cpp'
200--- src/server/frontend/default_configuration.cpp 2013-09-25 14:18:18 +0000
201+++ src/server/frontend/default_configuration.cpp 2013-09-30 14:16:13 +0000
202@@ -24,7 +24,6 @@
203 #include "mir/shell/session_container.h"
204 #include "mir/shell/session.h"
205 #include "published_socket_connector.h"
206-#include "mir/frontend/connector_report.h"
207
208 namespace mf = mir::frontend;
209 namespace msh = mir::shell;
210@@ -67,7 +66,7 @@
211 the_session_creator(),
212 threads,
213 force_requests_to_complete,
214- std::make_shared<mf::NullConnectorReport>());
215+ the_connector_report());
216 }
217 else
218 {
219@@ -76,7 +75,7 @@
220 the_session_creator(),
221 threads,
222 force_requests_to_complete,
223- std::make_shared<mf::NullConnectorReport>());
224+ the_connector_report());
225 }
226 });
227 }
228
229=== modified file 'src/server/frontend/published_socket_connector.cpp'
230--- src/server/frontend/published_socket_connector.cpp 2013-09-24 16:20:17 +0000
231+++ src/server/frontend/published_socket_connector.cpp 2013-09-30 14:16:13 +0000
232@@ -51,6 +51,8 @@
233
234 void mf::PublishedSocketConnector::start_accept()
235 {
236+ report->listening_on(socket_file);
237+
238 auto socket = std::make_shared<boost::asio::local::stream_protocol::socket>(io_service);
239
240 acceptor.async_accept(
241@@ -78,9 +80,9 @@
242 int threads,
243 std::function<void()> const& force_requests_to_complete,
244 std::shared_ptr<ConnectorReport> const& report)
245-: io_service_threads(threads),
246+: report(report),
247+ io_service_threads(threads),
248 force_requests_to_complete(force_requests_to_complete),
249- report(report),
250 session_creator{session_creator}
251 {
252 }
253@@ -92,7 +94,9 @@
254 while (true)
255 try
256 {
257+ report->thread_start();
258 io_service.run();
259+ report->thread_end();
260 return;
261 }
262 catch (std::exception const& e)
263@@ -101,6 +105,7 @@
264 }
265 };
266
267+ report->starting_threads(io_service_threads.size());
268 for (auto& thread : io_service_threads)
269 {
270 thread = std::thread(run_io_service);
271@@ -118,6 +123,8 @@
272 */
273 force_requests_to_complete();
274
275+ report->stopping_threads(io_service_threads.size());
276+
277 /* Wait for all io processing threads to finish */
278 for (auto& thread : io_service_threads)
279 {
280@@ -133,6 +140,7 @@
281
282 void mf::BasicConnector::create_session_for(std::shared_ptr<boost::asio::local::stream_protocol::socket> const& server_socket) const
283 {
284+ report->creating_session_for(server_socket->native_handle());
285 session_creator->create_session_for(server_socket);
286 }
287
288@@ -151,6 +159,8 @@
289 auto const server_socket = std::make_shared<boost::asio::local::stream_protocol::socket>(
290 io_service, boost::asio::local::stream_protocol(), socket_fd[server]);
291
292+ report->creating_socket_pair(socket_fd[server], socket_fd[client]);
293+
294 create_session_for(server_socket);
295
296 return socket_fd[client];
297@@ -161,6 +171,34 @@
298 stop();
299 }
300
301+void mf::NullConnectorReport::thread_start()
302+{
303+}
304+
305+void mf::NullConnectorReport::thread_end()
306+{
307+}
308+
309+void mf::NullConnectorReport::starting_threads(int /*count*/)
310+{
311+}
312+
313+void mf::NullConnectorReport::stopping_threads(int /*count*/)
314+{
315+}
316+
317+void mf::NullConnectorReport::creating_session_for(int /*socket_handle*/)
318+{
319+}
320+
321+void mf::NullConnectorReport::creating_socket_pair(int /*server_handle*/, int /*client_handle*/)
322+{
323+}
324+
325+void mf::NullConnectorReport::listening_on(std::string const& /*endpoint*/)
326+{
327+}
328+
329 void mf::NullConnectorReport::error(std::exception const& /*error*/)
330 {
331 }
332
333=== modified file 'src/server/frontend/published_socket_connector.h'
334--- src/server/frontend/published_socket_connector.h 2013-09-25 10:16:44 +0000
335+++ src/server/frontend/published_socket_connector.h 2013-09-30 14:16:13 +0000
336@@ -60,11 +60,11 @@
337 protected:
338 void create_session_for(std::shared_ptr<boost::asio::local::stream_protocol::socket> const& server_socket) const;
339 boost::asio::io_service mutable io_service;
340+ std::shared_ptr<ConnectorReport> const report;
341
342 private:
343 std::vector<std::thread> io_service_threads;
344 std::function<void()> const force_requests_to_complete;
345- std::shared_ptr<ConnectorReport> const report;
346 std::shared_ptr<SessionCreator> const session_creator;
347 };
348
349
350=== modified file 'src/server/logging/CMakeLists.txt'
351--- src/server/logging/CMakeLists.txt 2013-08-28 03:41:48 +0000
352+++ src/server/logging/CMakeLists.txt 2013-09-30 14:16:13 +0000
353@@ -1,6 +1,7 @@
354 set(
355 LOGGING_SOURCES
356
357+ connector_report.cpp
358 glog_logger.cpp
359 session_mediator_report.cpp
360 message_processor_report.cpp
361
362=== added file 'src/server/logging/connector_report.cpp'
363--- src/server/logging/connector_report.cpp 1970-01-01 00:00:00 +0000
364+++ src/server/logging/connector_report.cpp 2013-09-30 14:16:13 +0000
365@@ -0,0 +1,98 @@
366+/*
367+ * Copyright © 2013 Canonical Ltd.
368+ *
369+ * This program is free software: you can redistribute it and/or modify it
370+ * under the terms of the GNU General Public License version 3,
371+ * as published by the Free Software Foundation.
372+ *
373+ * This program is distributed in the hope that it will be useful,
374+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
375+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
376+ * GNU General Public License for more details.
377+ *
378+ * You should have received a copy of the GNU General Public License
379+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
380+ *
381+ * Authored by: Alan Griffiths <alan@octopull.co.uk>
382+ */
383+
384+#include "mir/logging/connector_report.h"
385+
386+#include "mir/logging/logger.h"
387+
388+#include <boost/exception/diagnostic_information.hpp>
389+
390+#include <sstream>
391+#include <thread>
392+
393+namespace ml = mir::logging;
394+
395+namespace
396+{
397+char const* const component = "frontend::SessionMediator";
398+}
399+
400+ml::ConnectorReport::ConnectorReport(std::shared_ptr<Logger> const& log) :
401+ logger(log)
402+{
403+}
404+
405+void ml::ConnectorReport::thread_start()
406+{
407+ std::stringstream ss;
408+ ss << "thread (" << std::this_thread::get_id() << ") started.";
409+ logger->log<Logger::informational>(ss.str(), component);
410+}
411+
412+void ml::ConnectorReport::thread_end()
413+{
414+ std::stringstream ss;
415+ ss << "thread (" << std::this_thread::get_id() << ") ended.";
416+ logger->log<Logger::informational>(ss.str(), component);
417+}
418+
419+void ml::ConnectorReport::starting_threads(int count)
420+{
421+ std::stringstream ss;
422+ ss << "Starting " << count << " threads.";
423+ logger->log<Logger::informational>(ss.str(), component);
424+}
425+
426+void ml::ConnectorReport::stopping_threads(int count)
427+{
428+ std::stringstream ss;
429+ ss << "Stopping " << count << " threads.";
430+ logger->log<Logger::informational>(ss.str(), component);
431+}
432+
433+void ml::ConnectorReport::creating_session_for(int socket_handle)
434+{
435+ std::stringstream ss;
436+ ss << "thread (" << std::this_thread::get_id() << ") Creating session for socket " << socket_handle;
437+ logger->log<Logger::informational>(ss.str(), component);
438+}
439+
440+void ml::ConnectorReport::creating_socket_pair(int server_handle, int client_handle)
441+{
442+ std::stringstream ss;
443+ ss << "thread (" << std::this_thread::get_id() << ") Creating socket pair (server=" << server_handle << ", client=" << client_handle << ").";
444+ logger->log<Logger::informational>(ss.str(), component);
445+}
446+
447+void ml::ConnectorReport::listening_on(std::string const& endpoint)
448+{
449+ std::stringstream ss;
450+ ss << "Listening on endpoint: " << endpoint;
451+ logger->log<Logger::informational>(ss.str(), component);
452+}
453+
454+void ml::ConnectorReport::error(std::exception const& error)
455+{
456+ std::stringstream ss;
457+ ss << "thread (" << std::this_thread::get_id() << ") Error: " << boost::diagnostic_information(error);
458+
459+ logger->log<ml::Logger::warning>(ss.str(), component);
460+}
461+
462+
463+
464
465=== modified file 'tests/unit-tests/frontend/test_published_socket_connector.cpp'
466--- tests/unit-tests/frontend/test_published_socket_connector.cpp 2013-09-25 10:16:44 +0000
467+++ tests/unit-tests/frontend/test_published_socket_connector.cpp 2013-09-30 14:16:13 +0000
468@@ -46,11 +46,21 @@
469
470 namespace
471 {
472-class MockCommunicatorReport : public mf::ConnectorReport
473+class MockConnectorReport : public mf::ConnectorReport
474 {
475 public:
476
477- ~MockCommunicatorReport() noexcept {}
478+ ~MockConnectorReport() noexcept {}
479+
480+ MOCK_METHOD0(thread_start, void ());
481+ MOCK_METHOD0(thread_end, void());
482+ MOCK_METHOD1(starting_threads, void (int count));
483+ MOCK_METHOD1(stopping_threads, void(int count));
484+
485+ MOCK_METHOD1(creating_session_for, void(int socket_handle));
486+ MOCK_METHOD2(creating_socket_pair, void(int server_handle, int client_handle));
487+
488+ MOCK_METHOD1(listening_on, void(std::string const& endpoint));
489
490 MOCK_METHOD1(error, void (std::exception const& error));
491 };
492@@ -64,7 +74,7 @@
493
494 void SetUp()
495 {
496- communicator_report = std::make_shared<MockCommunicatorReport>();
497+ communicator_report = std::make_shared<MockConnectorReport>();
498 stub_server_tool = std::make_shared<mt::StubServerTool>();
499 stub_server = std::make_shared<mt::TestProtobufServer>(
500 "./test_socket",
501@@ -94,13 +104,13 @@
502 }
503
504 std::shared_ptr<mt::TestProtobufClient> client;
505- static std::shared_ptr<MockCommunicatorReport> communicator_report;
506+ static std::shared_ptr<MockConnectorReport> communicator_report;
507 static std::shared_ptr<mt::StubServerTool> stub_server_tool;
508 static std::shared_ptr<mt::TestProtobufServer> stub_server;
509 };
510
511 std::shared_ptr<mt::StubServerTool> PublishedSocketConnector::stub_server_tool;
512-std::shared_ptr<MockCommunicatorReport> PublishedSocketConnector::communicator_report;
513+std::shared_ptr<MockConnectorReport> PublishedSocketConnector::communicator_report;
514 std::shared_ptr<mt::TestProtobufServer> PublishedSocketConnector::stub_server;
515
516 TEST_F(PublishedSocketConnector, create_surface_results_in_a_callback)

Subscribers

People subscribed via source and target branches