Service: plumb EnableTracing errors to the consumer
This change propagates service errors that occur when
invoking the consumer RPC method EnableTracing down to
the consumer. This allows the consumer to tell the user
what went wrong.
In order to do this, this CL moved TraceProcessor's
util::Status up to base and uses that in TracingServiceImpl.
Bug: chromium:1141377
Test: manual: run perfetto --txt ... passing a malformed config and
see logging both on service and client side.
Change-Id: I827cef8357de219301102c56cb66de7d0b9a303ddiff --git a/Android.bp b/Android.bp
index 2ebcd28..226c350 100644
--- a/Android.bp
+++ b/Android.bp
@@ -6081,6 +6081,7 @@
"src/base/metatrace.cc",
"src/base/paged_memory.cc",
"src/base/pipe.cc",
+ "src/base/status.cc",
"src/base/string_splitter.cc",
"src/base/string_utils.cc",
"src/base/string_view.cc",
diff --git a/BUILD b/BUILD
index 4b44849..7cb3963 100644
--- a/BUILD
+++ b/BUILD
@@ -273,6 +273,7 @@
"include/perfetto/base/flat_set.h",
"include/perfetto/base/logging.h",
"include/perfetto/base/proc_utils.h",
+ "include/perfetto/base/status.h",
"include/perfetto/base/task_runner.h",
"include/perfetto/base/thread_utils.h",
"include/perfetto/base/time.h",
@@ -548,6 +549,7 @@
"src/base/metatrace.cc",
"src/base/paged_memory.cc",
"src/base/pipe.cc",
+ "src/base/status.cc",
"src/base/string_splitter.cc",
"src/base/string_utils.cc",
"src/base/string_view.cc",
diff --git a/include/perfetto/base/BUILD.gn b/include/perfetto/base/BUILD.gn
index 85819b7..58d2552 100644
--- a/include/perfetto/base/BUILD.gn
+++ b/include/perfetto/base/BUILD.gn
@@ -22,6 +22,7 @@
"flat_set.h",
"logging.h",
"proc_utils.h",
+ "status.h",
"task_runner.h",
"thread_utils.h",
"time.h",
diff --git a/include/perfetto/base/status.h b/include/perfetto/base/status.h
new file mode 100644
index 0000000..7e2863d
--- /dev/null
+++ b/include/perfetto/base/status.h
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef INCLUDE_PERFETTO_BASE_STATUS_H_
+#define INCLUDE_PERFETTO_BASE_STATUS_H_
+
+#include <string>
+
+#include "perfetto/base/compiler.h"
+#include "perfetto/base/export.h"
+#include "perfetto/base/logging.h"
+
+namespace perfetto {
+namespace base {
+
+// Represents either the success or the failure message of a function.
+// This can used as the return type of functions which would usually return an
+// bool for success or int for errno but also wants to add some string context
+// (ususally for logging).
+class PERFETTO_EXPORT Status {
+ public:
+ Status() : ok_(true) {}
+ explicit Status(std::string msg) : ok_(false), message_(std::move(msg)) {
+ PERFETTO_DCHECK(!message_.empty());
+ }
+
+ // Copy operations.
+ Status(const Status&) = default;
+ Status& operator=(const Status&) = default;
+
+ // Move operations. The moved-from state is valid but unspecified.
+ Status(Status&&) noexcept = default;
+ Status& operator=(Status&&) = default;
+
+ bool ok() const { return ok_; }
+
+ // Only valid to call when this message has an Err status (i.e. ok() returned
+ // false or operator bool() returned true).
+ const std::string& message() const {
+ PERFETTO_DCHECK(!ok_);
+ return message_;
+ }
+
+ // Only valid to call when this message has an Err status (i.e. ok() returned
+ // false or operator bool() returned true).
+ const char* c_message() const {
+ PERFETTO_DCHECK(!ok_);
+ return message_.c_str();
+ }
+
+ private:
+ bool ok_ = false;
+ std::string message_;
+};
+
+// Returns a status object which represents the Ok status.
+inline Status OkStatus() {
+ return Status();
+}
+
+PERFETTO_PRINTF_FORMAT(1, 2) Status ErrStatus(const char* format, ...);
+
+} // namespace base
+} // namespace perfetto
+
+#endif // INCLUDE_PERFETTO_BASE_STATUS_H_
diff --git a/include/perfetto/ext/tracing/core/consumer.h b/include/perfetto/ext/tracing/core/consumer.h
index 0cbd158..fce7c65 100644
--- a/include/perfetto/ext/tracing/core/consumer.h
+++ b/include/perfetto/ext/tracing/core/consumer.h
@@ -47,8 +47,9 @@
// - The consumer explicitly called DisableTracing()
// - The TraceConfig's |duration_ms| has been reached.
// - The TraceConfig's |max_file_size_bytes| has been reached.
- // - An error occurred while trying to enable tracing.
- virtual void OnTracingDisabled() = 0;
+ // - An error occurred while trying to enable tracing. In this case |error|
+ // is non-empty.
+ virtual void OnTracingDisabled(const std::string& error) = 0;
// Called back by the Service (or transport layer) after invoking
// TracingService::ConsumerEndpoint::ReadBuffers(). This function can be
diff --git a/include/perfetto/trace_processor/BUILD.gn b/include/perfetto/trace_processor/BUILD.gn
index bdaf789..ed1f3a9 100644
--- a/include/perfetto/trace_processor/BUILD.gn
+++ b/include/perfetto/trace_processor/BUILD.gn
@@ -34,4 +34,5 @@
"basic_types.h",
"status.h",
]
+ public_deps = [ "../base" ]
}
diff --git a/include/perfetto/trace_processor/status.h b/include/perfetto/trace_processor/status.h
index ee7271a..6ee417e 100644
--- a/include/perfetto/trace_processor/status.h
+++ b/include/perfetto/trace_processor/status.h
@@ -17,67 +17,20 @@
#ifndef INCLUDE_PERFETTO_TRACE_PROCESSOR_STATUS_H_
#define INCLUDE_PERFETTO_TRACE_PROCESSOR_STATUS_H_
-#include <stdarg.h>
-#include <string>
+#include "perfetto/base/status.h"
-#include "perfetto/base/export.h"
+// Once upon a time Status used to live in perfetto::trace_processor. At some
+// point it has been moved up to base. This forwarding header stayed here
+// because of out-of-repo users.
namespace perfetto {
namespace trace_processor {
-
-// Status and related methods are inside util for consistency with embedders of
-// trace processor.
namespace util {
-// Represents either the success or the failure message of a function.
-// This can used as the return type of functions which would usually return an
-// bool for success or int for errno but also wants to add some string context
-// (ususally for logging).
-class PERFETTO_EXPORT Status {
- public:
- Status() : ok_(true) {}
- explicit Status(std::string error) : ok_(false), message_(std::move(error)) {}
+using Status = ::perfetto::base::Status;
- // Copy operations.
- Status(const Status&) = default;
- Status& operator=(const Status&) = default;
-
- // Move operations. The moved-from state is valid but unspecified.
- Status(Status&&) noexcept = default;
- Status& operator=(Status&&) = default;
-
- bool ok() const { return ok_; }
-
- // Only valid to call when this message has an Err status (i.e. ok() returned
- // false or operator bool() returned true).
- const std::string& message() const { return message_; }
-
- // Only valid to call when this message has an Err status (i.e. ok() returned
- // false or operator bool() returned true).
- const char* c_message() const { return message_.c_str(); }
-
- private:
- bool ok_ = false;
- std::string message_;
-};
-
-// Returns a status object which represents the Ok status.
-inline Status OkStatus() {
- return Status();
-}
-
-// Returns a status object which represents an error with the given message
-// formatted using printf.
-__attribute__((__format__(__printf__, 1, 2))) inline Status ErrStatus(
- const char* format,
- ...) {
- va_list ap;
- va_start(ap, format);
-
- char buffer[1024];
- vsnprintf(buffer, sizeof(buffer), format, ap);
- return Status(std::string(buffer));
-}
+constexpr auto OkStatus = ::perfetto::base::OkStatus;
+constexpr auto ErrStatus = ::perfetto::base::ErrStatus;
} // namespace util
} // namespace trace_processor
diff --git a/protos/perfetto/ipc/consumer_port.proto b/protos/perfetto/ipc/consumer_port.proto
index b8b929c..6b7147d 100644
--- a/protos/perfetto/ipc/consumer_port.proto
+++ b/protos/perfetto/ipc/consumer_port.proto
@@ -127,6 +127,10 @@
message EnableTracingResponse {
oneof state { bool disabled = 1; }
+
+ // If present and non-empty tracing was disabled because of an error.
+ // Introduced in Android S.
+ optional string error = 3;
}
// Arguments for rpc StartTracing().
diff --git a/src/base/BUILD.gn b/src/base/BUILD.gn
index 36d2865..26c4ba9 100644
--- a/src/base/BUILD.gn
+++ b/src/base/BUILD.gn
@@ -29,6 +29,7 @@
"logging.cc",
"metatrace.cc",
"paged_memory.cc",
+ "status.cc",
"string_splitter.cc",
"string_utils.cc",
"string_view.cc",
diff --git a/src/base/status.cc b/src/base/status.cc
new file mode 100644
index 0000000..30ccc47
--- /dev/null
+++ b/src/base/status.cc
@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "perfetto/base/status.h"
+
+#include <stdarg.h>
+
+namespace perfetto {
+namespace base {
+
+Status ErrStatus(const char* format, ...) {
+ char buffer[1024];
+ va_list ap;
+ va_start(ap, format);
+ vsnprintf(buffer, sizeof(buffer), format, ap);
+ va_end(ap);
+ Status status(buffer);
+ return status;
+}
+
+} // namespace base
+} // namespace perfetto
diff --git a/src/perfetto_cmd/perfetto_cmd.cc b/src/perfetto_cmd/perfetto_cmd.cc
index c0be487..e01778b 100644
--- a/src/perfetto_cmd/perfetto_cmd.cc
+++ b/src/perfetto_cmd/perfetto_cmd.cc
@@ -770,9 +770,12 @@
FinalizeTraceAndExit(); // Reached end of trace.
}
-void PerfettoCmd::OnTracingDisabled() {
+void PerfettoCmd::OnTracingDisabled(const std::string& error) {
LogUploadEvent(PerfettoStatsdAtom::kOnTracingDisabled);
+ if (!error.empty())
+ PERFETTO_ELOG("Service error: %s", error.c_str());
+
if (trace_config_->write_into_file()) {
// If write_into_file == true, at this point the passed file contains
// already all the packets.
diff --git a/src/perfetto_cmd/perfetto_cmd.h b/src/perfetto_cmd/perfetto_cmd.h
index 1d2aba4..93ac0e4 100644
--- a/src/perfetto_cmd/perfetto_cmd.h
+++ b/src/perfetto_cmd/perfetto_cmd.h
@@ -48,7 +48,7 @@
// perfetto::Consumer implementation.
void OnConnect() override;
void OnDisconnect() override;
- void OnTracingDisabled() override;
+ void OnTracingDisabled(const std::string& error) override;
void OnTraceData(std::vector<TracePacket>, bool has_more) override;
void OnDetach(bool) override;
void OnAttach(bool, const TraceConfig&) override;
diff --git a/src/tracing/consumer_api_deprecated/consumer_api_deprecated.cc b/src/tracing/consumer_api_deprecated/consumer_api_deprecated.cc
index 540e473..6d901d8 100644
--- a/src/tracing/consumer_api_deprecated/consumer_api_deprecated.cc
+++ b/src/tracing/consumer_api_deprecated/consumer_api_deprecated.cc
@@ -87,7 +87,7 @@
// perfetto::Consumer implementation.
void OnConnect() override;
void OnDisconnect() override;
- void OnTracingDisabled() override;
+ void OnTracingDisabled(const std::string& error) override;
void OnTraceData(std::vector<TracePacket>, bool has_more) override;
void OnDetach(bool) override;
void OnAttach(bool, const TraceConfig&) override;
@@ -198,9 +198,9 @@
consumer_endpoint_->StartTracing();
}
-void TracingSession::OnTracingDisabled() {
+void TracingSession::OnTracingDisabled(const std::string& error) {
PERFETTO_DCHECK_THREAD(thread_checker_);
- PERFETTO_DLOG("OnTracingDisabled");
+ PERFETTO_DLOG("OnTracingDisabled %s", error.c_str());
struct stat stat_buf {};
int res = fstat(buf_fd_.get(), &stat_buf);
diff --git a/src/tracing/core/tracing_service_impl.cc b/src/tracing/core/tracing_service_impl.cc
index d05f019..b1dd493 100644
--- a/src/tracing/core/tracing_service_impl.cc
+++ b/src/tracing/core/tracing_service_impl.cc
@@ -17,6 +17,7 @@
#include "src/tracing/core/tracing_service_impl.h"
#include "perfetto/base/build_config.h"
+#include "perfetto/base/status.h"
#include <errno.h>
#include <inttypes.h>
@@ -83,6 +84,10 @@
// from the IPC layer, but we should never assume that that the producer calls
// come in the right order or their arguments are sane / within bounds.
+// This is a macro because we want the call-site line number for the ELOG.
+#define PERFETTO_SVC_ERR(...) \
+ (PERFETTO_ELOG(__VA_ARGS__), ::perfetto::base::ErrStatus(__VA_ARGS__))
+
namespace perfetto {
namespace {
@@ -478,9 +483,9 @@
return true;
}
-bool TracingServiceImpl::EnableTracing(ConsumerEndpointImpl* consumer,
- const TraceConfig& cfg,
- base::ScopedFile fd) {
+base::Status TracingServiceImpl::EnableTracing(ConsumerEndpointImpl* consumer,
+ const TraceConfig& cfg,
+ base::ScopedFile fd) {
PERFETTO_DCHECK_THREAD(thread_checker_);
PERFETTO_DLOG("Enabling tracing for consumer %p",
reinterpret_cast<void*>(consumer));
@@ -491,19 +496,18 @@
TracingSession* tracing_session =
GetTracingSession(consumer->tracing_session_id_);
if (tracing_session) {
- PERFETTO_DLOG(
- "A Consumer is trying to EnableTracing() but another tracing session "
- "is already active (forgot a call to FreeBuffers() ?)");
- return false;
+ return PERFETTO_SVC_ERR(
+ "A Consumer is trying to EnableTracing() but another tracing "
+ "session is already active (forgot a call to FreeBuffers() ?)");
}
const uint32_t max_duration_ms = cfg.enable_extra_guardrails()
? kGuardrailsMaxTracingDurationMillis
: kMaxTracingDurationMillis;
if (cfg.duration_ms() > max_duration_ms) {
- PERFETTO_ELOG("Requested too long trace (%" PRIu32 "ms > %" PRIu32 " ms)",
- cfg.duration_ms(), max_duration_ms);
- return false;
+ return PERFETTO_SVC_ERR("Requested too long trace (%" PRIu32
+ "ms > %" PRIu32 " ms)",
+ cfg.duration_ms(), max_duration_ms);
}
const bool has_trigger_config = cfg.trigger_config().trigger_mode() !=
@@ -511,17 +515,15 @@
if (has_trigger_config && (cfg.trigger_config().trigger_timeout_ms() == 0 ||
cfg.trigger_config().trigger_timeout_ms() >
kGuardrailsMaxTracingDurationMillis)) {
- PERFETTO_ELOG(
+ return PERFETTO_SVC_ERR(
"Traces with START_TRACING triggers must provide a positive "
"trigger_timeout_ms < 7 days (received %" PRIu32 "ms)",
cfg.trigger_config().trigger_timeout_ms());
- return false;
}
if (has_trigger_config && cfg.duration_ms() != 0) {
- PERFETTO_ELOG(
+ return PERFETTO_SVC_ERR(
"duration_ms was set, this must not be set for traces with triggers.");
- return false;
}
if (cfg.trigger_config().trigger_mode() ==
@@ -532,56 +534,52 @@
// drain the events in ReadBuffers because we are waiting for STOP_TRACING,
// we can end up queueing up a lot of TracingServiceEvents and emitting them
// wildy out of order breaking windowed sorting in trace processor).
- PERFETTO_ELOG(
+ return PERFETTO_SVC_ERR(
"Specifying trigger mode STOP_TRACING and write_into_file together is "
"unsupported");
- return false;
}
std::unordered_set<std::string> triggers;
for (const auto& trigger : cfg.trigger_config().triggers()) {
if (!triggers.insert(trigger.name()).second) {
- PERFETTO_ELOG("Duplicate trigger name: %s", trigger.name().c_str());
- return false;
+ return PERFETTO_SVC_ERR("Duplicate trigger name: %s",
+ trigger.name().c_str());
}
}
if (cfg.enable_extra_guardrails()) {
if (cfg.deferred_start()) {
- PERFETTO_ELOG(
+ return PERFETTO_SVC_ERR(
"deferred_start=true is not supported in unsupervised traces");
- return false;
}
uint64_t buf_size_sum = 0;
for (const auto& buf : cfg.buffers()) {
if (buf.size_kb() % 4 != 0) {
- PERFETTO_ELOG("buffers.size_kb must be a multiple of 4, got %" PRIu32,
- buf.size_kb());
- return false;
+ return PERFETTO_SVC_ERR(
+ "buffers.size_kb must be a multiple of 4, got %" PRIu32,
+ buf.size_kb());
}
buf_size_sum += buf.size_kb();
}
if (buf_size_sum > kGuardrailsMaxTracingBufferSizeKb) {
- PERFETTO_ELOG("Requested too large trace buffer (%" PRIu64
- "kB > %" PRIu32 " kB)",
- buf_size_sum, kGuardrailsMaxTracingBufferSizeKb);
- return false;
+ return PERFETTO_SVC_ERR("Requested too large trace buffer (%" PRIu64
+ "kB > %" PRIu32 " kB)",
+ buf_size_sum, kGuardrailsMaxTracingBufferSizeKb);
}
}
if (cfg.buffers_size() > kMaxBuffersPerConsumer) {
- PERFETTO_ELOG("Too many buffers configured (%d)", cfg.buffers_size());
- return false;
+ return PERFETTO_SVC_ERR("Too many buffers configured (%d)",
+ cfg.buffers_size());
}
if (!cfg.unique_session_name().empty()) {
const std::string& name = cfg.unique_session_name();
for (auto& kv : tracing_sessions_) {
if (kv.second.config.unique_session_name() == name) {
- PERFETTO_ELOG(
+ return PERFETTO_SVC_ERR(
"A trace with this unique session name (%s) already exists",
name.c_str());
- return false;
}
}
}
@@ -606,11 +604,10 @@
if (previous_s == 0) {
previous_s = now_s;
} else {
- PERFETTO_ELOG(
+ return PERFETTO_SVC_ERR(
"A trace with unique session name \"%s\" began less than %" PRId64
"s ago (%" PRId64 "s)",
name.c_str(), kMinSecondsBetweenTracesGuardrail, now_s - previous_s);
- return false;
}
}
@@ -625,10 +622,9 @@
per_uid_limit = kMaxConcurrentTracingSessionsForStatsdUid;
}
if (sessions_for_uid >= per_uid_limit) {
- PERFETTO_ELOG(
+ return PERFETTO_SVC_ERR(
"Too many concurrent tracing sesions (%ld) for uid %d limit is %d",
sessions_for_uid, static_cast<int>(consumer->uid_), per_uid_limit);
- return false;
}
// TODO(primiano): This is a workaround to prevent that a producer gets stuck
@@ -636,9 +632,8 @@
// instances than free pages in the buffer. This is really a bug in
// trace_probes and the way it handles stalls in the shmem buffer.
if (tracing_sessions_.size() >= kMaxConcurrentTracingSessions) {
- PERFETTO_ELOG("Too many concurrent tracing sesions (%zu)",
- tracing_sessions_.size());
- return false;
+ return PERFETTO_SVC_ERR("Too many concurrent tracing sesions (%zu)",
+ tracing_sessions_.size());
}
const TracingSessionID tsid = ++last_tracing_session_id_;
@@ -648,17 +643,17 @@
if (cfg.write_into_file()) {
if (!fd ^ !cfg.output_path().empty()) {
- PERFETTO_ELOG(
+ tracing_sessions_.erase(tsid);
+ return PERFETTO_SVC_ERR(
"When write_into_file==true either a FD needs to be passed or "
"output_path must be populated (but not both)");
- tracing_sessions_.erase(tsid);
- return false;
}
if (!cfg.output_path().empty()) {
fd = CreateTraceFile(cfg.output_path());
if (!fd) {
tracing_sessions_.erase(tsid);
- return false;
+ return PERFETTO_SVC_ERR("Failed to create the trace file %s",
+ cfg.output_path().c_str());
}
}
tracing_session->write_into_file = std::move(fd);
@@ -719,7 +714,8 @@
buffers_.erase(global_id);
}
tracing_sessions_.erase(tsid);
- return false;
+ return PERFETTO_SVC_ERR(
+ "Failed to allocate tracing buffers: OOM or too many buffers");
}
consumer->tracing_session_id_ = tsid;
@@ -789,7 +785,7 @@
if (!cfg.deferred_start() && !has_start_trigger)
return StartTracing(tsid);
- return true;
+ return base::OkStatus();
}
void TracingServiceImpl::ChangeTraceConfig(ConsumerEndpointImpl* consumer,
@@ -910,18 +906,17 @@
}
}
-bool TracingServiceImpl::StartTracing(TracingSessionID tsid) {
+base::Status TracingServiceImpl::StartTracing(TracingSessionID tsid) {
PERFETTO_DCHECK_THREAD(thread_checker_);
TracingSession* tracing_session = GetTracingSession(tsid);
if (!tracing_session) {
- PERFETTO_DLOG("StartTracing() failed, invalid session ID %" PRIu64, tsid);
- return false;
+ return PERFETTO_SVC_ERR(
+ "StartTracing() failed, invalid session ID %" PRIu64, tsid);
}
if (tracing_session->state != TracingSession::CONFIGURED) {
- PERFETTO_DLOG("StartTracing() failed, invalid session state: %d",
- tracing_session->state);
- return false;
+ return PERFETTO_SVC_ERR("StartTracing() failed, invalid session state: %d",
+ tracing_session->state);
}
tracing_session->state = TracingSession::STARTED;
@@ -1012,7 +1007,7 @@
}
StartDataSourceInstance(producer, tracing_session, &data_source);
}
- return true;
+ return base::OkStatus();
}
void TracingServiceImpl::StartDataSourceInstance(
@@ -1353,7 +1348,7 @@
}
if (tracing_session->consumer_maybe_null)
- tracing_session->consumer_maybe_null->NotifyOnTracingDisabled();
+ tracing_session->consumer_maybe_null->NotifyOnTracingDisabled("");
}
void TracingServiceImpl::Flush(TracingSessionID tsid,
@@ -2860,12 +2855,13 @@
consumer_->OnDisconnect();
}
-void TracingServiceImpl::ConsumerEndpointImpl::NotifyOnTracingDisabled() {
+void TracingServiceImpl::ConsumerEndpointImpl::NotifyOnTracingDisabled(
+ const std::string& error) {
PERFETTO_DCHECK_THREAD(thread_checker_);
auto weak_this = GetWeakPtr();
- task_runner_->PostTask([weak_this] {
+ task_runner_->PostTask([weak_this, error /* deliberate copy */] {
if (weak_this)
- weak_this->consumer_->OnTracingDisabled();
+ weak_this->consumer_->OnTracingDisabled(error);
});
}
@@ -2873,8 +2869,9 @@
const TraceConfig& cfg,
base::ScopedFile fd) {
PERFETTO_DCHECK_THREAD(thread_checker_);
- if (!service_->EnableTracing(this, cfg, std::move(fd)))
- NotifyOnTracingDisabled();
+ auto status = service_->EnableTracing(this, cfg, std::move(fd));
+ if (!status.ok())
+ NotifyOnTracingDisabled(status.message());
}
void TracingServiceImpl::ConsumerEndpointImpl::ChangeTraceConfig(
diff --git a/src/tracing/core/tracing_service_impl.h b/src/tracing/core/tracing_service_impl.h
index b7f34d6..0b77eb6 100644
--- a/src/tracing/core/tracing_service_impl.h
+++ b/src/tracing/core/tracing_service_impl.h
@@ -27,6 +27,7 @@
#include <vector>
#include "perfetto/base/logging.h"
+#include "perfetto/base/status.h"
#include "perfetto/base/time.h"
#include "perfetto/ext/base/circular_queue.h"
#include "perfetto/ext/base/optional.h"
@@ -180,7 +181,7 @@
uid_t uid);
~ConsumerEndpointImpl() override;
- void NotifyOnTracingDisabled();
+ void NotifyOnTracingDisabled(const std::string& error);
base::WeakPtr<ConsumerEndpointImpl> GetWeakPtr();
// TracingService::ConsumerEndpoint implementation.
@@ -259,12 +260,12 @@
bool DetachConsumer(ConsumerEndpointImpl*, const std::string& key);
bool AttachConsumer(ConsumerEndpointImpl*, const std::string& key);
void DisconnectConsumer(ConsumerEndpointImpl*);
- bool EnableTracing(ConsumerEndpointImpl*,
- const TraceConfig&,
- base::ScopedFile);
+ base::Status EnableTracing(ConsumerEndpointImpl*,
+ const TraceConfig&,
+ base::ScopedFile);
void ChangeTraceConfig(ConsumerEndpointImpl*, const TraceConfig&);
- bool StartTracing(TracingSessionID);
+ base::Status StartTracing(TracingSessionID);
void DisableTracing(TracingSessionID, bool disable_immediately = false);
void Flush(TracingSessionID tsid,
uint32_t timeout_ms,
diff --git a/src/tracing/core/tracing_service_impl_unittest.cc b/src/tracing/core/tracing_service_impl_unittest.cc
index a426256..d6c315f 100644
--- a/src/tracing/core/tracing_service_impl_unittest.cc
+++ b/src/tracing/core/tracing_service_impl_unittest.cc
@@ -62,6 +62,7 @@
using ::testing::Property;
using ::testing::StrictMock;
using ::testing::StringMatchResultListener;
+using ::testing::StrNe;
namespace perfetto {
@@ -706,16 +707,18 @@
auto checkpoint_name = "on_tracing_disabled_consumer_1_and_2";
auto on_tracing_disabled = task_runner.CreateCheckpoint(checkpoint_name);
std::atomic<size_t> counter(0);
- EXPECT_CALL(*consumer_1, OnTracingDisabled()).WillOnce(Invoke([&]() {
- if (++counter == 2u) {
- on_tracing_disabled();
- }
- }));
- EXPECT_CALL(*consumer_2, OnTracingDisabled()).WillOnce(Invoke([&]() {
- if (++counter == 2u) {
- on_tracing_disabled();
- }
- }));
+ EXPECT_CALL(*consumer_1, OnTracingDisabled(_))
+ .WillOnce(InvokeWithoutArgs([&]() {
+ if (++counter == 2u) {
+ on_tracing_disabled();
+ }
+ }));
+ EXPECT_CALL(*consumer_2, OnTracingDisabled(_))
+ .WillOnce(InvokeWithoutArgs([&]() {
+ if (++counter == 2u) {
+ on_tracing_disabled();
+ }
+ }));
EXPECT_CALL(*producer, StopDataSource(id1));
EXPECT_CALL(*producer, StopDataSource(id2));
@@ -3333,7 +3336,8 @@
for (int i = 0; i <= kUids; i++) {
auto* consumer = start_new_session(/*uid=*/static_cast<uid_t>(i) % kUids);
auto on_fail = task_runner.CreateCheckpoint("uid_" + std::to_string(i));
- EXPECT_CALL(*consumer, OnTracingDisabled()).WillOnce(Invoke(on_fail));
+ EXPECT_CALL(*consumer, OnTracingDisabled(StrNe("")))
+ .WillOnce(InvokeWithoutArgs(on_fail));
}
// Wait for failure (only after both attempts).
diff --git a/src/tracing/internal/tracing_muxer_impl.cc b/src/tracing/internal/tracing_muxer_impl.cc
index d459347..e3a0522 100644
--- a/src/tracing/internal/tracing_muxer_impl.cc
+++ b/src/tracing/internal/tracing_muxer_impl.cc
@@ -279,9 +279,11 @@
service_.reset();
}
-void TracingMuxerImpl::ConsumerImpl::OnTracingDisabled() {
+void TracingMuxerImpl::ConsumerImpl::OnTracingDisabled(
+ const std::string& error) {
PERFETTO_DCHECK_THREAD(thread_checker_);
PERFETTO_DCHECK(!stopped_);
+ PERFETTO_ELOG("Service error: %s", error.c_str());
stopped_ = true;
// If we're still waiting for the start event, fire it now. This may happen if
// there are no active data sources in the session.
diff --git a/src/tracing/internal/tracing_muxer_impl.h b/src/tracing/internal/tracing_muxer_impl.h
index cf85101..a92cb5d 100644
--- a/src/tracing/internal/tracing_muxer_impl.h
+++ b/src/tracing/internal/tracing_muxer_impl.h
@@ -228,7 +228,7 @@
// perfetto::Consumer implementation.
void OnConnect() override;
void OnDisconnect() override;
- void OnTracingDisabled() override;
+ void OnTracingDisabled(const std::string& error) override;
void OnTraceData(std::vector<TracePacket>, bool has_more) override;
void OnDetach(bool success) override;
void OnAttach(bool success, const TraceConfig&) override;
diff --git a/src/tracing/ipc/consumer/consumer_ipc_client_impl.cc b/src/tracing/ipc/consumer/consumer_ipc_client_impl.cc
index 72b5de0..e9c1708 100644
--- a/src/tracing/ipc/consumer/consumer_ipc_client_impl.cc
+++ b/src/tracing/ipc/consumer/consumer_ipc_client_impl.cc
@@ -182,7 +182,7 @@
void ConsumerIPCClientImpl::OnEnableTracingResponse(
ipc::AsyncResult<protos::gen::EnableTracingResponse> response) {
if (!response || response->disabled())
- consumer_->OnTracingDisabled();
+ consumer_->OnTracingDisabled(response->error());
}
void ConsumerIPCClientImpl::FreeBuffers() {
diff --git a/src/tracing/ipc/service/consumer_ipc_service.cc b/src/tracing/ipc/service/consumer_ipc_service.cc
index 073bc74..5a06597 100644
--- a/src/tracing/ipc/service/consumer_ipc_service.cc
+++ b/src/tracing/ipc/service/consumer_ipc_service.cc
@@ -324,11 +324,14 @@
// |service_endpoint| (in the RemoteConsumer dtor).
void ConsumerIPCService::RemoteConsumer::OnDisconnect() {}
-void ConsumerIPCService::RemoteConsumer::OnTracingDisabled() {
+void ConsumerIPCService::RemoteConsumer::OnTracingDisabled(
+ const std::string& error) {
if (enable_tracing_response.IsBound()) {
auto result =
ipc::AsyncResult<protos::gen::EnableTracingResponse>::Create();
result->set_disabled(true);
+ if (!error.empty())
+ result->set_error(error);
enable_tracing_response.Resolve(std::move(result));
}
}
diff --git a/src/tracing/ipc/service/consumer_ipc_service.h b/src/tracing/ipc/service/consumer_ipc_service.h
index d513371..5693a6a 100644
--- a/src/tracing/ipc/service/consumer_ipc_service.h
+++ b/src/tracing/ipc/service/consumer_ipc_service.h
@@ -84,7 +84,7 @@
// no connection here, these methods are posted straight away.
void OnConnect() override;
void OnDisconnect() override;
- void OnTracingDisabled() override;
+ void OnTracingDisabled(const std::string& error) override;
void OnTraceData(std::vector<TracePacket>, bool has_more) override;
void OnDetach(bool) override;
void OnAttach(bool, const TraceConfig&) override;
diff --git a/src/tracing/test/mock_consumer.cc b/src/tracing/test/mock_consumer.cc
index e7cf3b1..2d7dc4c 100644
--- a/src/tracing/test/mock_consumer.cc
+++ b/src/tracing/test/mock_consumer.cc
@@ -73,7 +73,8 @@
static int i = 0;
auto checkpoint_name = "on_tracing_disabled_consumer_" + std::to_string(i++);
auto on_tracing_disabled = task_runner_->CreateCheckpoint(checkpoint_name);
- EXPECT_CALL(*this, OnTracingDisabled()).WillOnce(Invoke(on_tracing_disabled));
+ EXPECT_CALL(*this, OnTracingDisabled(_))
+ .WillOnce(testing::InvokeWithoutArgs(on_tracing_disabled));
task_runner_->RunUntilCheckpoint(checkpoint_name, timeout_ms);
}
diff --git a/src/tracing/test/mock_consumer.h b/src/tracing/test/mock_consumer.h
index a1f050e..9ba12b4 100644
--- a/src/tracing/test/mock_consumer.h
+++ b/src/tracing/test/mock_consumer.h
@@ -69,7 +69,7 @@
// Consumer implementation.
MOCK_METHOD0(OnConnect, void());
MOCK_METHOD0(OnDisconnect, void());
- MOCK_METHOD0(OnTracingDisabled, void());
+ MOCK_METHOD1(OnTracingDisabled, void(const std::string& /*error*/));
MOCK_METHOD2(OnTraceData,
void(std::vector<TracePacket>* /*packets*/, bool /*has_more*/));
MOCK_METHOD1(OnDetach, void(bool));
diff --git a/src/tracing/test/tracing_integration_test.cc b/src/tracing/test/tracing_integration_test.cc
index eb96e40..8674439 100644
--- a/src/tracing/test/tracing_integration_test.cc
+++ b/src/tracing/test/tracing_integration_test.cc
@@ -80,7 +80,7 @@
// Producer implementation.
MOCK_METHOD0(OnConnect, void());
MOCK_METHOD0(OnDisconnect, void());
- MOCK_METHOD0(OnTracingDisabled, void());
+ MOCK_METHOD1(OnTracingDisabled, void(const std::string& /*error*/));
MOCK_METHOD2(OnTracePackets, void(std::vector<TracePacket>*, bool));
MOCK_METHOD1(OnDetach, void(bool));
MOCK_METHOD2(OnAttach, void(bool, const TraceConfig&));
@@ -338,8 +338,8 @@
auto on_tracing_disabled =
task_runner_->CreateCheckpoint("on_tracing_disabled");
EXPECT_CALL(producer_, StopDataSource(_));
- EXPECT_CALL(consumer_, OnTracingDisabled())
- .WillOnce(Invoke(on_tracing_disabled));
+ EXPECT_CALL(consumer_, OnTracingDisabled(_))
+ .WillOnce(InvokeWithoutArgs(on_tracing_disabled));
task_runner_->RunUntilCheckpoint("on_tracing_disabled");
}
@@ -390,8 +390,8 @@
auto on_tracing_disabled =
task_runner_->CreateCheckpoint("on_tracing_disabled");
EXPECT_CALL(producer_, StopDataSource(_));
- EXPECT_CALL(consumer_, OnTracingDisabled())
- .WillOnce(Invoke(on_tracing_disabled));
+ EXPECT_CALL(consumer_, OnTracingDisabled(_))
+ .WillOnce(InvokeWithoutArgs(on_tracing_disabled));
task_runner_->RunUntilCheckpoint("on_tracing_disabled");
// Check that |tmp_file| contains a valid trace.proto message.
@@ -517,8 +517,8 @@
auto on_tracing_disabled =
task_runner_->CreateCheckpoint("on_tracing_disabled");
EXPECT_CALL(producer_, StopDataSource(_));
- EXPECT_CALL(consumer_, OnTracingDisabled())
- .WillOnce(Invoke(on_tracing_disabled));
+ EXPECT_CALL(consumer_, OnTracingDisabled(_))
+ .WillOnce(InvokeWithoutArgs(on_tracing_disabled));
task_runner_->RunUntilCheckpoint("on_tracing_disabled");
}
diff --git a/test/test_helper.cc b/test/test_helper.cc
index 55c08bc..2ffdd8c 100644
--- a/test/test_helper.cc
+++ b/test/test_helper.cc
@@ -54,7 +54,7 @@
PERFETTO_FATAL("Consumer unexpectedly disconnected from the service");
}
-void TestHelper::OnTracingDisabled() {
+void TestHelper::OnTracingDisabled(const std::string& /*error*/) {
std::move(on_stop_tracing_callback_)();
}
diff --git a/test/test_helper.h b/test/test_helper.h
index 095d133..5989a67 100644
--- a/test/test_helper.h
+++ b/test/test_helper.h
@@ -172,7 +172,7 @@
// Consumer implementation.
void OnConnect() override;
void OnDisconnect() override;
- void OnTracingDisabled() override;
+ void OnTracingDisabled(const std::string& error) override;
void OnTraceData(std::vector<TracePacket> packets, bool has_more) override;
void OnDetach(bool) override;
void OnAttach(bool, const TraceConfig&) override;