Closed
Bug 764260
Opened 12 years ago
Closed 12 years ago
Various networking telemetry probes are defined wrongly and may be giving misleading (wrong) results
Categories
(Core :: Networking, defect)
Core
Networking
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: briansmith, Assigned: mcmanus)
References
Details
Attachments
(1 file)
10.14 KB,
patch
|
briansmith
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #763351 +++ See the investigation in bug 763351 and bug 763359. I fixed the HTTP_*_DISPOSITION telemetry by creating new HTTP_*_DISPOSITION_2 probes using HISTOGRAM_ENUMERATED_VALUES. There are two other networking telemetry probes that may need to be fixed in a similar way: SPDY_VERSION DNS_LOOKUP_METHOD Should be a pretty simple change but I want to make sure that the people who work primarily on SPDY and DNS know exactly what change needs to be made, because it may mean that we have to start ignoring the above two probes and instead use new probes called SPDY_VERSION_2 and DNS_LOOKUP_METHOD_2 or similar.
Assignee | ||
Comment 1•12 years ago
|
||
brian, if I follow correctly you're saying that SPDY_VERSION and DNS_LOOKUP_METHOD need to be converted to the new ENUMERATED_VALUES style (and their names changed). we can do that under this bug.
Assignee: nobody → mcmanus
Reporter | ||
Comment 2•12 years ago
|
||
That is correct, Patrick.
Updated•12 years ago
|
Assignee | ||
Comment 3•12 years ago
|
||
Attachment #634578 -
Flags: review?(bsmith)
Assignee | ||
Comment 4•12 years ago
|
||
thanks btw brian for figuring this out.. those cache results were boggling.
Reporter | ||
Comment 5•12 years ago
|
||
Comment on attachment 634578 [details] [diff] [review] patch 0 Review of attachment 634578 [details] [diff] [review]: ----------------------------------------------------------------- r+ ::: toolkit/components/telemetry/TelemetryHistograms.h @@ +170,5 @@ > HTTP_HISTOGRAMS(PAGE, "page: ") > HTTP_HISTOGRAMS(SUB, "subitem: ") > > +// SPDY_VERSION was renamed to SPDY_VERSION2 as the old version did not use enumerated values > +HISTOGRAM_ENUMERATED_VALUES(SPDY_VERSION2, 16, "SPDY: Protocol Version Used") Make sure that this probe has a range that is as future-proof as you think is appropriate because we will have to rename the probe if we change the range. @@ +211,5 @@ > HISTOGRAM(CACHE_SERVICE_LOCK_WAIT, 1, 10000, 10000, LINEAR, "Time spent waiting on the cache service lock (ms)") > HISTOGRAM(CACHE_SERVICE_LOCK_WAIT_MAINTHREAD, 1, 10000, 10000, LINEAR, "Time spent waiting on the cache service lock on the main thread (ms)") > > +// DNS_LOOKUP_METHOD was renamed to DNS_LOOKUP_METHOD2 as the old version did not use enumerated values > +HISTOGRAM_ENUMERATED_VALUES(DNS_LOOKUP_METHOD2, 7, "DNS Lookup Type (hit, renewal, negative-hit, literal, overflow, network-first, network-shared)") If we ever add more methods, which would change the range, apparently we would have to rename the probe again. So, it might be a good idea to extend the range while we are at it.
Attachment #634578 -
Flags: review?(bsmith) → review+
Assignee | ||
Comment 6•12 years ago
|
||
(In reply to Brian Smith (:bsmith) from comment #5) > Comment on attachment 634578 [details] [diff] [review] > patch 0 > > Review of attachment 634578 [details] [diff] [review]: > ----------------------------------------------------------------- > > r+ > > ::: toolkit/components/telemetry/TelemetryHistograms.h > @@ +170,5 @@ > > HTTP_HISTOGRAMS(PAGE, "page: ") > > HTTP_HISTOGRAMS(SUB, "subitem: ") > > > > +// SPDY_VERSION was renamed to SPDY_VERSION2 as the old version did not use enumerated values > > +HISTOGRAM_ENUMERATED_VALUES(SPDY_VERSION2, 16, "SPDY: Protocol Version Used") > > Make sure that this probe has a range that is as future-proof as you think > is appropriate because we will have to rename the probe if we change the > range. > The fact that you say that about 16 versions is equal parts useful and depressing :)
Assignee | ||
Comment 7•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/bcf784073886
Target Milestone: --- → mozilla16
Comment 8•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/bcf784073886
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•