bug 1366294 - Part 5 - Send appropriately-structured Telemetry data to JS r=gfritzsche
☠☠ backed out by 6fe44a6c4e46 ☠ ☠
authorChris H-C <chutten@mozilla.com>
Tue, 20 Jun 2017 15:03:10 -0400
changeset 418456 e7ce6a06c131dfe37ceea24a3b09a5b5d6475127
parent 418455 1cc736607aab9744c015e9dc0734dc35511db825
child 418457 476daf9a58465e1954419c874148493cbac534cc
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgfritzsche
bugs1366294
milestone56.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 1366294 - Part 5 - Send appropriately-structured Telemetry data to JS r=gfritzsche Previously we were doing bad string manipulation nonsense. Now when asked for snapshots C++ can return a properly-formated Object tree. MozReview-Commit-ID: HAvIbgzUvMU
toolkit/components/telemetry/TelemetryHistogram.cpp
toolkit/components/telemetry/TelemetrySession.jsm
toolkit/components/telemetry/nsITelemetry.idl
--- a/toolkit/components/telemetry/TelemetryHistogram.cpp
+++ b/toolkit/components/telemetry/TelemetryHistogram.cpp
@@ -37,16 +37,17 @@ using mozilla::StaticMutexAutoLock;
 using mozilla::Telemetry::Accumulation;
 using mozilla::Telemetry::KeyedAccumulation;
 using mozilla::Telemetry::HistogramID;
 using mozilla::Telemetry::ProcessID;
 using mozilla::Telemetry::HistogramCount;
 using mozilla::Telemetry::Common::LogToBrowserConsole;
 using mozilla::Telemetry::Common::RecordedProcessType;
 using mozilla::Telemetry::Common::AutoHashtable;
+using mozilla::Telemetry::Common::GetNameForProcessID;
 using mozilla::Telemetry::Common::IsExpiredVersion;
 using mozilla::Telemetry::Common::CanRecordDataset;
 using mozilla::Telemetry::Common::IsInDataset;
 
 namespace TelemetryIPCAccumulator = mozilla::TelemetryIPCAccumulator;
 
 ////////////////////////////////////////////////////////////////////////
 ////////////////////////////////////////////////////////////////////////
@@ -1938,65 +1939,63 @@ TelemetryHistogram::CreateHistogramSnaps
       }
 
       mozilla::DebugOnly<Histogram*> h = nullptr;
       h = internal_GetHistogramById(HistogramID(histogramId), ProcessID(process), sessionType);
       MOZ_ASSERT(h);
     }
   }
 
