Implement argument filter in trace event data source

The argument filter is required to make sure only whitelisted arguments
are present in traces from users. To be able to use Perfetto backend in
slow reports in JSON mode, this CL implements argument filtering as it
exists in TraceLog. The metadata filtering is done by
TracingControllerImpl with both backends.

BUG=925148

Change-Id: I65ad53a8ccfea8f721b7fbf4dd7e66969e50fda2
Reviewed-on: https://chromium-review.googlesource.com/c/1464433
Commit-Queue: ssid <ssid@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632001}

base/test/trace_event_analyzer.cc

@@ -103,8 +103,14 @@ bool TraceEvent::SetFromJSON(const base::Value* event_value) {
     return false;
   }
   if (!dictionary->GetDictionary("args", &args)) {
-    LOG(ERROR) << "args is missing from TraceEvent JSON";
-    return false;
+    std::string stripped_args;
+    // If argument filter is enabled, the arguments field contains a string
+    // value.
+    if (!dictionary->GetString("args", &stripped_args) ||
+        stripped_args != "__stripped__") {
+      LOG(ERROR) << "args is missing from TraceEvent JSON";
+      return false;
+    }
   }
   if (require_id && !dictionary->GetString("id", &id)) {
     LOG(ERROR) << "id is missing from ASYNC_BEGIN/ASYNC_END TraceEvent JSON";
@@ -125,23 +131,25 @@ bool TraceEvent::SetFromJSON(const base::Value* event_value) {
   }
 
   // For each argument, copy the type and create a trace_analyzer::TraceValue.
-  for (base::DictionaryValue::Iterator it(*args); !it.IsAtEnd();
-       it.Advance()) {
-    std::string str;
-    bool boolean = false;
-    int int_num = 0;
-    double double_num = 0.0;
-    if (it.value().GetAsString(&str)) {
-      arg_strings[it.key()] = str;
-    } else if (it.value().GetAsInteger(&int_num)) {
-      arg_numbers[it.key()] = static_cast<double>(int_num);
-    } else if (it.value().GetAsBoolean(&boolean)) {
-      arg_numbers[it.key()] = static_cast<double>(boolean ? 1 : 0);
-    } else if (it.value().GetAsDouble(&double_num)) {
-      arg_numbers[it.key()] = double_num;
+  if (args) {
+    for (base::DictionaryValue::Iterator it(*args); !it.IsAtEnd();
+         it.Advance()) {
+      std::string str;
+      bool boolean = false;
+      int int_num = 0;
+      double double_num = 0.0;
+      if (it.value().GetAsString(&str)) {
+        arg_strings[it.key()] = str;
+      } else if (it.value().GetAsInteger(&int_num)) {
+        arg_numbers[it.key()] = static_cast<double>(int_num);
+      } else if (it.value().GetAsBoolean(&boolean)) {
+        arg_numbers[it.key()] = static_cast<double>(boolean ? 1 : 0);
+      } else if (it.value().GetAsDouble(&double_num)) {
+        arg_numbers[it.key()] = double_num;
+      }
+      // Record all arguments as values.
+      arg_values[it.key()] = it.value().CreateDeepCopy();
     }
-    // Record all arguments as values.
-    arg_values[it.key()] = it.value().CreateDeepCopy();
   }
 
   return true;

base/trace_event/trace_log.cc

@@ -652,6 +652,11 @@ void TraceLog::SetArgumentFilterPredicate(
   argument_filter_predicate_ = argument_filter_predicate;
 }
 
+ArgumentFilterPredicate TraceLog::GetArgumentFilterPredicate() const {
+  AutoLock lock(lock_);
+  return argument_filter_predicate_;
+}
+
 TraceLog::InternalTraceOptions TraceLog::GetInternalOptionsFromTraceConfig(
     const TraceConfig& config) {
   InternalTraceOptions ret = config.IsArgumentFilterEnabled()

base/trace_event/trace_log.h

@@ -165,6 +165,7 @@ class BASE_EXPORT TraceLog : public MemoryDumpProvider {
 
   void SetArgumentFilterPredicate(
       const ArgumentFilterPredicate& argument_filter_predicate);
+  ArgumentFilterPredicate GetArgumentFilterPredicate() const;
 
   // Flush all collected events to the given output callback. The callback will
   // be called one or more times either synchronously or asynchronously from

services/tracing/perfetto/json_trace_exporter.cc

@@ -85,10 +85,65 @@ const char* GetStringFromStringTable(
   return it->second.c_str();
 }
 
+void OutputJSONFromArgumentValue(
+    const perfetto::protos::ChromeTraceEvent::Arg& arg,
+    std::string* out) {
+  TraceEvent::TraceValue value;
+  if (arg.has_bool_value()) {
+    value.as_bool = arg.bool_value();
+    TraceEvent::AppendValueAsJSON(TRACE_VALUE_TYPE_BOOL, value, out);
+    return;
+  }
+
+  if (arg.has_uint_value()) {
+    value.as_uint = arg.uint_value();
+    TraceEvent::AppendValueAsJSON(TRACE_VALUE_TYPE_UINT, value, out);
+    return;
+  }
+
+  if (arg.has_int_value()) {
+    value.as_int = arg.int_value();
+    TraceEvent::AppendValueAsJSON(TRACE_VALUE_TYPE_INT, value, out);
+    return;
+  }
+
+  if (arg.has_double_value()) {
+    value.as_double = arg.double_value();
+    TraceEvent::AppendValueAsJSON(TRACE_VALUE_TYPE_DOUBLE, value, out);
+    return;
+  }
+
+  if (arg.has_pointer_value()) {
+    value.as_pointer = reinterpret_cast<void*>(arg.pointer_value());
+    TraceEvent::AppendValueAsJSON(TRACE_VALUE_TYPE_POINTER, value, out);
+    return;
+  }
+
+  if (arg.has_string_value()) {
+    std::string str = arg.string_value();
+    value.as_string = &str[0];
+    TraceEvent::AppendValueAsJSON(TRACE_VALUE_TYPE_STRING, value, out);
+    return;
+  }
+
+  if (arg.has_json_value()) {
+    *out += arg.json_value();
+    return;
+  }
+
+  if (arg.has_traced_value()) {
+    AppendProtoDictAsJSON(out, arg.traced_value());
+    return;
+  }
+
+  NOTREACHED();
+}
+
 void OutputJSONFromTraceEventProto(
     const perfetto::protos::ChromeTraceEvent& event,
-    std::string* out,
-    const std::unordered_map<int, std::string>& string_table) {
+    const std::unordered_map<int, std::string>& string_table,
+    const JSONTraceExporter::ArgumentFilterPredicate& argument_filter_predicate,
+    std::string* out) {
   char phase = static_cast<char>(event.phase());
   const char* name =
       event.has_name_index()
@@ -189,72 +244,46 @@ void OutputJSONFromTraceEventProto(
     base::StringAppendF(out, ",\"s\":\"%c\"", scope);
   }
 
-  *out += ",\"args\":{";
-  for (int i = 0; i < event.args_size(); ++i) {
-    auto& arg = event.args(i);
-
-    if (i > 0) {
-      *out += ",";
-    }
-
-    *out += "\"";
-    *out += arg.has_name_index()
-                ? GetStringFromStringTable(string_table, arg.name_index())
-                : arg.name();
-    *out += "\":";
+  *out += ",\"args\":";
 
-    TraceEvent::TraceValue value;
-    if (arg.has_bool_value()) {
-      value.as_bool = arg.bool_value();
-      TraceEvent::AppendValueAsJSON(TRACE_VALUE_TYPE_BOOL, value, out);
-      continue;
-    }
-
-    if (arg.has_uint_value()) {
-      value.as_uint = arg.uint_value();
-      TraceEvent::AppendValueAsJSON(TRACE_VALUE_TYPE_UINT, value, out);
-      continue;
-    }
-
-    if (arg.has_int_value()) {
-      value.as_int = arg.int_value();
-      TraceEvent::AppendValueAsJSON(TRACE_VALUE_TYPE_INT, value, out);
-      continue;
-    }
-
-    if (arg.has_double_value()) {
-      value.as_double = arg.double_value();
-      TraceEvent::AppendValueAsJSON(TRACE_VALUE_TYPE_DOUBLE, value, out);
-      continue;
-    }
+  JSONTraceExporter::ArgumentNameFilterPredicate argument_name_filter_predicate;
+  bool strip_args =
+      event.args_size() > 0 && !argument_filter_predicate.is_null() &&
+      !argument_filter_predicate.Run(category_group_name, name,
+                                     &argument_name_filter_predicate);
 
-    if (arg.has_pointer_value()) {
-      value.as_pointer = reinterpret_cast<void*>(arg.pointer_value());
-      TraceEvent::AppendValueAsJSON(TRACE_VALUE_TYPE_POINTER, value, out);
-      continue;
-    }
+  if (strip_args) {
+    *out += "\"__stripped__\"";
+  } else {
+    *out += "{";
 
-    if (arg.has_string_value()) {
-      std::string str = arg.string_value();
-      value.as_string = &str[0];
-      TraceEvent::AppendValueAsJSON(TRACE_VALUE_TYPE_STRING, value, out);
-      continue;
-    }
+    for (int i = 0; i < event.args_size(); ++i) {
+      auto& arg = event.args(i);
 
-    if (arg.has_json_value()) {
-      *out += arg.json_value();
-      continue;
-    }
+      if (i > 0) {
+        *out += ",";
+      }
 
-    if (arg.has_traced_value()) {
-      AppendProtoDictAsJSON(out, arg.traced_value());
-      continue;
+      *out += "\"";
+      std::string arg_name =
+          arg.has_name_index()
+              ? GetStringFromStringTable(string_table, arg.name_index())
+              : arg.name();
+      *out += arg_name;
+      *out += "\":";
+
+      if (!argument_name_filter_predicate.is_null() &&
+          !argument_name_filter_predicate.Run(arg_name.c_str())) {
+        *out += "\"__stripped__\"";
+        continue;
+      }
+      OutputJSONFromArgumentValue(arg, out);
     }
 
-    NOTREACHED();
+    *out += "}";
   }
 
-  *out += "}}";
+  *out += "}";
 }
 
 std::unique_ptr<base::DictionaryValue> ConvertTraceStatsToDict(
@@ -321,9 +350,12 @@ void AppendProtoDictAsJSON(std::string* out,
   out->append("}");
 }
 
-JSONTraceExporter::JSONTraceExporter(OnTraceEventJSONCallback callback)
+JSONTraceExporter::JSONTraceExporter(
+    ArgumentFilterPredicate argument_filter_predicate,
+    OnTraceEventJSONCallback callback)
     : json_callback_(callback),
-      metadata_(std::make_unique<base::DictionaryValue>()) {
+      metadata_(std::make_unique<base::DictionaryValue>()),
+      argument_filter_predicate_(argument_filter_predicate) {
   DCHECK(json_callback_);
 }
 
@@ -380,7 +412,8 @@ void JSONTraceExporter::OnTraceData(std::vector<perfetto::TracePacket> packets,
         has_output_first_event_ = true;
       }
 
-      OutputJSONFromTraceEventProto(event, &out, string_table);
+      OutputJSONFromTraceEventProto(event, string_table,
+                                    argument_filter_predicate_, &out);
     }
 
     for (auto& metadata : bundle.metadata()) {

services/tracing/perfetto/json_trace_exporter.h

@@ -31,18 +31,34 @@ void AppendProtoDictAsJSON(std::string* out,
 // Conversion happens on-the-fly as new trace packets are received.
 class JSONTraceExporter {
  public:
+  // Given argument name for the trace event, returns if the argument should be
+  // filtered or not.
+  using ArgumentNameFilterPredicate =
+      base::RepeatingCallback<bool(const char* arg_name)>;
+
+  // Given trace event name and category group name, returns a argument name
+  // filter predicate callback that can filter arguments for the given event.
+  using ArgumentFilterPredicate =
+      base::RepeatingCallback<bool(const char* category_group_name,
+                                   const char* event_name,
+                                   ArgumentNameFilterPredicate*)>;
+
   using OnTraceEventJSONCallback =
       base::RepeatingCallback<void(const std::string& json,
                                    base::DictionaryValue* metadata,
                                    bool has_more)>;
 
-  JSONTraceExporter(OnTraceEventJSONCallback callback);
+  JSONTraceExporter(ArgumentFilterPredicate, OnTraceEventJSONCallback callback);
   virtual ~JSONTraceExporter();
 
   // Called to notify the exporter of new trace packets. Will call the
   // |json_callback| passed in the constructor with the converted trace data.
   void OnTraceData(std::vector<perfetto::TracePacket> packets, bool has_more);
 
+  void SetArgumentFilterForTesting(const ArgumentFilterPredicate& predicate) {
+    argument_filter_predicate_ = predicate;
+  }
+
  private:
   OnTraceEventJSONCallback json_callback_;
   bool has_output_json_preamble_ = false;
@@ -51,6 +67,8 @@ class JSONTraceExporter {
   std::string legacy_system_ftrace_output_;
   std::string legacy_system_trace_events_;
 
+  ArgumentFilterPredicate argument_filter_predicate_;
+
   DISALLOW_COPY_AND_ASSIGN(JSONTraceExporter);
 };
 

services/tracing/perfetto/json_trace_exporter_unittest.cc

@@ -13,6 +13,7 @@
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
+#include "base/strings/pattern.h"
 #include "base/test/trace_event_analyzer.h"
 #include "base/trace_event/trace_event.h"
 #include "base/values.h"
@@ -24,19 +25,54 @@
 #include "third_party/perfetto/protos/perfetto/trace/chrome/chrome_trace_event.pbzero.h"
 #include "third_party/perfetto/protos/perfetto/trace/trace_packet.pb.h"
 
+using base::trace_event::TraceLog;
+
 namespace tracing {
 
+namespace {
+
+bool IsArgNameWhitelisted(const char* arg_name) {
+  return base::MatchPattern(arg_name, "granular_arg_whitelisted");
+}
+
+bool IsTraceEventArgsWhitelisted(
+    const char* category_group_name,
+    const char* event_name,
+    base::trace_event::ArgumentNameFilterPredicate* arg_filter) {
+  if (base::MatchPattern(category_group_name, "toplevel") &&
+      base::MatchPattern(event_name, "*")) {
+    return true;
+  }
+
+  if (base::MatchPattern(category_group_name, "benchmark") &&
+      base::MatchPattern(event_name, "granularly_whitelisted")) {
+    *arg_filter = base::BindRepeating(&IsArgNameWhitelisted);
+    return true;
+  }
+
+  return false;
+}
+
+}  // namespace
+
 class JSONTraceExporterTest : public testing::Test {
  public:
   void SetUp() override {
-    json_trace_exporter_.reset(new JSONTraceExporter(base::BindRepeating(
-        &JSONTraceExporterTest::OnTraceEventJSON, base::Unretained(this))));
+    json_trace_exporter_.reset(new JSONTraceExporter(
+        JSONTraceExporter::ArgumentFilterPredicate(),
+        base::BindRepeating(&JSONTraceExporterTest::OnTraceEventJSON,
+                            base::Unretained(this))));
   }
 
   void TearDown() override {
     json_trace_exporter_.reset();
   }
 
+  void EnableArgumentFilter() {
+    json_trace_exporter_->SetArgumentFilterForTesting(
+        base::BindRepeating(&IsTraceEventArgsWhitelisted));
+  }
+
   void OnTraceEventJSON(const std::string& json,
                         base::DictionaryValue* metadata,
                         bool has_more) {
@@ -57,6 +93,7 @@ class JSONTraceExporterTest : public testing::Test {
     base::JSONWriter::Write(*events_value, &raw_events);
 
     trace_analyzer_.reset(trace_analyzer::TraceAnalyzer::Create(raw_events));
+    EXPECT_TRUE(trace_analyzer_);
   }
 
   void SetTestPacketBasicData(
@@ -597,4 +634,75 @@ TEST_F(JSONTraceExporterTest, TestLegacySystemTraceEvents) {
   EXPECT_EQ(content->GetList()[0].FindKey("name")->GetString(), "bar_name");
 }
 
+TEST_F(JSONTraceExporterTest, ArgsWhitelisting) {
+  EnableArgumentFilter();
+
+  perfetto::protos::TracePacket trace_packet_proto;
+
+  {
+    auto* new_trace_event =
+        trace_packet_proto.mutable_chrome_events()->add_trace_events();
+    SetTestPacketBasicData(new_trace_event);
+    new_trace_event->set_name("event1");
+    new_trace_event->set_category_group_name("toplevel");
+    auto* new_arg = new_trace_event->add_args();
+    new_arg->set_name("int_one");
+    new_arg->set_uint_value(1);
+  }
+
+  {
+    auto* new_trace_event =
+        trace_packet_proto.mutable_chrome_events()->add_trace_events();
+    SetTestPacketBasicData(new_trace_event);
+    new_trace_event->set_name("event2");
+    new_trace_event->set_category_group_name("whitewashed");
+    auto* new_arg = new_trace_event->add_args();
+    new_arg->set_name("int_two");
+    new_arg->set_uint_value(1);
+  }
+
+  {
+    auto* new_trace_event =
+        trace_packet_proto.mutable_chrome_events()->add_trace_events();
+    SetTestPacketBasicData(new_trace_event);
+    new_trace_event->set_name("granularly_whitelisted");
+    new_trace_event->set_category_group_name("benchmark");
+    auto* new_arg1 = new_trace_event->add_args();
+    new_arg1->set_name("granular_arg_whitelisted");
+    new_arg1->set_string_value("whitelisted_value");
+    auto* new_arg2 = new_trace_event->add_args();
+    new_arg2->set_name("granular_arg_blacklisted");
+    new_arg2->set_string_value("blacklisted_value");
+  }
+
+  FinalizePacket(trace_packet_proto);
+
+  {
+    const auto* trace_event = trace_analyzer()->FindFirstOf(
+        trace_analyzer::Query(trace_analyzer::Query::EVENT_NAME) ==
+        trace_analyzer::Query::String("event1"));
+    EXPECT_TRUE(trace_event);
+    EXPECT_EQ(1, trace_event->GetKnownArgAsDouble("int_one"));
+  }
+
+  {
+    const auto* trace_event = trace_analyzer()->FindFirstOf(
+        trace_analyzer::Query(trace_analyzer::Query::EVENT_NAME) ==
+        trace_analyzer::Query::String("event2"));
+    EXPECT_TRUE(trace_event);
+    EXPECT_FALSE(trace_event->HasArg(("int_two")));
+  }
+
+  {
+    const auto* trace_event = trace_analyzer()->FindFirstOf(
+        trace_analyzer::Query(trace_analyzer::Query::EVENT_NAME) ==
+        trace_analyzer::Query::String("granularly_whitelisted"));
+    EXPECT_TRUE(trace_event);
+    EXPECT_EQ("whitelisted_value",
+              trace_event->GetKnownArgAsString(("granular_arg_whitelisted")));
+    EXPECT_EQ("__stripped__",
+              trace_event->GetKnownArgAsString(("granular_arg_blacklisted")));
+  }
+}
+
 }  // namespace tracing

services/tracing/perfetto/perfetto_tracing_coordinator.cc

@@ -8,6 +8,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/trace_event/trace_log.h"
 #include "build/build_config.h"
 #include "mojo/public/cpp/system/data_pipe_utils.h"
 #include "services/tracing/perfetto/json_trace_exporter.h"
@@ -20,6 +21,15 @@
 
 namespace tracing {
 
+namespace {
+
+bool IsArgumentFilterEnabled(const std::string& config) {
+  base::trace_event::TraceConfig chrome_trace_config_obj(config);
+  return chrome_trace_config_obj.IsArgumentFilterEnabled();
+}
+
+}  // namespace
+
 // A TracingSession acts as a perfetto consumer and is used to wrap all the
 // associated state of an on-going tracing session, for easy setup and cleanup.
 //
@@ -31,6 +41,10 @@ class PerfettoTracingCoordinator::TracingSession : public perfetto::Consumer {
   TracingSession(const std::string& config,
                  base::OnceClosure tracing_over_callback)
       : json_trace_exporter_(std::make_unique<JSONTraceExporter>(
+            IsArgumentFilterEnabled(config)
+                ? base::trace_event::TraceLog::GetInstance()
+                      ->GetArgumentFilterPredicate()
+                : JSONTraceExporter::ArgumentFilterPredicate(),
             base::BindRepeating(&TracingSession::OnJSONTraceEventCallback,
                                 base::Unretained(this)))),
         tracing_over_callback_(std::move(tracing_over_callback)) {