Background tracing: Support defining custom tracing categories and not just hardcoded presets

R=ssid@chromium.org
CC=etienneb@chromium.org

Change-Id: If3d32313d8202889a6765a4eef80a799b99d1508
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1630232
Commit-Queue: oysteine <oysteine@chromium.org>
Reviewed-by: ssid <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#666529}

content/browser/tracing/background_tracing_active_scenario.cc

@@ -28,10 +28,6 @@
 using base::trace_event::TraceConfig;
 using Metrics = content::BackgroundTracingManagerImpl::Metrics;
 
-namespace {
-const size_t kDefaultTraceBufferSizeInKb = 10 * 1024;
-}  // namespace
-
 namespace content {
 
 class BackgroundTracingActiveScenario::TracingTimer {
@@ -75,10 +71,8 @@ class PerfettoTracingSession
       public mojo::DataPipeDrainer::Client {
  public:
   PerfettoTracingSession(BackgroundTracingActiveScenario* parent_scenario,
-                         const TraceConfig& chrome_config,
-                         BackgroundTracingConfigImpl::CategoryPreset preset)
+                         const TraceConfig& chrome_config)
       : parent_scenario_(parent_scenario),
-        category_preset_(preset),
         raw_data_(std::make_unique<std::string>()) {
 #if !defined(OS_ANDROID)
     // TODO(crbug.com/941318): Re-enable startup tracing for Android once all
@@ -143,7 +137,7 @@ class PerfettoTracingSession
   // tracing::mojom::TracingSession implementation:
   void OnTracingEnabled() override {
     BackgroundTracingManagerImpl::GetInstance()->OnStartTracingDone(
-        category_preset_);
+        parent_scenario_->GetConfig()->category_preset());
   }
 
   void OnTracingDisabled() override {
@@ -182,7 +176,6 @@ class PerfettoTracingSession
   tracing::mojom::TracingSessionHostPtr tracing_session_host_;
   std::unique_ptr<mojo::DataPipeDrainer> drainer_;
   tracing::mojom::ConsumerHostPtr consumer_host_;
-  BackgroundTracingConfigImpl::CategoryPreset category_preset_;
   std::unique_ptr<std::string> raw_data_;
   bool has_finished_read_buffers_ = false;
   bool has_finished_receiving_data_ = false;
@@ -192,8 +185,7 @@ class LegacyTracingSession
     : public BackgroundTracingActiveScenario::TracingSession {
  public:
   LegacyTracingSession(BackgroundTracingActiveScenario* parent_scenario,
-                       const TraceConfig& chrome_config,
-                       BackgroundTracingConfigImpl::CategoryPreset preset)
+                       const TraceConfig& chrome_config)
       : parent_scenario_(parent_scenario) {
 #if !defined(OS_ANDROID)
     // TODO(crbug.com/941318): Re-enable startup tracing for Android once all
@@ -210,7 +202,7 @@ class LegacyTracingSession
         base::BindOnce(
             &BackgroundTracingManagerImpl::OnStartTracingDone,
             base::Unretained(BackgroundTracingManagerImpl::GetInstance()),
-            preset));
+            parent_scenario->GetConfig()->category_preset()));
     // We check IsEnabled() before creating the LegacyTracingSession,
     // so any failures to start tracing at this point would be due to invalid
     // configs which we treat as a failure scenario.
@@ -309,18 +301,8 @@ void BackgroundTracingActiveScenario::SetState(State new_state) {
     // which means that we're left in a state where the Mojo interface doesn't
     // think we're tracing but TraceLog is still enabled. If that's the case,
     // we abort tracing here.
-    auto record_mode =
-        (config_->tracing_mode() == BackgroundTracingConfigImpl::PREEMPTIVE)
-            ? base::trace_event::RECORD_CONTINUOUSLY
-            : base::trace_event::RECORD_UNTIL_FULL;
-    TraceConfig config =
-        BackgroundTracingConfigImpl::GetConfigForCategoryPreset(
-            config_->category_preset(), record_mode);
-
-    uint8_t modes = base::trace_event::TraceLog::RECORDING_MODE;
-    if (!config.event_filters().empty())
-      modes |= base::trace_event::TraceLog::FILTERING_MODE;
-    base::trace_event::TraceLog::GetInstance()->SetDisabled(modes);
+    base::trace_event::TraceLog::GetInstance()->SetDisabled(
+        base::trace_event::TraceLog::GetInstance()->enabled_modes());
   }
 
   if (scenario_state_ == State::kAborted) {
@@ -347,32 +329,15 @@ BackgroundTracingActiveScenario::GetWeakPtr() {
 void BackgroundTracingActiveScenario::StartTracingIfConfigNeedsIt() {
   DCHECK(config_);
   if (config_->tracing_mode() == BackgroundTracingConfigImpl::PREEMPTIVE) {
-    StartTracing(config_->category_preset(),
-                 base::trace_event::RECORD_CONTINUOUSLY);
+    StartTracing();
   }
 
   // There is nothing to do in case of reactive tracing.
 }
 
-bool BackgroundTracingActiveScenario::StartTracing(
-    BackgroundTracingConfigImpl::CategoryPreset preset,
-    base::trace_event::TraceRecordMode record_mode) {
+bool BackgroundTracingActiveScenario::StartTracing() {
   TraceConfig chrome_config =
-      BackgroundTracingConfigImpl::GetConfigForCategoryPreset(preset,
-                                                              record_mode);
-  if (requires_anonymized_data_) {
-    chrome_config.EnableArgumentFilter();
-  }
-
-  chrome_config.SetTraceBufferSizeInKb(kDefaultTraceBufferSizeInKb);
-
-#if defined(OS_ANDROID)
-  // Set low trace buffer size on Android in order to upload small trace files.
-  if (config_->tracing_mode() == BackgroundTracingConfigImpl::PREEMPTIVE) {
-    chrome_config.SetTraceBufferSizeInEvents(20000);
-    chrome_config.SetTraceBufferSizeInKb(500);
-  }
-#endif
+      config_->GetTraceConfig(requires_anonymized_data_);
 
   // If the tracing controller is tracing, i.e. DevTools or about://tracing,
   // we don't start background tracing to not interfere with the user activity.
@@ -393,10 +358,10 @@ bool BackgroundTracingActiveScenario::StartTracing(
   DCHECK(!tracing_session_);
   if (base::FeatureList::IsEnabled(features::kBackgroundTracingProtoOutput)) {
     tracing_session_ =
-        std::make_unique<PerfettoTracingSession>(this, chrome_config, preset);
+        std::make_unique<PerfettoTracingSession>(this, chrome_config);
   } else {
     tracing_session_ =
-        std::make_unique<LegacyTracingSession>(this, chrome_config, preset);
+        std::make_unique<LegacyTracingSession>(this, chrome_config);
   }
 
   SetState(State::kTracing);
@@ -574,8 +539,7 @@ void BackgroundTracingActiveScenario::OnRuleTriggered(
 
     if (state() != State::kTracing) {
       // It was not already tracing, start a new trace.
-      if (!StartTracing(triggered_rule->category_preset(),
-                        base::trace_event::RECORD_UNTIL_FULL)) {
+      if (!StartTracing()) {
         return;
       }
     } else {

content/browser/tracing/background_tracing_active_scenario.h

@@ -68,8 +68,7 @@ class BackgroundTracingActiveScenario {
       const base::RepeatingClosure& callback);
 
  private:
-  bool StartTracing(BackgroundTracingConfigImpl::CategoryPreset,
-                    base::trace_event::TraceRecordMode);
+  bool StartTracing();
   void BeginFinalizing(
       BackgroundTracingManager::StartedFinalizingCallback callback);
 

content/browser/tracing/background_tracing_config_impl.cc

@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "base/values.h"
+#include "build/build_config.h"
 #include "components/tracing/common/trace_startup_config.h"
 #include "content/browser/tracing/background_tracing_rule.h"
 
@@ -27,6 +28,7 @@ const char kConfigModeReactive[] = "REACTIVE_TRACING_MODE";
 const char kConfigScenarioName[] = "scenario_name";
 
 const char kConfigCategoryKey[] = "category";
+const char kConfigCustomCategoriesKey[] = "custom_categories";
 const char kConfigCategoryBenchmark[] = "BENCHMARK";
 const char kConfigCategoryBenchmarkDeep[] = "BENCHMARK_DEEP";
 const char kConfigCategoryBenchmarkGPU[] = "BENCHMARK_GPU";
@@ -42,6 +44,9 @@ const char kConfigCategoryBenchmarkRenderers[] = "BENCHMARK_RENDERERS";
 const char kConfigCategoryBenchmarkServiceworker[] = "BENCHMARK_SERVICEWORKER";
 const char kConfigCategoryBenchmarkPower[] = "BENCHMARK_POWER";
 const char kConfigCategoryBlinkStyle[] = "BLINK_STYLE";
+const char kConfigCategoryCustom[] = "CUSTOM";
+
+const size_t kDefaultTraceBufferSizeInKb = 10 * 1024;
 
 }  // namespace
 
@@ -84,6 +89,8 @@ std::string BackgroundTracingConfigImpl::CategoryPresetToString(
       return kConfigCategoryBenchmarkPower;
     case BackgroundTracingConfigImpl::BLINK_STYLE:
       return kConfigCategoryBlinkStyle;
+    case BackgroundTracingConfigImpl::CUSTOM_CATEGORY_PRESET:
+      return kConfigCategoryCustom;
     case BackgroundTracingConfigImpl::CATEGORY_PRESET_UNSET:
       NOTREACHED();
   }
@@ -169,6 +176,10 @@ bool BackgroundTracingConfigImpl::StringToCategoryPreset(
 }
 
 void BackgroundTracingConfigImpl::IntoDict(base::DictionaryValue* dict) {
+  if (category_preset_ == CUSTOM_CATEGORY_PRESET) {
+    dict->SetString(kConfigCustomCategoriesKey, custom_categories_);
+  }
+
   switch (tracing_mode()) {
     case BackgroundTracingConfigImpl::PREEMPTIVE:
       dict->SetString(kConfigModeKey, kConfigModePreemptive);
@@ -214,6 +225,35 @@ void BackgroundTracingConfigImpl::AddReactiveRule(
   }
 }
 
+base::trace_event::TraceConfig BackgroundTracingConfigImpl::GetTraceConfig(
+    bool requires_anonymized_data) {
+  base::trace_event::TraceRecordMode record_mode =
+      (tracing_mode() == BackgroundTracingConfigImpl::REACTIVE)
+          ? base::trace_event::RECORD_UNTIL_FULL
+          : base::trace_event::RECORD_CONTINUOUSLY;
+
+  base::trace_event::TraceConfig chrome_config =
+      (category_preset() == CUSTOM_CATEGORY_PRESET
+           ? base::trace_event::TraceConfig(custom_categories_, record_mode)
+           : GetConfigForCategoryPreset(category_preset(), record_mode));
+
+  if (requires_anonymized_data) {
+    chrome_config.EnableArgumentFilter();
+  }
+
+  chrome_config.SetTraceBufferSizeInKb(kDefaultTraceBufferSizeInKb);
+
+#if defined(OS_ANDROID)
+  // Set low trace buffer size on Android in order to upload small trace files.
+  if (tracing_mode() == BackgroundTracingConfigImpl::PREEMPTIVE) {
+    chrome_config.SetTraceBufferSizeInEvents(20000);
+    chrome_config.SetTraceBufferSizeInKb(500);
+  }
+#endif
+
+  return chrome_config;
+}
+
 // static
 std::unique_ptr<BackgroundTracingConfigImpl>
 BackgroundTracingConfigImpl::FromDict(const base::DictionaryValue* dict) {
@@ -249,13 +289,19 @@ BackgroundTracingConfigImpl::PreemptiveFromDict(
   std::unique_ptr<BackgroundTracingConfigImpl> config(
       new BackgroundTracingConfigImpl(BackgroundTracingConfigImpl::PREEMPTIVE));
 
-  std::string category_preset_string;
-  if (!dict->GetString(kConfigCategoryKey, &category_preset_string))
-    return nullptr;
+  if (dict->GetString(kConfigCustomCategoriesKey,
+                      &config->custom_categories_)) {
+    config->category_preset_ = CUSTOM_CATEGORY_PRESET;
+  } else {
+    std::string category_preset_string;
+    if (!dict->GetString(kConfigCategoryKey, &category_preset_string))
+      return nullptr;
 
-  if (!StringToCategoryPreset(category_preset_string,
-                              &config->category_preset_))
-    return nullptr;
+    if (!StringToCategoryPreset(category_preset_string,
+                                &config->category_preset_)) {
+      return nullptr;
+    }
+  }
 
   const base::ListValue* configs_list = nullptr;
   if (!dict->GetList(kConfigsKey, &configs_list))
@@ -284,6 +330,20 @@ BackgroundTracingConfigImpl::ReactiveFromDict(
   std::unique_ptr<BackgroundTracingConfigImpl> config(
       new BackgroundTracingConfigImpl(BackgroundTracingConfigImpl::REACTIVE));
 
+  std::string category_preset_string;
+  bool has_global_categories = false;
+  if (dict->GetString(kConfigCustomCategoriesKey,
+                      &config->custom_categories_)) {
+    config->category_preset_ = CUSTOM_CATEGORY_PRESET;
+    has_global_categories = true;
+  } else if (dict->GetString(kConfigCategoryKey, &category_preset_string)) {
+    if (!StringToCategoryPreset(category_preset_string,
+                                &config->category_preset_)) {
+      return nullptr;
+    }
+    has_global_categories = true;
+  }
+
   const base::ListValue* configs_list = nullptr;
   if (!dict->GetList(kConfigsKey, &configs_list))
     return nullptr;
@@ -293,15 +353,17 @@ BackgroundTracingConfigImpl::ReactiveFromDict(
     if (!it.GetAsDictionary(&config_dict))
       return nullptr;
 
-    std::string category_preset_string;
-    if (!config_dict->GetString(kConfigCategoryKey, &category_preset_string))
-      return nullptr;
-
-    BackgroundTracingConfigImpl::CategoryPreset new_category_preset;
-    if (!StringToCategoryPreset(category_preset_string, &new_category_preset))
-      return nullptr;
+    // TODO(oysteine): Remove the per-rule category preset when configs have
+    // been updated to just specify the per-config category preset.
+    if (!has_global_categories &&
+        config_dict->GetString(kConfigCategoryKey, &category_preset_string)) {
+      if (!StringToCategoryPreset(category_preset_string,
+                                  &config->category_preset_)) {
+        return nullptr;
+      }
+    }
 
-    config->AddReactiveRule(config_dict, new_category_preset);
+    config->AddReactiveRule(config_dict, config->category_preset_);
   }
 
   if (config->rules().empty())
@@ -391,6 +453,7 @@ TraceConfig BackgroundTracingConfigImpl::GetConfigForCategoryPreset(
       return config;
     }
     case BackgroundTracingConfigImpl::CategoryPreset::CATEGORY_PRESET_UNSET:
+    case BackgroundTracingConfigImpl::CategoryPreset::CUSTOM_CATEGORY_PRESET:
       NOTREACHED();
   }
   NOTREACHED();

content/browser/tracing/background_tracing_config_impl.h

@@ -30,6 +30,7 @@ class CONTENT_EXPORT BackgroundTracingConfigImpl
 
   enum CategoryPreset {
     CATEGORY_PRESET_UNSET,
+    CUSTOM_CATEGORY_PRESET,
     BENCHMARK,
     BENCHMARK_DEEP,
     BENCHMARK_GPU,
@@ -61,6 +62,8 @@ class CONTENT_EXPORT BackgroundTracingConfigImpl
       const base::DictionaryValue* dict,
       BackgroundTracingConfigImpl::CategoryPreset category_preset);
 
+  base::trace_event::TraceConfig GetTraceConfig(bool requires_anonymized_data);
+
   static std::unique_ptr<BackgroundTracingConfigImpl> PreemptiveFromDict(
       const base::DictionaryValue* dict);
   static std::unique_ptr<BackgroundTracingConfigImpl> ReactiveFromDict(
@@ -75,17 +78,18 @@ class CONTENT_EXPORT BackgroundTracingConfigImpl
       const std::string& category_preset_string,
       BackgroundTracingConfigImpl::CategoryPreset* category_preset);
 
-  static base::trace_event::TraceConfig GetConfigForCategoryPreset(
-      BackgroundTracingConfigImpl::CategoryPreset,
-      base::trace_event::TraceRecordMode);
-
  private:
   FRIEND_TEST_ALL_PREFIXES(BackgroundTracingConfigTest,
                            ValidPreemptiveConfigToString);
 
+  static base::trace_event::TraceConfig GetConfigForCategoryPreset(
+      BackgroundTracingConfigImpl::CategoryPreset,
+      base::trace_event::TraceRecordMode);
+
   CategoryPreset category_preset_;
   std::vector<std::unique_ptr<BackgroundTracingRule>> rules_;
   std::string scenario_name_;
+  std::string custom_categories_;
 
   DISALLOW_COPY_AND_ASSIGN(BackgroundTracingConfigImpl);
 };

content/browser/tracing/background_tracing_config_unittest.cc

@@ -256,6 +256,21 @@ TEST_F(BackgroundTracingConfigTest, PreemptiveConfigFromValidString) {
   EXPECT_EQ(RuleToString(config->rules()[1]),
             "{\"rule\":\"MONITOR_AND_DUMP_WHEN_TRIGGER_NAMED\","
             "\"trigger_name\":\"foo2\"}");
+
+  config = ReadFromJSONString(
+      "{\"mode\":\"PREEMPTIVE_TRACING_MODE\", \"custom_categories\": "
+      "\"toplevel,benchmark\",\"configs\": [{\"rule\": "
+      "\"MONITOR_AND_DUMP_WHEN_TRIGGER_NAMED\", \"trigger_name\":\"foo1\"}]}");
+  EXPECT_TRUE(config);
+  EXPECT_EQ(config->tracing_mode(), BackgroundTracingConfig::PREEMPTIVE);
+  EXPECT_EQ(config->category_preset(),
+            BackgroundTracingConfigImpl::CUSTOM_CATEGORY_PRESET);
+  EXPECT_EQ(config->rules().size(), 1u);
+  EXPECT_EQ(
+      ConfigToString(config.get()),
+      "{\"category\":\"CUSTOM\",\"configs\":[{\"rule\":\"MONITOR_AND_DUMP_WHEN_"
+      "TRIGGER_NAMED\",\"trigger_name\":\"foo1\"}],\"custom_categories\":"
+      "\"toplevel,benchmark\",\"mode\":\"PREEMPTIVE_TRACING_MODE\"}");
 }
 
 TEST_F(BackgroundTracingConfigTest, ValidPreemptiveCategoryToString) {
@@ -375,6 +390,7 @@ TEST_F(BackgroundTracingConfigTest, ReactiveConfigFromValidString) {
             "{\"category\":\"BENCHMARK_DEEP\","
             "\"rule\":\"TRACE_ON_NAVIGATION_UNTIL_TRIGGER_OR_FULL\","
             "\"trigger_delay\":30,\"trigger_name\":\"foo2\"}");
+
   config = ReadFromJSONString(
       "{\"mode\":\"REACTIVE_TRACING_MODE\",\"configs\": [{\"rule\": "
       "\"TRACE_AT_RANDOM_INTERVALS\","
@@ -388,6 +404,22 @@ TEST_F(BackgroundTracingConfigTest, ReactiveConfigFromValidString) {
             "{\"category\":\"BENCHMARK_DEEP\",\"rule\":\"TRACE_AT_RANDOM_"
             "INTERVALS\",\"stop_tracing_on_repeated_reactive\":true,"
             "\"timeout_max\":20,\"timeout_min\":10}");
+
+  config = ReadFromJSONString(
+      "{\"mode\":\"REACTIVE_TRACING_MODE\","
+      "\"custom_categories\": \"benchmark,toplevel\","
+      "\"configs\": [{\"rule\": "
+      "\"TRACE_AT_RANDOM_INTERVALS\","
+      "\"stop_tracing_on_repeated_reactive\": true, "
+      "\"timeout_max\":20,\"timeout_min\":10}]}");
+  EXPECT_TRUE(config);
+  EXPECT_EQ(config->tracing_mode(), BackgroundTracingConfig::REACTIVE);
+  EXPECT_EQ(config->rules().size(), 1u);
+  EXPECT_EQ(ConfigToString(config.get()),
+            "{\"configs\":[{\"category\":\"CUSTOM\",\"rule\":\"TRACE_AT_RANDOM_"
+            "INTERVALS\",\"stop_tracing_on_repeated_reactive\":true,\"timeout_"
+            "max\":20,\"timeout_min\":10}],\"custom_categories\":\"benchmark,"
+            "toplevel\",\"mode\":\"REACTIVE_TRACING_MODE\"}");
 }
 
 TEST_F(BackgroundTracingConfigTest, ValidPreemptiveConfigToString) {
@@ -502,6 +534,28 @@ TEST_F(BackgroundTracingConfigTest, ValidPreemptiveConfigToString) {
               "SPECIFIC_HISTOGRAM_AND_VALUE\",\"trigger_delay\":10}],\"mode\":"
               "\"PREEMPTIVE_TRACING_MODE\"}");
   }
+
+  {
+    config.reset(
+        new BackgroundTracingConfigImpl(BackgroundTracingConfig::PREEMPTIVE));
+
+    std::unique_ptr<base::DictionaryValue> second_dict(
+        new base::DictionaryValue());
+    second_dict->SetString(
+        "rule", "MONITOR_AND_DUMP_WHEN_SPECIFIC_HISTOGRAM_AND_VALUE");
+    second_dict->SetString("histogram_name", "foo");
+    second_dict->SetInteger("histogram_lower_value", 1);
+    second_dict->SetInteger("histogram_upper_value", 2);
+    second_dict->SetInteger("trigger_delay", 10);
+    config->AddPreemptiveRule(second_dict.get());
+
+    EXPECT_EQ(ConfigToString(config.get()),
+              "{\"category\":\"BENCHMARK\",\"configs\":[{\"histogram_lower_"
+              "value\":1,\"histogram_name\":\"foo\",\"histogram_repeat\":true,"
+              "\"histogram_upper_value\":2,\"rule\":\"MONITOR_AND_DUMP_WHEN_"
+              "SPECIFIC_HISTOGRAM_AND_VALUE\",\"trigger_delay\":10}],\"mode\":"
+              "\"PREEMPTIVE_TRACING_MODE\"}");
+  }
 }
 
 TEST_F(BackgroundTracingConfigTest, InvalidPreemptiveConfigToString) {