Merge lp:~didrocks/ubiquity/intall-metrics into lp:ubiquity

Proposed by Didier Roche-Tolomelli
Status: Merged
Merged at revision: 6596
Proposed branch: lp:~didrocks/ubiquity/intall-metrics
Merge into: lp:ubiquity
Diff against target: 390 lines (+150/-21)
8 files modified
debian/changelog (+4/-0)
ubiquity/frontend/debconf_ui.py (+5/-1)
ubiquity/frontend/gtk_ui.py (+8/-1)
ubiquity/frontend/kde_components/PartAuto.py (+7/-5)
ubiquity/frontend/kde_ui.py (+7/-1)
ubiquity/frontend/noninteractive.py (+5/-1)
ubiquity/plugins/ubi-partman.py (+18/-12)
ubiquity/telemetry.py (+96/-0)
To merge this branch: bzr merge lp:~didrocks/ubiquity/intall-metrics
Reviewer Review Type Date Requested Status
Jean-Baptiste Lallement Approve
Iain Lane Approve
Review via email: mp+341229@code.launchpad.net

Description of the change

Add telemetry function

Always report install metrics for optional reporting later on.
Tested on both Kubuntu and Ubuntu, with:
  - side-by-side install
  - use biggest free space
  - reinstall over existing installation, keeping files
  - reinstall and replace existing installation
  - full disk used
  - lvm
  - crypto
  - manual
  - oem
  - automatic installation

It should also work in debconf UI mode, but I'm unsure how to test it.

To post a comment you must log in.
lp:~didrocks/ubiquity/intall-metrics updated
6593. By Jean-Baptiste Lallement

[ Sebastien Bacher ]
  * gui/gtk/stepPrepare.ui:
    - updated the non-free-software text to not include the mention to the
      MP3 fluendo codecs since those have been superseeded in the recent
      gstreamer version.

6594. By Jean-Baptiste Lallement

[ Sebastien Bacher ]
  * Rename the "Preparing to install Ubuntu" installer step to "Updates and
    other software" according to the design, thanks Matthew for pointing out
    the inconsistency

6595. By Jean-Baptiste Lallement

* debian/control: fixed dependency on sensible-utils

Revision history for this message
Iain Lane (laney) wrote :

Thanks, this is great. I have some comments inline, nothing too major. :)

After fixing those, I don't think that you'd need to re-test the full matrix: if it works for one it should work equally for all the others.

Just for the record, as jibel said on IRC, it would be good to test if this works with preseeding. Even if not, that could be fixed later on though.

review: Needs Fixing
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Waiting for your comments on my answer before going further :)

Revision history for this message
Didier Roche-Tolomelli (didrocks) :
Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

ok, sounds like I can't make it a singleton easily without breaking the MVC semantic in place.

I have changed the global as a class variable and made the time measurement relative + adding a "start time"

Let me know what you think about class singleton vs code consistency and "Steps" naming

Revision history for this message
Iain Lane (laney) wrote :

some replies to your comments but I had to do them on the old version of the diff

thanks for considering. As commented I think we should discuss what the requirements for timing data are as this seems a bit strange to me. After getting that settled and maybe adding some protection to the file saving I'm OK with this once jibel has tested and given his thumbs up. Cheers!

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Done of protecting telemetry. Let's wait on finishing our discussion on what to collect :)

Revision history for this message
Sebastien Bacher (seb128) wrote :

Ok, I've replied on the "recording the steps" comments, those seem like they could be useful to validate design changes and see their impact on the users

Revision history for this message
Didier Roche-Tolomelli (didrocks) wrote :

Tested latest rebased version with the GTK installer, all working!

Revision history for this message
Iain Lane (laney) wrote :

ok from my side, waiting for QA to approve too

review: Approve
Revision history for this message
Jean-Baptiste Lallement (jibel) wrote :

LGTM too. Thanks!

review: Approve
lp:~didrocks/ubiquity/intall-metrics updated
6596. By Jean-Baptiste Lallement

