New API Metrics, and start of a libtsapi.so structure

.gitignore

@@ -102,6 +102,7 @@ src/tscore/test_tscore
 src/tscpp/util/test_tscpputil
 src/records/test_librecords
 src/records/test_librecords_on_eventsystem
+src/api/test_Metrics
 lib/perl/lib/Apache/TS.pm
 
 iocore/net/test_certlookup

CMakeLists.txt

@@ -355,6 +355,7 @@ configure_file(include/ts/apidefs.h.in include/ts/apidefs.h)
 
 enable_testing()
 
+add_subdirectory(src/api)
 add_subdirectory(src/tscpp/util)
 add_subdirectory(src/tscpp/api)
 add_subdirectory(src/tscore)

configure.ac

@@ -2400,6 +2400,7 @@ AC_CONFIG_FILES([
   src/tscpp/util/Makefile
   src/tscore/Makefile
   src/records/Makefile
+  src/api/Makefile
   tools/Makefile
   tools/trafficserver.pc
   tools/tsxs

include/api/Metrics.h

@@ -0,0 +1,237 @@
+/** @file
+
+  A brief file description
+
+  @section license License
+
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+ */
+
+#pragma once
+
+#include <array>
+#include <unordered_map>
+#include <tuple>
+#include <mutex>
+#include <thread>
+#include <atomic>
+#include <cstdint>
+#include <string>
+#include <string_view>
+
+#include "tscore/ink_assert.h"
+
+namespace ts
+{
+class Metrics
+{
+private:
+  using self_type  = Metrics;
+  using IdType     = int32_t; // Could be a tuple, but one way or another, they have to be combined to an int32_t.
+  using AtomicType = std::atomic<int64_t>;
+
+public:
+  static constexpr uint16_t METRICS_MAX_BLOBS = 8192;
+  static constexpr uint16_t METRICS_MAX_SIZE  = 2048;                               // For a total of 16M metrics
+  static constexpr IdType NOT_FOUND           = std::numeric_limits<IdType>::min(); // <16-bit,16-bit> = <blob-index,offset>
+
+private:
+  using NameAndId       = std::tuple<std::string, IdType>;
+  using NameContainer   = std::array<NameAndId, METRICS_MAX_SIZE>;
+  using AtomicContainer = std::array<AtomicType, METRICS_MAX_SIZE>;
+  using MetricStorage   = std::tuple<NameContainer, AtomicContainer>;
+  using MetricBlobs     = std::array<MetricStorage *, METRICS_MAX_BLOBS>;
+  using LookupTable     = std::unordered_map<std::string_view, IdType>;
+
+public:
+  Metrics(const self_type &)              = delete;
+  self_type &operator=(const self_type &) = delete;
+  Metrics &operator=(Metrics &&)          = delete;
+  Metrics(Metrics &&)                     = delete;
+
+  virtual ~Metrics() = default;
+
+  Metrics()
+  {
+    _blobs[0] = new MetricStorage();
+    ink_release_assert(_blobs[0]);
+    ink_release_assert(0 == newMetric("proxy.node.api.metrics.bad_id")); // Reserve slot 0 for errors, this should always be 0
+  }
+
+  // Singleton
+  static Metrics &getInstance();
+
+  // Yes, we don't return objects here, but rather ID's and atomic's directly. Treat
+  // the std::atomic<int64_t> as the underlying class for a single metric, and be happy.
+  IdType newMetric(const std::string_view name);
+  IdType lookup(const std::string_view name) const;
+  AtomicType *lookup(IdType id, std::string_view *name = nullptr) const;
+
+  AtomicType &
+  operator[](IdType id)
+  {
+    return *lookup(id);
+  }
+
+  IdType
+  operator[](const std::string_view name) const
+  {
+    return lookup(name);
+  }
+
+  int64_t
+  increment(IdType id, uint64_t val = 1)
+  {
+    auto metric = lookup(id);
+
+    return (metric ? metric->fetch_add(val) : NOT_FOUND);
+  }
+
+  // ToDo: Do we even need these inc/dec functions?
+  int64_t
+  decrement(IdType id, uint64_t val = 1)
+  {
+    auto metric = lookup(id);
+
+    return (metric ? metric->fetch_sub(val) : NOT_FOUND);
+  }
+
+  std::string_view name(IdType id) const;
+
+  bool
+  valid(IdType id) const
+  {
+    auto [blob, entry] = _splitID(id);
+
+    return (id >= 0 && ((blob < _cur_blob && entry < METRICS_MAX_SIZE) || (blob == _cur_blob && entry <= _cur_off)));
+  }
+
+  class iterator
+  {
+  public:
+    using iterator_category = std::input_iterator_tag;
+    using value_type        = std::tuple<std::string_view, int64_t>;
+    using difference_type   = ptrdiff_t;
+    using pointer           = value_type *;
+    using reference         = value_type &;
+
+    iterator(const Metrics &m, IdType pos) : _metrics(m), _it(pos) {}
+
+    iterator &
+    operator++()
+    {
+      next();
+
+      return *this;
+    }
+
+    iterator
+    operator++(int)
+    {
+      iterator result = *this;
+
+      next();
+
+      return result;
+    }
+
+    value_type
+    operator*() const
+    {
+      std::string_view name;
+      auto metric = _metrics.lookup(_it, &name);
+
+      return std::make_tuple(name, metric->load());
+    }
+
+    bool
+    operator==(const iterator &o) const
+    {
+      return _it == o._it && std::addressof(_metrics) == std::addressof(o._metrics);
+    }
+
+    bool
+    operator!=(const iterator &o) const
+    {
+      return _it != o._it || std::addressof(_metrics) != std::addressof(o._metrics);
+    }
+
+  private:
+    void next();
+
+    const Metrics &_metrics;
+    IdType _it;
+  };
+
+  iterator
+  begin() const
+  {
+    return iterator(*this, 0);
+  }
+
+  iterator
+  end() const
+  {
+    _mutex.lock();
+    int16_t blob   = _cur_blob;
+    int16_t offset = _cur_off;
+    _mutex.unlock();
+
+    return iterator(*this, _makeId(blob, offset));
+  }
+
+  iterator
+  find(const std::string_view name) const
+  {
+    auto id = lookup(name);
+
+    if (id == NOT_FOUND) {
+      return end();
+    } else {
+      return iterator(*this, id);
+    }
+  }
+
+private:
+  static constexpr std::tuple<uint16_t, uint16_t>
+  _splitID(IdType value)
+  {
+    return std::make_tuple(static_cast<uint16_t>(value >> 16), static_cast<uint16_t>(value & 0xFFFF));
+  }
+
+  static constexpr IdType
+  _makeId(uint16_t blob, uint16_t offset)
+  {
+    return (blob << 16 | offset);
+  }
+
+  static constexpr IdType
+  _makeId(std::tuple<uint16_t, uint16_t> id)
+  {
+    return _makeId(std::get<0>(id), std::get<1>(id));
+  }
+
+  void _addBlob();
+
+  mutable std::mutex _mutex;
+  LookupTable _lookups;
+  MetricBlobs _blobs;
+  uint16_t _cur_blob = 0;
+  uint16_t _cur_off  = 0;
+}; // class Metrics
+
+} // namespace ts

include/records/I_RecordsConfig.h

@@ -25,9 +25,6 @@
 
 #include "records/P_RecCore.h"
 
-// This is to manage the librecords table sizes. Not awesome, but better than the earlier recompiling of ATS requirement...
-extern int max_records_entries;
-
 enum RecordRequiredType {
   RR_NULL,    // config is _not_ required to be defined in records.yaml
   RR_REQUIRED // config _is_ required to be defined in record.config

include/records/P_RecCore.h

@@ -37,7 +37,7 @@
 #include <swoc/Errata.h>
 
 // records, record hash-table, and hash-table rwlock
-extern RecRecord *g_records;
+extern RecRecord g_records[REC_MAX_RECORDS];
 extern std::unordered_map<std::string, RecRecord *> g_records_ht;
 extern ink_rwlock g_records_rwlock;
 extern int g_num_records;

include/records/P_RecDefs.h

@@ -27,12 +27,10 @@
 
 #define REC_MESSAGE_ELE_MAGIC 0xF00DF00D
 
-// We need at least this many internal record entries for our configurations and metrics. Any
-// additional slots in librecords will be allocated to the plugin metrics. These should be
-// updated if we change the internal librecords size significantly.
-#define REC_INTERNAL_RECORDS    1100
-#define REC_DEFAULT_API_RECORDS 1400
-
+// We need at least this many internal record entries for our configurations and metrics.
+// This may need adjustments if we make significant additions to librecords. Note that
+// plugins are using their own metrics systems.
+#define REC_MAX_RECORDS               1500
 #define REC_CONFIG_UPDATE_INTERVAL_MS 3000
 #define REC_REMOTE_SYNC_INTERVAL_MS   5000
 

iocore/aio/Makefile.am

@@ -56,6 +56,7 @@ test_AIO_LDADD = \
 	$(top_builddir)/src/records/librecords_p.a \
 	$(top_builddir)/iocore/eventsystem/libinkevent.a \
 	$(top_builddir)/src/tscore/libtscore.a \
+	$(top_builddir)/src/api/libtsapi.la \
 	$(top_builddir)/src/tscpp/util/libtscpputil.la \
 	@SWOC_LIBS@ @HWLOC_LIBS@ @YAMLCPP_LIBS@ @LIBPCRE@ @LIBCAP@
 

iocore/cache/Makefile.am

@@ -99,6 +99,7 @@ test_LDADD = \
 	$(top_builddir)/iocore/utils/libinkutils.a \
 	$(top_builddir)/iocore/aio/libinkaio.a \
 	$(top_builddir)/src/records/librecords_p.a \
+	$(top_builddir)/src/api/libtsapi.la \
 	$(top_builddir)/src/tscore/libtscore.a \
 	$(top_builddir)/lib/fastlz/libfastlz.a \
 	$(top_builddir)/iocore/eventsystem/libinkevent.a \

iocore/eventsystem/Makefile.am

@@ -97,6 +97,7 @@ test_LD_ADD = \
 	$(top_builddir)/src/records/librecords_p.a \
 	$(top_builddir)/iocore/eventsystem/libinkevent.a \
 	$(top_builddir)/src/tscore/libtscore.a \
+	$(top_builddir)/src/api/libtsapi.la \
 	$(top_builddir)/src/tscpp/util/libtscpputil.la \
 	@HWLOC_LIBS@ @SWOC_LIBS@ @YAMLCPP_LIBS@ @LIBPCRE@ @LIBCAP@
 

iocore/hostdb/Makefile.am

@@ -67,6 +67,7 @@ test_LD_ADD = \
 	$(top_builddir)/src/records/librecords_p.a \
 	$(top_builddir)/iocore/eventsystem/libinkevent.a \
 	$(top_builddir)/src/tscore/libtscore.a \
+	$(top_builddir)/src/api/libtsapi.la \
 	$(top_builddir)/src/tscpp/util/libtscpputil.la \
 	@SWOC_LIBS@ @HWLOC_LIBS@ @YAMLCPP_LIBS@ @OPENSSL_LIBS@ @LIBPCRE@ @LIBCAP@
 

iocore/net/Makefile.am

@@ -51,12 +51,13 @@ test_certlookup_SOURCES = \
 	SSLCertLookup.cc
 
 test_certlookup_LDADD = \
-	@OPENSSL_LIBS@ \
-	$(top_builddir)/src/tscore/libtscore.a $(top_builddir)/src/tscpp/util/libtscpputil.la \
+	$(top_builddir)/src/tscore/libtscore.a \
+	$(top_builddir)/src/tscpp/util/libtscpputil.la \
 	$(top_builddir)/iocore/eventsystem/libinkevent.a \
 	$(top_builddir)/proxy/ParentSelectionStrategy.o \
 	@YAMLCPP_LIBS@ \
 	@SWOC_LIBS@ \
+	@OPENSSL_LIBS@ \
 	@LIBPCRE@ \
 	@LIBCAP@
 
@@ -84,7 +85,9 @@ test_UDPNet_LDADD = \
 	$(top_builddir)/iocore/eventsystem/libinkevent.a \
 	$(top_builddir)/src/records/librecords_p.a \
 	$(top_builddir)/proxy/hdrs/libhdrs.a \
-	$(top_builddir)/src/tscore/libtscore.a $(top_builddir)/src/tscpp/util/libtscpputil.la \
+	$(top_builddir)/src/tscore/libtscore.a \
+	$(top_builddir)/src/tscpp/util/libtscpputil.la \
+	$(top_builddir)/src/api/libtsapi.la \
 	$(top_builddir)/proxy/ParentSelectionStrategy.o \
 	@HWLOC_LIBS@ @OPENSSL_LIBS@ @LIBPCRE@ @YAMLCPP_LIBS@ @SWOC_LIBS@ @LIBPCRE@ @LIBCAP@
 if ENABLE_QUIC
@@ -125,6 +128,7 @@ test_libinknet_LDADD = \
 	$(top_builddir)/src/records/librecords_p.a \
 	$(top_builddir)/proxy/hdrs/libhdrs.a \
 	$(top_builddir)/src/tscore/libtscore.a \
+	$(top_builddir)/src/api/libtsapi.la \
 	$(top_builddir)/src/tscpp/util/libtscpputil.la \
 	$(top_builddir)/proxy/ParentSelectionStrategy.o \
 	@HWLOC_LIBS@ @OPENSSL_LIBS@ @LIBPCRE@ @YAMLCPP_LIBS@ @SWOC_LIBS@ @LIBCAP@

mgmt/rpc/Makefile.am

@@ -106,6 +106,7 @@ test_jsonrpcserver_LDADD = \
 	$(top_builddir)/src/tscore/libtscore.a \
 	$(top_builddir)/iocore/eventsystem/libinkevent.a \
 	$(top_builddir)/src/records/librecords_p.a \
+	$(top_builddir)/src/api/libtsapi.la \
 	$(top_builddir)/src/tscpp/util/libtscpputil.la \
 	$(top_builddir)/src/tscore/libtscore.a \
 	@YAMLCPP_LIBS@ @HWLOC_LIBS@  @SWOC_LIBS@ @YAMLCPP_LIBS@ @LIBPCRE@ @LIBCAP@

plugins/experimental/remap_stats/remap_stats.cc

@@ -46,9 +46,6 @@ struct config_t {
   int txn_slot{-1};
 };
 
-// From "core".... sigh, but we need it for now at least.
-extern int max_records_entries;
-
 namespace
 {
 void

proxy/hdrs/Makefile.am

@@ -89,6 +89,7 @@ test_proxy_hdrs_LDADD = \
 	$(top_builddir)/src/records/librecords_p.a \
 	$(top_builddir)/src/tscore/libtscore.a \
 	$(top_builddir)/src/tscpp/util/libtscpputil.la \
+	$(top_builddir)/src/api/libtsapi.la \
 	@SWOC_LIBS@ @YAMLCPP_LIBS@ @HWLOC_LIBS@ @LIBPCRE@ @OPENSSL_LIBS@ @LIBCAP@
 
 test_hdr_heap_CPPFLAGS = $(AM_CPPFLAGS) \
@@ -104,6 +105,7 @@ test_hdr_heap_LDADD = \
 	$(top_builddir)/src/records/librecords_p.a \
 	$(top_builddir)/src/tscore/libtscore.a \
 	$(top_builddir)/src/tscpp/util/libtscpputil.la \
+	$(top_builddir)/src/api/libtsapi.la \
 	@SWOC_LIBS@ @HWLOC_LIBS@ \
 	@LIBPCRE@ @OPENSSL_LIBS@ @LIBCAP@