From 5570291ec49c244ce21df61f202d2f88f36c21ce Mon Sep 17 00:00:00 2001 From: Valentin Gosu Date: Fri, 29 May 2020 21:23:41 +0000 Subject: [PATCH] Bug 1640867 - Record TRR steering telemetry separately r=dragana,tdsmith,necko-reviewers This change makes a set of probes keyed. The key is "(default)" if no steering is currently active, or "(auto-detected)" is the current URL has been set by steering (autodetection). Differential Revision: https://phabricator.services.mozilla.com/D77022 --- netwerk/dns/TRR.cpp | 6 ++- netwerk/dns/TRRService.cpp | 26 +++++++--- netwerk/dns/TRRService.h | 3 ++ netwerk/dns/nsHostResolver.cpp | 53 +++++++++++++------- toolkit/components/telemetry/Histograms.json | 49 ++++++++++-------- 5 files changed, 89 insertions(+), 48 deletions(-) diff --git a/netwerk/dns/TRR.cpp b/netwerk/dns/TRR.cpp index b5eba91e9572..db61df861408 100644 --- a/netwerk/dns/TRR.cpp +++ b/netwerk/dns/TRR.cpp @@ -254,14 +254,16 @@ nsresult TRR::SendHTTPRequest() { gTRRService->IsTRRBlacklisted(mHost, mOriginSuffix, mPB, true)) { if (mType == TRRTYPE_A) { // count only blacklist for A records to avoid double counts - Telemetry::Accumulate(Telemetry::DNS_TRR_BLACKLISTED, true); + Telemetry::Accumulate(Telemetry::DNS_TRR_BLACKLISTED2, + TRRService::AutoDetectedKey(), true); } // not really an error but no TRR is issued return NS_ERROR_UNKNOWN_HOST; } if (UseDefaultServer() && (mType == TRRTYPE_A)) { - Telemetry::Accumulate(Telemetry::DNS_TRR_BLACKLISTED, false); + Telemetry::Accumulate(Telemetry::DNS_TRR_BLACKLISTED2, + TRRService::AutoDetectedKey(), false); } } diff --git a/netwerk/dns/TRRService.cpp b/netwerk/dns/TRRService.cpp index c9765953cf26..c2a8de1e2b76 100644 --- a/netwerk/dns/TRRService.cpp +++ b/netwerk/dns/TRRService.cpp @@ -102,6 +102,17 @@ bool TRRService::CheckCaptivePortalIsPassed() { return result; } +NS_NAMED_LITERAL_CSTRING(kTRRIsAutoDetectedKey, "(auto-detected)"); +NS_NAMED_LITERAL_CSTRING(kTRRNotAutoDetectedKey, "(default)"); +// static +const nsCString& TRRService::AutoDetectedKey() { + if (gTRRService && gTRRService->IsUsingAutoDetectedURL()) { + return kTRRIsAutoDetectedKey.AsString(); + } + + return kTRRNotAutoDetectedKey.AsString(); +} + nsresult TRRService::Init() { MOZ_ASSERT(NS_IsMainThread(), "wrong thread"); if (mInitialized) { @@ -895,11 +906,13 @@ void TRRService::TRRIsOkay(enum TrrOkay aReason) { XRE_IsSocketProcess(), NS_IsMainThread()); - Telemetry::AccumulateCategorical( - aReason == OKAY_NORMAL ? Telemetry::LABELS_DNS_TRR_SUCCESS::Fine - : (aReason == OKAY_TIMEOUT - ? Telemetry::LABELS_DNS_TRR_SUCCESS::Timeout - : Telemetry::LABELS_DNS_TRR_SUCCESS::Bad)); + Telemetry::AccumulateCategoricalKeyed( + AutoDetectedKey(), + aReason == OKAY_NORMAL + ? Telemetry::LABELS_DNS_TRR_SUCCESS2::Fine + : (aReason == OKAY_TIMEOUT + ? Telemetry::LABELS_DNS_TRR_SUCCESS2::Timeout + : Telemetry::LABELS_DNS_TRR_SUCCESS2::Bad)); if (aReason == OKAY_NORMAL) { mTRRFailures = 0; } else if ((mMode == MODE_TRRFIRST) && (mConfirmationState == CONFIRM_OK)) { @@ -960,7 +973,8 @@ AHostResolver::LookupStatus TRRService::CompleteLookup( // don't accumulate trronly data here since trronly failures are // handled above by trying again, so counting the successes here would // skew the numbers - Telemetry::Accumulate(Telemetry::DNS_TRR_NS_VERFIFIED, + Telemetry::Accumulate(Telemetry::DNS_TRR_NS_VERFIFIED2, + TRRService::AutoDetectedKey(), (mConfirmationState == CONFIRM_OK)); } mRetryConfirmInterval = 1000; diff --git a/netwerk/dns/TRRService.h b/netwerk/dns/TRRService.h index 086cd2f4efc6..d63f2dfb8028 100644 --- a/netwerk/dns/TRRService.h +++ b/netwerk/dns/TRRService.h @@ -74,6 +74,9 @@ class TRRService : public TRRServiceBase, already_AddRefed TRRThread(); bool IsOnTRRThread(); + bool IsUsingAutoDetectedURL() { return mURISetByDetection; } + static const nsCString& AutoDetectedKey(); + private: virtual ~TRRService(); diff --git a/netwerk/dns/nsHostResolver.cpp b/netwerk/dns/nsHostResolver.cpp index 551808da03ff..dcca7d1936fd 100644 --- a/netwerk/dns/nsHostResolver.cpp +++ b/netwerk/dns/nsHostResolver.cpp @@ -414,51 +414,66 @@ void AddrHostRecord::ResolveComplete() { uint32_t millis = static_cast(mNativeDuration.ToMilliseconds()); Telemetry::Accumulate(Telemetry::DNS_NATIVE_LOOKUP_TIME, millis); } - AccumulateCategorical( - mNativeSuccess ? Telemetry::LABELS_DNS_LOOKUP_DISPOSITION::osOK - : Telemetry::LABELS_DNS_LOOKUP_DISPOSITION::osFail); + AccumulateCategoricalKeyed( + TRRService::AutoDetectedKey(), + mNativeSuccess ? Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::osOK + : Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::osFail); } if (mTRRUsed) { if (mTRRSuccess) { uint32_t millis = static_cast(mTrrDuration.ToMilliseconds()); - Telemetry::Accumulate(Telemetry::DNS_TRR_LOOKUP_TIME, millis); + Telemetry::Accumulate(Telemetry::DNS_TRR_LOOKUP_TIME2, + TRRService::AutoDetectedKey(), millis); } - AccumulateCategorical( - mTRRSuccess ? Telemetry::LABELS_DNS_LOOKUP_DISPOSITION::trrOK - : Telemetry::LABELS_DNS_LOOKUP_DISPOSITION::trrFail); + AccumulateCategoricalKeyed( + TRRService::AutoDetectedKey(), + mTRRSuccess ? Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::trrOK + : Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::trrFail); if (mTrrAUsed == OK) { - AccumulateCategorical(Telemetry::LABELS_DNS_LOOKUP_DISPOSITION::trrAOK); + AccumulateCategoricalKeyed( + TRRService::AutoDetectedKey(), + Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::trrAOK); } else if (mTrrAUsed == FAILED) { - AccumulateCategorical(Telemetry::LABELS_DNS_LOOKUP_DISPOSITION::trrAFail); + AccumulateCategoricalKeyed( + TRRService::AutoDetectedKey(), + Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::trrAFail); } if (mTrrAAAAUsed == OK) { - AccumulateCategorical( - Telemetry::LABELS_DNS_LOOKUP_DISPOSITION::trrAAAAOK); + AccumulateCategoricalKeyed( + TRRService::AutoDetectedKey(), + Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::trrAAAAOK); } else if (mTrrAAAAUsed == FAILED) { - AccumulateCategorical( - Telemetry::LABELS_DNS_LOOKUP_DISPOSITION::trrAAAAFail); + AccumulateCategoricalKeyed( + TRRService::AutoDetectedKey(), + Telemetry::LABELS_DNS_LOOKUP_DISPOSITION2::trrAAAAFail); } } if (mEffectiveTRRMode == nsIRequest::TRR_FIRST_MODE) { if (flags & nsIDNSService::RESOLVE_DISABLE_TRR) { // TRR is disabled on request, which is a next-level back-off method. - Telemetry::Accumulate(Telemetry::DNS_TRR_DISABLED, mNativeSuccess); + Telemetry::Accumulate(Telemetry::DNS_TRR_DISABLED2, + TRRService::AutoDetectedKey(), mNativeSuccess); } else { if (mTRRSuccess) { - AccumulateCategorical(Telemetry::LABELS_DNS_TRR_FIRST2::TRR); + AccumulateCategoricalKeyed(TRRService::AutoDetectedKey(), + Telemetry::LABELS_DNS_TRR_FIRST3::TRR); } else if (mNativeSuccess) { if (mTRRUsed) { - AccumulateCategorical( - Telemetry::LABELS_DNS_TRR_FIRST2::NativeAfterTRR); + AccumulateCategoricalKeyed( + TRRService::AutoDetectedKey(), + Telemetry::LABELS_DNS_TRR_FIRST3::NativeAfterTRR); } else { - AccumulateCategorical(Telemetry::LABELS_DNS_TRR_FIRST2::Native); + AccumulateCategoricalKeyed(TRRService::AutoDetectedKey(), + Telemetry::LABELS_DNS_TRR_FIRST3::Native); } } else { - AccumulateCategorical(Telemetry::LABELS_DNS_TRR_FIRST2::BothFailed); + AccumulateCategoricalKeyed( + TRRService::AutoDetectedKey(), + Telemetry::LABELS_DNS_TRR_FIRST3::BothFailed); } } } diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 6741b39d4647..a191c4a1d501 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -3943,17 +3943,18 @@ "n_buckets": 50, "description": "Time for a successful DNS resolution (msec)" }, - "DNS_TRR_LOOKUP_TIME": { + "DNS_TRR_LOOKUP_TIME2": { "record_in_processes": ["main"], "products": ["firefox", "fennec", "geckoview"], "expires_in_version": "never", "kind": "exponential", "high": 60000, + "keyed": true, "releaseChannelCollection": "opt-out", "alert_emails": ["necko@mozilla.com"], - "bug_numbers": [1434852], + "bug_numbers": [1434852, 1640867], "n_buckets": 50, - "description": "Time for a completed TRR resolution (msec)" + "description": "Time for a completed TRR resolution (msec); Keyed by TRR auto-detected" }, "DNS_TRR_PROCESSING_TIME": { "record_in_processes": ["main"], @@ -4037,43 +4038,47 @@ "releaseChannelCollection": "opt-out", "description": "DNS: success distribution when both native and TRR were used" }, - "DNS_TRR_FIRST2": { + "DNS_TRR_FIRST3": { "record_in_processes": ["main"], "products": ["firefox", "fennec", "geckoview"], "alert_emails": ["necko@mozilla.com"], "expires_in_version": "never", "kind": "categorical", + "keyed": true, "labels": ["TRR", "NativeAfterTRR", "Native", "BothFailed"], - "bug_numbers": [1497252], + "bug_numbers": [1497252, 1640867], "releaseChannelCollection": "opt-out", - "description": "TRR-first mode distribution. 0=Worked, 1=fell back fine after TRR fail, 2=native worked, 3=both failed" + "description": "TRR-first mode distribution. 0=Worked, 1=fell back fine after TRR fail, 2=native worked, 3=both failed; Keyed by TRR auto-detected" }, - "DNS_TRR_DISABLED": { + "DNS_TRR_DISABLED2": { "record_in_processes": ["main"], "products": ["firefox", "fennec", "geckoview"], "expires_in_version": "never", "kind": "boolean", - "description": "Resolve success rate when in TRR-first and called TRR-disabled (fall-back mode)", - "bug_numbers": [1472659], + "keyed": true, + "description": "Resolve success rate when in TRR-first and called TRR-disabled (fall-back mode); Keyed by TRR auto-detected", + "bug_numbers": [1472659, 1640867], "releaseChannelCollection": "opt-out", "alert_emails": ["necko@mozilla.com"] }, - "DNS_TRR_BLACKLISTED": { + "DNS_TRR_BLACKLISTED2": { "record_in_processes": ["main"], "products": ["firefox", "fennec", "geckoview"], "expires_in_version": "never", "kind": "boolean", - "description": "DNS check for TRR was blocked by blacklist", - "bug_numbers": [1434852], + "keyed": true, + "description": "DNS check for TRR was blocked by blacklist; Keyed by TRR auto-detected", + "bug_numbers": [1434852, 1640867], "alert_emails": ["necko@mozilla.com"] }, - "DNS_TRR_NS_VERFIFIED": { + "DNS_TRR_NS_VERFIFIED2": { "record_in_processes": ["main"], "products": ["firefox", "fennec", "geckoview"], "expires_in_version": "never", "kind": "boolean", - "description": "TRR managed to verify NS entry", - "bug_numbers": [1453825], + "keyed": true, + "description": "TRR managed to verify NS entry; Keyed by TRR auto-detected", + "bug_numbers": [1453825, 1640867], "alert_emails": ["necko@mozilla.com"] }, "DNS_TRR_REQUEST_PER_CONN": { @@ -4087,14 +4092,15 @@ "bug_numbers": [1470853], "description": "Number of DOH requests per connection" }, - "DNS_TRR_SUCCESS": { + "DNS_TRR_SUCCESS2": { "record_in_processes": ["main"], "products": ["firefox", "fennec", "geckoview"], "expires_in_version": "never", "kind": "categorical", + "keyed": true, "labels": ["Fine", "Timeout", "Bad"], - "description": "How often TRR (Trusted Recursive Resolver) requests are fine, time-out or error.", - "bug_numbers": [1497438], + "description": "How often TRR (Trusted Recursive Resolver) requests are fine, time-out or error. Keyed by TRR auto-detected", + "bug_numbers": [1497438, 1640867], "releaseChannelCollection": "opt-out", "alert_emails": ["necko@mozilla.com"] }, @@ -4118,17 +4124,18 @@ "bug_numbers": [1434852], "description": "DNS: lookup algorithm" }, - "DNS_LOOKUP_DISPOSITION": { + "DNS_LOOKUP_DISPOSITION2": { "record_in_processes": ["main"], "products": ["firefox", "fennec", "geckoview"], "alert_emails": ["necko@mozilla.com"], "expires_in_version": "never", "kind": "categorical", + "keyed": true, "labels": ["trrOK", "trrFail", "trrAOK", "trrAFail", "trrAAAAOK", "trrAAAAFail", "osOK", "osFail"], - "bug_numbers": [1434852], + "bug_numbers": [1434852, 1640867], "releaseChannelCollection": "opt-out", - "description": "DNS: lookup algorithm" + "description": "DNS: lookup algorithm; Keyed by TRR auto-detected" }, "DNS_RENEWAL_TIME": { "record_in_processes": ["main"],