perfetto: remove user build guardrail

It causes more problems than it solves these days, just remove it. Change-Id: I5f4a2b4ea8056f6672b802b7fc788c139f7499c7
google · Oct 18, 2024 · 6d9f291 · 6d9f291
1 parent 1d2daf2
commit 6d9f291
Show file tree

Hide file tree

Showing 14 changed files with 11 additions and 207 deletions.
diff --git a/Android.bp b/Android.bp
@@ -11216,7 +11216,6 @@ filegroup {
         "src/perfetto_cmd/packet_writer.cc",
         "src/perfetto_cmd/perfetto_cmd.cc",
         "src/perfetto_cmd/perfetto_cmd_android.cc",
-        "src/perfetto_cmd/rate_limiter.cc",
     ],
 }
 
@@ -11287,7 +11286,6 @@ filegroup {
         "src/perfetto_cmd/config_unittest.cc",
         "src/perfetto_cmd/packet_writer_unittest.cc",
         "src/perfetto_cmd/pbtxt_to_pb_unittest.cc",
-        "src/perfetto_cmd/rate_limiter_unittest.cc",
     ],
 }
 

diff --git a/BUILD b/BUILD
@@ -1267,8 +1267,6 @@ perfetto_filegroup(
         "src/perfetto_cmd/packet_writer.h",
         "src/perfetto_cmd/perfetto_cmd.cc",
         "src/perfetto_cmd/perfetto_cmd.h",
-        "src/perfetto_cmd/rate_limiter.cc",
-        "src/perfetto_cmd/rate_limiter.h",
     ],
 )
 

diff --git a/protos/perfetto/config/perfetto_config.proto b/protos/perfetto/config/perfetto_config.proto
@@ -3959,11 +3959,8 @@ message TraceConfig {
   }
   optional IncrementalStateConfig incremental_state_config = 21;
 
-  // Additional guardrail used by the Perfetto command line client.
-  // On user builds when --dropbox is set perfetto will refuse to trace unless
-  // this is also set.
-  // Added in Q.
-  optional bool allow_user_build_tracing = 19;
+  // No longer needed as we unconditionally allow tracing on user builds.
+  optional bool allow_user_build_tracing = 19 [deprecated = true];
 
   // If set the tracing service will ensure there is at most one tracing session
   // with this key.

diff --git a/protos/perfetto/config/trace_config.proto b/protos/perfetto/config/trace_config.proto
@@ -438,11 +438,8 @@ message TraceConfig {
   }
   optional IncrementalStateConfig incremental_state_config = 21;
 
-  // Additional guardrail used by the Perfetto command line client.
-  // On user builds when --dropbox is set perfetto will refuse to trace unless
-  // this is also set.
-  // Added in Q.
-  optional bool allow_user_build_tracing = 19;
+  // No longer needed as we unconditionally allow tracing on user builds.
+  optional bool allow_user_build_tracing = 19 [deprecated = true];
 
   // If set the tracing service will ensure there is at most one tracing session
   // with this key.

diff --git a/protos/perfetto/trace/perfetto_trace.proto b/protos/perfetto/trace/perfetto_trace.proto
@@ -3959,11 +3959,8 @@ message TraceConfig {
   }
   optional IncrementalStateConfig incremental_state_config = 21;
 
-  // Additional guardrail used by the Perfetto command line client.
-  // On user builds when --dropbox is set perfetto will refuse to trace unless
-  // this is also set.
-  // Added in Q.
-  optional bool allow_user_build_tracing = 19;
+  // No longer needed as we unconditionally allow tracing on user builds.
+  optional bool allow_user_build_tracing = 19 [deprecated = true];
 
   // If set the tracing service will ensure there is at most one tracing session
   // with this key.

