Add an override flag to enable metrics reporting.

Add an override flag to enable metrics reporting.

This simplifies local testing of metrics collection, particularly in dev builds.

BUG=717792
Review-Url: https://codereview.chromium.org/2861563002
Cr-Commit-Position: refs/heads/master@{#469526}
Committed: https://chromium.googlesource.com/chromium/src/+/8d28e669ef279c46b174cc78a5a24c729fb720a5

[Steven Holte, 2017-05-03 00:05:15]

Description was changed from

==========
Add an override flag to enable metrics reporting.

This can be used for testing metrics collection, and generating example logs.

BUG=
==========

to

==========
Add an override flag to enable metrics reporting.

This can be used for testing metrics collection, and generating example logs.

BUG=717792
==========

[Steven Holte, 2017-05-03 00:08:36]

holte@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Alexei Svitkine (slow), 2017-05-03 19:09:36]

I'm not sure we really need this. Especially since we're currently not doing any
tagging of example logs and that we didn't get that requirement from privacy.

[Steven Holte, 2017-05-03 19:46:40]

On 2017/05/03 19:09:36, Alexei Svitkine (slow) wrote:
> I'm not sure we really need this. Especially since we're currently not doing
any
> tagging of example logs and that we didn't get that requirement from privacy.

Even without the tagging I think this is valuable, since metrics are
conditioning on so many things.  Currently turning metrics on in developer mode
requires source modifications since the consent checks are gated by Chrome
branding checks.  You also need to either make another modification to skip the
sync check, or build with API keys so you can sign in a profile.  On regular
builds, there are multiple field trial / sampling checks to contend with.

I'd like to make confirming that your metrics recording is working be as simple
as start chrome with this flag and check chrome://ukm (cl/2858683006) after
you've done whatever should record it.

[Alexei Svitkine (slow), 2017-05-03 20:13:50]

I'd rather we call the flag just --enable-metrics-reporting-for-testing or
similar, then. Since there's no affect of it on any "sample data" marking
right now. We can always revisit the name if we do decide to go with that
in the future.

On Wed, May 3, 2017 at 3:46 PM, <holte@chromium.org> wrote:

> On 2017/05/03 19:09:36, Alexei Svitkine (slow) wrote:
> > I'm not sure we really need this. Especially since we're currently not
> doing
> any
> > tagging of example logs and that we didn't get that requirement from
> privacy.
>
> Even without the tagging I think this is valuable, since metrics are
> conditioning on so many things. Currently turning metrics on in developer
> mode
> requires source modifications since the consent checks are gated by Chrome
> branding checks. You also need to either make another modification to skip
> the
> sync check, or build with API keys so you can sign in a profile. On regular
> builds, there are multiple field trial / sampling checks to contend with.
>
> I'd like to make confirming that your metrics recording is working be as
> simple
> as start chrome with this flag and check chrome://ukm (cl/2858683006
> <https://critique.corp.google.com/#review/2858683006>) after
> you've done whatever should record it.
>
> https://codereview.chromium.org/2861563002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Steven Holte, 2017-05-04 21:30:49]

On 2017/05/03 20:13:50, Alexei Svitkine (slow) wrote:
> I'd rather we call the flag just --enable-metrics-reporting-for-testing or
> similar, then. Since there's no affect of it on any "sample data" marking
> right now. We can always revisit the name if we do decide to go with that
> in the future.
> 
> On Wed, May 3, 2017 at 3:46 PM, <mailto:holte@chromium.org> wrote:
> 
> > On 2017/05/03 19:09:36, Alexei Svitkine (slow) wrote:
> > > I'm not sure we really need this. Especially since we're currently not
> > doing
> > any
> > > tagging of example logs and that we didn't get that requirement from
> > privacy.
> >
> > Even without the tagging I think this is valuable, since metrics are
> > conditioning on so many things. Currently turning metrics on in developer
> > mode
> > requires source modifications since the consent checks are gated by Chrome
> > branding checks. You also need to either make another modification to skip
> > the
> > sync check, or build with API keys so you can sign in a profile. On regular
> > builds, there are multiple field trial / sampling checks to contend with.
> >
> > I'd like to make confirming that your metrics recording is working be as
> > simple
> > as start chrome with this flag and check chrome://ukm (cl/2858683006
> > <https://critique.corp.google.com/#review/2858683006>) after
> > you've done whatever should record it.
> >
> > https://codereview.chromium.org/2861563002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Renamed the flag and functions.  One drawback of this name is that calling the
function XyzForTesting() scares presubmit.

