chromeos: Simplify user action code.

chromeos: Simplify user action code.

This makes us (almost) just forward through user actions that
we receive from other programs rather than having them listed
in Chrome's source.  Note that new actions that aren't
present in Chrome still need to be added to
chrome/tools/extract_actions.py, which needs to be rerun to
generate new hashes.

I say "almost" above because metrics still need to be added
to external_metrics.cc due to http://crosbug.com/11095.  Once
it's fixed, that requirement can be easily removed.

I also cleaned up some earlier additions that I made to
extract_actions.py and re-ran it to pick up missing actions.

BUG=chromium-os:10403
TEST=built it.  checked that i see no errors in chrome's log when using the window manager to report a valid user action, but that i still see an error when running "metrics_client -u BogusMetric"

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=72173

[Daniel Erat, 2011-01-20 00:44:23]

Is there some way for me to test this?

http://codereview.chromium.org/6266011/diff/2001/chrome/browser/chromeos/exte...
File chrome/browser/chromeos/external_metrics.cc (left):

http://codereview.chromium.org/6266011/diff/2001/chrome/browser/chromeos/exte...
chrome/browser/chromeos/external_metrics.cc:62:
DefineUserAction("TabOverviewExitMouse", RecordTabOverviewExitMouse);
I don't think that these exist anymore, so I'm not bothering to add them to the
script.

[petkov, 2011-01-20 01:00:53]

LGTM

I assume there're no relevant tests in external_metrics_unittest.cc?

In terms of testing, you could generate the key events, check that they appear
in /var/log/metrics/uma-events, then check that they disappear from that file
within 30 seconds or so (Chrome consumes them). All assuming that you have the
/home/chronos/Consent To Send Stats file.

Checking that the data is actually sent to UMA is probably a bit more difficult.
One way is to run an official Chrome build (or comment out the proper defines in
browser_main.cc:InitializeMetrics) so that the metrics service runs and sends
data to UMA. Then modify all inerlog_duration_ assignments in metrics_service.cc
so that you don't have to wait for 30 minutes for the next XML to be sent. And
then look for the Action's hashes in the XML that's being logged -- you probably
need to look at the second XML, not the first one that gets sent after 2
minutes....

... or just trust that it works :-)

http://codereview.chromium.org/6266011/diff/2001/chrome/browser/chromeos/exte...
File chrome/browser/chromeos/external_metrics.cc (left):

http://codereview.chromium.org/6266011/diff/2001/chrome/browser/chromeos/exte...
chrome/browser/chromeos/external_metrics.cc:62:
DefineUserAction("TabOverviewExitMouse", RecordTabOverviewExitMouse);
On 2011/01/20 00:44:23, Daniel Erat wrote:
> I don't think that these exist anymore, so I'm not bothering to add them to
the
> script.

I'm not sure these ever existed...

[Daniel Erat, 2011-01-20 01:13:53]

On Wed, Jan 19, 2011 at 5:00 PM,  <petkov@chromium.org> wrote:
> LGTM
>
> I assume there're no relevant tests in external_metrics_unittest.cc?

Yeah, I don't see anything there that checks the actual metrics-sending code.

> In terms of testing, you could generate the key events, check that they
> appear
> in /var/log/metrics/uma-events, then check that they disappear from that
> file
> within 30 seconds or so (Chrome consumes them). All assuming that you have
> the
> /home/chronos/Consent To Send Stats file.

This is exactly what I did to test my earlier changes to make the
window manager and power manager report the actions, but I don't think
it'll help here -- it shows that Chrome is consuming the events but
not that it's reporting them (hence me not realizing that the metrics
from the previous changes were getting dropped by Chrome since I
hadn't registered them in external_metrics.cc).

> Checking that the data is actually sent to UMA is probably a bit more
> difficult.
> One way is to run an official Chrome build (or comment out the proper
> defines in
> browser_main.cc:InitializeMetrics) so that the metrics service runs and
> sends
> data to UMA. Then modify all inerlog_duration_ assignments in
> metrics_service.cc
> so that you don't have to wait for 30 minutes for the next XML to be sent.
> And
> then look for the Action's hashes in the XML that's being logged -- you
> probably
> need to look at the second XML, not the first one that gets sent after 2
> minutes....

Thanks, I'll look into this...

> ... or just trust that it works :-)

... maybe. :-)

>
>
>
http://codereview.chromium.org/6266011/diff/2001/chrome/browser/chromeos/exte...
> File chrome/browser/chromeos/external_metrics.cc (left):
>
>
http://codereview.chromium.org/6266011/diff/2001/chrome/browser/chromeos/exte...
> chrome/browser/chromeos/external_metrics.cc:62:
> DefineUserAction("TabOverviewExitMouse", RecordTabOverviewExitMouse);
> On 2011/01/20 00:44:23, Daniel Erat wrote:
>>
>> I don't think that these exist anymore, so I'm not bothering to add
>
> them to the
>>
>> script.
>
> I'm not sure these ever existed...
>
> http://codereview.chromium.org/6266011/
>

[Daniel Erat, 2011-01-20 01:41:03]

On Wed, Jan 19, 2011 at 5:13 PM, Daniel Erat <derat@chromium.org> wrote:
> On Wed, Jan 19, 2011 at 5:00 PM,  <petkov@chromium.org> wrote:
[snip]
>> Checking that the data is actually sent to UMA is probably a bit more
>> difficult.
>> One way is to run an official Chrome build (or comment out the proper
>> defines in
>> browser_main.cc:InitializeMetrics) so that the metrics service runs and
>> sends
>> data to UMA. Then modify all inerlog_duration_ assignments in
>> metrics_service.cc
>> so that you don't have to wait for 30 minutes for the next XML to be sent.
>> And
>> then look for the Action's hashes in the XML that's being logged -- you
>> probably
>> need to look at the second XML, not the first one that gets sent after 2
>> minutes....

So in this suggestion, where does the XML get logged?

>> http://codereview.chromium.org/6266011/

[petkov, 2011-01-20 01:45:44]

The chrome log.

Sent from my mobile phone.
On Jan 19, 2011 5:41 PM, "Daniel Erat" <derat@chromium.org> wrote:
> On Wed, Jan 19, 2011 at 5:13 PM, Daniel Erat <derat@chromium.org> wrote:
>> On Wed, Jan 19, 2011 at 5:00 PM,  <petkov@chromium.org> wrote:
> [snip]
>>> Checking that the data is actually sent to UMA is probably a bit more
>>> difficult.
>>> One way is to run an official Chrome build (or comment out the proper
>>> defines in
>>> browser_main.cc:InitializeMetrics) so that the metrics service runs and
>>> sends
>>> data to UMA. Then modify all inerlog_duration_ assignments in
>>> metrics_service.cc
>>> so that you don't have to wait for 30 minutes for the next XML to be
sent.
>>> And
>>> then look for the Action's hashes in the XML that's being logged -- you
>>> probably
>>> need to look at the second XML, not the first one that gets sent after 2
>>> minutes....
>
> So in this suggestion, where does the XML get logged?
>
>>> http://codereview.chromium.org/6266011/

[Daniel Erat, 2011-01-20 01:48:02]

Okay, thanks (came to the same conclusion from the code but think I
didn't get it since it's VLOG(1)).

On Wed, Jan 19, 2011 at 5:45 PM, Darin Petkov <petkov@chromium.org> wrote:
> The chrome log.
>
> Sent from my mobile phone.
>
> On Jan 19, 2011 5:41 PM, "Daniel Erat" <derat@chromium.org> wrote:
>> On Wed, Jan 19, 2011 at 5:13 PM, Daniel Erat <derat@chromium.org> wrote:
>>> On Wed, Jan 19, 2011 at 5:00 PM,  <petkov@chromium.org> wrote:
>> [snip]
>>>> Checking that the data is actually sent to UMA is probably a bit more
>>>> difficult.
>>>> One way is to run an official Chrome build (or comment out the proper
>>>> defines in
>>>> browser_main.cc:InitializeMetrics) so that the metrics service runs and
>>>> sends
>>>> data to UMA. Then modify all inerlog_duration_ assignments in
>>>> metrics_service.cc
>>>> so that you don't have to wait for 30 minutes for the next XML to be
>>>> sent.
>>>> And
>>>> then look for the Action's hashes in the XML that's being logged -- you
>>>> probably
>>>> need to look at the second XML, not the first one that gets sent after 2
>>>> minutes....
>>
>> So in this suggestion, where does the XML get logged?
>>
>>>> http://codereview.chromium.org/6266011/
>

[Daniel Erat, 2011-01-20 02:16:15]

Now that I'm looking at the logs some more, I see entries like this
(pre-dating my not-yet-checked-in change):

chrome_20110111-175153:[355:355:36466644:ERROR:external_metrics.cc(70)]
undefined UMA action: latform.BootSectorsRead 204294 1 1000000 50
chrome_20110111-175153:[355:355:36466854:ERROR:external_metrics.cc(70)]
undefined UMA action: latform.BootSectorsWritten 1104 1 10000 50
chrome_20110111-175153:[355:355:36467141:ERROR:external_metrics.cc(70)]
undefined UMA action: etwork.Wifi.TimeOnline 12 1 28800 50
chrome_20110111-175153:[355:355:36467375:ERROR:external_metrics.cc(70)]
undefined UMA action: gram
chrome_20110111-175153:[355:355:36467643:ERROR:external_metrics.cc(70)]
undefined UMA action: gram
chrome_20110111-175153:[355:355:36467859:ERROR:external_metrics.cc(70)]
undefined UMA action: gram

Is there any concern that we're going to be reporting malformed
actions if I remove the checks from external_metrics.cc?  Does this
indicate a problem in the code that connects the metrics library to
Chrome?

On Wed, Jan 19, 2011 at 5:47 PM, Daniel Erat <derat@chromium.org> wrote:
> Okay, thanks (came to the same conclusion from the code but think I
> didn't get it since it's VLOG(1)).
>
> On Wed, Jan 19, 2011 at 5:45 PM, Darin Petkov <petkov@chromium.org> wrote:
>> The chrome log.
>>
>> Sent from my mobile phone.
>>
>> On Jan 19, 2011 5:41 PM, "Daniel Erat" <derat@chromium.org> wrote:
>>> On Wed, Jan 19, 2011 at 5:13 PM, Daniel Erat <derat@chromium.org> wrote:
>>>> On Wed, Jan 19, 2011 at 5:00 PM,  <petkov@chromium.org> wrote:
>>> [snip]
>>>>> Checking that the data is actually sent to UMA is probably a bit more
>>>>> difficult.
>>>>> One way is to run an official Chrome build (or comment out the proper
>>>>> defines in
>>>>> browser_main.cc:InitializeMetrics) so that the metrics service runs and
>>>>> sends
>>>>> data to UMA. Then modify all inerlog_duration_ assignments in
>>>>> metrics_service.cc
>>>>> so that you don't have to wait for 30 minutes for the next XML to be
>>>>> sent.
>>>>> And
>>>>> then look for the Action's hashes in the XML that's being logged -- you
>>>>> probably
>>>>> need to look at the second XML, not the first one that gets sent after 2
>>>>> minutes....
>>>
>>> So in this suggestion, where does the XML get logged?
>>>
>>>>> http://codereview.chromium.org/6266011/
>>
>

[petkov, 2011-01-20 05:24:27]

This looks bad. A couple of issues:

- Yes, there might be a concern about malformed user actions although that's not
too different than malformed histograms -- we have very few if any checks for
histogram data. It would have been nice if the protocol ensured some integrity
of the data.

- I'm concerned why you're seeing this. Reading the code, you'd get a log
message like this only if you had something like "useraction\0gram" or
"useraction\0etwork.Wifi.TimeOnline 12 1 28800 50" in the uma-events file. I
don't see how you end up in this state though... Both libmetrics and Chrome use
locks to write/read the file respectively. Do you see this consistently?

