New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 701608 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

event_type DISABLED_BY_PREF incorrectly logged

Project Member Reported by pdyson@chromium.org, Mar 15 2017

Issue description

DISABLED_BY_PREF is being logged alongside INITIATION_STATUS_LANGUAGE_IN_ULP. This is not the correct place.

https://cs.chromium.org/chromium/src/components/translate/core/browser/translate_manager.cc?l=301

It should probably be logged alongside INITIATION_STATUS_DISABLED_BY_PREFS here:

https://cs.chromium.org/chromium/src/components/translate/core/browser/translate_manager.cc?l=183
 
Cc: hamelphi@chromium.org
+ hamelphi - this looks to be an error that affects the TranslateEventProto metrics.

Comment 2 by pdyson@chromium.org, Mar 15 2017

Cc: rogerm@chromium.org
+ rogerm - Rachel suggested that I ask you about this. Should I just move this logging up to near line 183?
FRom what I can see, I suggest we add an enum for the ULP case in TEP event_types, replace the DISABLED_BY_PREFS there with this new case, and add the DISABLED_BY_PREFS at line 183.
Status: Assigned (was: Unconfirmed)
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a12e52d81346dd23d8284763d44c4ee657f11cea

commit a12e52d81346dd23d8284763d44c4ee657f11cea
Author: pdyson <pdyson@chromium.org>
Date: Mon Mar 27 01:27:37 2017

Fix counting of ULP and DISABLED_BY_PREF.

* Move counting of DISABLED_BY_PREF to the correct place.
* Count when translate is not used due to ULP using a new enum: LANGUAGE_IN_ULP.

BUG= 701608 

Review-Url: https://codereview.chromium.org/2757173002
Cr-Commit-Position: refs/heads/master@{#459687}

[modify] https://crrev.com/a12e52d81346dd23d8284763d44c4ee657f11cea/components/metrics/proto/translate_event.proto
[modify] https://crrev.com/a12e52d81346dd23d8284763d44c4ee657f11cea/components/translate/core/browser/translate_manager.cc

Comment 6 by pdyson@chromium.org, Mar 27 2017

Status: Fixed (was: Assigned)
Components: -UI>Browser>Translate UI>Browser>Language>Translate

Sign in to add a comment