Adds logging metrics to translate infobar.

Adds logging metrics to translate infobar.

Adds the following:
1. Differenciate between always translate and undo always translate
from the options menu,
2. Translate decline and close infobar events.
3. Count of number of times a page was translated.

BUG=720231
Review-Url: https://codereview.chromium.org/2878363002
Cr-Commit-Position: refs/heads/master@{#472351}
Committed: https://chromium.googlesource.com/chromium/src/+/1e56466a77adc37fef96e5069293c0b22ad2c49d

[ramyasharma, 2017-05-15 04:30:21]

Patchset #1 (id:1) has been deleted

[ramyasharma, 2017-05-15 04:30:48]

The CQ bit was checked by ramyasharma@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-15 04:31:07]

Dry run: CQ is trying da patch.

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

[ramyasharma, 2017-05-15 04:34:43]

ramyasharma@chromium.org changed reviewers:
+ isherman@chromium.org, mdjones@chromium.org

[ramyasharma, 2017-05-15 04:34:44]

isherman@chromium.org: Please review changes in 

mdjones@chromium.org: Please review changes in

[commit-bot: I haz the power, 2017-05-15 05:40:36]

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

[commit-bot: I haz the power, 2017-05-15 05:40:38]

Dry run: This issue passed the CQ dry run.

[mdjones, 2017-05-15 16:00:30]

https://codereview.chromium.org/2878363002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java
(right):

https://codereview.chromium.org/2878363002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:41:
private int mTotalTranslationCount = 0;
Is this only supposed to count the number of translations while the infobar is
open? It seems like this would only ever be 0 or 1.

[Ilya Sherman, 2017-05-15 22:59:54]

https://codereview.chromium.org/2878363002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2878363002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:75750: +<histogram
name="Translate.CompactInfobar.Count">
nit: "Foo.Count" reads about the same as "Foo" for a histogram -- all histograms
are counts.  Please update this name to clarify what's being counted -- e.g.
"TranslationsPerPage".

https://codereview.chromium.org/2878363002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:75752: +  <summary>Records the total
number of times a page was translated.</summary>
This is not quite detailed enough.  Could you please document when this is
recorded?  Also, it's not clear to me why a page might be translated multiple
times -- is that a bug?  It might be helpful to provide a motivating example for
why a page might undergo multiple translations.

[ramyasharma, 2017-05-16 04:41:38]

The CQ bit was checked by ramyasharma@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-16 04:41:56]

Dry run: CQ is trying da patch.

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

[ramyasharma, 2017-05-16 05:40:10]

Thanks Ilya and Mathew. PTAL?

https://codereview.chromium.org/2878363002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java
(right):

https://codereview.chromium.org/2878363002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/infobar/TranslateCompactInfoBar.java:41:
private int mTotalTranslationCount = 0;
On 2017/05/15 16:00:30, mdjones wrote:
> Is this only supposed to count the number of translations while the infobar is
> open? It seems like this would only ever be 0 or 1.

Yes, it counts the number of times the page was translated. We can translate to
multiple languages in the new UI, and go back and forth between original and
translated language many times. Hence this can be any number and not just 0 or
1.
This will give us a sense of how often people try out several translates on a
page, or flip back and forth.

https://codereview.chromium.org/2878363002/diff/20001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2878363002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:75750: +<histogram
name="Translate.CompactInfobar.Count">
On 2017/05/15 22:59:53, Ilya Sherman wrote:
> nit: "Foo.Count" reads about the same as "Foo" for a histogram -- all
histograms
> are counts.  Please update this name to clarify what's being counted -- e.g.
> "TranslationsPerPage".

Thanks Done.

https://codereview.chromium.org/2878363002/diff/20001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:75752: +  <summary>Records the total
number of times a page was translated.</summary>
On 2017/05/15 22:59:53, Ilya Sherman wrote:
> This is not quite detailed enough.  Could you please document when this is
> recorded?  Also, it's not clear to me why a page might be translated multiple
> times -- is that a bug?  It might be helpful to provide a motivating example
for
> why a page might undergo multiple translations.

This is not a bug, its a feature. User has the flexibility to pick any number of
target languages and translate. Not many users will obviously translate to more
than one language, but this is what we want to measure. 
Also reverting back to the original language counts as 1 translation.

[Ilya Sherman, 2017-05-16 05:54:38]

https://codereview.chromium.org/2878363002/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2878363002/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:75907: +<histogram
name="Translate.CompactInfobar.TranslationsPerPageCount">
Optional nit: I'd omit the word "Count" in this metric name.

https://codereview.chromium.org/2878363002/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:75907: +<histogram
name="Translate.CompactInfobar.TranslationsPerPageCount">
nit: Please add units="translations"

https://codereview.chromium.org/2878363002/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:75910: +    Records the total number of
times a page was translated. Reverting back to
Please document when this metric is recorded.  It looks like you currently
record a value every time a translation occurs.  That means that if a user
translates a page 5 times, you'll record each of 1, 2, 3, 4, and 5 to this
histogram.   If you could instead implement the metric to only record '5', that
would be better.  If that's infeasible, then please at least document the
behavior.

https://codereview.chromium.org/2878363002/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:75912: +    language results in 1
translation. This will give us a sense of how often
The second sentence is not very clear to me.  I think you're using almost
identical language to describe two different events, but I'm not entirely sure. 
Here's my guess at what you mean:

Case A: A user translates a page from French to English, and then reverts back
to French.  A total of 1 translations are emitted to this histogram.

Case B: A user translates a page from French to English, and then to Spanish.  A
total of 2 translations are emitted to this histogram.

Is my understanding of the above two correct?  Or is 2 emitted in each case? 
Also, I'm confused about the cases below:

Case C: A user translates a page from French to English, then to Spanish, then
back to English.  A total of ??? translations are emitted to this histogram.

Case D: A user translates a page from French to English, then to Spanish, then
back to French.  A total of ??? translations are emitted. to this histogram.

Please update the description to clarify all of these details.

[commit-bot: I haz the power, 2017-05-16 06:08:28]

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

[commit-bot: I haz the power, 2017-05-16 06:08:29]

Dry run: This issue passed the CQ dry run.

[ramyasharma, 2017-05-16 06:31:27]

Patchset #3 (id:60001) has been deleted

[ramyasharma, 2017-05-16 06:34:21]

Thanks Ilya, PTAL?

https://codereview.chromium.org/2878363002/diff/40001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2878363002/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:75907: +<histogram
name="Translate.CompactInfobar.TranslationsPerPageCount">
On 2017/05/16 05:54:38, Ilya Sherman wrote:
> Optional nit: I'd omit the word "Count" in this metric name.

Done.

https://codereview.chromium.org/2878363002/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:75907: +<histogram
name="Translate.CompactInfobar.TranslationsPerPageCount">
On 2017/05/16 05:54:38, Ilya Sherman wrote:
> nit: Please add units="translations"

Done.

https://codereview.chromium.org/2878363002/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:75910: +    Records the total number of
times a page was translated. Reverting back to
On 2017/05/16 05:54:38, Ilya Sherman wrote:
> Please document when this metric is recorded.  It looks like you currently
> record a value every time a translation occurs.  That means that if a user
> translates a page 5 times, you'll record each of 1, 2, 3, 4, and 5 to this
> histogram.   If you could instead implement the metric to only record '5',
that
> would be better.  If that's infeasible, then please at least document the
> behavior.

Yes, that is correct. It's not feasible to record only the final translation
number due to various reasons, so we fall back to recording each time, so that 1
means >= 1 translations, 2 is >=2 and so on.Have updated the behaviour. Let me
know if this is clear?

https://codereview.chromium.org/2878363002/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:75912: +    language results in 1
translation. This will give us a sense of how often
On 2017/05/16 05:54:38, Ilya Sherman wrote:
> The second sentence is not very clear to me.  I think you're using almost
> identical language to describe two different events, but I'm not entirely
sure. 
> Here's my guess at what you mean:
> 
> Case A: A user translates a page from French to English, and then reverts back
> to French.  A total of 1 translations are emitted to this histogram.
> 
> Case B: A user translates a page from French to English, and then to Spanish. 
A
> total of 2 translations are emitted to this histogram.
> 
> Is my understanding of the above two correct?  Or is 2 emitted in each case? 
> Also, I'm confused about the cases below:
> 
> Case C: A user translates a page from French to English, then to Spanish, then
> back to English.  A total of ??? translations are emitted to this histogram.
> 
> Case D: A user translates a page from French to English, then to Spanish, then
> back to French.  A total of ??? translations are emitted. to this histogram.
> 
> Please update the description to clarify all of these details.

I have updated the behaviour. Let me know if this is clear?

[Ilya Sherman, 2017-05-16 06:39:40]

Metrics LGTM, thanks.

[mdjones, 2017-05-16 15:28:02]

lgtm

[ramyasharma, 2017-05-17 00:34:24]

The CQ bit was checked by ramyasharma@chromium.org

[commit-bot: I haz the power, 2017-05-17 00:36:38]

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-17 04:06:08]

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

[commit-bot: I haz the power, 2017-05-17 04:06:09]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[ramyasharma, 2017-05-17 04:21:14]

The CQ bit was checked by ramyasharma@chromium.org

[commit-bot: I haz the power, 2017-05-17 04:22:44]

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-17 06:29:43]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1494994874000950,
"parent_rev": "93e568fe667c7bf74a30b59659f07cafe1fdee54", "commit_rev":
"1e56466a77adc37fef96e5069293c0b22ad2c49d"}

[commit-bot: I haz the power, 2017-05-17 06:29:52]

Description was changed from

==========
Adds logging metrics to translate infobar.

Adds the following:
1. Differenciate between always translate and undo always translate
from the options menu,
2. Translate decline and close infobar events.
3. Count of number of times a page was translated.

BUG=720231
==========

to

==========
Adds logging metrics to translate infobar.

Adds the following:
1. Differenciate between always translate and undo always translate
from the options menu,
2. Translate decline and close infobar events.
3. Count of number of times a page was translated.

BUG=720231

Review-Url: https://codereview.chromium.org/2878363002
Cr-Commit-Position: refs/heads/master@{#472351}
Committed:
https://chromium.googlesource.com/chromium/src/+/1e56466a77adc37fef96e5069293...
==========

[commit-bot: I haz the power, 2017-05-17 06:29:53]

Committed patchset #3 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/1e56466a77adc37fef96e5069293...