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 490117 b6780702981d863bfd913ca404d5b5209b952066
parent 490116 7bddefa538666290f285c7f7ca8904b64dee352d
child 490118 43db36acc04fc464e9f75b79bf635e7cc2ba328b
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersfrancois, valentin
bugs1499149
milestone64.0a1
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"],