Commit 08b78eae authored by Xinghui Lu's avatar Xinghui Lu Committed by Commit Bot

Add cache related metrics for real time URL check.

Metrics added in this CL:
1. SafeBrowsing.RT.HasValidCacheManager
This metrics is for checking if cache manager is valid when real time
url check is enabled.
2. SafeBrowsing.RT.GetCacheResult
This metrics is useful to identify the cache hit rate and the proportion
between safe verdicts and dangerous verdicts.
3. SafeBrowsing.RT.GetCache.Time
This metrics is useful to identify if the time to get cache is too long
compared with its effectiveness.

Bug: 1030989
Change-Id: I616ac3ad3bc207c907bd401801a09a9e46742818
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1974843
Commit-Queue: Xinghui Lu <xinghuilu@chromium.org>
Reviewed-by: default avatarIlya Sherman <isherman@chromium.org>
Reviewed-by: default avatarVarun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#726621}
parent c13b5c9a
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "components/safe_browsing/browser/safe_browsing_url_checker_impl.h" #include "components/safe_browsing/browser/safe_browsing_url_checker_impl.h"
#include "base/bind.h" #include "base/bind.h"
#include "base/metrics/histogram_functions.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/metrics/histogram_macros_local.h" #include "base/metrics/histogram_macros_local.h"
#include "base/task/post_task.h" #include "base/task/post_task.h"
...@@ -452,20 +453,24 @@ void SafeBrowsingUrlCheckerImpl::OnCheckUrlForHighConfidenceAllowlist( ...@@ -452,20 +453,24 @@ void SafeBrowsingUrlCheckerImpl::OnCheckUrlForHighConfidenceAllowlist(
FROM_HERE, {content::BrowserThread::UI}, FROM_HERE, {content::BrowserThread::UI},
base::BindOnce( base::BindOnce(
&SafeBrowsingUrlCheckerImpl::StartGetCachedRealTimeUrlVerdictOnUI, &SafeBrowsingUrlCheckerImpl::StartGetCachedRealTimeUrlVerdictOnUI,
weak_factory_.GetWeakPtr(), cache_manager_on_ui_, url)); weak_factory_.GetWeakPtr(), cache_manager_on_ui_, url,
base::TimeTicks::Now()));
} }
// static // static
void SafeBrowsingUrlCheckerImpl::StartGetCachedRealTimeUrlVerdictOnUI( void SafeBrowsingUrlCheckerImpl::StartGetCachedRealTimeUrlVerdictOnUI(
base::WeakPtr<SafeBrowsingUrlCheckerImpl> weak_checker_on_io, base::WeakPtr<SafeBrowsingUrlCheckerImpl> weak_checker_on_io,
base::WeakPtr<VerdictCacheManager> cache_manager_on_ui, base::WeakPtr<VerdictCacheManager> cache_manager_on_ui,
const GURL& url) { const GURL& url,
base::TimeTicks get_cache_start_time) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI); DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
std::unique_ptr<RTLookupResponse::ThreatInfo> cached_threat_info = std::unique_ptr<RTLookupResponse::ThreatInfo> cached_threat_info =
std::make_unique<RTLookupResponse::ThreatInfo>(); std::make_unique<RTLookupResponse::ThreatInfo>();
// TODO(crbug.com/1030989): Add metrics for the proportion of cache_manager
// not valid. base::UmaHistogramBoolean("SafeBrowsing.RT.HasValidCacheManager",
!!cache_manager_on_ui);
RTLookupResponse::ThreatInfo::VerdictType verdict_type = RTLookupResponse::ThreatInfo::VerdictType verdict_type =
cache_manager_on_ui cache_manager_on_ui
? cache_manager_on_ui->GetCachedRealTimeUrlVerdict( ? cache_manager_on_ui->GetCachedRealTimeUrlVerdict(
...@@ -475,17 +480,21 @@ void SafeBrowsingUrlCheckerImpl::StartGetCachedRealTimeUrlVerdictOnUI( ...@@ -475,17 +480,21 @@ void SafeBrowsingUrlCheckerImpl::StartGetCachedRealTimeUrlVerdictOnUI(
FROM_HERE, {content::BrowserThread::IO}, FROM_HERE, {content::BrowserThread::IO},
base::BindOnce( base::BindOnce(
&SafeBrowsingUrlCheckerImpl::OnGetCachedRealTimeUrlVerdictDoneOnIO, &SafeBrowsingUrlCheckerImpl::OnGetCachedRealTimeUrlVerdictDoneOnIO,
weak_checker_on_io, verdict_type, std::move(cached_threat_info), weak_checker_on_io, verdict_type, std::move(cached_threat_info), url,
url)); get_cache_start_time));
} }
void SafeBrowsingUrlCheckerImpl::OnGetCachedRealTimeUrlVerdictDoneOnIO( void SafeBrowsingUrlCheckerImpl::OnGetCachedRealTimeUrlVerdictDoneOnIO(
RTLookupResponse::ThreatInfo::VerdictType verdict_type, RTLookupResponse::ThreatInfo::VerdictType verdict_type,
std::unique_ptr<RTLookupResponse::ThreatInfo> cached_threat_info, std::unique_ptr<RTLookupResponse::ThreatInfo> cached_threat_info,
const GURL& url) { const GURL& url,
base::TimeTicks get_cache_start_time) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO); DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
// TODO(crbug.com/1030989): Add metrics for cache hit rate and latency. base::UmaHistogramSparse("SafeBrowsing.RT.GetCacheResult", verdict_type);
UMA_HISTOGRAM_TIMES("SafeBrowsing.RT.GetCache.Time",
base::TimeTicks::Now() - get_cache_start_time);
if (verdict_type == RTLookupResponse::ThreatInfo::SAFE) { if (verdict_type == RTLookupResponse::ThreatInfo::SAFE) {
OnUrlResult(url, SB_THREAT_TYPE_SAFE, ThreatMetadata()); OnUrlResult(url, SB_THREAT_TYPE_SAFE, ThreatMetadata());
return; return;
......
...@@ -126,13 +126,15 @@ class SafeBrowsingUrlCheckerImpl : public mojom::SafeBrowsingUrlChecker, ...@@ -126,13 +126,15 @@ class SafeBrowsingUrlCheckerImpl : public mojom::SafeBrowsingUrlChecker,
static void StartGetCachedRealTimeUrlVerdictOnUI( static void StartGetCachedRealTimeUrlVerdictOnUI(
base::WeakPtr<SafeBrowsingUrlCheckerImpl> weak_checker_on_io, base::WeakPtr<SafeBrowsingUrlCheckerImpl> weak_checker_on_io,
base::WeakPtr<VerdictCacheManager> cache_manager_on_ui, base::WeakPtr<VerdictCacheManager> cache_manager_on_ui,
const GURL& url); const GURL& url,
base::TimeTicks get_cache_start_time);
// This function will start real time url lookup if there is no cache match. // This function will start real time url lookup if there is no cache match.
void OnGetCachedRealTimeUrlVerdictDoneOnIO( void OnGetCachedRealTimeUrlVerdictDoneOnIO(
RTLookupResponse::ThreatInfo::VerdictType verdict_type, RTLookupResponse::ThreatInfo::VerdictType verdict_type,
std::unique_ptr<RTLookupResponse::ThreatInfo> cached_threat_info, std::unique_ptr<RTLookupResponse::ThreatInfo> cached_threat_info,
const GURL& url); const GURL& url,
base::TimeTicks get_cache_start_time);
void OnTimeout(); void OnTimeout();
......
...@@ -55325,6 +55325,12 @@ Called by update_net_trust_anchors.py.--> ...@@ -55325,6 +55325,12 @@ Called by update_net_trust_anchors.py.-->
<int value="2" label="BLACKLISTED"/> <int value="2" label="BLACKLISTED"/>
</enum> </enum>
<enum name="SafeBrowsingRTLookupResponseVerdictType">
<int value="0" label="VERDICT_TYPE_UNSPECIFIED"/>
<int value="1" label="SAFE"/>
<int value="100" label="DANGEROUS"/>
</enum>
<enum name="SafeBrowsingStoreAvailabilityResult"> <enum name="SafeBrowsingStoreAvailabilityResult">
<int value="0" label="Unknown"/> <int value="0" label="Unknown"/>
<int value="1" label="Not Enabled"/> <int value="1" label="Not Enabled"/>
...@@ -128093,6 +128093,40 @@ uploading your change for review. ...@@ -128093,6 +128093,40 @@ uploading your change for review.
</summary> </summary>
</histogram> </histogram>
<histogram name="SafeBrowsing.RT.GetCache.Time" units="ms"
expires_after="2020-12-18">
<owner>xinghuilu@chromium.org</owner>
<owner>chrome-safebrowsing-alerts@google.com</owner>
<summary>
Logs the latency between the start of getting a result from the cache and
when the cache is actually obtained. The time includes bouncing between IO
and UI threads. This is an indicator of the efficiency of loading the cache.
</summary>
</histogram>
<histogram name="SafeBrowsing.RT.GetCacheResult"
enum="SafeBrowsingRTLookupResponseVerdictType" expires_after="2020-12-18">
<owner>xinghuilu@chromium.org</owner>
<owner>chrome-safebrowsing-alerts@google.com</owner>
<summary>
Logs the result of real time URL cache lookup. If the result is
VERDICT_TYPE_UNSPECIFIED, that means cache miss and a ping will be sent
afterwards. Otherwise, ping won't be sent. This is an indicator of cache hit
rate.
</summary>
</histogram>
<histogram name="SafeBrowsing.RT.HasValidCacheManager" enum="BooleanValid"
expires_after="2020-12-18">
<owner>xinghuilu@chromium.org</owner>
<owner>chrome-safebrowsing-alerts@google.com</owner>
<summary>
Logs if cache manager is valid when real time URL check is enabled. Logged
each time if the URL doesn't match the high confidence allowlist. Ideally,
cache manager should be valid whenever real time URL check is enabled.
</summary>
</histogram>
<histogram name="SafeBrowsing.RT.LocalMatch.Result" <histogram name="SafeBrowsing.RT.LocalMatch.Result"
enum="SafeBrowsingAllowlistAsyncMatch" expires_after="2020-08-05"> enum="SafeBrowsingAllowlistAsyncMatch" expires_after="2020-08-05">
<owner>vakh@chromium.org</owner> <owner>vakh@chromium.org</owner>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment