Trace the stats report as JSON instead of each stat separately.

webrtc/pc/rtcstats_integrationtest.cc

 /*
  *  Copyright 2016 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include <set>
 #include <vector>
 
 #include "webrtc/api/audio_codecs/builtin_audio_decoder_factory.h"
 #include "webrtc/api/audio_codecs/builtin_audio_encoder_factory.h"
 #include "webrtc/api/datachannelinterface.h"
 #include "webrtc/api/peerconnectioninterface.h"
 #include "webrtc/api/stats/rtcstats_objects.h"
 #include "webrtc/api/stats/rtcstatsreport.h"
 #include "webrtc/pc/test/peerconnectiontestwrapper.h"
 #include "webrtc/pc/test/rtcstatsobtainer.h"
 #include "webrtc/rtc_base/checks.h"
 #include "webrtc/rtc_base/event_tracer.h"
 #include "webrtc/rtc_base/gunit.h"
 #include "webrtc/rtc_base/refcountedobject.h"
 #include "webrtc/rtc_base/scoped_ref_ptr.h"
+#include "webrtc/rtc_base/trace_event.h"
 #include "webrtc/rtc_base/virtualsocketserver.h"
 
 namespace webrtc {
 
 namespace {
 
 const int64_t kGetStatsTimeoutMs = 10000;
 
 const unsigned char* GetCategoryEnabledHandler(const char* name) {
-  return reinterpret_cast<const unsigned char*>("webrtc_stats");
+  if (strcmp("webrtc_stats", name) != 0) {
+    return reinterpret_cast<const unsigned char*>("\0");

[hbos, 2017/08/14 13:15:39]

All string literals are null-terminated, should this be ""? (['\0'] in memory as
opposed to ['\0','\0'])

[ehmaldonado_webrtc, 2017/08/14 13:43:24]

Right, thanks.

+  }
+  return reinterpret_cast<const unsigned char*>(name);
 }
 
-void AddTraceEventHandler(char phase,
-                          const unsigned char* category_enabled,
-                          const char* name,
-                          unsigned long long id,
-                          int num_args,
-                          const char** arg_names,
-                          const unsigned char* arg_types,
-                          const unsigned long long* arg_values,
-                          unsigned char flags) {
-  // Do nothing
-}
+class RTCStatsTracedReport {
+ public:
+  static RTCStatsTracedReport* Get() {
+    static RTCStatsTracedReport* traced_report = nullptr;
+    if (!traced_report)
+      traced_report = new RTCStatsTracedReport();
+    return traced_report;
+  }
+
+  static void AddTraceEventHandler(char phase,
+                                   const unsigned char* category_enabled,
+                                   const char* name,
+                                   unsigned long long id,
+                                   int num_args,
+                                   const char** arg_names,
+                                   const unsigned char* arg_types,
+                                   const unsigned long long* arg_values,
+                                   unsigned char flags) {
+    RTCStatsTracedReport* traced_report = RTCStatsTracedReport::Get();
+    EXPECT_FALSE(traced_report->report_received_);
+    EXPECT_STREQ("webrtc_stats",
+                 reinterpret_cast<const char*>(category_enabled));
+    EXPECT_STREQ("webrtc_stats", name);
+    EXPECT_EQ(1, num_args);
+    EXPECT_STREQ("report", arg_names[0]);
+    EXPECT_EQ(TRACE_VALUE_TYPE_COPY_STRING, arg_types[0]);
+
+    traced_report->report_received_ = true;
+    traced_report->traced_report_ =
+        reinterpret_cast<const char*>(arg_values[0]);
+  }
+
+  void Reset() { report_received_ = false; }
+
+  const std::string& GetTracedReport() const {
+    EXPECT_TRUE(report_received_);
+    return traced_report_;
+  }
+
+ private:
+  std::string traced_report_;
+  bool report_received_;

[hbos, 2017/08/14 13:15:39]

Add a constructor that default-initializes this to false,
"RTCStatsTracedReport() : report_received_(false) {}" if we keep
report_received_, though I think it is sufficient to use traced_report_ = ""
(which is default-constructed automatically, no need for a constructor).

[ehmaldonado_webrtc, 2017/08/14 13:43:24]

Done.

+};
 
 class RTCStatsIntegrationTest : public testing::Test {
  public:
   RTCStatsIntegrationTest()
       : network_thread_(new rtc::Thread(&virtual_socket_server_)),
         worker_thread_(rtc::Thread::Create()) {
-    SetupEventTracer(&GetCategoryEnabledHandler, &AddTraceEventHandler);
+    RTCStatsTracedReport::Get()->Reset();
+    SetupEventTracer(&GetCategoryEnabledHandler,
+                     &RTCStatsTracedReport::AddTraceEventHandler);

[hbos, 2017/08/14 13:15:39]

Because of the usage of a global variable, it is wrong to call SetupEventTracer
without safely doing Reset() in case this isn't the first time the tracer is set
up. I suggest doing these two steps in a static member function of
RTCStatsTracedReport instead.

I view both Reset() and Get() as implementation details, the only "public" thing
should be initialization and GetTracedReport(), how about:

class RTCStatsTracedReport {
 public:
  static void SetupEventTracer();
  static std::string GetLastTrace() {
    DCHECK(traced_report_);
    return traced_report_->last_trace_;
  }

 private:
  static void AddTraceEventHandler(...) {
    DCHECK(traced_report_);
    ...
  }

  static RTCStatsTracedReport* traced_report_ = nullptr;

  std::string last_trace_;
};

SetupEventTracer() could simply reset last_trace_ to "", no need for a separate
bool or Reset method.

[ehmaldonado_webrtc, 2017/08/14 13:43:24]

Done.

 
     RTC_CHECK(network_thread_->Start());
     RTC_CHECK(worker_thread_->Start());
 
     caller_ = new rtc::RefCountedObject<PeerConnectionTestWrapper>(
         "caller", network_thread_.get(), worker_thread_.get());
     callee_ = new rtc::RefCountedObject<PeerConnectionTestWrapper>(
         "callee", network_thread_.get(), worker_thread_.get());
   }
 
@@ skipping 576 matching lines @@
  private:
   rtc::scoped_refptr<const RTCStatsReport> report_;
 };
 
 #ifdef HAVE_SCTP
 TEST_F(RTCStatsIntegrationTest, GetStatsFromCaller) {
   StartCall();
 
   rtc::scoped_refptr<const RTCStatsReport> report = GetStatsFromCaller();
   RTCStatsReportVerifier(report.get()).VerifyReport();
+  EXPECT_EQ(report->ToJson(), RTCStatsTracedReport::Get()->GetTracedReport());
 }
 
 TEST_F(RTCStatsIntegrationTest, GetStatsFromCallee) {
   StartCall();
 
   rtc::scoped_refptr<const RTCStatsReport> report = GetStatsFromCallee();
   RTCStatsReportVerifier(report.get()).VerifyReport();
+  EXPECT_EQ(report->ToJson(), RTCStatsTracedReport::Get()->GetTracedReport());
 }
 
 TEST_F(RTCStatsIntegrationTest, GetsStatsWhileDestroyingPeerConnections) {
   StartCall();
 
   rtc::scoped_refptr<RTCStatsObtainer> stats_obtainer =
       RTCStatsObtainer::Create();
   caller_->pc()->GetStats(stats_obtainer);
   // This will destroy the peer connection.
   caller_ = nullptr;
   // Any pending stats requests should have completed in the act of destroying
   // the peer connection.
   EXPECT_TRUE(stats_obtainer->report());
+  EXPECT_EQ(stats_obtainer->report()->ToJson(),
+            RTCStatsTracedReport::Get()->GetTracedReport());
 }
 #endif  // HAVE_SCTP
 
 }  // namespace
 
 }  // namespace webrtc