[Domain Reliability: Monitor] Report beacon if the go away is detected, regardless of request statu…

[Domain Reliability: Monitor] Report beacon if the go away is detected, regardless of request status.
BUG=593163
Committed: https://crrev.com/93fa5a95beaa9837dd0da8c9ab1f18026398f954
Cr-Commit-Position: refs/heads/master@{#381008}

[Zhongyi Shi, 2016-03-11 19:41:26]

zhongyi@chromium.org changed reviewers:
+ rch@chromium.org, ttuttle@chromium.org

[Ryan Hamilton, 2016-03-11 22:49:54]

looks good to me, but I'm pretty unfamiliar with this code so Julia is the
important voice :>

[Zhongyi Shi, 2016-03-12 00:43:07]

Set sample rate to 1.0 if the beacon is sent for goaway received due to port
migration.

[Deprecated (see juliatuttle), 2016-03-14 13:19:20]

lgtm

https://codereview.chromium.org/1785143003/diff/40001/components/domain_relia...
File components/domain_reliability/context.cc (right):

https://codereview.chromium.org/1785143003/diff/40001/components/domain_relia...
components/domain_reliability/context.cc:73: double sample_rate =
beacon->details.quic_port_migration_detected
nit: I'd prefer you do this with should_report, not sample_rate, since I think
it is clearer, but either is fine, especially as this is temporary debugging
code.

[Zhongyi Shi, 2016-03-14 17:12:42]

https://codereview.chromium.org/1785143003/diff/40001/components/domain_relia...
File components/domain_reliability/context.cc (right):

https://codereview.chromium.org/1785143003/diff/40001/components/domain_relia...
components/domain_reliability/context.cc:73: double sample_rate =
beacon->details.quic_port_migration_detected
On 2016/03/14 13:19:20, Julia Tuttle (deprecated) wrote:
> nit: I'd prefer you do this with should_report, not sample_rate, since I think
> it is clearer, but either is fine, especially as this is temporary debugging
> code.

I think the idea here is that 1. we want to report whatever the request status
is, "should_report" should always be true. 2. the sample rate for this beacon is
1.0 as it only represents itself.

So setting sample_rate to 1.0 sounds more reasonable?

[Zhongyi Shi, 2016-03-14 17:15:52]

The CQ bit was checked by zhongyi@chromium.org

[commit-bot: I haz the power, 2016-03-14 17:16:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1785143003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1785143003/40001

[Deprecated (see juliatuttle), 2016-03-14 17:18:35]

https://codereview.chromium.org/1785143003/diff/40001/components/domain_relia...
File components/domain_reliability/context.cc (right):

https://codereview.chromium.org/1785143003/diff/40001/components/domain_relia...
components/domain_reliability/context.cc:73: double sample_rate =
beacon->details.quic_port_migration_detected
On 2016/03/14 17:12:42, Zhongyi Shi wrote:
> On 2016/03/14 13:19:20, Julia Tuttle (deprecated) wrote:
> > nit: I'd prefer you do this with should_report, not sample_rate, since I
think
> > it is clearer, but either is fine, especially as this is temporary debugging
> > code.
> 
> I think the idea here is that 1. we want to report whatever the request status
> is, "should_report" should always be true. 2. the sample rate for this beacon
is
> 1.0 as it only represents itself.
> 
> So setting sample_rate to 1.0 sounds more reasonable?

Oh, you're right, I forgot we were recording sample_rate below. Sounds good to
me.

[commit-bot: I haz the power, 2016-03-14 18:11:13]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-14 18:12:47]

Description was changed from

==========
[Domain Reliability: Monitor] Report beacon if the go away is detected,
regardless of request status.
BUG=593163
==========

to

==========
[Domain Reliability: Monitor] Report beacon if the go away is detected,
regardless of request status.
BUG=593163

Committed: https://crrev.com/93fa5a95beaa9837dd0da8c9ab1f18026398f954
Cr-Commit-Position: refs/heads/master@{#381008}
==========

[commit-bot: I haz the power, 2016-03-14 18:12:48]

Patchset 3 (id:??) landed as
https://crrev.com/93fa5a95beaa9837dd0da8c9ab1f18026398f954
Cr-Commit-Position: refs/heads/master@{#381008}