Bug 1499149 - Better telemetry for alt-svc headers seen in the wild. r=francois,valentin
authorNicholas Hurley <hurley@mozilla.com>
Wed, 17 Oct 2018 18:14:56 +0000
changeset 500189 b6780702981d863bfd913ca404d5b5209b952066
parent 500188 7bddefa538666290f285c7f7ca8904b64dee352d
child 500190 43db36acc04fc464e9f75b79bf635e7cc2ba328b
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfrancois, valentin
bugs1499149
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1499149 - Better telemetry for alt-svc headers seen in the wild. r=francois,valentin Right now, we have no idea how often an origin may offer us multiple alt-svc options. As we are considering racing multiple alt-svc connections (if they're available), it would be good to know how often we actually have (or would have, were we to store them) multiple options available. It would also be good to know how often an origin may change the target of its alt-svc mapping (even if there is only one target), as changes in target may make it useful to store/race multiple targets, as well. Differential Revision: https://phabricator.services.mozilla.com/D8878
netwerk/protocol/http/AlternateServices.cpp
toolkit/components/telemetry/Histograms.json
--- a/netwerk/protocol/http/AlternateServices.cpp
+++ b/netwerk/protocol/http/AlternateServices.cpp
@@ -74,16 +74,17 @@ AltSvcMapping::ProcessHeader(const nsCSt
   }
   if (!isHTTPS && !gHttpHandler->AllowAltSvcOE()) {
     LOG(("Alt-Svc Response Header for http:// origin but OE disabled\n"));
     return;
   }
 
   LOG(("Alt-Svc Response Header %s\n", buf.get()));
   ParsedHeaderValueListList parsedAltSvc(buf);
+  int32_t numEntriesInHeader = parsedAltSvc.mValues.Length();
 
   for (uint32_t index = 0; index < parsedAltSvc.mValues.Length(); ++index) {
     uint32_t maxage = 86400; // default
     nsAutoCString hostname;
     nsAutoCString npnToken;
     int32_t portno = originPort;
     bool clearEntry = false;
 
@@ -93,16 +94,17 @@ AltSvcMapping::ProcessHeader(const nsCSt
       nsDependentCSubstring &currentName =
         parsedAltSvc.mValues[index].mValues[pairIndex].mName;
       nsDependentCSubstring &currentValue =
         parsedAltSvc.mValues[index].mValues[pairIndex].mValue;
 
       if (!pairIndex) {
         if (currentName.EqualsLiteral("clear")) {
           clearEntry = true;
+          --numEntriesInHeader; // Only want to keep track of actual alt-svc maps, not clearing
           break;
         }
 
         // h2=[hostname]:443
         npnToken = currentName;
         int32_t colonIndex = currentValue.FindChar(':');
         if (colonIndex >= 0) {
           portno =
@@ -155,16 +157,20 @@ AltSvcMapping::ProcessHeader(const nsCSt
       // since this isn't a parse error, let's clear any existing mapping
       // as that would have happened if we had accepted the parameters.
       gHttpHandler->ConnMgr()->ClearHostMapping(originHost, originPort, originAttributes);
     } else {
       gHttpHandler->UpdateAltServiceMapping(mapping, proxyInfo, callbacks, caps,
                                             originAttributes);
     }
   }
+
+  if (numEntriesInHeader) { // Ignore headers that were just "alt-svc: clear"
+    Telemetry::Accumulate(Telemetry::HTTP_ALTSVC_ENTRIES_PER_HEADER, numEntriesInHeader);
+  }
 }
 
 AltSvcMapping::AltSvcMapping(DataStorage *storage, int32_t epoch,
                              const nsACString &originScheme,
                              const nsACString &originHost,
                              int32_t originPort,
                              const nsACString &username,
                              bool privateBrowsing,
@@ -880,28 +886,30 @@ AltSvcCache::UpdateAltServiceMapping(Alt
                this, map, existing.get()));
           existing->SetExpiresAt(map->GetExpiresAt());
         } else {
           LOG(("AltSvcCache::UpdateAltServiceMapping %p map %p tries to extend %p but"
                " cannot as without .wk\n",
                this, map, existing.get()));
         }
       }
+      Telemetry::Accumulate(Telemetry::HTTP_ALTSVC_MAPPING_CHANGED_TARGET, false);
       return;
     }
 
     if (map->GetExpiresAt() < existing->GetExpiresAt()) {
       LOG(("AltSvcCache::UpdateAltServiceMapping %p map %p ttl shorter than %p, ignoring",
            this, map, existing.get()));
       return;
     }
 
     // new alternate. start new validation
     LOG(("AltSvcCache::UpdateAltServiceMapping %p map %p may overwrite %p\n",
          this, map, existing.get()));
+    Telemetry::Accumulate(Telemetry::HTTP_ALTSVC_MAPPING_CHANGED_TARGET, true);
   }
 
   if (existing && !existing->Validated()) {
     LOG(("AltSvcCache::UpdateAltServiceMapping %p map %p ignored because %p "
          "still in progress\n", this, map, existing.get()));
     return;
   }
 
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -1705,16 +1705,33 @@
     "description": "Whether a HTTP transaction was routed via Alt-Svc or not."
   },
   "HTTP_TRANSACTION_USE_ALTSVC_OE": {
     "record_in_processes": ["main", "content"],
     "expires_in_version": "never",
     "kind": "boolean",
     "description": "Whether a HTTP transaction routed via Alt-Svc was scheme=http"
   },
+  "HTTP_ALTSVC_ENTRIES_PER_HEADER": {
+    "record_in_processes": ["main", "content"],
+    "expires_in_version": "never",
+    "bug_numbers": [1499149],
+    "alert_emails": ["hurley@mozilla.com"],
+    "kind": "enumerated",
+    "n_values": 5,
+    "description": "How many alt-svc productions were seen in a single Alt-Svc header"
+  },
+  "HTTP_ALTSVC_MAPPING_CHANGED_TARGET": {
+    "record_in_processes": ["main", "content"],
+    "expires_in_version": "never",
+    "bug_numbers": [1499149],
+    "alert_emails": ["hurley@mozilla.com"],
+    "kind": "boolean",
+    "description": "Whether or not a new alt-svc mapping would change the target hostname of the existing mapping"
+  },
   "HTTP_SCHEME_UPGRADE_TYPE": {
     "record_in_processes": ["main", "content"],
     "alert_emails": ["seceng-telemetry@mozilla.com", "jkt@mozilla.com"],
     "bug_numbers": [1340021, 1435733],
     "releaseChannelCollection": "opt-out",
     "expires_in_version": "never",
     "kind": "categorical",
     "labels": ["AlreadyHTTPS", "NoReasonToUpgrade", "PrefBlockedSTS", "STS", "CSP", "BrowserDisplay"],