Add Android fingerprint to metrics logs

chrome/browser/metrics/metrics_log.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/metrics/metrics_log.h"
 
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
@@ skipping 27 matching lines @@
 #include "chrome/common/pref_names.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/gpu_data_manager.h"
 #include "content/public/common/content_client.h"
 #include "content/public/common/gpu_info.h"
 #include "googleurl/src/gurl.h"
 #include "ui/gfx/screen.h"
 #include "webkit/plugins/webplugininfo.h"
 
+#if defined(OS_ANDROID)
+#include "base/android/build_info.h"
+#endif
+
 #define OPEN_ELEMENT_FOR_SCOPE(name) ScopedElement scoped_element(this, name)
 
 #if defined(OS_WIN)
 #include "base/win/metro.h"
 
 // http://blogs.msdn.com/oldnewthing/archive/2004/10/25/247180.aspx
 extern "C" IMAGE_DOS_HEADER __ImageBase;
 #endif
 
 using content::GpuDataManager;
@@ skipping 260 matching lines @@
 
 void MetricsLog::RecordIncrementalStabilityElements(
     const std::vector<webkit::WebPluginInfo>& plugin_list) {
   DCHECK(!locked());
 
   PrefService* pref = GetPrefService();
   DCHECK(pref);
 
   OPEN_ELEMENT_FOR_SCOPE("profile");
   WriteCommonEventAttributes();
-
   WriteInstallElement();
+#if defined(OS_ANDROID)
+  WriteOSInfoElement();
+#endif

[Ilya Sherman, 2012/10/01 23:01:38]

I don't think it's appropriate to add the OS info as part of recording
incremental stability data.  For the protocol buffer uploads, this data should
already be included as part of RecordEnvironmentProto(), which is called
immediately before RecordIncrementalStabilityElements() in the MetricsService.

[gone, 2012/10/02 18:24:24]

With nilesh's comment as context, does your comment about RecordEnvironmentProto
still hold?  MetricsService::StopRecording() does seem to send the proto
immediately before the XML, meaning that we probably get the fingerprint twice
on m18...  It'd be nice to confirm this somehow, though.

[Ilya Sherman, 2012/10/02 21:37:45]

TL;DR: You can ignore my previous comment w.r.t. the XML uploads, as long as the
changes only affect Android.

To clarify:  There are no protocol buffer uploads in m18; just XML ones.  In
m21+, there are both XML and protocol buffer uploads, which upload more or less
the same data.  The XML uploading mechanism is deprecated, and will soon be
removed.  Adding a new field to the XML uploads requires server-side changes for
parsing the XML, which just isn't worth at this point.

So, there are two questions: (1) What, if anything, should we upstream for the
XML uploads?  (2) How should we add the fingerprints to the protocol buffer
uploads?

For (1), I don't have a strong preference for whether or not we upstream some
Android-only code; but we definitely shouldn't add any new XML fields (tags). 
If you want to upstream the changes currently in Android, that's fine, but in
that case please leave the fingerprint as part of the OS name.

[gone, 2012/10/02 23:10:00]

If that's the case then we really shouldn't be forking it on the Android side;
I'm removing the diff from our downstream master branch.  The diff in the m18
release branch isn't being upstreamed and will continue overriding the OS
version in the XML.

For (2) I'd still lean towards adding the fingerprint as a separate field in the
protobuf definition.  Apparently the only thing we need to change after adding
the new field is a dremel query or two.

 
   {
     OPEN_ELEMENT_FOR_SCOPE("stability");  // Minimal set of stability elements.
     WriteRequiredStabilityAttributes(pref);
     WriteRealtimeStabilityAttributes(pref);
 
     WritePluginStabilityElements(plugin_list, pref);
   }
 }
 
