[VL] Add histogram to track addition of 404 VisitRows [chromium/src : main]
Svend Larsen (Gerrit)
New activity on the change![]( "Open in Gerrit")
Related details
- manuk hovanesian
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
manuk hovanesian (Gerrit)
manuk hovanesian added 2 comments![]( "Open in Gerrit")
function because AddPageVisit() is called in a loop for redirects, andshould include in histograms.xml that this is recorded multiple times if 1 user-click results in multiple redirect visits.
also, would the metric be more interpretable if we only recorded `VisitIs404` for the final visit in the redirect chain?
e.g. a.com -> b.com -> ... -> z.com -> 404 would record 25 successes, and 1 404; which would be misleading.
Related details
- Svend Larsen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
manuk hovanesian (Gerrit)
manuk hovanesian voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Svend Larsen
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Svend Larsen (Gerrit)
Svend Larsen added 1 comment![]( "Open in Gerrit")
function because AddPageVisit() is called in a loop for redirects, andshould include in histograms.xml that this is recorded multiple times if 1 user-click results in multiple redirect visits.
also, would the metric be more interpretable if we only recorded `VisitIs404` for the final visit in the redirect chain?
e.g. a.com -> b.com -> ... -> z.com -> 404 would record 25 successes, and 1 404; which would be misleading.
should include in histograms.xml that this is recorded multiple times if 1 user-click results in multiple redirect visits.
Done!
My goal is to measure the impact on table insertions of making 404s eligible for History, rather than to get a sense of how many navigations end in 404, so I think emitting for each redirect is a better measure. But I think I should change the histogram from `VisitIs404` to `VisitAddedDueTo404`, and then for redirects in chains that end in a 404, I should emit `true`, since the redirect visits otherwise wouldn't have been saved to History. I'll upload a new patchset doing that soon.
Related details
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Svend Larsen (Gerrit)
Svend Larsen added 1 comment![]( "Open in Gerrit")
function because AddPageVisit() is called in a loop for redirects, andSvend Larsenshould include in histograms.xml that this is recorded multiple times if 1 user-click results in multiple redirect visits.
also, would the metric be more interpretable if we only recorded `VisitIs404` for the final visit in the redirect chain?
e.g. a.com -> b.com -> ... -> z.com -> 404 would record 25 successes, and 1 404; which would be misleading.
should include in histograms.xml that this is recorded multiple times if 1 user-click results in multiple redirect visits.
Done!
would the metric be more interpretable if we only recorded `VisitIs404` for the final visit in the redirect chain?
e.g. a.com -> b.com -> ... -> z.com -> 404 would record 25 successes, and 1 404; which would be misleading.My goal is to measure the impact on table insertions of making 404s eligible for History, rather than to get a sense of how many navigations end in 404, so I think emitting for each redirect is a better measure. But I think I should change the histogram from `VisitIs404` to `VisitAddedDueTo404`, and then for redirects in chains that end in a 404, I should emit `true`, since the redirect visits otherwise wouldn't have been saved to History. I'll upload a new patchset doing that soon.
for redirects in chains that end in a 404, I should emit true, since the redirect visits otherwise wouldn't have been saved to History. I'll upload a new patchset doing that soon.
I updated the histogram name and description, but it turns out the code was already doing the new behavior! Since I was surprised, I added a unit test (which I should have done in the first place) and more comments. PTAL again.
Related details
- manuk hovanesian
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |