1253379 - Not all timing data are carried to the child process

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Honza Bambas (:mayhemer)]

Apparently the patch [1], bug 1064706, doesn't carry at least the cache load timings.  This is a major telemetry and timing break.

Valentin, can you please take a look and check what is missing and fix it?


[1] https://bugzilla.mozilla.org/attachment.cgi?id=8500326&action=diff#a/netwerk/protocol/http/HttpChannelParent.cpp_sec3

Comment 2

[Valentin Gosu [:valentin] (he/him)]

Attachment: MozReview Request: Bug 1253379 - Cache timings not send to HttpChannelChild r=honzab

Review commit: https://reviewboard.mozilla.org/r/37951/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37951/

Comment 3

[Honza Bambas (:mayhemer)]

Comment on attachment 8726385 [details]
MozReview Request: Bug 1253379 - Cache timings not send to HttpChannelChild r=honzab

https://reviewboard.mozilla.org/r/37951/#review34517

::: netwerk/protocol/http/TimingStruct.h:33
(Diff revision 1)
> +  // the rest of the timings.

please say also WHY.

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8726385 [details]
MozReview Request: Bug 1253379 - Cache timings not send to HttpChannelChild r=honzab

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37951/diff/1-2/

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/96bf29982e94

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/96bf29982e94

Comment 7

[Honza Bambas (:mayhemer)]

Valentin, please nominate this up to beta.

Comment 8

[Valentin Gosu [:valentin] (he/him)]

Comment on attachment 8726385 [details]
MozReview Request: Bug 1253379 - Cache timings not send to HttpChannelChild r=honzab

Approval Request Comment

[Feature/regressing bug #]:
Bug 1064706 did not pass all of the timing data to the content process.

[User impact if declined]:
Telementry of cache timing on the content process will report wrong values.

[Describe test coverage new/current, TreeHerder]:
We have tests for other timings, but not cacheReadStart/End.

[Risks and why]: 
This is low risk. It only sends the missing cacheReadStart/End timestamps to the content process. It does not change behaviour, only the data reported in the child process.

[String/UUID change made/needed]:
None.

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8726385 [details]
MozReview Request: Bug 1253379 - Cache timings not send to HttpChannelChild r=honzab

Too late for 45

Comment 10

[Liz Henry (:lizzard Please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8726385 [details]
MozReview Request: Bug 1253379 - Cache timings not send to HttpChannelChild r=honzab

Fix for telemetry issue, we want good info in 46. 
Please uplift to beta (46 is beta now, we just did the merge an hour ago)

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/bbaf0ef3deea