@@ skipping 301 matching lines @@
 void MetricsLog::RecordEnvironment(
          const std::vector<webkit::WebPluginInfo>& plugin_list,
          const GoogleUpdateMetrics& google_update_metrics,
          const DictionaryValue* profile_metrics) {
   DCHECK(!locked());
 
   PrefService* pref = GetPrefService();
 
   OPEN_ELEMENT_FOR_SCOPE("profile");
   WriteCommonEventAttributes();
-
   WriteInstallElement();
 
   // Write the XML version.
   // We'll write the protobuf version in RecordEnvironmentProto().
   bool write_as_xml = true;
   WritePluginList(plugin_list, write_as_xml);
 
   WriteStabilityElement(plugin_list, pref);
 
   {
     // Write the XML version.
     // We'll write the protobuf version in RecordEnvironmentProto().
     OPEN_ELEMENT_FOR_SCOPE("cpu");
     WriteAttribute("arch", base::SysInfo::CPUArchitecture());
   }
 
   {
     // Write the XML version.
     // We'll write the protobuf version in RecordEnvironmentProto().
     OPEN_ELEMENT_FOR_SCOPE("memory");
     WriteIntAttribute("mb", base::SysInfo::AmountOfPhysicalMemoryMB());
 #if defined(OS_WIN)
     WriteIntAttribute("dllbase", reinterpret_cast<int>(&__ImageBase));
 #endif
   }
 
-  {
-    // Write the XML version.
-    // We'll write the protobuf version in RecordEnvironmentProto().
-    OPEN_ELEMENT_FOR_SCOPE("os");
-    WriteAttribute("name", base::SysInfo::OperatingSystemName());
-    WriteAttribute("version", base::SysInfo::OperatingSystemVersion());
-  }
+  WriteOSInfoElement();
 
   {
     OPEN_ELEMENT_FOR_SCOPE("gpu");
     const content::GPUInfo& gpu_info =
         GpuDataManager::GetInstance()->GetGPUInfo();
 
     // Write the XML version.
     // We'll write the protobuf version in RecordEnvironmentProto().
     WriteIntAttribute("vendorid", gpu_info.gpu.vendor_id);
     WriteIntAttribute("deviceid", gpu_info.gpu.device_id);
@@ skipping 74 matching lines @@
   // TODO(mad): This only checks whether the main process is a Metro process at
   // upload time; not whether the collected metrics were all gathered from
   // Metro.  This is ok as an approximation for now, since users will rarely be
   // switching from Metro to Desktop mode; but we should re-evaluate whether we
   // can distinguish metrics more cleanly in the future: http://crbug.com/140568
   if (base::win::IsMetroProcess())
     os_name += " (Metro)";
 #endif
   os->set_name(os_name);
   os->set_version(base::SysInfo::OperatingSystemVersion());
+#if defined(OS_ANDROID)
+  os->set_fingerprint(
+      base::android::BuildInfo::GetInstance()->android_build_fp());
+#endif
 
   const content::GPUInfo& gpu_info =
       GpuDataManager::GetInstance()->GetGPUInfo();
   SystemProfileProto::Hardware::Graphics* gpu = hardware->mutable_gpu();
   gpu->set_vendor_id(gpu_info.gpu.vendor_id);
   gpu->set_device_id(gpu_info.gpu.device_id);
   gpu->set_driver_version(gpu_info.driver_version);
   gpu->set_driver_date(gpu_info.driver_date);
   SystemProfileProto::Hardware::Graphics::PerformanceStatistics*
       gpu_performance = gpu->mutable_performance_statistics();
@@ skipping 98 matching lines @@
         }
 
         default:
           NOTREACHED();
           break;
       }
     }
   }
 }
 
+void MetricsLog::WriteOSInfoElement() {
+  // Write the XML version.

[Ilya Sherman, 2012/10/01 23:01:38]

As I mentioned above, I don't think you should move this code.  However, if you
do, you should document why this method only writes the XML version, and
probably name it accordingly.

[gone, 2012/10/02 23:45:10]

Undid the change.  We'll go with the protobuf on master.

+  OPEN_ELEMENT_FOR_SCOPE("os");
+  WriteAttribute("name", base::SysInfo::OperatingSystemName());
+  WriteAttribute("version", base::SysInfo::OperatingSystemVersion());
+#if defined(OS_ANDROID)
+  WriteAttribute("fingerprint",
+      base::android::BuildInfo::GetInstance()->android_build_fp());

[Ilya Sherman, 2012/10/01 23:01:38]

I don't recommend trying to add this field to the XML-based uploads, as that
pipeline is deprecated.  The only reason to add this field to the XML uploads is
if you need it like yesterday for M18-based Android builds... but then there's
not much point to upstreaming the change to the main chrome/ repository.

[gone, 2012/10/02 23:45:10]

Removed.

+#endif
+}
+
 void MetricsLog::RecordOmniboxOpenedURL(const AutocompleteLog& log) {
   DCHECK(!locked());
 
   // Write the XML version.
   OPEN_ELEMENT_FOR_SCOPE("uielement");
   WriteAttribute("action", "autocomplete");
   WriteAttribute("targetidhash", "");
   // TODO(kochi): Properly track windows.
   WriteIntAttribute("window", 0);
   if (log.tab_id != -1) {
@@ skipping 100 matching lines @@
     ProductDataToProto(google_update_metrics.google_update_data,
                        google_update->mutable_google_update_status());
   }
 
   if (!google_update_metrics.product_data.version.empty()) {
     ProductDataToProto(google_update_metrics.product_data,
                        google_update->mutable_client_status());
   }
 #endif  // defined(GOOGLE_CHROME_BUILD) && defined(OS_WIN)
 }