DeviceInfo into GenericSets

tracing/tracing/value/add_device_info.py

Index: tracing/tracing/value/add_device_info.py
diff --git a/tracing/tracing/value/add_device_info.py b/tracing/tracing/value/add_device_info.py
index 03ae81522e3beb368f2e7e1784f87746e2f971d3..a2dfee6e0bbae7aa7cebb54f8bc4f775e7e6d54e 100644
--- a/tracing/tracing/value/add_device_info.py
+++ b/tracing/tracing/value/add_device_info.py
@@ -2,23 +2,32 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-import json
-import tempfile
+from tracing.value import histogram
+from tracing.value.diagnostics import reserved_infos
 
-from tracing.value import add_shared_diagnostic
-
-def AddDeviceInfo(histograms_json_filename, chrome_version, os_name, os_version,
+def AddDeviceInfo(histogram_set, chrome_version, os_name, os_version,

[phsilva, 2017/07/25 21:15:39]

Currently, this function is not being used anywhere, as far as I could tell. I
figured I would update it rather than entirely deleting it for now. As part of
the effort to populate DeviceInfo in the near future, we may or may not need it,
so I suggest we push the decision to then and keep as an available resource for
now.

                   gpu_info, arch, ram):
+  """Adds a shared diagnostics containing pieces of device information to a
+     HistogramSet.
+
+  Args:
+    histograms_json_filename: a HistogramSet to add the diagnostics to.
+    chrome_version: name of the device's Chrome version.
+    os_name: name of the device's OS.
+    os_version: name of the device's OS version.
+    gpu_info: GPU information of the device
+    arch: name of the device's OS architecture.
+    ram: device's total available ram.
+  """
   device_info = {
-      'chromeVersion': chrome_version,
-      'osName': os_name,
-      'osVersion': os_version,
-      'gpuInfo': gpu_info,
-      'arch': arch,
-      'ram': ram,
+      reserved_infos.PRODUCT_VERSIONS.name: chrome_version,
+      reserved_infos.OS_NAMES.name: os_name,
+      reserved_infos.OS_VERSIONS.name: os_version,
+      reserved_infos.GPUS.name: gpu_info,
+      reserved_infos.ARCHITECTURES.name: arch,
+      reserved_infos.MEMORY_AMOUNTS.name: ram,
   }
 
-  with tempfile.NamedTemporaryFile() as diagnostic_file:
-    json.dump(device_info, diagnostic_file)
-    return add_shared_diagnostic.AddSharedDiagnostic(
-        histograms_json_filename, 'device', diagnostic_file.name)
+  for device_info_name, device_info_value in device_info.iteritems():

[phsilva, 2017/07/25 21:15:39]

This part of the code adds GenericSet diagnostics for all of the above, even if
they turn out to be "falsy" -- None, or an empty list. Do we want to ignore the
ones that are going to be empty, in order to avoid volume, or is it not
significant enough.

[benjhayden, 2017/07/26 05:21:17]

I think you can say 'if device_info_value:' here in order to avoid storing empty
GenericSets.
I think that callers are going to need to be prepared for histograms to lack
these diagnostics regardless of what you do here because not all histograms will
necessarily pass through here.
Does that sound right?

+    histogram_set.AddSharedDiagnostic(
+        device_info_name, histogram.GenericSet(device_info_value))