On 2011/01/20 02:16:15, Daniel Erat wrote:
> Now that I'm looking at the logs some more, I see entries like this
> (pre-dating my not-yet-checked-in change):
> 
> chrome_20110111-175153:[355:355:36466644:ERROR:external_metrics.cc(70)]
> undefined UMA action: latform.BootSectorsRead 204294 1 1000000 50
> chrome_20110111-175153:[355:355:36466854:ERROR:external_metrics.cc(70)]
> undefined UMA action: latform.BootSectorsWritten 1104 1 10000 50
> chrome_20110111-175153:[355:355:36467141:ERROR:external_metrics.cc(70)]
> undefined UMA action: etwork.Wifi.TimeOnline 12 1 28800 50
> chrome_20110111-175153:[355:355:36467375:ERROR:external_metrics.cc(70)]
> undefined UMA action: gram
> chrome_20110111-175153:[355:355:36467643:ERROR:external_metrics.cc(70)]
> undefined UMA action: gram
> chrome_20110111-175153:[355:355:36467859:ERROR:external_metrics.cc(70)]
> undefined UMA action: gram
> 
> Is there any concern that we're going to be reporting malformed
> actions if I remove the checks from external_metrics.cc?  Does this
> indicate a problem in the code that connects the metrics library to
> Chrome?
> 
> On Wed, Jan 19, 2011 at 5:47 PM, Daniel Erat <mailto:derat@chromium.org>
wrote:
> > Okay, thanks (came to the same conclusion from the code but think I
> > didn't get it since it's VLOG(1)).
> >
> > On Wed, Jan 19, 2011 at 5:45 PM, Darin Petkov <mailto:petkov@chromium.org>
wrote:
> >> The chrome log.
> >>
> >> Sent from my mobile phone.
> >>
> >> On Jan 19, 2011 5:41 PM, "Daniel Erat" <mailto:derat@chromium.org> wrote:
> >>> On Wed, Jan 19, 2011 at 5:13 PM, Daniel Erat <mailto:derat@chromium.org>
wrote:
> >>>> On Wed, Jan 19, 2011 at 5:00 PM, mailto: <petkov@chromium.org> wrote:
> >>> [snip]
> >>>>> Checking that the data is actually sent to UMA is probably a bit more
> >>>>> difficult.
> >>>>> One way is to run an official Chrome build (or comment out the proper
> >>>>> defines in
> >>>>> browser_main.cc:InitializeMetrics) so that the metrics service runs and
> >>>>> sends
> >>>>> data to UMA. Then modify all inerlog_duration_ assignments in
> >>>>> metrics_service.cc
> >>>>> so that you don't have to wait for 30 minutes for the next XML to be
> >>>>> sent.
> >>>>> And
> >>>>> then look for the Action's hashes in the XML that's being logged -- you
> >>>>> probably
> >>>>> need to look at the second XML, not the first one that gets sent after 2
> >>>>> minutes....
> >>>
> >>> So in this suggestion, where does the XML get logged?
> >>>
> >>>>> http://codereview.chromium.org/6266011/
> >>
> >

[Daniel Erat, 2011-01-20 22:24:56]

I filed http://crosbug.com/11125 to track this.  I can hold off on
these changes and just explicitly add my new actions to the .cc file
in Chrome until you figure out what's going on (assuming that
reporting bogus metrics would clutter up the dashboard).