diff --git a/src/android_stats/perfetto_atoms.h b/src/android_stats/perfetto_atoms.h
@@ -35,7 +35,6 @@ enum class PerfettoStatsdAtom {
 
   // Guardrails inside perfetto_cmd before tracing is finished.
   kOnTimeout = 16,
-  kCmdUserBuildTracingNotAllowed = 43,
 
   // Checkpoints inside traced.
   kTracedEnableTracing = 37,
@@ -113,6 +112,10 @@ enum class PerfettoStatsdAtom {
   // Contained status of guardrail state initialization and upload limit in
   // perfetto_cmd. Removed as perfetto no longer manages stateful guardrails
   // reserved 44, 45, 46;
+
+  // Contained the guardrail for user build tracing. Removed as this guardrail
+  // causes more problem than it solves these days.
+  // reserved 43;
 };
 
 // This must match the values of the PerfettoTrigger::TriggerType enum in:

diff --git a/src/perfetto_cmd/BUILD.gn b/src/perfetto_cmd/BUILD.gn
@@ -71,8 +71,6 @@ source_set("perfetto_cmd") {
     "packet_writer.h",
     "perfetto_cmd.cc",
     "perfetto_cmd.h",
-    "rate_limiter.cc",
-    "rate_limiter.h",
   ]
   if (is_android) {
     deps += [ "../android_internal:lazy_library_loader" ]
@@ -163,6 +161,5 @@ perfetto_unittest_source_set("unittests") {
     "config_unittest.cc",
     "packet_writer_unittest.cc",
     "pbtxt_to_pb_unittest.cc",
-    "rate_limiter_unittest.cc",
   ]
 }
diff --git a/src/perfetto_cmd/perfetto_cmd.cc b/src/perfetto_cmd/perfetto_cmd.cc
@@ -78,7 +78,6 @@
 #include "src/perfetto_cmd/config.h"
 #include "src/perfetto_cmd/packet_writer.h"
 #include "src/perfetto_cmd/pbtxt_to_pb.h"
-#include "src/perfetto_cmd/rate_limiter.h"
 #include "src/perfetto_cmd/trigger_producer.h"
 
 #include "protos/perfetto/common/ftrace_descriptor.gen.h"
@@ -159,30 +158,6 @@ bool ParseTraceConfigPbtxt(const std::string& file_name,
   return true;
 }
 
-bool IsUserBuild() {
-#if PERFETTO_BUILDFLAG(PERFETTO_ANDROID_BUILD)
-  std::string build_type = base::GetAndroidProp("ro.build.type");
-  if (build_type.empty()) {
-    PERFETTO_ELOG("Unable to read ro.build.type: assuming user build");
-    return true;
-  }
-  return build_type == "user";
-#else
-  return false;
-#endif  // PERFETTO_BUILDFLAG(PERFETTO_ANDROID_BUILD)
-}
-
-std::optional<PerfettoStatsdAtom> ConvertRateLimiterResponseToAtom(
-    RateLimiter::ShouldTraceResponse resp) {
-  switch (resp) {
-    case RateLimiter::kNotAllowedOnUserBuild:
-      return PerfettoStatsdAtom::kCmdUserBuildTracingNotAllowed;
-    case RateLimiter::kOkToTrace:
-      return std::nullopt;
-  }
-  PERFETTO_FATAL("For GCC");
-}
-
 void ArgsAppend(std::string* str, const std::string& arg) {
   str->append(arg);
   str->append("\0", 1);
@@ -334,7 +309,6 @@ std::optional<int> PerfettoCmd::ParseCmdlineAndMaybeDaemonize(int argc,
   std::string trace_config_raw;
   bool parse_as_pbtxt = false;
   TraceConfig::StatsdMetadata statsd_metadata;
-  limiter_.reset(new RateLimiter());
 
   ConfigOptions config_options;
   bool has_config_options = false;
@@ -976,11 +950,6 @@ int PerfettoCmd::ConnectToServiceAndRun() {
     return 1;  // We can legitimately get here if the service disconnects.
   }
 
-  RateLimiter::Args args{};
-  args.is_user_build = IsUserBuild();
-  args.is_uploading = save_to_incidentd_ || report_to_android_framework_;
-  args.allow_user_build_tracing = trace_config_->allow_user_build_tracing();
-
   if (!trace_config_->unique_session_name().empty())
     base::MaybeSetThreadName("p-" + trace_config_->unique_session_name());
 
@@ -1017,12 +986,6 @@ int PerfettoCmd::ConnectToServiceAndRun() {
     LogUploadEvent(PerfettoStatsdAtom::kBackgroundTraceBegin);
   }
 
-  auto err_atom = ConvertRateLimiterResponseToAtom(limiter_->ShouldTrace(args));
-  if (err_atom) {
-    LogUploadEvent(err_atom.value());
-    return 1;
-  }
-
 #if PERFETTO_BUILDFLAG(PERFETTO_OS_ANDROID)
   if (!background_ && !is_detach() && !upload_flag_ &&
       triggers_to_activate_.empty() && !isatty(STDIN_FILENO) &&
@@ -1125,7 +1088,7 @@ void PerfettoCmd::OnConnect() {
   // Failsafe mechanism to avoid waiting indefinitely if the service hangs.
   // Note: when using prefer_suspend_clock_for_duration the actual duration
   // might be < expected_duration_ms_ measured in in wall time. But this is fine
-  // because the resulting timeout will be conservative (it will be accurate
+  // because the resulting timeout will be conservative (it will be accut
   // if the device never suspends, and will be more lax if it does).
   if (expected_duration_ms_) {
     uint32_t trace_timeout = expected_duration_ms_ + 60000 +

diff --git a/src/perfetto_cmd/perfetto_cmd.h b/src/perfetto_cmd/perfetto_cmd.h
@@ -42,8 +42,6 @@
 
 namespace perfetto {
 
-class RateLimiter;
-
 // Directory for local state and temporary files. This is automatically
 // created by the system by setting setprop persist.traced.enable=1.
 extern const char* kStateDir;
@@ -145,7 +143,6 @@ class PerfettoCmd : public Consumer {
 
   base::UnixTaskRunner task_runner_;
 
-  std::unique_ptr<RateLimiter> limiter_;
   std::unique_ptr<perfetto::TracingService::ConsumerEndpoint>
       consumer_endpoint_;
   std::unique_ptr<TraceConfig> trace_config_;

diff --git a/src/perfetto_cmd/rate_limiter.cc b/src/perfetto_cmd/rate_limiter.cc
diff --git a/src/perfetto_cmd/rate_limiter.h b/src/perfetto_cmd/rate_limiter.h
diff --git a/src/perfetto_cmd/rate_limiter_unittest.cc b/src/perfetto_cmd/rate_limiter_unittest.cc
diff --git a/test/cmdline_integrationtest.cc b/test/cmdline_integrationtest.cc
@@ -551,7 +551,6 @@ TEST_F(PerfettoCmdlineTest, AndroidOnly(NoDataNoFileWithoutTrigger)) {
   constexpr size_t kMessageSize = 32;
   protos::gen::TraceConfig trace_config;
   trace_config.add_buffers()->set_size_kb(1024);
-  trace_config.set_allow_user_build_tracing(true);
   auto* incident_config = trace_config.mutable_incident_report_config();
   incident_config->set_destination_package("foo.bar.baz");
   auto* ds_config = trace_config.add_data_sources()->mutable_config();

diff --git a/test/cts/reporter/reporter_test_cts.cc b/test/cts/reporter/reporter_test_cts.cc
@@ -41,7 +41,6 @@ TEST(PerfettoReporterTest, TestEndToEndReport) {
   TraceConfig trace_config;
   trace_config.add_buffers()->set_size_kb(1024);
   trace_config.set_duration_ms(200);
-  trace_config.set_allow_user_build_tracing(true);
   trace_config.set_unique_session_name("TestEndToEndReport");
 
   // Make the trace as small as possible (see b/282508742).