Slow Reports - Embed Metadata in Traces

content/browser/tracing/background_tracing_manager_impl.cc

Index: content/browser/tracing/background_tracing_manager_impl.cc
diff --git a/content/browser/tracing/background_tracing_manager_impl.cc b/content/browser/tracing/background_tracing_manager_impl.cc
index 74d63800e6a63aa4c7a6225d162dcd5494e79298..b7a0ea913aef41bac41474a41b6854c6c6af442e 100644
--- a/content/browser/tracing/background_tracing_manager_impl.cc
+++ b/content/browser/tracing/background_tracing_manager_impl.cc
@@ -4,6 +4,7 @@
 
 #include "content/browser/tracing/background_tracing_manager_impl.h"
 
+#include "base/json/json_writer.h"
 #include "base/macros.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/time/time.h"
@@ -21,6 +22,9 @@ namespace {
 base::LazyInstance<BackgroundTracingManagerImpl>::Leaky g_controller =
     LAZY_INSTANCE_INITIALIZER;
 
+const char kMetaDataConfigKey[] = "config";
+const char kMetaDataVersionKey[] = "version";
+
 // These values are used for a histogram. Do not reorder.
 enum BackgroundTracingMetrics {
   SCENARIO_ACTIVATION_REQUESTED = 0,
@@ -370,11 +374,25 @@ void BackgroundTracingManagerImpl::OnFinalizeStarted(
   UMA_HISTOGRAM_MEMORY_KB("Tracing.Background.FinalizingTraceSizeInKB",
                           file_contents->size() / 1024);
 
-  if (!receive_callback_.is_null())
+  if (!receive_callback_.is_null()) {
+    std::map<std::string, std::string> metadata;
+
+    auto metadata_dict = GenerateMetadataDict();

[oystein (OOO til 10th of July), 2015/06/12 18:14:17]

Why not pass the base::DictionaryValue around directly, instead of converting to
map<string, string>?

[shatch, 2015/06/12 19:05:54]

Figured it looked cleaner and easier to work with vs passing the dict around.
TraceCrashServiceUploader just wants a bunch of string/string pairs to attach to
the upload, so felt like it shouldn't have to deal with some json dict. We could
pass the dict up there as a param of DoUpload(), and move this iteration code
into the uploader if you feel strongly about this.

[oystein (OOO til 10th of July), 2015/06/12 19:29:16]

Up to you, it just feels like a bit of an unnecessary step atm.

Basically, right now it's:
loop through to convert -> pass -> loop through
instead of just
pass -> loop through (assuming the uploader gets a json dict member too.

[shatch, 2015/06/12 20:53:47]

Done.

+    if (metadata_dict) {
+      for (base::DictionaryValue::Iterator it(*metadata_dict); !it.IsAtEnd();
+           it.Advance()) {
+        std::string value;
+        if (!it.value().GetAsString(&value))
+          continue;
+        metadata.insert(std::pair<std::string, std::string>(it.key(), value));

[oystein (OOO til 10th of July), 2015/06/12 18:14:17]

nit: std::make_pair() maybe?

[shatch, 2015/06/12 19:05:54]

Done.

+      }
+    }
+
     receive_callback_.Run(
-        file_contents,
+        file_contents, metadata,
         base::Bind(&BackgroundTracingManagerImpl::OnFinalizeComplete,
                    base::Unretained(this)));
+  }
 }
 
 void BackgroundTracingManagerImpl::OnFinalizeComplete() {
@@ -398,6 +416,27 @@ void BackgroundTracingManagerImpl::OnFinalizeComplete() {
   RecordBackgroundTracingMetric(FINALIZATION_COMPLETE);
 }
 
+scoped_ptr<base::DictionaryValue>
+BackgroundTracingManagerImpl::GenerateMetadataDict() const {
+  std::string chrome_version;
+  if (!delegate_->GetChromeVersion(&chrome_version))

[oystein (OOO til 10th of July), 2015/06/12 18:14:16]

Shouldn't be "chrome"; maybe product_version.

+    return nullptr;
+
+  scoped_ptr<base::DictionaryValue> config_dict(new base::DictionaryValue());
+
+  BackgroundTracingConfig::IntoDict(config_.get(), config_dict.get());
+
+  std::string config_string;
+  if (!base::JSONWriter::Write(*config_dict.get(), &config_string))
+    return nullptr;
+
+  scoped_ptr<base::DictionaryValue> metadata_dict(new base::DictionaryValue());
+  metadata_dict->SetString(kMetaDataConfigKey, config_string);
+  metadata_dict->SetString(kMetaDataVersionKey, chrome_version);
+
+  return metadata_dict.Pass();
+}
+
 void BackgroundTracingManagerImpl::BeginFinalizing(
     StartedFinalizingCallback callback) {
   is_gathering_ = true;
@@ -413,6 +452,13 @@ void BackgroundTracingManagerImpl::BeginFinalizing(
     trace_data_sink = content::TracingController::CreateCompressedStringSink(
         data_endpoint_wrapper_);
     RecordBackgroundTracingMetric(FINALIZATION_ALLOWED);
+
+    auto metadata_dict = GenerateMetadataDict();
+    if (metadata_dict) {
+      std::string results;
+      if (base::JSONWriter::Write(*metadata_dict.get(), &results))
+        trace_data_sink->SetMetadata(results);
+    }
   } else {
     RecordBackgroundTracingMetric(FINALIZATION_DISALLOWED);
   }