On Wed, Jan 19, 2011 at 9:24 PM,  <petkov@chromium.org> wrote:
> This looks bad. A couple of issues:
>
> - Yes, there might be a concern about malformed user actions although that's
> not
> too different than malformed histograms -- we have very few if any checks
> for
> histogram data. It would have been nice if the protocol ensured some
> integrity
> of the data.
>
> - I'm concerned why you're seeing this. Reading the code, you'd get a log
> message like this only if you had something like "useraction\0gram" or
> "useraction\0etwork.Wifi.TimeOnline 12 1 28800 50" in the uma-events file. I
> don't see how you end up in this state though... Both libmetrics and Chrome
> use
> locks to write/read the file respectively. Do you see this consistently?
>
> On 2011/01/20 02:16:15, Daniel Erat wrote:
>>
>> Now that I'm looking at the logs some more, I see entries like this
>> (pre-dating my not-yet-checked-in change):
>
>> chrome_20110111-175153:[355:355:36466644:ERROR:external_metrics.cc(70)]
>> undefined UMA action: latform.BootSectorsRead 204294 1 1000000 50
>> chrome_20110111-175153:[355:355:36466854:ERROR:external_metrics.cc(70)]
>> undefined UMA action: latform.BootSectorsWritten 1104 1 10000 50
>> chrome_20110111-175153:[355:355:36467141:ERROR:external_metrics.cc(70)]
>> undefined UMA action: etwork.Wifi.TimeOnline 12 1 28800 50
>> chrome_20110111-175153:[355:355:36467375:ERROR:external_metrics.cc(70)]
>> undefined UMA action: gram
>> chrome_20110111-175153:[355:355:36467643:ERROR:external_metrics.cc(70)]
>> undefined UMA action: gram
>> chrome_20110111-175153:[355:355:36467859:ERROR:external_metrics.cc(70)]
>> undefined UMA action: gram
>
>> Is there any concern that we're going to be reporting malformed
>> actions if I remove the checks from external_metrics.cc?  Does this
>> indicate a problem in the code that connects the metrics library to
>> Chrome?
>
>> On Wed, Jan 19, 2011 at 5:47 PM, Daniel Erat <mailto:derat@chromium.org>
>
> wrote:
>>
>> > Okay, thanks (came to the same conclusion from the code but think I
>> > didn't get it since it's VLOG(1)).
>> >
>> > On Wed, Jan 19, 2011 at 5:45 PM, Darin Petkov
>> > <mailto:petkov@chromium.org>
>
> wrote:
>>
>> >> The chrome log.
>> >>
>> >> Sent from my mobile phone.
>> >>
>> >> On Jan 19, 2011 5:41 PM, "Daniel Erat" <mailto:derat@chromium.org>
>> >> wrote:
>> >>> On Wed, Jan 19, 2011 at 5:13 PM, Daniel Erat
>> >>> <mailto:derat@chromium.org>
>
> wrote:
>>
>> >>>> On Wed, Jan 19, 2011 at 5:00 PM, mailto: <petkov@chromium.org> wrote:
>> >>> [snip]
>> >>>>> Checking that the data is actually sent to UMA is probably a bit
>> >>>>> more
>> >>>>> difficult.
>> >>>>> One way is to run an official Chrome build (or comment out the
>> >>>>> proper
>> >>>>> defines in
>> >>>>> browser_main.cc:InitializeMetrics) so that the metrics service runs
>> >>>>> and
>> >>>>> sends
>> >>>>> data to UMA. Then modify all inerlog_duration_ assignments in
>> >>>>> metrics_service.cc
>> >>>>> so that you don't have to wait for 30 minutes for the next XML to be
>> >>>>> sent.
>> >>>>> And
>> >>>>> then look for the Action's hashes in the XML that's being logged --
>> >>>>> you
>> >>>>> probably
>> >>>>> need to look at the second XML, not the first one that gets sent
>> >>>>> after 2
>> >>>>> minutes....
>> >>>
>> >>> So in this suggestion, where does the XML get logged?
>> >>>
>> >>>>> http://codereview.chromium.org/6266011/
>> >>
>> >
>
>
>
> http://codereview.chromium.org/6266011/
>

[petkov, 2011-01-20 22:33:03]

On 2011/01/20 22:24:56, Daniel Erat wrote:
> I filed http://crosbug.com/11125 to track this.  I can hold off on
> these changes and just explicitly add my new actions to the .cc file
> in Chrome until you figure out what's going on (assuming that
> reporting bogus metrics would clutter up the dashboard).

