Skip to content

Commit

Permalink
tracing: Add a system tracing integration test
Browse files Browse the repository at this point in the history 
The new test verifies that system tracing output on CrOS, Cast and Win
actually ends up in a trace - in preparation for porting the respective
tracing agents to the new perfetto service.

Bug: 900603
Change-Id: I8383a907d2aa843cb2f67f5300e82c7684f116fc
Reviewed-on: https://chromium-review.googlesource.com/c/1309835
Commit-Queue: Eric Seckler <eseckler@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Reviewed-by: Michael Spang <spang@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604546}
  • Loading branch information
@betasheet
betasheet authored and Commit Bot committed Nov 1, 2018
1 parent 3ca0b0f commit 4d2bcd3
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 69 deletions.
1 change: 1 addition & 0 deletions chromecast/tracing/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ cast_source_set("system_tracer") {
public_deps = [
":system_tracing_common",
"//base",
"//chromecast:chromecast_buildflags",
]
}

Expand Down
112 changes: 91 additions & 21 deletions chromecast/tracing/system_tracer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "base/posix/eintr_wrapper.h"
#include "base/posix/unix_domain_socket.h"
#include "base/trace_event/trace_config.h"
#include "chromecast/chromecast_buildflags.h"
#include "chromecast/tracing/system_tracing_common.h"

namespace chromecast {
Expand Down Expand Up @@ -43,16 +44,57 @@ base::ScopedFD CreateClientSocket() {
return socket_fd;
}

} // namespace

SystemTracer::SystemTracer() : buffer_(new char[kBufferSize]) {}

SystemTracer::~SystemTracer() {
Cleanup();
}

void SystemTracer::StartTracing(base::StringPiece categories,
StartTracingCallback callback) {
class SystemTracerImpl : public SystemTracer {
public:
SystemTracerImpl() : buffer_(new char[kBufferSize]) {}
~SystemTracerImpl() override { Cleanup(); }

void StartTracing(base::StringPiece categories,
StartTracingCallback callback) override;

void StopTracing(const StopTracingCallback& callback) override;

private:
enum class State {
INITIAL, // Not yet started.
STARTING, // Sent start message, waiting for ack.
TRACING, // Tracing, not yet requested stop.
READING, // Trace stopped, reading output.
FINISHED, // All done.
};

void ReceiveStartAckAndTracePipe();
void ReceiveTraceData();
void FailStartTracing();
void FailStopTracing();
void SendPartialTraceData();
void FinishTracing();
void Cleanup();

// Current state of tracing attempt.
State state_ = State::INITIAL;

// Unix socket connection to tracing daemon.
base::ScopedFD connection_fd_;
std::unique_ptr<base::FileDescriptorWatcher::Controller> connection_watcher_;

// Pipe for trace data.
base::ScopedFD trace_pipe_fd_;
std::unique_ptr<base::FileDescriptorWatcher::Controller> trace_pipe_watcher_;

// Read buffer (of size kBufferSize).
std::unique_ptr<char[]> buffer_;

// Callbacks for StartTracing() and StopTracing().
StartTracingCallback start_tracing_callback_;
StopTracingCallback stop_tracing_callback_;

// Trace data.
std::string trace_data_;
};

void SystemTracerImpl::StartTracing(base::StringPiece categories,
StartTracingCallback callback) {
start_tracing_callback_ = std::move(callback);
if (state_ != State::INITIAL) {
FailStartTracing();
Expand Down Expand Up @@ -80,12 +122,12 @@ void SystemTracer::StartTracing(base::StringPiece categories,

connection_watcher_ = base::FileDescriptorWatcher::WatchReadable(
connection_fd_.get(),
base::BindRepeating(&SystemTracer::ReceiveStartAckAndTracePipe,
base::BindRepeating(&SystemTracerImpl::ReceiveStartAckAndTracePipe,
base::Unretained(this)));
state_ = State::STARTING;
}

void SystemTracer::StopTracing(const StopTracingCallback& callback) {
void SystemTracerImpl::StopTracing(const StopTracingCallback& callback) {
stop_tracing_callback_ = callback;
if (state_ != State::TRACING) {
FailStopTracing();
Expand All @@ -102,12 +144,13 @@ void SystemTracer::StopTracing(const StopTracingCallback& callback) {
}

trace_pipe_watcher_ = base::FileDescriptorWatcher::WatchReadable(
trace_pipe_fd_.get(), base::BindRepeating(&SystemTracer::ReceiveTraceData,
base::Unretained(this)));
trace_pipe_fd_.get(),
base::BindRepeating(&SystemTracerImpl::ReceiveTraceData,
base::Unretained(this)));
state_ = State::READING;
}

void SystemTracer::ReceiveStartAckAndTracePipe() {
void SystemTracerImpl::ReceiveStartAckAndTracePipe() {
DCHECK_EQ(state_, State::STARTING);

std::vector<base::ScopedFD> fds;
Expand Down Expand Up @@ -135,7 +178,7 @@ void SystemTracer::ReceiveStartAckAndTracePipe() {
std::move(start_tracing_callback_).Run(Status::OK);
}

void SystemTracer::ReceiveTraceData() {
void SystemTracerImpl::ReceiveTraceData() {
DCHECK_EQ(state_, State::READING);

for (;;) {
Expand Down Expand Up @@ -164,29 +207,29 @@ void SystemTracer::ReceiveTraceData() {
}
}

void SystemTracer::FailStartTracing() {
void SystemTracerImpl::FailStartTracing() {
std::move(start_tracing_callback_).Run(Status::FAIL);
Cleanup();
}

void SystemTracer::FailStopTracing() {
void SystemTracerImpl::FailStopTracing() {
stop_tracing_callback_.Run(Status::FAIL, "");
Cleanup();
}

void SystemTracer::SendPartialTraceData() {
void SystemTracerImpl::SendPartialTraceData() {
DCHECK_EQ(state_, State::READING);
stop_tracing_callback_.Run(Status::KEEP_GOING, std::move(trace_data_));
trace_data_ = "";
}

void SystemTracer::FinishTracing() {
void SystemTracerImpl::FinishTracing() {
DCHECK_EQ(state_, State::READING);
stop_tracing_callback_.Run(Status::OK, std::move(trace_data_));
Cleanup();
}

void SystemTracer::Cleanup() {
void SystemTracerImpl::Cleanup() {
connection_watcher_.reset();
connection_fd_.reset();
trace_pipe_watcher_.reset();
Expand All @@ -196,4 +239,31 @@ void SystemTracer::Cleanup() {
state_ = State::FINISHED;
}

class FakeSystemTracer : public SystemTracer {
public:
FakeSystemTracer() = default;
~FakeSystemTracer() override = default;

void StartTracing(base::StringPiece categories,
StartTracingCallback callback) override {
std::move(callback).Run(Status::OK);
}

void StopTracing(const StopTracingCallback& callback) override {
std::string trace_data = "# tracer: nop\n";
std::move(callback).Run(Status::OK, std::move(trace_data));
}
};

} // namespace

// static
std::unique_ptr<SystemTracer> SystemTracer::Create() {
#if BUILDFLAG(IS_CAST_DESKTOP_BUILD)
return std::make_unique<FakeSystemTracer>();
#else
return std::make_unique<SystemTracerImpl>();
#endif
}

} // namespace chromecast
51 changes: 9 additions & 42 deletions chromecast/tracing/system_tracer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ namespace chromecast {

class SystemTracer {
public:
SystemTracer();
~SystemTracer();
static std::unique_ptr<SystemTracer> Create();

virtual ~SystemTracer() = default;

enum class Status {
OK,
Expand All @@ -32,54 +33,20 @@ class SystemTracer {
base::RepeatingCallback<void(Status status, std::string trace_data)>;

// Start system tracing for categories in |categories| (comma separated).
void StartTracing(base::StringPiece categories,
StartTracingCallback callback);
virtual void StartTracing(base::StringPiece categories,
StartTracingCallback callback) = 0;

// Stop system tracing.
//
// This will call |callback| on the current thread with the trace data. If
// |status| is Status::KEEP_GOING, another call will be made with additional
// data.
void StopTracing(const StopTracingCallback& callback);

private:
enum class State {
INITIAL, // Not yet started.
STARTING, // Sent start message, waiting for ack.
TRACING, // Tracing, not yet requested stop.
READING, // Trace stopped, reading output.
FINISHED, // All done.
};

void ReceiveStartAckAndTracePipe();
void ReceiveTraceData();
void FailStartTracing();
void FailStopTracing();
void SendPartialTraceData();
void FinishTracing();
void Cleanup();

// Current state of tracing attempt.
State state_ = State::INITIAL;
virtual void StopTracing(const StopTracingCallback& callback) = 0;

// Unix socket connection to tracing daemon.
base::ScopedFD connection_fd_;
std::unique_ptr<base::FileDescriptorWatcher::Controller> connection_watcher_;

// Pipe for trace data.
base::ScopedFD trace_pipe_fd_;
std::unique_ptr<base::FileDescriptorWatcher::Controller> trace_pipe_watcher_;

// Read buffer (of size kBufferSize).
std::unique_ptr<char[]> buffer_;

// Callbacks for StartTracing() and StopTracing().
StartTracingCallback start_tracing_callback_;
StopTracingCallback stop_tracing_callback_;

// Trace data.
std::string trace_data_;
protected:
SystemTracer() = default;

private:
DISALLOW_COPY_AND_ASSIGN(SystemTracer);
};

Expand Down
4 changes: 2 additions & 2 deletions chromeos/dbus/fake_debug_daemon_client.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ void FakeDebugDaemonClient::StartAgentTracing(

void FakeDebugDaemonClient::StopAgentTracing(
StopAgentTracingCallback callback) {
std::string no_data;
std::string trace_data = "# tracer: nop\n";
std::move(callback).Run(GetTracingAgentName(), GetTraceEventLabel(),
base::RefCountedString::TakeString(&no_data));
base::RefCountedString::TakeString(&trace_data));
}

void FakeDebugDaemonClient::SetStopAgentTracingTaskRunner(
Expand Down
2 changes: 1 addition & 1 deletion content/browser/tracing/cast_tracing_agent.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ void CastTracingAgent::GetCategories(Agent::GetCategoriesCallback callback) {
void CastTracingAgent::StartTracingOnIO(
scoped_refptr<base::TaskRunner> reply_task_runner,
const std::string& categories) {
system_tracer_ = std::make_unique<chromecast::SystemTracer>();
system_tracer_ = chromecast::SystemTracer::Create();

system_tracer_->StartTracing(
categories, base::BindOnce(&CastTracingAgent::FinishStartOnIO,
Expand Down
29 changes: 26 additions & 3 deletions content/browser/tracing/tracing_controller_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "content/test/test_content_browser_client.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/tracing/public/cpp/trace_event_agent.h"
#include "services/tracing/public/cpp/tracing_features.h"

using base::trace_event::RECORD_CONTINUOUSLY;
using base::trace_event::RECORD_UNTIL_FULL;
Expand Down Expand Up @@ -198,7 +199,7 @@ class TracingControllerTest : public ContentBrowserTest {
return last_data_;
}

void TestStartAndStopTracingString() {
void TestStartAndStopTracingString(bool enable_systrace = false) {
Navigate(shell());

TracingController* controller = TracingController::GetInstance();
Expand All @@ -208,8 +209,10 @@ class TracingControllerTest : public ContentBrowserTest {
TracingController::StartTracingDoneCallback callback =
base::BindOnce(&TracingControllerTest::StartTracingDoneCallbackTest,
base::Unretained(this), run_loop.QuitClosure());
bool result =
controller->StartTracing(TraceConfig(), std::move(callback));
TraceConfig config;
if (enable_systrace)
config.EnableSystrace();
bool result = controller->StartTracing(config, std::move(callback));
ASSERT_TRUE(result);
run_loop.Run();
EXPECT_EQ(enable_recording_done_callback_count(), 1);
Expand Down Expand Up @@ -537,4 +540,24 @@ IN_PROC_BROWSER_TEST_F(TracingControllerTest, MAYBE_DoubleStopTracing) {
run_loop.Run();
}

// TODO(crbug.com/871770): Disabled for failing on ASAN.
#if defined(ADDRESS_SANITIZER)
#define MAYBE_SystemTraceEvents DISABLED_SystemTraceEvents
// Only CrOS, Cast, and Windows support system tracing.
#elif defined(OS_CHROMEOS) || (defined(IS_CHROMECAST) && defined(OS_LINUX)) || \
defined(OS_WIN)
#define MAYBE_SystemTraceEvents SystemTraceEvents
#else
#define MAYBE_SystemTraceEvents DISABLED_SystemTraceEvents
#endif
IN_PROC_BROWSER_TEST_F(TracingControllerTest, MAYBE_SystemTraceEvents) {
// TODO(crbug.com/900603): Enable this test for perfetto once passing.
if (tracing::TracingUsesPerfettoBackend())
return;

TestStartAndStopTracingString(true /* enable_systrace */);
EXPECT_TRUE(last_data().size() > 0);
EXPECT_TRUE(last_data().find("systemTraceEvents") != std::string::npos);
}

} // namespace content

0 comments on commit 4d2bcd3

Please sign in to comment.