From 272cc7775186f9462ac076da99f701e976d13b0e Mon Sep 17 00:00:00 2001
From: mrambacher <mrambach@gmail.com>
Date: Fri, 17 Sep 2021 06:54:40 -0700
Subject: [PATCH] Added a default Name method to Statistics (#8918)

Summary:
This keeps the implementations/API backward compatible.  Implementations of Statistics will need to override this method (and be registered with the ObjectRegistry) in order to be created via CreateFromString.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/8918

Reviewed By: pdillinger

Differential Revision: D30958916

Pulled By: mrambacher

fbshipit-source-id: 75b99a84e9e11fda2a9e8eff9ee1ef69a17517b2
---
 HISTORY.md                    |  1 +
 include/rocksdb/statistics.h  |  7 +++++
 monitoring/statistics_test.cc | 49 +++++++++++++++++++++++++++++++++--
 3 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/HISTORY.md b/HISTORY.md
index eccbe3c97..18ff661b0 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -27,6 +27,7 @@
 ### Public API change
 * Remove obsolete implementation details FullKey and ParseFullKey from public API
 * Add a public API RateLimiter::GetTotalPendingRequests() for the total number of requests that are pending for bytes in the rate limiter.
+* Made Statistics extend the Customizable class and added a CreateFromString method.  Implementations of Statistics need to be registered with the ObjectRegistry and to implement a Name() method in order to be created via this method.
 * Extended `FlushJobInfo` and `CompactionJobInfo` in listener.h to provide information about the blob files generated by a flush/compaction and garbage collected during compaction in Integrated BlobDB. Added struct members `blob_file_addition_infos` and `blob_file_garbage_infos` that contain this information.
 * Extended parameter `output_file_names` of `CompactFiles` API to also include paths of the blob files generated by the compaction in Integrated BlobDB.
 
diff --git a/include/rocksdb/statistics.h b/include/rocksdb/statistics.h
index 57d173cd6..c4f2d5154 100644
--- a/include/rocksdb/statistics.h
+++ b/include/rocksdb/statistics.h
@@ -576,6 +576,10 @@ class Statistics : public Customizable {
   static Status CreateFromString(const ConfigOptions& opts,
                                  const std::string& value,
                                  std::shared_ptr<Statistics>* result);
+  // Default name of empty, for backwards compatibility.  Derived classes should
+  // override this method.
+  // This default implementation will likely be removed in a future release
+  const char* Name() const override { return ""; }
   virtual uint64_t getTickerCount(uint32_t tickerType) const = 0;
   virtual void histogramData(uint32_t type,
                              HistogramData* const data) const = 0;
@@ -607,6 +611,9 @@ class Statistics : public Customizable {
   // Resets all ticker and histogram stats
   virtual Status Reset() { return Status::NotSupported("Not implemented"); }
 
+#ifndef ROCKSDB_LITE
+  using Customizable::ToString;
+#endif  // ROCKSDB_LITE
   // String representation of the statistic object. Must be thread-safe.
   virtual std::string ToString() const {
     // Do nothing by default
diff --git a/monitoring/statistics_test.cc b/monitoring/statistics_test.cc
index e497afcbe..cffa5054a 100644
--- a/monitoring/statistics_test.cc
+++ b/monitoring/statistics_test.cc
@@ -4,12 +4,14 @@
 //  (found in the LICENSE.Apache file in the root directory).
 //
 
+#include "rocksdb/statistics.h"
+
 #include "port/stack_trace.h"
+#include "rocksdb/convenience.h"
+#include "rocksdb/utilities/options_type.h"
 #include "test_util/testharness.h"
 #include "test_util/testutil.h"
 
-#include "rocksdb/statistics.h"
-
 namespace ROCKSDB_NAMESPACE {
 
 class StatisticsTest : public testing::Test {};
@@ -38,6 +40,49 @@ TEST_F(StatisticsTest, SanityHistograms) {
   }
 }
 
+TEST_F(StatisticsTest, NoNameStats) {
+  static std::unordered_map<std::string, OptionTypeInfo> no_name_opt_info = {
+#ifndef ROCKSDB_LITE
+      {"inner",
+       OptionTypeInfo::AsCustomSharedPtr<Statistics>(
+           0, OptionVerificationType::kByName,
+           OptionTypeFlags::kAllowNull | OptionTypeFlags::kCompareNever)},
+#endif  // ROCKSDB_LITE
+  };
+
+  class DefaultNameStatistics : public Statistics {
+   public:
+    DefaultNameStatistics(const std::shared_ptr<Statistics>& stats = nullptr)
+        : inner(stats) {
+      RegisterOptions("", &inner, &no_name_opt_info);
+    }
+
+    uint64_t getTickerCount(uint32_t /*tickerType*/) const override {
+      return 0;
+    }
+    void histogramData(uint32_t /*type*/,
+                       HistogramData* const /*data*/) const override {}
+    void recordTick(uint32_t /*tickerType*/, uint64_t /*count*/) override {}
+    void setTickerCount(uint32_t /*tickerType*/, uint64_t /*count*/) override {}
+    uint64_t getAndResetTickerCount(uint32_t /*tickerType*/) override {
+      return 0;
+    }
+    std::shared_ptr<Statistics> inner;
+  };
+  ConfigOptions options;
+  options.ignore_unsupported_options = false;
+  auto stats = std::make_shared<DefaultNameStatistics>();
+  ASSERT_STREQ(stats->Name(), "");
+#ifndef ROCKSDB_LITE
+  ASSERT_EQ("", stats->ToString(
+                    options));  // A stats with no name with have no options...
+  ASSERT_OK(stats->ConfigureFromString(options, "inner="));
+  ASSERT_EQ("", stats->ToString(
+                    options));  // A stats with no name with have no options...
+  ASSERT_NE(stats->inner, nullptr);
+  ASSERT_NE("", stats->inner->ToString(options));  // ... even if it does...
+#endif                                             // ROCKSDB_LITE
+}
 }  // namespace ROCKSDB_NAMESPACE
 
 int main(int argc, char** argv) {
-- 
GitLab