bug 1366294 - Part 5 - Send appropriately-structured Telemetry data to JS r=gfritzsche
authorChris H-C <chutten@mozilla.com>
Tue, 20 Jun 2017 15:03:10 -0400
changeset 419305 2c1ac79386ecfa725fbb17335d975938baaa0e12
parent 419304 b376e9bcdfbaa41c96a947044cd0aa64b0064294
child 419306 208fd1dccdd89613b06e16885b919e03415236dc
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