metrics cleanup and fixes.

metrics cleanup and fixes.

- value is int now.
- add seconds to milliseconds option to metrics_client and use it
- chmod chronos/root fix
- style fixes

[sosa, 2010-04-15 18:06:44]

lgtm

[petkov, 2010-04-15 20:08:15]

PTAL, moved message formatting into a separate routine so that I can use it for
the register-histogram message (separate CL).

[sosa, 2010-04-15 23:15:54]

http://codereview.chromium.org/1649007/diff/4001/5005
File src/platform/metrics/metrics_library.cc (right):

http://codereview.chromium.org/1649007/diff/4001/5005#newcode45
src/platform/metrics/metrics_library.cc:45: static void
SendMessageToChrome(int32_t length, const char *message) {
I should have noticed this earlier, but is there any particular reason you are
masking error codes i.e. why not return an int or boolean to indicate
success/failure for the caller?

http://codereview.chromium.org/1649007/diff/4001/5005#newcode132
src/platform/metrics/metrics_library.cc:132: if (message_length <= 0)
Doesn't match with < 0 ... i.e. should we log some error/warning if they try to
send something that is 0 bytes.  It might be nice for debugging later (i.e. why
doesn't this stat appear ... oh it's cause the log says you're sending a blank
message so it's not sending)

[petkov, 2010-04-15 23:33:33]

PTAL

http://codereview.chromium.org/1649007/diff/4001/5005
File src/platform/metrics/metrics_library.cc (right):

http://codereview.chromium.org/1649007/diff/4001/5005#newcode45
src/platform/metrics/metrics_library.cc:45: static void
SendMessageToChrome(int32_t length, const char *message) {
On 2010/04/15 23:15:54, sosa wrote:
> I should have noticed this earlier, but is there any particular reason you are
> masking error codes i.e. why not return an int or boolean to indicate
> success/failure for the caller?

This is how the original library was implemented. Changed to expose the status
to the caller.

http://codereview.chromium.org/1649007/diff/4001/5005#newcode132
src/platform/metrics/metrics_library.cc:132: if (message_length <= 0)
On 2010/04/15 23:15:54, sosa wrote:
> Doesn't match with < 0 ... i.e. should we log some error/warning if they try
to
> send something that is 0 bytes.  It might be nice for debugging later (i.e.
why
> doesn't this stat appear ... oh it's cause the log says you're sending a blank
> message so it's not sending)

0 will never actually happen. Anyway, changed the <= to < for clarity. It might
make sense to add some assertion capabilities to the libraries at some point.

[sosa, 2010-04-15 23:36:59]

lgtm