Measure and log the latency of VariationsService requests.

Measure and log the latency of VariationsService requests.

TEST=Start Chrome, navigate to about:histograms, and ensure that there is an entry for Variations.RequestSuccessLatency or Variations.RequestNonSuccessLatency.
BUG=152958
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=159989

[SteveT, 2012-09-28 18:44:27]

Not quite done yet... What do you think about these options:
1) Logging just the latency of successful.
2) Logging successful vs non-successful latency (non 200). What this patch does
now.
3) Logging success (200), empty response (something), and everything else.

?

I worry a bit that (3) is too much stuff. Especially since we might not care
about the latency of empty vs full responses after x thousand samples.


Steve

[Alexei Svitkine (slow), 2012-09-28 18:56:27]

I'm okay with 3.

I think its reasonable to monitor these over time (e.g. if we make changes to
the server or get more clients, what happens to latency, etc.)

[SteveT, 2012-09-28 19:11:27]

Okay, here is a stab at doing all three.

I suppose that we can observe these for a bit and remove the ones we deem
useless. Which is better than wishing we had something that wasn't implemented.


Steve

[SteveT, 2012-10-02 14:38:32]

Friendly neighbourhood spider ping.

[jwd, 2012-10-02 20:54:19]

On 2012/10/02 14:38:32, SteveT wrote:
> Friendly neighbourhood spider ping.

LGTM

[Ilya Sherman, 2012-10-02 21:09:32]

http://codereview.chromium.org/10996067/diff/7003/chrome/browser/metrics/vari...
File chrome/browser/metrics/variations/variations_service.cc (right):

http://codereview.chromium.org/10996067/diff/7003/chrome/browser/metrics/vari...
chrome/browser/metrics/variations/variations_service.cc:101:
UMA_HISTOGRAM_CUSTOM_TIMES(name,
This won't work.  The name passed to any of the UMA_HISTOGRAM macros needs to be
a fixed string during the entire run of Chrome, as the macro internally caches
the histogram in a static variable.

http://codereview.chromium.org/10996067/diff/7003/chrome/browser/metrics/vari...
chrome/browser/metrics/variations/variations_service.cc:101:
UMA_HISTOGRAM_CUSTOM_TIMES(name,
nit: Why do we need custom times for this histogram, rather than using one of
MEDIUM_ or LONG_TIMES?

[SteveT, 2012-10-03 19:51:33]

PTAL.

http://codereview.chromium.org/10996067/diff/7003/chrome/browser/metrics/vari...
File chrome/browser/metrics/variations/variations_service.cc (right):

http://codereview.chromium.org/10996067/diff/7003/chrome/browser/metrics/vari...
chrome/browser/metrics/variations/variations_service.cc:101:
UMA_HISTOGRAM_CUSTOM_TIMES(name,
Darn, okay. It was odd that my testing seemed to work.

I've reverted to calling UMA_HISTOGRAMS_... three times.

http://codereview.chromium.org/10996067/diff/7003/chrome/browser/metrics/vari...
chrome/browser/metrics/variations/variations_service.cc:101:
UMA_HISTOGRAM_CUSTOM_TIMES(name,
I was following what how some Net code was measuring some Http request
latencies... But it looked like that code was just for the handshake.

3 minutes seems like a reasonable upper bound on this histogram, so I'll use
MEDIUM_TIMES unless you think otherwise.

[Ilya Sherman, 2012-10-03 20:15:12]

LGTM, thanks :)

[commit-bot: I haz the power, 2012-10-03 20:16:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevet@chromium.org/10996067/17001

[commit-bot: I haz the power, 2012-10-03 22:11:58]

Change committed as 159989