Sure. I'm not sure how to reproduce though.

> 
> On Wed, Jan 19, 2011 at 9:24 PM,  <mailto:petkov@chromium.org> wrote:
> > This looks bad. A couple of issues:
> >
> > - Yes, there might be a concern about malformed user actions although that's
> > not
> > too different than malformed histograms -- we have very few if any checks
> > for
> > histogram data. It would have been nice if the protocol ensured some
> > integrity
> > of the data.
> >
> > - I'm concerned why you're seeing this. Reading the code, you'd get a log
> > message like this only if you had something like "useraction\0gram" or
> > "useraction\0etwork.Wifi.TimeOnline 12 1 28800 50" in the uma-events file. I
> > don't see how you end up in this state though... Both libmetrics and Chrome
> > use
> > locks to write/read the file respectively. Do you see this consistently?
> >
> > On 2011/01/20 02:16:15, Daniel Erat wrote:
> >>
> >> Now that I'm looking at the logs some more, I see entries like this
> >> (pre-dating my not-yet-checked-in change):
> >
> >> chrome_20110111-175153:[355:355:36466644:ERROR:external_metrics.cc(70)]
> >> undefined UMA action: latform.BootSectorsRead 204294 1 1000000 50
> >> chrome_20110111-175153:[355:355:36466854:ERROR:external_metrics.cc(70)]
> >> undefined UMA action: latform.BootSectorsWritten 1104 1 10000 50
> >> chrome_20110111-175153:[355:355:36467141:ERROR:external_metrics.cc(70)]
> >> undefined UMA action: etwork.Wifi.TimeOnline 12 1 28800 50
> >> chrome_20110111-175153:[355:355:36467375:ERROR:external_metrics.cc(70)]
> >> undefined UMA action: gram
> >> chrome_20110111-175153:[355:355:36467643:ERROR:external_metrics.cc(70)]
> >> undefined UMA action: gram
> >> chrome_20110111-175153:[355:355:36467859:ERROR:external_metrics.cc(70)]
> >> undefined UMA action: gram
> >
> >> Is there any concern that we're going to be reporting malformed
> >> actions if I remove the checks from external_metrics.cc? &nbsp;Does this
> >> indicate a problem in the code that connects the metrics library to
> >> Chrome?
> >
> >> On Wed, Jan 19, 2011 at 5:47 PM, Daniel Erat <mailto:derat@chromium.org>
> >
> > wrote:
> >>
> >> > Okay, thanks (came to the same conclusion from the code but think I
> >> > didn't get it since it's VLOG(1)).
> >> >
> >> > On Wed, Jan 19, 2011 at 5:45 PM, Darin Petkov
> >> > <mailto:petkov@chromium.org>
> >
> > wrote:
> >>
> >> >> The chrome log.
> >> >>
> >> >> Sent from my mobile phone.
> >> >>
> >> >> On Jan 19, 2011 5:41 PM, "Daniel Erat" <mailto:derat@chromium.org>
> >> >> wrote:
> >> >>> On Wed, Jan 19, 2011 at 5:13 PM, Daniel Erat
> >> >>> <mailto:derat@chromium.org>
> >
> > wrote:
> >>
> >> >>>> On Wed, Jan 19, 2011 at 5:00 PM, mailto:&nbsp;<petkov@chromium.org>
wrote:
> >> >>> [snip]
> >> >>>>> Checking that the data is actually sent to UMA is probably a bit
> >> >>>>> more
> >> >>>>> difficult.
> >> >>>>> One way is to run an official Chrome build (or comment out the
> >> >>>>> proper
> >> >>>>> defines in
> >> >>>>> browser_main.cc:InitializeMetrics) so that the metrics service runs
> >> >>>>> and
> >> >>>>> sends
> >> >>>>> data to UMA. Then modify all inerlog_duration_ assignments in
> >> >>>>> metrics_service.cc
> >> >>>>> so that you don't have to wait for 30 minutes for the next XML to be
> >> >>>>> sent.
> >> >>>>> And
> >> >>>>> then look for the Action's hashes in the XML that's being logged --
> >> >>>>> you
> >> >>>>> probably
> >> >>>>> need to look at the second XML, not the first one that gets sent
> >> >>>>> after 2
> >> >>>>> minutes....
> >> >>>
> >> >>> So in this suggestion, where does the XML get logged?
> >> >>>
> >> >>>>> http://codereview.chromium.org/6266011/
> >> >>
> >> >
> >
> >
> >
> > http://codereview.chromium.org/6266011/
> >

