Daemonize (fork()) and install metrics_daemon in the image for execution.

Daemonize (fork()) and install metrics_daemon in the image for execution.
With this change ChromeOS begins to send metrics to the UMA server.

[Luigi Semenzato, 2009-12-17 23:20:33]

Ted, I would be very much obliged if you can review this.  This change makes
metrics_daemon a proper system daemon (it forks by default), packages it for
installation, and adds an upstart script for running it in the image.

Testing: I verified that changes in the network state are logged in Chrome under
about:histograms.

[tedbo, 2009-12-17 23:42:43]

See comments below:

http://codereview.chromium.org/500117/diff/28/1003
File src/platform/init/metrics_daemon.conf (right):

http://codereview.chromium.org/500117/diff/28/1003#newcode8
src/platform/init/metrics_daemon.conf:8: start on started connman
FYI- We start connman early in boot so that we can try to get on the network in
parallel with starting up the windowing system. By having this "start on started
connman" you are putting loading and starting of the metrics daemon in a
critical part of the boot. If possible, it is recommended to "start on stopped
dump-boot-stats".

http://codereview.chromium.org/500117/diff/28/1003#newcode14
src/platform/init/metrics_daemon.conf:14: stop on starting halt or starting
reboot
Is it necessary to shut-down the metrics daemon cleanly on shutdown or reboot?
If it is not, then please remove.

If you keep this, the convention that I've been using is to put this under the
"start on" at the top of these files.

http://codereview.chromium.org/500117/diff/28/1007
File src/platform/metrics_daemon/debian/rules (right):

http://codereview.chromium.org/500117/diff/28/1007#newcode36
src/platform/metrics_daemon/debian/rules:36: mkdir -p
$(CURDIR)/debian/chromeos-metrics-daemon
You can leave this as is, or put the dir in the "dirs" file. In the latter case,
the dh_installdirs will create the directory for you. I'm fine with either way;
at some point we should update to the newest method for creating debian packages
across the board anyway.

http://codereview.chromium.org/500117/diff/28/1010
File src/platform/metrics_daemon/metrics_daemon.cc (right):

http://codereview.chromium.org/500117/diff/28/1010#newcode34
src/platform/metrics_daemon/metrics_daemon.cc:34: if (!do_fork || fork() == 0) {
Maybe it would be better to use use daemon(0, 0) instead of fork()? See "man 3
daemon".

Maybe the option should be "--daemon / --nodaemon" as well?

[Luigi Semenzato, 2009-12-18 00:55:30]

Thanks.

http://codereview.chromium.org/500117/diff/28/1003
File src/platform/init/metrics_daemon.conf (right):

http://codereview.chromium.org/500117/diff/28/1003#newcode8
src/platform/init/metrics_daemon.conf:8: start on started connman
On 2009/12/17 23:42:43, tedbo wrote:
> FYI- We start connman early in boot so that we can try to get on the network
in
> parallel with starting up the windowing system. By having this "start on
started
> connman" you are putting loading and starting of the metrics daemon in a
> critical part of the boot. If possible, it is recommended to "start on stopped
> dump-boot-stats".

Yes.  I am hoping that the overhead is small.  The text size of metrics_daemon
is 160K, most of the overhead will be reading that from disk.  I am trying to
get early numbers from connman.  When we optimize, I can change connman so that
it puts those numbers directly into the chromeos-metrics file (read by chrome)
instead of sending state changes to the DBUS.

http://codereview.chromium.org/500117/diff/28/1003#newcode14
src/platform/init/metrics_daemon.conf:14: stop on starting halt or starting
reboot
On 2009/12/17 23:42:43, tedbo wrote:
> Is it necessary to shut-down the metrics daemon cleanly on shutdown or reboot?
> If it is not, then please remove.

OK---will remove.

> If you keep this, the convention that I've been using is to put this under the
> "start on" at the top of these files.
>

http://codereview.chromium.org/500117/diff/28/1007
File src/platform/metrics_daemon/debian/rules (right):

http://codereview.chromium.org/500117/diff/28/1007#newcode36
src/platform/metrics_daemon/debian/rules:36: mkdir -p
$(CURDIR)/debian/chromeos-metrics-daemon
On 2009/12/17 23:42:43, tedbo wrote:
> You can leave this as is, or put the dir in the "dirs" file. In the latter
case,
> the dh_installdirs will create the directory for you. I'm fine with either
way;
> at some point we should update to the newest method for creating debian
packages
> across the board anyway.

OK, I will leave as is then.

http://codereview.chromium.org/500117/diff/28/1010
File src/platform/metrics_daemon/metrics_daemon.cc (right):

http://codereview.chromium.org/500117/diff/28/1010#newcode34
src/platform/metrics_daemon/metrics_daemon.cc:34: if (!do_fork || fork() == 0) {
On 2009/12/17 23:42:43, tedbo wrote:
> Maybe it would be better to use use daemon(0, 0) instead of fork()? See "man 3
> daemon".
> 
> Maybe the option should be "--daemon / --nodaemon" as well?
> 

Thanks, fully agree.  Daemon(3) does useful things.

[tedbo, 2009-12-18 01:12:17]

LGTM

Since you changed the flag name (thanks!) I think it makes sense to change the
code. After that feel free to submit.

http://codereview.chromium.org/500117/diff/3001/4010
File src/platform/metrics_daemon/metrics_daemon.cc (right):

http://codereview.chromium.org/500117/diff/3001/4010#newcode33
src/platform/metrics_daemon/metrics_daemon.cc:33: void MetricsDaemon::Run(bool
do_fork, bool testing) {
s/do_fork/do_daemon

http://codereview.chromium.org/500117/diff/3001/4011
File src/platform/metrics_daemon/metrics_daemon.h (right):

http://codereview.chromium.org/500117/diff/3001/4011#newcode20
src/platform/metrics_daemon/metrics_daemon.h:20: // Does all the work.  If
do_fork is true, daemonize by forking.  If
Please update this comment and function prototype below to s/do_fork/do_daemon.