Also made this function override the UKM feature check.

[Alexei Svitkine (slow), 2017-05-04 21:36:01]

Hmm, good point about the presubmits - that's not ideal.

So maybe instead of for-testing we can name it
"--force-enable-metrics-reporting"?

LGTM with the above flag name and functions renamed to something like
IsMetricsReportingForceEnabled().

https://codereview.chromium.org/2861563002/diff/20001/chrome/common/chrome_sw...
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/2861563002/diff/20001/chrome/common/chrome_sw...
chrome/common/chrome_switches.cc:359: // Forces metrics reporting to be enabled
and may tag logs as examples.
Nit: Omit the "and may tag logs as examples" part. It can be re-added if we ever
implement something like that.

[Steven Holte, 2017-05-04 22:07:04]

https://codereview.chromium.org/2861563002/diff/20001/chrome/common/chrome_sw...
File chrome/common/chrome_switches.cc (right):

https://codereview.chromium.org/2861563002/diff/20001/chrome/common/chrome_sw...
chrome/common/chrome_switches.cc:359: // Forces metrics reporting to be enabled
and may tag logs as examples.
On 2017/05/04 21:36:01, Alexei Svitkine (slow) wrote:
> Nit: Omit the "and may tag logs as examples" part. It can be re-added if we
ever
> implement something like that.

Done.

[Steven Holte, 2017-05-04 22:09:30]

Description was changed from

==========
Add an override flag to enable metrics reporting.

This can be used for testing metrics collection, and generating example logs.

BUG=717792
==========

to

==========
Add an override flag to enable metrics reporting.

This simplifies local testing of metrics collection, particularly in dev builds.

BUG=717792
==========

[Steven Holte, 2017-05-04 22:14:08]

The CQ bit was checked by holte@chromium.org

[Steven Holte, 2017-05-04 22:14:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2861563002/#ps40001
(title: "Rename to IsMetricsReportingForceEnabled")

[commit-bot: I haz the power, 2017-05-04 22:14:53]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-04 22:18:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-04 22:18:57]

Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Steven Holte, 2017-05-04 22:40:50]

The CQ bit was checked by holte@chromium.org

[Steven Holte, 2017-05-04 22:40:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2861563002/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2017-05-04 22:41:29]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-04 23:50:46]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1493937650049800,
"parent_rev": "f05a24a46d1c6b00e76ccdf23f8fae3c39b0aab6", "commit_rev":
"8d28e669ef279c46b174cc78a5a24c729fb720a5"}

[commit-bot: I haz the power, 2017-05-04 23:50:54]

Description was changed from

==========
Add an override flag to enable metrics reporting.

This simplifies local testing of metrics collection, particularly in dev builds.

BUG=717792
==========

to

==========
Add an override flag to enable metrics reporting.

This simplifies local testing of metrics collection, particularly in dev builds.

BUG=717792

Review-Url: https://codereview.chromium.org/2861563002
Cr-Commit-Position: refs/heads/master@{#469526}
Committed:
https://chromium.googlesource.com/chromium/src/+/8d28e669ef279c46b174cc78a5a2...
==========

[commit-bot: I haz the power, 2017-05-04 23:50:55]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/8d28e669ef279c46b174cc78a5a2...