-  // TODO: This won't get us data from different processes.
-  // We'll have to refactor this function to return {"parent": {...}, "content": {...}}.
-
-  // OK, now we can actually reflect things.
-  JS::Rooted<JSObject*> hobj(cx);
-  for (size_t i = 0; i < HistogramCount; ++i) {
-    const HistogramInfo& info = gHistogramInfos[i];
-    if (info.keyed) {
-      continue;
+  for (uint32_t process = 0; process < static_cast<uint32_t>(ProcessID::Count); ++process) {
+    JS::Rooted<JSObject*> processObject(cx, JS_NewPlainObject(cx));
+    if (!processObject) {
+      return NS_ERROR_FAILURE;
     }
-
-    HistogramID id = HistogramID(i);
-
-    // TODO: support multiple processes.
-    //for (uint32_t process = 0; process < static_cast<uint32_t>(ProcessID::Count); ++process) {
-
-    uint32_t process = uint32_t(ProcessID::Parent);
-    if (!CanRecordInProcess(info.record_in_processes, ProcessID(process)) ||
-      ((ProcessID(process) == ProcessID::Gpu) && !includeGPUProcess)) {
-      continue;
-    }
-
-    Histogram* h = internal_GetHistogramById(id, ProcessID(process), sessionType, /* instantiate = */ false);
-    if (!h || internal_IsExpired(h) || !internal_ShouldReflectHistogram(h, id)) {
-      continue;
-    }
-
-    hobj = JS_NewPlainObject(cx);
-    if (!hobj) {
+    if (!JS_DefineProperty(cx, root_obj, GetNameForProcessID(ProcessID(process)),
+                           processObject, JSPROP_ENUMERATE)) {
       return NS_ERROR_FAILURE;
     }
-    switch (internal_ReflectHistogramSnapshot(cx, hobj, h)) {
-    case REFLECT_FAILURE:
-      return NS_ERROR_FAILURE;
-    case REFLECT_OK:
-      if (!JS_DefineProperty(cx, root_obj, gHistogramInfos[id].name(),
-                             hobj, JSPROP_ENUMERATE)) {
+    for (size_t i = 0; i < HistogramCount; ++i) {
+      const HistogramInfo& info = gHistogramInfos[i];
+      if (info.keyed) {
+        continue;
+      }
+
+      HistogramID id = HistogramID(i);
+
+      if (!CanRecordInProcess(info.record_in_processes, ProcessID(process)) ||
+        ((ProcessID(process) == ProcessID::Gpu) && !includeGPUProcess)) {
+        continue;
+      }
+
+      Histogram* h = internal_GetHistogramById(id, ProcessID(process), sessionType, /* instantiate = */ false);
+      if (!h || internal_IsExpired(h) || !internal_ShouldReflectHistogram(h, id)) {
+        continue;
+      }
+
+      JS::Rooted<JSObject*> hobj(cx, JS_NewPlainObject(cx));
+      if (!hobj) {
         return NS_ERROR_FAILURE;
       }
-    }
+      switch (internal_ReflectHistogramSnapshot(cx, hobj, h)) {
+      case REFLECT_FAILURE:
+        return NS_ERROR_FAILURE;
+      case REFLECT_OK:
+        if (!JS_DefineProperty(cx, processObject, gHistogramInfos[id].name(),
+                               hobj, JSPROP_ENUMERATE)) {
+          return NS_ERROR_FAILURE;
+        }
+      }
 
 #if !defined(MOZ_WIDGET_ANDROID)
-    if ((sessionType == SessionType::Subsession) && clearSubsession) {
-      h->Clear();
-    }
+      if ((sessionType == SessionType::Subsession) && clearSubsession) {
+        h->Clear();
+      }
 #endif
-
-    // TODO: support multiple processes.
-    // }
+    }
   }
   return NS_OK;
 }
 
 nsresult
 TelemetryHistogram::RegisteredHistograms(uint32_t aDataset, uint32_t *aCount,
                                          char*** aHistograms)
 {
--- a/toolkit/components/telemetry/TelemetrySession.jsm
+++ b/toolkit/components/telemetry/TelemetrySession.jsm
@@ -931,23 +931,21 @@ var Impl = {
       registered = registered.filter(n => !n.startsWith("TELEMETRY_TEST_"));
     }
     registered = registered.concat(registered.map(n => "STARTUP_" + n));
 
     let hls = subsession ? Telemetry.snapshotSubsessionHistograms(clearSubsession)
                          : Telemetry.histogramSnapshots;
     let ret = {};
 
-    for (let name of registered) {
-      for (let suffix of Object.values(HISTOGRAM_SUFFIXES)) {
-        if (name + suffix in hls) {
-          if (!(suffix in ret)) {
-            ret[suffix] = {};
-          }
-          ret[suffix][name] = this.packHistogram(hls[name + suffix]);
+    for (let [process, histograms] of Object.entries(hls)) {
+      ret[process] = {};
+      for (let [name, value] of Object.entries(histograms)) {
+        if (registered.includes(name)) {
+          ret[process][name] = this.packHistogram(value);
         }
       }
     }
 
     return ret;
   },
 
   getKeyedHistograms(subsession, clearSubsession) {
@@ -1321,50 +1319,50 @@ var Impl = {
 
     // Additional payload for chrome process.
     let histograms = protect(() => this.getHistograms(isSubsession, clearSubsession), {});
     let keyedHistograms = protect(() => this.getKeyedHistograms(isSubsession, clearSubsession), {});
     let scalars = protect(() => this.getScalars(isSubsession, clearSubsession), {});
     let keyedScalars = protect(() => this.getScalars(isSubsession, clearSubsession, true), {});
     let events = protect(() => this.getEvents(isSubsession, clearSubsession))
 
-    payloadObj.histograms = histograms[HISTOGRAM_SUFFIXES.PARENT] || {};
-    payloadObj.keyedHistograms = keyedHistograms[HISTOGRAM_SUFFIXES.PARENT] || {};
+    payloadObj.histograms = histograms.parent || {};
+    payloadObj.keyedHistograms = keyedHistograms.parent || {};
     payloadObj.processes = {
       parent: {
         scalars: scalars["parent"] || {},
         keyedScalars: keyedScalars["parent"] || {},
         events: events["parent"] || [],
       },
       content: {
         scalars: scalars["content"],
         keyedScalars: keyedScalars["content"],
-        histograms: histograms[HISTOGRAM_SUFFIXES.CONTENT],
-        keyedHistograms: keyedHistograms[HISTOGRAM_SUFFIXES.CONTENT],
+        histograms: histograms["content"],
+        keyedHistograms: keyedHistograms["content"],
         events: events["content"] || [],
       },
       extension: {
         scalars: scalars["extension"],
         keyedScalars: keyedScalars["extension"],
-        histograms: histograms[HISTOGRAM_SUFFIXES.EXTENSION],
-        keyedHistograms: keyedHistograms[HISTOGRAM_SUFFIXES.EXTENSION],
+        histograms: histograms["extension"],
+        keyedHistograms: keyedHistograms["extension"],
         events: events["extension"] || [],
       },
     };
 
     // Only include the GPU process if we've accumulated data for it.
-    if (HISTOGRAM_SUFFIXES.GPU in histograms ||
-        HISTOGRAM_SUFFIXES.GPU in keyedHistograms ||
+    if ("gpu" in histograms ||
+        "gpu" in keyedHistograms ||
         "gpu" in scalars ||
         "gpu" in keyedScalars) {
       payloadObj.processes.gpu = {
         scalars: scalars["gpu"],
         keyedScalars: keyedScalars["gpu"],
-        histograms: histograms[HISTOGRAM_SUFFIXES.GPU],
-        keyedHistograms: keyedHistograms[HISTOGRAM_SUFFIXES.GPU],
+        histograms: histograms["gpu"],
+        keyedHistograms: keyedHistograms["gpu"],
         events: events["gpu"] || [],
       };
     }
 
     payloadObj.info = info;
 
     // Add extended set measurements for chrome process.
     if (Telemetry.canRecordExtended) {
--- a/toolkit/components/telemetry/nsITelemetry.idl
+++ b/toolkit/components/telemetry/nsITelemetry.idl
@@ -47,18 +47,18 @@ interface nsITelemetry : nsISupports
    * DATASET_RELEASE_CHANNEL_OPTIN - the extended dataset that is opt-in on release,
    *                                 opt-out on pre-release channels.
    */
   const unsigned long DATASET_RELEASE_CHANNEL_OPTOUT = 0;
   const unsigned long DATASET_RELEASE_CHANNEL_OPTIN = 1;
 
 
   /**
-   * An object containing a snapshot from all of the currently registered histograms.
-   * { name1: {data1}, name2:{data2}...}
+   * An object containing a snapshot from all of the currently registered histograms from all processes.
+   * { process1: {name1: {data1}, name2:{data2}...} }
    * where data is consists of the following properties:
    *   min - Minimal bucket size
    *   max - Maximum bucket size
    *   histogram_type - HISTOGRAM_EXPONENTIAL, HISTOGRAM_LINEAR, HISTOGRAM_BOOLEAN
    *                    or HISTOGRAM_COUNT
    *   counts - array representing contents of the buckets in the histogram
    *   ranges -  an array with calculated bucket sizes
    *   sum - sum of the bucket contents