Track network stack error codes from UMA and UKM

Track network stack error codes from UMA and UKM

BUG=695538
Review-Url: https://codereview.chromium.org/2774503002
Cr-Commit-Position: refs/heads/master@{#461811}
Committed: https://chromium.googlesource.com/chromium/src/+/035ec7fb1af183cb9a902541b273208ddb6931ab

[Steven Holte, 2017-03-23 00:16:55]

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

[Alexei Svitkine (slow), 2017-03-23 15:23:43]

https://codereview.chromium.org/2774503002/diff/20001/components/metrics/metr...
File components/metrics/metrics_log_uploader.h (right):

https://codereview.chromium.org/2774503002/diff/20001/components/metrics/metr...
components/metrics/metrics_log_uploader.h:20: typedef base::Callback<void(int,
int)> UploadCallback;
Nit: Document the parameters please.

https://codereview.chromium.org/2774503002/diff/20001/components/metrics/metr...
File components/metrics/metrics_reporting_service.cc (right):

https://codereview.chromium.org/2774503002/diff/20001/components/metrics/metr...
components/metrics/metrics_reporting_service.cc:100:
UMA_HISTOGRAM_SPARSE_SLOWLY("UMA.FailedUploadErrorCode", -error_code);
I think error and response code should be logged in the same histogram, like we
do for Variations.SeedFetchResponseOrErrorCode.

We can deprecate UMA.UploadResponseStatus.Protobuf at the same time.

[Steven Holte, 2017-03-23 20:38:07]

https://codereview.chromium.org/2774503002/diff/20001/components/metrics/metr...
File components/metrics/metrics_log_uploader.h (right):

https://codereview.chromium.org/2774503002/diff/20001/components/metrics/metr...
components/metrics/metrics_log_uploader.h:20: typedef base::Callback<void(int,
int)> UploadCallback;
On 2017/03/23 15:23:42, Alexei Svitkine (slow) wrote:
> Nit: Document the parameters please.

Done.

https://codereview.chromium.org/2774503002/diff/20001/components/metrics/metr...
File components/metrics/metrics_reporting_service.cc (right):

https://codereview.chromium.org/2774503002/diff/20001/components/metrics/metr...
components/metrics/metrics_reporting_service.cc:100:
UMA_HISTOGRAM_SPARSE_SLOWLY("UMA.FailedUploadErrorCode", -error_code);
On 2017/03/23 15:23:42, Alexei Svitkine (slow) wrote:
> I think error and response code should be logged in the same histogram, like
we
> do for Variations.SeedFetchResponseOrErrorCode.
> 
> We can deprecate UMA.UploadResponseStatus.Protobuf at the same time.

Done.

[Alexei Svitkine (slow), 2017-03-23 20:41:58]

lgtm

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

https://codereview.chromium.org/2774503002/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:72923: +    HTTP response codes and
network error encountered by UKM when attempting to
Nit: errors (plural)

https://codereview.chromium.org/2774503002/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:73474: +    HTTP response codes and
network error encountered by UMA when attempting to
Nit: errors (plural)

[Steven Holte, 2017-03-23 21:08:53]

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

https://codereview.chromium.org/2774503002/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:72923: +    HTTP response codes and
network error encountered by UKM when attempting to
On 2017/03/23 20:41:58, Alexei Svitkine (slow) wrote:
> Nit: errors (plural)

Done.

https://codereview.chromium.org/2774503002/diff/40001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:73474: +    HTTP response codes and
network error encountered by UMA when attempting to
On 2017/03/23 20:41:58, Alexei Svitkine (slow) wrote:
> Nit: errors (plural)

Done.

[Steven Holte, 2017-03-27 22:10:47]

Description was changed from

==========
Track network stack error codes from UMA and UKM

BUG=695538
==========

to

==========
Track network stack error codes from UMA and UKM

TBR=olivierrobin,boliu

BUG=695538
==========

[Steven Holte, 2017-03-27 22:10:47]

holte@chromium.org changed reviewers:
+ boliu@chromium.org, olivierrobin@chromium.org

[Steven Holte, 2017-03-27 22:11:51]

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

[commit-bot: I haz the power, 2017-03-27 22:12:42]

Dry run: 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-03-27 23:39:13]

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

[commit-bot: I haz the power, 2017-03-27 23:39:14]

Dry run: This issue passed the CQ dry run.

[Olivier, 2017-03-31 07:50:16]

ios LGTM

[boliu, 2017-03-31 17:38:57]

android_webview rs lgtm

[paulmiller, 2017-03-31 17:46:40]

On 2017/03/31 17:38:57, boliu wrote:
> android_webview rs lgtm

Shouldn't you also change the callback declaration in
android_webview/native/aw_metrics_log_uploader.h?

[Steven Holte, 2017-04-04 17:53:02]

On 2017/03/31 17:46:40, paulmiller wrote:
> On 2017/03/31 17:38:57, boliu wrote:
> > android_webview rs lgtm
> 
> Shouldn't you also change the callback declaration in
> android_webview/native/aw_metrics_log_uploader.h?

Rebased onto your change adding it, and updated it call the callback with 0
error codes.

[Steven Holte, 2017-04-04 17:53:24]

Description was changed from

==========
Track network stack error codes from UMA and UKM

TBR=olivierrobin,boliu

BUG=695538
==========

to

==========
Track network stack error codes from UMA and UKM

BUG=695538
==========

[Steven Holte, 2017-04-04 17:53:45]

The CQ bit was checked by holte@chromium.org

[Steven Holte, 2017-04-04 17:53:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org, olivierrobin@chromium.org, boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/2774503002/#ps100001
(title: "Rebase on")

[commit-bot: I haz the power, 2017-04-04 17:54:18]

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

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1491328425434360,
"parent_rev": "0c150c2f05a1bc1e46f8bbace6228d365d4d50d2", "commit_rev":
"035ec7fb1af183cb9a902541b273208ddb6931ab"}

[commit-bot: I haz the power, 2017-04-04 20:17:52]

Description was changed from

==========
Track network stack error codes from UMA and UKM

BUG=695538
==========

to

==========
Track network stack error codes from UMA and UKM

BUG=695538

Review-Url: https://codereview.chromium.org/2774503002
Cr-Commit-Position: refs/heads/master@{#461811}
Committed:
https://chromium.googlesource.com/chromium/src/+/035ec7fb1af183cb9a902541b273...
==========

[commit-bot: I haz the power, 2017-04-04 20:17:53]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/035ec7fb1af183cb9a902541b273...