[trafficserver] branch NewAPIMetricsPOC updated: Adds operator[] lookups, and some error checking

[zwoop]

This is an automated email from the ASF dual-hosted git repository.

zwoop pushed a commit to branch NewAPIMetricsPOC
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/NewAPIMetricsPOC by this push:
     new dfb8100d77 Adds operator[] lookups, and some error checking
dfb8100d77 is described below

commit dfb8100d77df6c7954d5c8f7175716060d7781d6
Author: Leif Hedstrom <zw...@apache.org>
AuthorDate: Thu Jul 6 22:53:18 2023 -0600

    Adds operator[] lookups, and some error checking
---
 include/api/Metrics.h        | 19 ++++++++++++++++++-
 src/api/Metrics.cc           | 13 +++++++++----
 src/traffic_server/InkAPI.cc | 15 ++++++++-------
 3 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/include/api/Metrics.h b/include/api/Metrics.h
index 1c6a7e95a3..35f573bc8b 100644
--- a/include/api/Metrics.h
+++ b/include/api/Metrics.h
@@ -64,7 +64,12 @@ public:
 
   virtual ~Metrics() = default;
 
-  Metrics() { _blobs[0] = new MetricStorage(); }
+  Metrics()
+  {
+    _blobs[0] = new MetricStorage();
+    ink_release_assert(_blobs[0]);
+    ink_release_assert(0 == newMetric("proxy.node.bad_id.metrics")); // 
Reserve slot 0 for errors, this should always be 0
+  }
 
   // Singleton
   static Metrics &getInstance();
@@ -77,6 +82,18 @@ public:
   IdType lookup(const std::string_view name);
   AtomicType *lookup(IdType id) const;
 
+  AtomicType &
+  operator[](IdType id)
+  {
+    return *lookup(id);
+  }
+
+  IdType
+  operator[](const std::string_view name)
+  {
+    return lookup(name);
+  }
+
   // Inlined, for performance
   int64_t
   increment(IdType id, uint64_t val = 1)
diff --git a/src/api/Metrics.cc b/src/api/Metrics.cc
index fc500df46a..d9bb0981c6 100644
--- a/src/api/Metrics.cc
+++ b/src/api/Metrics.cc
@@ -90,11 +90,16 @@ Metrics::lookup(const std::string_view name)
 Metrics::AtomicType *
 Metrics::lookup(IdType id) const
 {
-  std::tuple<uint16_t, uint16_t> idx = _splitID(id);
-  Metrics::MetricStorage *blob       = _blobs[std::get<0>(idx)];
-  Metrics::AtomicContainer &atomics  = std::get<1>(*blob);
+  auto [blob_ix, offset]       = _splitID(id);
+  Metrics::MetricStorage *blob = _blobs[blob_ix];
 
-  return &(atomics[std::get<1>(idx)]);
+  // Do a sanity check on the ID, to make sure we don't index outside of the 
realm of possibility.
+  if (!blob || (blob_ix == _cur_blob && offset > _cur_off)) {
+    blob   = _blobs[0];
+    offset = 0;
+  }
+
+  return &((std::get<1>(*blob)[offset]));
 }
 
 // ToDo: This is probably not great, and likely we should have some better
diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc
index 157f2c8929..b1c7805240 100644
--- a/src/traffic_server/InkAPI.cc
+++ b/src/traffic_server/InkAPI.cc
@@ -386,6 +386,7 @@ HttpAPIHooks *http_global_hooks        = nullptr;
 SslAPIHooks *ssl_hooks                 = nullptr;
 LifecycleAPIHooks *lifecycle_hooks     = nullptr;
 ConfigUpdateCbTable *global_config_cbs = nullptr;
+static ts::Metrics &global_api_metrics = ts::Metrics::getInstance();
 
 static char traffic_server_version[128] = "";
 static int ts_major_version             = 0;
@@ -749,7 +750,7 @@ sdk_sanity_check_null_ptr(void const *ptr)
 static TSReturnCode
 sdk_sanity_check_stat_id(int id)
 {
-  return (ts::Metrics::getInstance().valid(id) ? TS_SUCCESS : TS_ERROR);
+  return (global_api_metrics.valid(id) ? TS_SUCCESS : TS_ERROR);
 }
 
 static TSReturnCode
@@ -7492,7 +7493,7 @@ TSCacheScan(TSCont contp, TSCacheKey key, int 
KB_per_second)
 int
 TSStatCreate(const char *the_name, TSRecordDataType the_type, 
TSStatPersistence persist, TSStatSync sync)
 {
-  int id = ts::Metrics::getInstance().newMetric(the_name);
+  int id = global_api_metrics.newMetric(the_name);
 
   if (id == ts::Metrics::NOT_FOUND) {
     return TS_ERROR;
@@ -7505,28 +7506,28 @@ void
 TSStatIntIncrement(int id, TSMgmtInt amount)
 {
   sdk_assert(sdk_sanity_check_stat_id(id) == TS_SUCCESS);
-  ts::Metrics::getInstance().increment(id, amount);
+  global_api_metrics[id].fetch_add(amount);
 }
 
 void
 TSStatIntDecrement(int id, TSMgmtInt amount)
 {
   sdk_assert(sdk_sanity_check_stat_id(id) == TS_SUCCESS);
-  ts::Metrics::getInstance().decrement(id, amount);
+  global_api_metrics[id].fetch_sub(amount);
 }
 
 TSMgmtInt
 TSStatIntGet(int id)
 {
   sdk_assert(sdk_sanity_check_stat_id(id) == TS_SUCCESS);
-  return ts::Metrics::getInstance().get(id);
+  return global_api_metrics.get(id);
 }
 
 void
 TSStatIntSet(int id, TSMgmtInt value)
 {
   sdk_assert(sdk_sanity_check_stat_id(id) == TS_SUCCESS);
-  ts::Metrics::getInstance().set(id, value);
+  global_api_metrics.set(id, value);
 }
 
 TSReturnCode
@@ -7535,7 +7536,7 @@ TSStatFindName(const char *name, int *idp)
   sdk_assert(sdk_sanity_check_null_ptr((void *)name) == TS_SUCCESS);
   sdk_assert(sdk_sanity_check_null_ptr((void *)idp) == TS_SUCCESS);
 
-  int id = ts::Metrics::getInstance().lookup(name);
+  int id = global_api_metrics.lookup(name);
 
   if (id == ts::Metrics::NOT_FOUND) {
     return TS_ERROR;