Fix datachannel stats id and timestamp.

Makes the id now be "datachannel_#####" where '####' is the id number for the datachannel.

Adds a timestamp to the data channel reports.

Implements unit tests to verify that the timestamp is set correctly.

BUG=1805
R=juberti@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/33119004

Cr-Commit-Position: refs/heads/master@{#8236}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8236 4adac7df-926f-26a2-2b94-8c16560cd09d

talk/app/webrtc/statscollector.cc

@@ -56,10 +56,6 @@ const char* STATSREPORT_ADAPTER_TYPE_WIFI = "wlan";
 const char* STATSREPORT_ADAPTER_TYPE_WWAN = "wwan";
 const char* STATSREPORT_ADAPTER_TYPE_VPN = "vpn";
 
-double GetTimeNow() {
-  return rtc::Timing::WallTimeNow() * rtc::kNumMillisecsPerSec;
-}
-
 bool GetTransportIdFromProxy(const cricket::ProxyTransportMap& map,
                              const std::string& proxy,
                              std::string* transport) {
@@ -379,6 +375,10 @@ StatsCollector::~StatsCollector() {
   ASSERT(session_->signaling_thread()->IsCurrent());
 }
 
+double StatsCollector::GetTimeNow() {
+  return rtc::Timing::WallTimeNow() * rtc::kNumMillisecsPerSec;
+}
+
 // Adds a MediaStream with tracks that can be used as a |selector| in a call
 // to GetStats.
 void StatsCollector::AddStream(MediaStreamInterface* stream) {
@@ -830,10 +830,10 @@ void StatsCollector::ExtractDataInfo() {
 
   for (const auto& dc :
            session_->mediastream_signaling()->sctp_data_channels()) {
-    rtc::scoped_ptr<StatsReport::Id> id(
-        StatsReport::NewTypedId(StatsReport::kStatsReportTypeDataChannel,
-                                dc->label()));
+    rtc::scoped_ptr<StatsReport::Id> id(StatsReport::NewTypedIntId(
+        StatsReport::kStatsReportTypeDataChannel, dc->id()));
     StatsReport* report = reports_.ReplaceOrAddNew(id.Pass());
+    report->set_timestamp(stats_gathering_started_);
     report->AddValue(StatsReport::kStatsValueNameLabel, dc->label());
     report->AddValue(StatsReport::kStatsValueNameDataChannelId, dc->id());
     report->AddValue(StatsReport::kStatsValueNameProtocol, dc->protocol());

talk/app/webrtc/statscollector.h

@@ -97,6 +97,9 @@ class StatsCollector {
  private:
   friend class StatsCollectorTest;
 
+  // Overridden in unit tests to fake timing.
+  virtual double GetTimeNow();
+
   bool CopySelectedReports(const std::string& selector, StatsReports* reports);
 
   // Helper method for AddCertificateReports.

talk/app/webrtc/statscollector_unittest.cc

@@ -455,6 +455,20 @@ void InitVoiceReceiverInfo(cricket::VoiceReceiverInfo* voice_receiver_info) {
   voice_receiver_info->expand_rate = 121;
 }
 
+class StatsCollectorForTest : public webrtc::StatsCollector {
+ public:
+  explicit StatsCollectorForTest(WebRtcSession* session) :
+      StatsCollector(session), time_now_(19477) {
+  }
+
+  double GetTimeNow() override {
+    return time_now_;
+  }
+
+ private:
+  double time_now_;
+};
+
 class StatsCollectorTest : public testing::Test {
  protected:
   StatsCollectorTest()
@@ -615,7 +629,7 @@ class StatsCollectorTest : public testing::Test {
                               const std::vector<std::string>& local_ders,
                               const rtc::FakeSSLCertificate& remote_cert,
                               const std::vector<std::string>& remote_ders) {
-    webrtc::StatsCollector stats(&session_);
+    StatsCollectorForTest stats(&session_);
 
     StatsReports reports;  // returned values.
 
@@ -718,12 +732,22 @@ TEST_F(StatsCollectorTest, ExtractDataInfo) {
   config.id = id;
   signaling_.AddDataChannel(DataChannel::Create(
       &data_channel_provider_, cricket::DCT_SCTP, label, config));
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   stats.UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard);
 
   StatsReports reports;
   stats.GetStats(NULL, &reports);
+
+  const StatsReport* report =
+      FindNthReportByType(reports, StatsReport::kStatsReportTypeDataChannel, 1);
+
+  scoped_ptr<StatsReport::Id> reportId = StatsReport::NewTypedIntId(
+      StatsReport::kStatsReportTypeDataChannel, id);
+
+  EXPECT_TRUE(reportId->Equals(report->id()));
+
+  EXPECT_EQ(stats.GetTimeNow(), report->timestamp());
   EXPECT_EQ(label, ExtractStatsValue(StatsReport::kStatsReportTypeDataChannel,
                                      reports,
                                      StatsReport::kStatsValueNameLabel));
@@ -741,7 +765,7 @@ TEST_F(StatsCollectorTest, ExtractDataInfo) {
 
 // This test verifies that 64-bit counters are passed successfully.
 TEST_F(StatsCollectorTest, BytesCounterHandles64Bits) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
   cricket::VideoChannel video_channel(rtc::Thread::Current(),
@@ -775,7 +799,7 @@ TEST_F(StatsCollectorTest, BytesCounterHandles64Bits) {
 
 // Test that BWE information is reported via stats.
 TEST_F(StatsCollectorTest, BandwidthEstimationInfoIsReported) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
   cricket::VideoChannel video_channel(rtc::Thread::Current(),
@@ -820,7 +844,7 @@ TEST_F(StatsCollectorTest, BandwidthEstimationInfoIsReported) {
 // This test verifies that an object of type "googSession" always
 // exists in the returned stats.
 TEST_F(StatsCollectorTest, SessionObjectExists) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   StatsReports reports;  // returned values.
   EXPECT_CALL(session_, video_channel()).WillRepeatedly(ReturnNull());
@@ -835,7 +859,7 @@ TEST_F(StatsCollectorTest, SessionObjectExists) {
 // This test verifies that only one object of type "googSession" exists
 // in the returned stats.
 TEST_F(StatsCollectorTest, OnlyOneSessionObjectExists) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   StatsReports reports;  // returned values.
   EXPECT_CALL(session_, video_channel()).WillRepeatedly(ReturnNull());
@@ -854,7 +878,7 @@ TEST_F(StatsCollectorTest, OnlyOneSessionObjectExists) {
 // This test verifies that the empty track report exists in the returned stats
 // without calling StatsCollector::UpdateStats.
 TEST_F(StatsCollectorTest, TrackObjectExistsWithoutUpdateStats) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
   cricket::VideoChannel video_channel(rtc::Thread::Current(),
@@ -878,7 +902,7 @@ TEST_F(StatsCollectorTest, TrackObjectExistsWithoutUpdateStats) {
 // This test verifies that the empty track report exists in the returned stats
 // when StatsCollector::UpdateStats is called with ssrc stats.
 TEST_F(StatsCollectorTest, TrackAndSsrcObjectExistAfterUpdateSsrcStats) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
   cricket::VideoChannel video_channel(rtc::Thread::Current(),
@@ -934,7 +958,7 @@ TEST_F(StatsCollectorTest, TrackAndSsrcObjectExistAfterUpdateSsrcStats) {
 // This test verifies that an SSRC object has the identifier of a Transport
 // stats object, and that this transport stats object exists in stats.
 TEST_F(StatsCollectorTest, TransportObjectLinkedFromSsrcObject) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   // Ignore unused callback (logspam).
   EXPECT_CALL(session_, GetTransport(_))
@@ -995,7 +1019,7 @@ TEST_F(StatsCollectorTest, TransportObjectLinkedFromSsrcObject) {
 // This test verifies that a remote stats object will not be created for
 // an outgoing SSRC where remote stats are not returned.
 TEST_F(StatsCollectorTest, RemoteSsrcInfoIsAbsent) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
   // The content_name known by the video channel.
@@ -1019,7 +1043,7 @@ TEST_F(StatsCollectorTest, RemoteSsrcInfoIsAbsent) {
 // This test verifies that a remote stats object will be created for
 // an outgoing SSRC where stats are returned.
 TEST_F(StatsCollectorTest, RemoteSsrcInfoIsPresent) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   // Ignore unused callback (logspam).
   EXPECT_CALL(session_, GetTransport(_))
@@ -1062,13 +1086,13 @@ TEST_F(StatsCollectorTest, RemoteSsrcInfoIsPresent) {
   const StatsReport* remote_report = FindNthReportByType(reports,
       StatsReport::kStatsReportTypeRemoteSsrc, 1);
   EXPECT_FALSE(remote_report == NULL);
-  EXPECT_NE(0, remote_report->timestamp());
+  EXPECT_EQ(12345.678, remote_report->timestamp());
 }
 
 // This test verifies that the empty track report exists in the returned stats
 // when StatsCollector::UpdateStats is called with ssrc stats.
 TEST_F(StatsCollectorTest, ReportsFromRemoteTrack) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
   cricket::VideoChannel video_channel(rtc::Thread::Current(),
@@ -1114,7 +1138,7 @@ TEST_F(StatsCollectorTest, ReportsFromRemoteTrack) {
 // This test verifies the Ice Candidate report should contain the correct
 // information from local/remote candidates.
 TEST_F(StatsCollectorTest, IceCandidateReport) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   StatsReports reports;                     // returned values.
 
@@ -1244,7 +1268,7 @@ TEST_F(StatsCollectorTest, ChainlessCertificateReportsCreated) {
 // This test verifies that the stats are generated correctly when no
 // transport is present.
 TEST_F(StatsCollectorTest, NoTransport) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   StatsReports reports;  // returned values.
 
@@ -1291,7 +1315,7 @@ TEST_F(StatsCollectorTest, NoTransport) {
 // This test verifies that the stats are generated correctly when the transport
 // does not have any certificates.
 TEST_F(StatsCollectorTest, NoCertificates) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   StatsReports reports;  // returned values.
 
@@ -1360,7 +1384,7 @@ TEST_F(StatsCollectorTest, UnsupportedDigestIgnored) {
 // Verifies the correct optons are passed to the VideoMediaChannel when using
 // verbose output level.
 TEST_F(StatsCollectorTest, StatsOutputLevelVerbose) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
   cricket::VideoChannel video_channel(rtc::Thread::Current(),
@@ -1406,7 +1430,7 @@ TEST_F(StatsCollectorTest, StatsOutputLevelVerbose) {
 // This test verifies that a local stats object can get statistics via
 // AudioTrackInterface::GetStats() method.
 TEST_F(StatsCollectorTest, GetStatsFromLocalAudioTrack) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   // Ignore unused callback (logspam).
   EXPECT_CALL(session_, GetTransport(_))
@@ -1440,7 +1464,7 @@ TEST_F(StatsCollectorTest, GetStatsFromLocalAudioTrack) {
 // This test verifies that audio receive streams populate stats reports
 // correctly.
 TEST_F(StatsCollectorTest, GetStatsFromRemoteStream) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   // Ignore unused callback (logspam).
   EXPECT_CALL(session_, GetTransport(_))
@@ -1467,7 +1491,7 @@ TEST_F(StatsCollectorTest, GetStatsFromRemoteStream) {
 // This test verifies that a local stats object won't update its statistics
 // after a RemoveLocalAudioTrack() call.
 TEST_F(StatsCollectorTest, GetStatsAfterRemoveAudioStream) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   // Ignore unused callback (logspam).
   EXPECT_CALL(session_, GetTransport(_))
@@ -1525,7 +1549,7 @@ TEST_F(StatsCollectorTest, GetStatsAfterRemoveAudioStream) {
 // This test verifies that when ongoing and incoming audio tracks are using
 // the same ssrc, they populate stats reports correctly.
 TEST_F(StatsCollectorTest, LocalAndRemoteTracksWithSameSsrc) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   // Ignore unused callback (logspam).
   EXPECT_CALL(session_, GetTransport(_))
@@ -1608,7 +1632,7 @@ TEST_F(StatsCollectorTest, LocalAndRemoteTracksWithSameSsrc) {
 // TODO(xians): Figure out if it is possible to encapsulate the setup and
 // avoid duplication of code in test cases.
 TEST_F(StatsCollectorTest, TwoLocalTracksWithSameSsrc) {
-  webrtc::StatsCollector stats(&session_);
+  StatsCollectorForTest stats(&session_);
 
   // Ignore unused callback (logspam).
   EXPECT_CALL(session_, GetTransport(_))

talk/app/webrtc/statstypes.cc

@@ -76,7 +76,6 @@ class BandwidthEstimationId : public StatsReport::Id {
 
 class TypedId : public StatsReport::Id {
  public:
-  static const char kSeparator = '_';
   TypedId(StatsReport::StatsType type, const std::string& id)
       : StatsReport::Id(type), id_(id) {}
 
@@ -93,6 +92,26 @@ class TypedId : public StatsReport::Id {
   const std::string id_;
 };
 
+class TypedIntId : public StatsReport::Id {
+ public:
+  TypedIntId(StatsReport::StatsType type, int id)
+      : StatsReport::Id(type), id_(id) {}
+
+  bool Equals(const Id& other) const override {
+    return Id::Equals(other) &&
+           static_cast<const TypedIntId&>(other).id_ == id_;
+  }
+
+  std::string ToString() const override {
+    return std::string(InternalTypeToString(type_)) +
+           kSeparator +
+           rtc::ToString<int>(id_);
+  }
+
+ protected:
+  const int id_;
+};
+
 class IdWithDirection : public TypedId {
  public:
   IdWithDirection(StatsReport::StatsType type, const std::string& id,
@@ -106,7 +125,7 @@ class IdWithDirection : public TypedId {
 
   std::string ToString() const override {
     std::string ret(TypedId::ToString());
-    ret += '_';
+    ret += kSeparator;
     ret += direction_ == StatsReport::kSend ? "send" : "recv";
     return ret;
   }
@@ -436,6 +455,11 @@ scoped_ptr<StatsReport::Id> StatsReport::NewTypedId(
   return scoped_ptr<Id>(new TypedId(type, id)).Pass();
 }
 
+// static
+scoped_ptr<StatsReport::Id> StatsReport::NewTypedIntId(StatsType type, int id) {
+  return scoped_ptr<Id>(new TypedIntId(type, id)).Pass();
+}
+
 // static
 scoped_ptr<StatsReport::Id> StatsReport::NewIdWithDirection(
     StatsType type, const std::string& id, StatsReport::Direction direction) {

talk/app/webrtc/statstypes.h

@@ -229,6 +229,8 @@ class StatsReport {
    protected:
     Id(StatsType type);  // Only meant for derived classes.
     const StatsType type_;
+
+    static const char kSeparator = '_';
   };
 
   struct Value {
@@ -254,6 +256,7 @@ class StatsReport {
   // Factory functions for various types of stats IDs.
   static rtc::scoped_ptr<Id> NewBandwidthEstimationId();
   static rtc::scoped_ptr<Id> NewTypedId(StatsType type, const std::string& id);
+  static rtc::scoped_ptr<Id> NewTypedIntId(StatsType type, int id);
   static rtc::scoped_ptr<Id> NewIdWithDirection(
       StatsType type, const std::string& id, Direction direction);
   static rtc::scoped_ptr<Id> NewCandidateId(bool local, const std::string& id);