[Daniel Erat, 2011-01-21 02:52:54]

Please take another look.

On 2011/01/20 22:33:03, petkov wrote:
> On 2011/01/20 22:24:56, Daniel Erat wrote:
> > I filed http://crosbug.com/11125 to track this.  I can hold off on
> > these changes and just explicitly add my new actions to the .cc file
> > in Chrome until you figure out what's going on (assuming that
> > reporting bogus metrics would clutter up the dashboard).
> 
> Sure. I'm not sure how to reproduce though.
> 
> > 
> > On Wed, Jan 19, 2011 at 9:24 PM,  <mailto:petkov@chromium.org> wrote:
> > > This looks bad. A couple of issues:
> > >
> > > - Yes, there might be a concern about malformed user actions although
that's
> > > not
> > > too different than malformed histograms -- we have very few if any checks
> > > for
> > > histogram data. It would have been nice if the protocol ensured some
> > > integrity
> > > of the data.
> > >
> > > - I'm concerned why you're seeing this. Reading the code, you'd get a log
> > > message like this only if you had something like "useraction\0gram" or
> > > "useraction\0etwork.Wifi.TimeOnline 12 1 28800 50" in the uma-events file.
I
> > > don't see how you end up in this state though... Both libmetrics and
Chrome
> > > use
> > > locks to write/read the file respectively. Do you see this consistently?
> > >
> > > On 2011/01/20 02:16:15, Daniel Erat wrote:
> > >>
> > >> Now that I'm looking at the logs some more, I see entries like this
> > >> (pre-dating my not-yet-checked-in change):
> > >
> > >> chrome_20110111-175153:[355:355:36466644:ERROR:external_metrics.cc(70)]
> > >> undefined UMA action: latform.BootSectorsRead 204294 1 1000000 50
> > >> chrome_20110111-175153:[355:355:36466854:ERROR:external_metrics.cc(70)]
> > >> undefined UMA action: latform.BootSectorsWritten 1104 1 10000 50
> > >> chrome_20110111-175153:[355:355:36467141:ERROR:external_metrics.cc(70)]
> > >> undefined UMA action: etwork.Wifi.TimeOnline 12 1 28800 50
> > >> chrome_20110111-175153:[355:355:36467375:ERROR:external_metrics.cc(70)]
> > >> undefined UMA action: gram
> > >> chrome_20110111-175153:[355:355:36467643:ERROR:external_metrics.cc(70)]
> > >> undefined UMA action: gram
> > >> chrome_20110111-175153:[355:355:36467859:ERROR:external_metrics.cc(70)]
> > >> undefined UMA action: gram
> > >
> > >> Is there any concern that we're going to be reporting malformed
> > >> actions if I remove the checks from external_metrics.cc? &nbsp;Does this
> > >> indicate a problem in the code that connects the metrics library to
> > >> Chrome?
> > >
> > >> On Wed, Jan 19, 2011 at 5:47 PM, Daniel Erat <mailto:derat@chromium.org>
> > >
> > > wrote:
> > >>
> > >> > Okay, thanks (came to the same conclusion from the code but think I
> > >> > didn't get it since it's VLOG(1)).
> > >> >
> > >> > On Wed, Jan 19, 2011 at 5:45 PM, Darin Petkov
> > >> > <mailto:petkov@chromium.org>
> > >
> > > wrote:
> > >>
> > >> >> The chrome log.
> > >> >>
> > >> >> Sent from my mobile phone.
> > >> >>
> > >> >> On Jan 19, 2011 5:41 PM, "Daniel Erat" <mailto:derat@chromium.org>
> > >> >> wrote:
> > >> >>> On Wed, Jan 19, 2011 at 5:13 PM, Daniel Erat
> > >> >>> <mailto:derat@chromium.org>
> > >
> > > wrote:
> > >>
> > >> >>>> On Wed, Jan 19, 2011 at 5:00 PM, mailto:&nbsp;<petkov@chromium.org>
> wrote:
> > >> >>> [snip]
> > >> >>>>> Checking that the data is actually sent to UMA is probably a bit
> > >> >>>>> more
> > >> >>>>> difficult.
> > >> >>>>> One way is to run an official Chrome build (or comment out the
> > >> >>>>> proper
> > >> >>>>> defines in
> > >> >>>>> browser_main.cc:InitializeMetrics) so that the metrics service runs
> > >> >>>>> and
> > >> >>>>> sends
> > >> >>>>> data to UMA. Then modify all inerlog_duration_ assignments in
> > >> >>>>> metrics_service.cc
> > >> >>>>> so that you don't have to wait for 30 minutes for the next XML to
be
> > >> >>>>> sent.
> > >> >>>>> And
> > >> >>>>> then look for the Action's hashes in the XML that's being logged --
> > >> >>>>> you
> > >> >>>>> probably
> > >> >>>>> need to look at the second XML, not the first one that gets sent
> > >> >>>>> after 2
> > >> >>>>> minutes....
> > >> >>>
> > >> >>> So in this suggestion, where does the XML get logged?
> > >> >>>
> > >> >>>>> http://codereview.chromium.org/6266011/
> > >> >>
> > >> >
> > >
> > >
> > >
> > > http://codereview.chromium.org/6266011/
> > >

[petkov, 2011-01-21 18:03:42]

LGTM w/ a comment/suggestion.

http://codereview.chromium.org/6266011/diff/15001/chrome/browser/chromeos/ext...
File chrome/browser/chromeos/external_metrics.cc (right):

http://codereview.chromium.org/6266011/diff/15001/chrome/browser/chromeos/ext...
chrome/browser/chromeos/external_metrics.cc:37: // TODO(derat): We shouldn't
need to verify actions before reporting them;
Or we can modify the regular expression in extract_actions.py:GrepForActions to
recognize the pattern below too. This way we guarantee that we never send
unknown actions to the server and at the same time keep only one place under
Chrome that needs to be updated whenever a new action needs to be registered.

Maybe add this as an alternative to the TODO.

[Daniel Erat, 2011-01-21 18:26:09]

On Fri, Jan 21, 2011 at 10:03 AM,  <petkov@chromium.org> wrote:
> LGTM w/ a comment/suggestion.
>
>
http://codereview.chromium.org/6266011/diff/15001/chrome/browser/chromeos/ext...
> File chrome/browser/chromeos/external_metrics.cc (right):
>
>
http://codereview.chromium.org/6266011/diff/15001/chrome/browser/chromeos/ext...
> chrome/browser/chromeos/external_metrics.cc:37: // TODO(derat): We
> shouldn't need to verify actions before reporting them;
> Or we can modify the regular expression in
> extract_actions.py:GrepForActions to recognize the pattern below too.
> This way we guarantee that we never send unknown actions to the server
> and at the same time keep only one place under Chrome that needs to be
> updated whenever a new action needs to be registered.
>
> Maybe add this as an alternative to the TODO.

I'd still like to see us get to a point where only the hash file needs
to be updated.  Even if I changed extract_actions.py as you suggest,
it would still be possible for people to add new actions to
external_metrics.cc but forget to run the script (this happens
frequently with other metrics), so I don't think that unknown actions
can be prevented here.  Not needing to modify Chrome C++ code and send
changes to trybots when adding new external actions seems like it'd be
a pretty big improvement.

> http://codereview.chromium.org/6266011/
>