* Added telemetry feature to collect installation metrics and optional reporting later on. Thanks didrocks

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2018-03-09 10:43:07 +0000
3+++ debian/changelog 2018-03-13 11:08:32 +0000
4@@ -12,6 +12,10 @@
5 other software" according to the design, thanks Matthew for pointing
6 out the inconsistency
7
8+ [ Didier Roche ]
9+ * Collect and dump telemetry info as a log file: (LP: #1755456)
10+ Those installation telemetry info are for optional upload later on.
11+
12 -- Mathieu Trudel-Lapierre <cyphermox@ubuntu.com> Wed, 28 Feb 2018 14:36:54 -0500
13
14 ubiquity (18.04.3) bionic; urgency=medium
15
16=== modified file 'ubiquity/frontend/debconf_ui.py'
17--- ubiquity/frontend/debconf_ui.py 2013-02-19 11:33:07 +0000
18+++ ubiquity/frontend/debconf_ui.py 2018-03-13 11:08:32 +0000
19@@ -33,7 +33,7 @@
20
21 import debconf
22
23-from ubiquity import i18n
24+from ubiquity import i18n, telemetry
25 from ubiquity.components import install, plugininstall
26 from ubiquity.frontend.base import BaseFrontend, Controller
27 from ubiquity.plugin import Plugin
28@@ -94,6 +94,9 @@
29 'cannot continue without them.'), file=sys.stderr)
30 sys.exit(1)
31
32+ telemetry.get().set_installer_type('DebConf')
33+ telemetry.get().add_stage(telemetry.START_INSTALL_STAGE_TAG)
34+
35 self.pagesindex = 0
36 self.pageslen = 0
37 self.pages = []
38@@ -152,6 +155,7 @@
39 "Install failed with exit code %s; see "
40 "/var/log/syslog" % ret)
41
42+ telemetry.get().done(self.db)
43 return 0
44 else:
45 return 10
46
47=== modified file 'ubiquity/frontend/gtk_ui.py'
48--- ubiquity/frontend/gtk_ui.py 2017-09-28 21:25:09 +0000
49+++ ubiquity/frontend/gtk_ui.py 2018-03-13 11:08:32 +0000
50@@ -54,7 +54,7 @@
51 from ubiquity import gtkwidgets
52
53 from ubiquity import (
54- filteredcommand, gsettings, i18n, validation, misc, osextras)
55+ filteredcommand, gsettings, i18n, validation, misc, osextras, telemetry)
56 from ubiquity.components import install, plugininstall, partman_commit
57 import ubiquity.frontend.base
58 from ubiquity.frontend.base import BaseFrontend
59@@ -757,6 +757,8 @@
60 self.debconf_progress_cancellable(False)
61 self.refresh()
62
63+ telemetry.get().set_installer_type('GTK')
64+
65 self.set_current_page(0)
66 self.live_installer.show()
67
68@@ -821,6 +823,8 @@
69 # postinstall will exit here by calling Gtk.main_quit in
70 # find_next_step.
71
72+ telemetry.get().done(self.db)
73+
74 self.unlock_environment()
75 if self.oem_user_config:
76 self.quit_installer()
77@@ -1555,6 +1559,7 @@
78 for i in range(len(self.pages))[index + 1:]:
79 self.dot_grid.get_child_at(i, 0).set_fraction(0)
80
81+ telemetry.get().add_stage(name)
82 syslog.syslog('switched to page %s' % name)
83
84 # Callbacks provided to components.
85@@ -1645,6 +1650,8 @@
86 return False
87
88 def switch_to_install_interface(self):
89+ if not self.installing:
90+ telemetry.get().add_stage(telemetry.START_INSTALL_STAGE_TAG)
91 self.installing = True
92 self.lockdown_environment()
93
94
95=== modified file 'ubiquity/frontend/kde_components/PartAuto.py'
96--- ubiquity/frontend/kde_components/PartAuto.py 2016-08-26 07:13:37 +0000
97+++ ubiquity/frontend/kde_components/PartAuto.py 2018-03-13 11:08:32 +0000
98@@ -252,18 +252,20 @@
99 disk_id = self.extra_options['use_device'][1][comboText][0]
100 disk_id = disk_id.rsplit('/', 1)[1]
101 option = self.extra_options['resize'][disk_id][0]
102- return option, '%d B' % self.resizeSize
103+ return option, '%d B' % self.resizeSize, 'resize_use_free'
104 elif choice == self.useDeviceChoice:
105 return (self.extra_options['use_device'][0],
106- str(self.part_auto_disk_box.currentText()))
107+ str(self.part_auto_disk_box.currentText()), 'use_device')
108 elif choice == self.lvm_choice:
109 return (choice,
110- str(self.part_auto_disk_box.currentText()))
111+ str(self.part_auto_disk_box.currentText()), 'use_lvm')
112 elif choice == self.crypto_choice:
113 return (choice,
114- str(self.part_auto_disk_box.currentText()))
115+ str(self.part_auto_disk_box.currentText()), 'use_crypto')
116+ elif choice == self.manualChoice:
117+ return choice, None, 'manual'
118 else:
119- return choice, None
120+ return choice, None, 'unknown'
121
122 def on_disks_combo_changed(self, index):
123 for e in self.bar_frames:
124
125=== modified file 'ubiquity/frontend/kde_ui.py'
126--- ubiquity/frontend/kde_ui.py 2017-01-09 20:15:33 +0000
127+++ ubiquity/frontend/kde_ui.py 2018-03-13 11:08:32 +0000
128@@ -41,7 +41,7 @@
129 sip.setapi("QVariant", 1)
130 from PyQt5 import QtCore, QtGui, QtWidgets, uic
131
132-from ubiquity import filteredcommand, i18n, misc
133+from ubiquity import filteredcommand, i18n, misc, telemetry
134 from ubiquity.components import partman_commit, install, plugininstall
135 import ubiquity.frontend.base
136 from ubiquity.frontend.base import BaseFrontend
137@@ -454,6 +454,8 @@
138 self.get_string('ubiquity/install/title'))
139 self.refresh()
140
141+ telemetry.get().set_installer_type('KDE')
142+
143 # Start the interface
144 self.set_current_page(0)
145
146@@ -529,6 +531,8 @@
147 self.start_slideshow()
148 self.run_main_loop()
149
150+ telemetry.get().done(self.db)
151+
152 quitText = '<qt>%s</qt>' % self.get_string("finished_label")
153 rebootButtonText = self.get_string("reboot_button")
154 shutdownButtonText = self.get_string("shutdown_button")
155@@ -1121,6 +1125,7 @@
156 self.ui.content_widget.show()
157 self.current_page = newPageID
158 name = self.step_name(newPageID)
159+ telemetry.get().add_stage(name)
160 syslog.syslog('switched to page %s' % name)
161 if 'UBIQUITY_GREETER' in os.environ:
162 if name == 'language':
163@@ -1291,6 +1296,7 @@
164 elif finished_step == 'ubi-partman':
165 # Flush changes to the database so that when the parallel db
166 # starts, it does so with the most recent changes.
167+ telemetry.get().add_stage(telemetry.START_INSTALL_STAGE_TAG)
168 self.stop_debconf()
169 self.start_debconf()
170 self._show_progress_bar(True)
171
172=== modified file 'ubiquity/frontend/noninteractive.py'
173--- ubiquity/frontend/noninteractive.py 2014-06-30 13:09:16 +0000
174+++ ubiquity/frontend/noninteractive.py 2018-03-13 11:08:32 +0000
175@@ -32,7 +32,7 @@
176
177 from gi.repository import GLib
178
179-from ubiquity import filteredcommand, i18n, misc
180+from ubiquity import filteredcommand, i18n, misc, telemetry
181 from ubiquity.components import install, plugininstall, partman_commit
182 import ubiquity.frontend.base
183 from ubiquity.frontend.base import BaseFrontend
184@@ -77,6 +77,9 @@
185 file=self.console)
186 sys.exit(1)
187
188+ telemetry.get().set_installer_type('NonInteractive')
189+ telemetry.get().add_stage(telemetry.START_INSTALL_STAGE_TAG)
190+
191 for x in self.pages:
192 if issubclass(x.filter_class, Plugin):
193 ui = x.ui
194@@ -110,6 +113,7 @@
195 if ret == 0:
196 self.run_success_cmd()
197 print('Installation complete.', file=self.console)
198+ telemetry.get().done(self.db)
199 if self.get_reboot():
200 misc.execute("reboot")
201 if ret != 0:
202
203=== modified file 'ubiquity/plugins/ubi-partman.py'
204--- ubiquity/plugins/ubi-partman.py 2016-12-02 13:20:16 +0000
205+++ ubiquity/plugins/ubi-partman.py 2018-03-13 11:08:32 +0000
206@@ -25,7 +25,8 @@
207
208 import debconf
209
210-from ubiquity import misc, osextras, parted_server, plugin, validation
211+from ubiquity import (misc, osextras, parted_server, plugin,
212+ telemetry, validation)
213 from ubiquity.install_misc import archdetect
214
215
216@@ -673,22 +674,24 @@
217
218 def get_autopartition_choice(self):
219 if self.reuse_partition.get_active():
220- return self.extra_options['reuse'][0][0], None
221+ return self.extra_options['reuse'][0][0], None, 'reuse_partition'
222
223 if self.replace_partition.get_active():
224- return self.extra_options['replace'][0], None
225+ return (self.extra_options['replace'][0], None,
226+ 'reinstall_partition')
227
228 elif self.custom_partitioning.get_active():
229- return self.extra_options['manual'], None
230+ return self.extra_options['manual'], None, 'manual'
231
232 elif self.resize_use_free.get_active():
233 if 'biggest_free' in self.extra_options:
234 choice = self.extra_options['biggest_free'][0]
235- return choice, None
236+ return choice, None, 'resize_use_free'
237 else:
238 disk_id = self.get_current_disk_partman_id()
239 choice = self.extra_options['resize'][disk_id][0]
240- return choice, '%s B' % self.resizewidget.get_size()
241+ return (choice, '%s B' % self.resizewidget.get_size(),
242+ 'resize_use_free')
243
244 elif self.use_device.get_active():
245 def choose_recipe():
246@@ -702,13 +705,15 @@
247
248 if not ((want_crypto and have_crypto) or
249 (want_lvm and have_lvm)):
250- return self.extra_options['use_device'][0]
251+ return self.extra_options['use_device'][0], 'use_device'
252
253 if want_crypto:
254- return self.extra_options['some_device_crypto']
255+ return (self.extra_options['some_device_crypto'],
256+ 'use_crypto')
257
258 if want_lvm:
259- return self.extra_options['some_device_lvm']
260+ return (self.extra_options['some_device_lvm'],
261+ 'use_lvm')
262
263 # Something went horribly wrong, we should have returned
264 # earlier
265@@ -717,9 +722,9 @@
266 i = self.part_auto_select_drive.get_active_iter()
267 m = self.part_auto_select_drive.get_model()
268 disk = m.get_value(i, 0)
269- choice = choose_recipe()
270+ choice, method = choose_recipe()
271 # Is the encoding necessary?
272- return choice, misc.utf8(disk, errors='replace')
273+ return choice, misc.utf8(disk, errors='replace'), method
274
275 else:
276 raise AssertionError("Couldn't get autopartition choice")
277@@ -3249,10 +3254,11 @@
278 self.preseed('grub-installer/bootdev', self.ui.get_grub_choice())
279
280 if self.current_question.endswith('automatically_partition'):
281- (autopartition_choice, self.extra_choice) = \
282+ (autopartition_choice, self.extra_choice, method) = \
283 self.ui.get_autopartition_choice()
284 self.preseed_as_c(self.current_question, autopartition_choice,
285 seen=False)
286+ telemetry.get().set_partition_method(method)
287 # Don't exit partman yet.
288 else:
289 self.finish_partitioning = True
290
291=== added file 'ubiquity/telemetry.py'
292--- ubiquity/telemetry.py 1970-01-01 00:00:00 +0000
293+++ ubiquity/telemetry.py 2018-03-13 11:08:32 +0000
294@@ -0,0 +1,96 @@
295+# -*- coding: utf-8; Mode: Python; indent-tabs-mode: nil; tab-width: 4 -*-
296+
297+# Copyright (C) 2018 Canonical Ltd.
298+#
299+# Functions useful for the final install.py script and for ubiquity
300+# plugins to use
301+#
302+# This program is free software; you can redistribute it and/or modify
303+# it under the terms of the GNU General Public License as published by
304+# the Free Software Foundation; either version 2 of the License, or
305+# (at your option) any later version.
306+#
307+# This program is distributed in the hope that it will be useful,
308+# but WITHOUT ANY WARRANTY; without even the implied warranty of
309+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
310+# GNU General Public License for more details.
311+#
312+# You should have received a copy of the GNU General Public License
313+# along with this program; if not, write to the Free Software
314+# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
315+
316+
317+import json
318+import os
319+import stat
320+import syslog
321+import time
322+
323+from ubiquity.misc import raise_privileges
324+
325+START_INSTALL_STAGE_TAG = 'start_install'
326+
327+
328+def get():
329+ """Return a singleton _Telemetry instance."""
330+ if _Telemetry._telemetry is None:
331+ _Telemetry._telemetry = _Telemetry()
332+ return _Telemetry._telemetry
333+
334+
335+class _Telemetry():
336+
337+ _telemetry = None
338+
339+ def __init__(self):
340+ self._metrics = {}
341+ self._stages_hist = {}
342+ self._start_time = time.time()
343+ self.add_stage('start')
344+ self._dest_path = '/target/var/log/installer/telemetry'
345+ try:
346+ with open('/cdrom/.disk/info') as f:
347+ self._metrics['Media'] = f.readline()
348+ except FileNotFoundError:
349+ self._metrics['Media'] = 'unknown'
350+
351+ def add_stage(self, stage_name):
352+ """Record installer stage with current time"""
353+ self._stages_hist[int(time.time() - self._start_time)] = stage_name
354+
355+ def set_installer_type(self, installer_type):
356+ """Record installer type"""
357+ self._metrics['Type'] = installer_type
358+
359+ def set_partition_method(self, method):
360+ """Record anynomized partition method"""
361+ self._metrics['PartitionMethod'] = method
362+
363+ @raise_privileges
364+ def done(self, db):
365+ """Close telemetry collection
366+
367+ Set as installation done, add additional info and save to
368+ destination file"""
369+ self.add_stage('done')
370+
371+ self._metrics['DownloadUpdates'] = db.get('ubiquity/download_updates')
372+ self._metrics['Language'] = db.get('localechooser/languagelist')
373+ self._metrics['Minimal'] = db.get('ubiquity/minimal_install')
374+ self._metrics['RestrictedAddons'] = db.get('ubiquity/use_nonfree')
375+ self._metrics['Stages'] = self._stages_hist
376+
377+ target_dir = os.path.dirname(self._dest_path)
378+ try:
379+ if not os.path.exists(target_dir):
380+ os.makedirs(target_dir)
381+ with open(self._dest_path, 'w') as f:
382+ json.dump(self._metrics, f)
383+ os.chmod(self._dest_path,
384+ stat.S_IRUSR | stat.S_IWUSR |
385+ stat.S_IRGRP | stat.S_IROTH)
386+ except OSError as e:
387+ syslog.syslog(syslog.LOG_ERR,
388+ "Exception while storing telemetry data: " + str(e))
389+
390+# vim:ai:et:sts=4:tw=80:sw=4:

Subscribers

People subscribed via source and target branches

to status/vote changes: