Revert "Replace the usage of MetricsObserverInterface by RTC_HISTOGRA…

…M_*." This reverts commit 870bca1. Reason for revert: it breaks internal tests and builds Original change's description: > Replace the usage of MetricsObserverInterface by RTC_HISTOGRAM_*. > > We now use RTC_HISTOGRAM_* macros in system_wrappers/include/metrics.h > to report the metrics in pc/ and p2p/ that are currently been reported > using MetricsObserverInterface. > > [email protected] > > Bug: webrtc:9409 > Change-Id: I47c9975402293c72250203fa1ec19eb1668766f6 > Reviewed-on: https://webrtc-review.googlesource.com/83782 > Commit-Queue: Qingsi Wang <[email protected]> > Reviewed-by: Harald Alvestrand <[email protected]> > Reviewed-by: Taylor (left Google) <[email protected]> > Reviewed-by: Steve Anton <[email protected]> > Cr-Commit-Position: refs/heads/master@{#23914} [email protected],[email protected],[email protected],[email protected] Change-Id: I1afd92d44f3b8cf3ae9aa6e6daa9a3a272e8097f No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: webrtc:9409 Reviewed-on: https://webrtc-review.googlesource.com/88040 Reviewed-by: Qingsi Wang <[email protected]> Commit-Queue: Qingsi Wang <[email protected]> Cr-Commit-Position: refs/heads/master@{#23916}
china20 · Jul 10, 2018 · 13f4c89 · 13f4c89
1 parent 79abc3d
commit 13f4c89
Show file tree

Hide file tree

Showing 25 changed files with 510 additions and 307 deletions.
diff --git a/api/BUILD.gn b/api/BUILD.gn
@@ -86,6 +86,7 @@ rtc_static_library("libjingle_peerconnection_api") {
     "statstypes.cc",
     "statstypes.h",
     "turncustomizer.h",
+    "umametrics.cc",
     "umametrics.h",
     "videosourceproxy.h",
   ]
@@ -439,6 +440,26 @@ if (rtc_include_tests) {
     ]
   }
 
+  rtc_source_set("fakemetricsobserver") {
+    testonly = true
+    sources = [
+      "fakemetricsobserver.cc",
+      "fakemetricsobserver.h",
+    ]
+    deps = [
+      "../media:rtc_media_base",
+      "../rtc_base:checks",
+      "../rtc_base:rtc_base_approved",
+    ]
+    if (!build_with_chromium && is_clang) {
+      # Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163).
+      suppressed_configs += [ "//build/config/clang:find_bad_constructs" ]
+    }
+    if (!build_with_mozilla) {
+      deps += [ ":libjingle_peerconnection_api" ]
+    }
+  }
+
   rtc_source_set("rtc_api_unittests") {
     testonly = true
 

diff --git a/api/fakemetricsobserver.cc b/api/fakemetricsobserver.cc
@@ -0,0 +1,87 @@
+/*
+ *  Copyright 2015 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 "api/fakemetricsobserver.h"
+#include "rtc_base/checks.h"
+
+namespace webrtc {
+
+FakeMetricsObserver::FakeMetricsObserver() {
+  Reset();
+}
+
+void FakeMetricsObserver::Reset() {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  counters_.clear();
+  memset(histogram_samples_, 0, sizeof(histogram_samples_));
+}
+
+void FakeMetricsObserver::IncrementEnumCounter(
+    PeerConnectionEnumCounterType type,
+    int counter,
+    int counter_max) {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  if (counters_.size() <= static_cast<size_t>(type)) {
+    counters_.resize(type + 1);
+  }
+  auto& counters = counters_[type];
+  ++counters[counter];
+}
+
+void FakeMetricsObserver::AddHistogramSample(PeerConnectionMetricsName type,
+                                             int value) {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  RTC_DCHECK_EQ(histogram_samples_[type], 0);
+  histogram_samples_[type] = value;
+}
+
+int FakeMetricsObserver::GetEnumCounter(PeerConnectionEnumCounterType type,
+                                        int counter) const {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  if (counters_.size() <= static_cast<size_t>(type)) {
+    return 0;
+  }
+  const auto& it = counters_[type].find(counter);
+  if (it == counters_[type].end()) {
+    return 0;
+  }
+  return it->second;
+}
+
+int FakeMetricsObserver::GetHistogramSample(
+    PeerConnectionMetricsName type) const {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  return histogram_samples_[type];
+}
+
+bool FakeMetricsObserver::ExpectOnlySingleEnumCount(
+    PeerConnectionEnumCounterType type,
+    int counter) const {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  if (counters_.size() <= static_cast<size_t>(type)) {
+    // If a counter has not been allocated then there has been no call to
+    // |IncrementEnumCounter| so all the values are 0.
+    return false;
+  }
+  bool pass = true;
+  if (GetEnumCounter(type, counter) != 1) {
+    RTC_LOG(LS_ERROR) << "Expected single count for counter: " << counter;
+    pass = false;
+  }
+  for (const auto& entry : counters_[type]) {
+    if (entry.first != counter && entry.second > 0) {
+      RTC_LOG(LS_ERROR) << "Expected no count for counter: " << entry.first;
+      pass = false;
+    }
+  }
+  return pass;
+}
+
+}  // namespace webrtc
diff --git a/api/fakemetricsobserver.h b/api/fakemetricsobserver.h
@@ -0,0 +1,56 @@
+/*
+ *  Copyright 2015 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.
+ */
+
+#ifndef API_FAKEMETRICSOBSERVER_H_
+#define API_FAKEMETRICSOBSERVER_H_
+
+#include <map>
+#include <string>
+#include <vector>
+
+#include "api/peerconnectioninterface.h"
+#include "rtc_base/thread_checker.h"
+
+namespace webrtc {
+
+class FakeMetricsObserver : public MetricsObserverInterface {
+ public:
+  FakeMetricsObserver();
+  void Reset();
+
+  void IncrementEnumCounter(PeerConnectionEnumCounterType,
+                            int counter,
+                            int counter_max) override;
+  void AddHistogramSample(PeerConnectionMetricsName type, int value) override;
+
+  // Accessors to be used by the tests.
+  int GetEnumCounter(PeerConnectionEnumCounterType type, int counter) const;
+  int GetHistogramSample(PeerConnectionMetricsName type) const;
+
+  // Returns true if and only if there is a count of 1 for the given counter and
+  // a count of 0 for all other counters of the given enum type.
+  bool ExpectOnlySingleEnumCount(PeerConnectionEnumCounterType type,
+                                 int counter) const;
+
+ protected:
+  ~FakeMetricsObserver() {}
+
+ private:
+  rtc::ThreadChecker thread_checker_;
+  // The vector contains maps for each counter type. In the map, it's a mapping
+  // from individual counter to its count, such that it's memory efficient when
+  // comes to sparse enum types, like the SSL ciphers in the IANA registry.
+  std::vector<std::map<int, int>> counters_;
+  int histogram_samples_[kPeerConnectionMetricsName_Max];
+};
+
+}  // namespace webrtc
+
+#endif  // API_FAKEMETRICSOBSERVER_H_
diff --git a/api/umametrics.cc b/api/umametrics.cc
@@ -0,0 +1,21 @@
+/*
+ *  Copyright 2017 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 "api/umametrics.h"
+
+namespace webrtc {
+
+void MetricsObserverInterface::IncrementSparseEnumCounter(
+    PeerConnectionEnumCounterType type,
+    int counter) {
+  IncrementEnumCounter(type, counter, 0 /* Ignored */);
+}
+
+}  // namespace webrtc
diff --git a/api/umametrics.h b/api/umametrics.h
@@ -176,13 +176,13 @@ class MetricsObserverInterface : public rtc::RefCountInterface {
   // number after the highest counter.
   virtual void IncrementEnumCounter(PeerConnectionEnumCounterType type,
                                     int counter,
-                                    int counter_max) = 0;
+                                    int counter_max) {}
 
   // This is used to handle sparse counters like SSL cipher suites.
   // TODO(guoweis): Remove the implementation once the dependency's interface
   // definition is updated.
   virtual void IncrementSparseEnumCounter(PeerConnectionEnumCounterType type,
-                                          int counter) = 0;
+                                          int counter);
 
   virtual void AddHistogramSample(PeerConnectionMetricsName type,
                                   int value) = 0;

diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn
@@ -91,7 +91,6 @@ rtc_static_library("rtc_p2p") {
     "../rtc_base:safe_minmax",
     "../rtc_base:stringutils",
     "../system_wrappers:field_trial_api",
-    "../system_wrappers:metrics_api",
     "//third_party/abseil-cpp/absl/memory",
     "//third_party/abseil-cpp/absl/types:optional",
   ]
@@ -172,13 +171,13 @@ if (rtc_include_tests) {
     deps = [
       ":p2p_test_utils",
       ":rtc_p2p",
+      "../api:fakemetricsobserver",
       "../api:ortc_api",
       "../rtc_base:checks",
       "../rtc_base:rtc_base",
       "../rtc_base:rtc_base_approved",
       "../rtc_base:rtc_base_tests_utils",
       "../rtc_base:stringutils",
-      "../system_wrappers:metrics_default",
       "../test:test_support",
       "//testing/gtest",
       "//third_party/abseil-cpp/absl/memory",

diff --git a/p2p/base/p2ptransportchannel.cc b/p2p/base/p2ptransportchannel.cc
@@ -28,7 +28,6 @@
 #include "rtc_base/stringencode.h"
 #include "rtc_base/timeutils.h"
 #include "system_wrappers/include/field_trial.h"
-#include "system_wrappers/include/metrics.h"
 
 namespace {
 
@@ -676,7 +675,7 @@ void P2PTransportChannel::MaybeStartGathering() {
       SignalGatheringState(this);
     }
 
-    if (!allocator_sessions_.empty()) {
+    if (metrics_observer_ && !allocator_sessions_.empty()) {
       IceRestartState state;
       if (writable()) {
         state = IceRestartState::CONNECTED;
@@ -685,9 +684,9 @@ void P2PTransportChannel::MaybeStartGathering() {
       } else {
         state = IceRestartState::DISCONNECTED;
       }
-      RTC_HISTOGRAM_ENUMERATION("WebRTC.PeerConnection.IceRestartState",
-                                static_cast<int>(state),
-                                static_cast<int>(IceRestartState::MAX_VALUE));
+      metrics_observer_->IncrementEnumCounter(
+          webrtc::kEnumCounterIceRestart, static_cast<int>(state),
+          static_cast<int>(IceRestartState::MAX_VALUE));
     }
 
     // Time for a new allocator.