You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2023/07/06 19:55:08 UTC
[trafficserver] 01/01: Initial cut at a new Metrics API
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
commit e550beb6257ab3d764b7e8264b4a78b6e11b1b60
Author: Leif Hedstrom <zw...@apache.org>
AuthorDate: Wed Jul 5 17:18:31 2023 -0600
Initial cut at a new Metrics API
---
configure.ac | 1 +
include/api/Metrics.h | 166 ++++++++++++++++++++++++++++++++++++++
src/Makefile.am | 2 +-
src/{ => api}/Makefile.am | 48 ++++++-----
src/api/Metrics.cc | 100 +++++++++++++++++++++++
src/records/RecCore.cc | 5 ++
src/traffic_crashlog/Makefile.inc | 1 +
src/traffic_layout/Makefile.inc | 1 +
src/traffic_logcat/Makefile.inc | 3 +-
src/traffic_logstats/Makefile.inc | 3 +-
src/traffic_server/InkAPI.cc | 85 ++++---------------
src/traffic_server/Makefile.inc | 1 +
12 files changed, 321 insertions(+), 95 deletions(-)
diff --git a/configure.ac b/configure.ac
index b70ead313a..b6c9b25bcc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2403,6 +2403,7 @@ AC_CONFIG_FILES([
src/tscpp/util/Makefile
src/tscore/Makefile
src/records/Makefile
+ src/api/Makefile
tools/Makefile
tools/trafficserver.pc
tools/tsxs
diff --git a/include/api/Metrics.h b/include/api/Metrics.h
new file mode 100644
index 0000000000..aa7ab61168
--- /dev/null
+++ b/include/api/Metrics.h
@@ -0,0 +1,166 @@
+/** @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 "records/P_RecDefs.h"
+
+namespace ts
+{
+class Metrics
+{
+private:
+ using self_type = Metrics;
+ using IdType = int32_t;
+ using AtomicType = std::atomic<int64_t>;
+
+public:
+ static constexpr uint16_t METRICS_MAX_BLOBS = 2048;
+ static constexpr uint16_t METRICS_MAX_SIZE = 8192; // For a total of 16M metrics
+ static constexpr IdType NOT_FOUND = INT32_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() { _addBlob(); }
+
+ // Singleton
+ static Metrics &getInstance();
+
+ IdType newMetric(const std::string_view name);
+ IdType lookup(const std::string_view name);
+ AtomicType *lookup(IdType id) const;
+
+ // Inlined, for performance
+ int64_t
+ increment(IdType id, uint64_t val = 1)
+ {
+ auto metric = lookup(id);
+
+ return (metric ? metric->fetch_add(val) : NOT_FOUND);
+ }
+
+ int64_t
+ decrement(IdType id, uint64_t val = 1)
+ {
+ auto metric = lookup(id);
+
+ return (metric ? metric->fetch_sub(val) : NOT_FOUND);
+ }
+
+ int64_t
+ get(IdType id) const
+ {
+ auto metric = lookup(id);
+
+ return (metric ? metric->load() : NOT_FOUND);
+ }
+
+ void
+ set(IdType id, int64_t val)
+ {
+ auto metric = lookup(id);
+
+ if (metric) {
+ metric->store(val);
+ }
+ }
+
+ bool
+ valid(IdType id) const
+ {
+ std::tuple<uint16_t, uint16_t> idx = _splitID(id);
+
+ return (id >= 0 && std::get<0>(idx) < _cur_blob && std::get<1>(idx) < METRICS_MAX_SIZE);
+ }
+
+ void
+ recordsDump(RecDumpEntryCb callback, void *edata)
+ {
+ return;
+ int16_t blob_ix, off_ix;
+ int16_t off_max = METRICS_MAX_SIZE;
+
+ {
+ std::lock_guard<std::mutex> lock(_mutex);
+
+ // Capture these while protected, in case the blobs change
+ blob_ix = _cur_blob;
+ off_ix = _cur_off;
+ }
+
+ for (int i = 0; i <= blob_ix; ++i) {
+ auto blob = _blobs[i];
+ auto &names = std::get<0>(*blob);
+ auto &metrics = std::get<1>(*blob);
+ RecData datum;
+
+ if (i == blob_ix) {
+ off_max = off_ix;
+ }
+ for (int j = 0; j < off_max; ++j) {
+ datum.rec_int = metrics[j].load();
+ // ToDo: The recordtype here is fine for now, but we should probably make this generic
+ callback(RECT_PLUGIN, edata, 1, std::get<0>(names[i]).c_str(), TS_RECORDDATATYPE_INT, &datum);
+ }
+ }
+ }
+
+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));
+ }
+
+ void _addBlob();
+
+ std::mutex _mutex;
+ LookupTable _lookups;
+ MetricBlobs _blobs;
+ uint16_t _cur_blob = 0;
+ uint16_t _cur_off = 0;
+}; // class Metrics
+
+} // namespace ts
diff --git a/src/Makefile.am b/src/Makefile.am
index 8e95c0c4a9..e6aaddec1d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -23,7 +23,7 @@ TESTS =
lib_LTLIBRARIES =
noinst_PROGRAMS =
-SUBDIRS = tscpp/api
+SUBDIRS = tscpp/api api
if BUILD_WCCP
SUBDIRS += wccp
diff --git a/src/Makefile.am b/src/api/Makefile.am
similarity index 58%
copy from src/Makefile.am
copy to src/api/Makefile.am
index 8e95c0c4a9..4a78c0741d 100644
--- a/src/Makefile.am
+++ b/src/api/Makefile.am
@@ -1,3 +1,4 @@
+# libts Makefile.am
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
@@ -17,28 +18,31 @@
include $(top_srcdir)/mk/tidy.mk
-bin_PROGRAMS =
-check_PROGRAMS =
-TESTS =
-lib_LTLIBRARIES =
-noinst_PROGRAMS =
-
-SUBDIRS = tscpp/api
-
-if BUILD_WCCP
-SUBDIRS += wccp
-include traffic_wccp/Makefile.inc
-endif
-
-include traffic_cache_tool/Makefile.inc
-include traffic_via/Makefile.inc
-include traffic_top/Makefile.inc
-include traffic_server/Makefile.inc
-include traffic_logstats/Makefile.inc
-include traffic_crashlog/Makefile.inc
-include traffic_layout/Makefile.inc
-include traffic_logcat/Makefile.inc
-include traffic_ctl/Makefile.inc
+# check_PROGRAMS = test_api
+
+# TESTS_ENVIRONMENT = LSAN_OPTIONS=suppressions=$(abs_top_srcdir)/ci/asan_leak_suppression/unit_tests.txt
+# TESTS = $(check_PROGRAMS)
+
+AM_CPPFLAGS += \
+ @SWOC_INCLUDES@ \
+ $(TS_INCLUDES)
+
+noinst_LIBRARIES = libtsapi.a
+
+libtsapi_a_SOURCES = \
+ Metrics.cc
+
+# test_tsapi_CPPFLAGS = $(AM_CPPFLAGS)\
+# -I$(abs_top_srcdir)/lib/catch2
+
+# test_tsapi_CXXFLAGS = -Wno-array-bounds $(AM_CXXFLAGS)
+# test_tsapi_LDADD = libtsapi.la @SWOC_LIBS@
+# test_tsapi_SOURCES = \
+# unit_tests/test_Metrics.cc
+
+# clean-local:
+# rm -f ParseRulesCType ParseRulesCTypeToLower ParseRulesCTypeToUpper
+
clang-tidy-local: $(DIST_SOURCES)
$(CXX_Clang_Tidy)
diff --git a/src/api/Metrics.cc b/src/api/Metrics.cc
new file mode 100644
index 0000000000..a95588c172
--- /dev/null
+++ b/src/api/Metrics.cc
@@ -0,0 +1,100 @@
+/** @file
+
+ The implementations of the Metrics API class.
+
+ @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.
+ */
+
+#include "ts/ts.h"
+#include "api/Metrics.h"
+
+namespace ts
+{
+
+// This is the singleton instance of the metrics class.
+Metrics &
+Metrics::getInstance()
+{
+ static ts::Metrics _instance;
+
+ return _instance;
+}
+
+void
+Metrics::_addBlob() // The mutex must be held before calling this!
+{
+ auto blob = new Metrics::MetricStorage();
+
+ ink_assert(blob);
+ ink_assert(_cur_blob < Metrics::METRICS_MAX_BLOBS);
+
+ _blobs[_cur_blob++] = blob;
+ _cur_off = 0;
+}
+
+Metrics::IdType
+Metrics::newMetric(std::string_view name)
+{
+ std::lock_guard<std::mutex> lock(_mutex);
+ auto it = _lookups.find(name);
+
+ if (it != _lookups.end()) {
+ return it->second;
+ }
+
+ Metrics::IdType id = (_cur_blob << 16 | _cur_off);
+ Metrics::MetricStorage *blob = _blobs[_cur_blob];
+ Metrics::NameContainer &names = std::get<0>(*blob);
+ Metrics::AtomicContainer &atomics = std::get<1>(*blob);
+
+ atomics[_cur_off].store(0);
+ names[_cur_off] = std::make_tuple(std::string(name), id);
+ _lookups.emplace(std::get<0>(names[_cur_off]), id);
+
+ if (++_cur_off >= Metrics::METRICS_MAX_SIZE) {
+ _addBlob(); // This resets _cur_off to 0 as well
+ }
+
+ return id;
+}
+
+Metrics::IdType
+Metrics::lookup(const std::string_view name)
+{
+ std::lock_guard<std::mutex> lock(_mutex);
+ auto it = _lookups.find(name);
+
+ if (it != _lookups.end()) {
+ return it->second;
+ }
+
+ return NOT_FOUND;
+}
+
+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);
+
+ return &(atomics[std::get<1>(idx)]);
+}
+
+} // namespace ts
diff --git a/src/records/RecCore.cc b/src/records/RecCore.cc
index 8fa774aadd..1bf0bd43fe 100644
--- a/src/records/RecCore.cc
+++ b/src/records/RecCore.cc
@@ -34,6 +34,7 @@
#include "records/P_RecUtils.h"
#include "tscore/I_Layout.h"
#include "tscpp/util/ts_errata.h"
+#include "api/Metrics.h"
// This is needed to manage the size of the librecords record. It can't be static, because it needs to be modified
// and used (read) from several binaries / modules.
@@ -1062,6 +1063,10 @@ RecDumpRecords(RecT rec_type, RecDumpEntryCb callback, void *edata)
rec_mutex_release(&(r->lock));
}
}
+ // Also dump the new ts::Metrics if asked for
+ if (rec_type & TS_RECORDTYPE_PLUGIN) {
+ ts::Metrics::getInstance().recordsDump(callback, edata);
+ }
}
void
diff --git a/src/traffic_crashlog/Makefile.inc b/src/traffic_crashlog/Makefile.inc
index ceee194fc6..1b05641718 100644
--- a/src/traffic_crashlog/Makefile.inc
+++ b/src/traffic_crashlog/Makefile.inc
@@ -43,4 +43,5 @@ traffic_crashlog_traffic_crashlog_LDADD = \
$(top_builddir)/iocore/net/libinknet.a \
$(top_builddir)/src/tscore/libtscore.la \
$(top_builddir)/src/tscpp/util/libtscpputil.la \
+ $(top_builddir)/src/api/libtsapi.a \
@HWLOC_LIBS@ @SWOC_LIBS@ @YAMLCPP_LIBS@
diff --git a/src/traffic_layout/Makefile.inc b/src/traffic_layout/Makefile.inc
index 0bdde7f6a0..cdc8f72789 100644
--- a/src/traffic_layout/Makefile.inc
+++ b/src/traffic_layout/Makefile.inc
@@ -46,4 +46,5 @@ traffic_layout_traffic_layout_LDADD = \
$(top_builddir)/src/records/librecords_p.a \
$(top_builddir)/src/tscore/libtscore.la \
$(top_builddir)/src/tscpp/util/libtscpputil.la \
+ $(top_builddir)/src/api/libtsapi.a \
@SWOC_LIBS@ @HWLOC_LIBS@ @YAMLCPP_LIBS@ @LIBLZMA@
diff --git a/src/traffic_logcat/Makefile.inc b/src/traffic_logcat/Makefile.inc
index c9c5a09dca..cb23da73f0 100644
--- a/src/traffic_logcat/Makefile.inc
+++ b/src/traffic_logcat/Makefile.inc
@@ -47,7 +47,8 @@ traffic_logcat_traffic_logcat_LDADD = \
$(top_builddir)/iocore/utils/libinkutils.a \
$(top_builddir)/src/tscore/libtscore.la \
$(top_builddir)/src/tscpp/util/libtscpputil.la \
- $(top_builddir)/mgmt/utils/libutils_p.la
+ $(top_builddir)/mgmt/utils/libutils_p.la \
+ $(top_builddir)/src/api/libtsapi.a
traffic_logcat_traffic_logcat_LDADD += \
@SWOC_LIBS@ @HWLOC_LIBS@ \
diff --git a/src/traffic_logstats/Makefile.inc b/src/traffic_logstats/Makefile.inc
index 7a2ac7f0d1..5f1e40b545 100644
--- a/src/traffic_logstats/Makefile.inc
+++ b/src/traffic_logstats/Makefile.inc
@@ -52,7 +52,8 @@ traffic_logstats_traffic_logstats_LDADD = \
$(top_builddir)/iocore/utils/libinkutils.a \
$(top_builddir)/src/tscore/libtscore.la \
$(top_builddir)/src/tscpp/util/libtscpputil.la \
- $(top_builddir)/mgmt/utils/libutils_p.la
+ $(top_builddir)/mgmt/utils/libutils_p.la \
+ $(top_builddir)/src/api/libtsapi.a
traffic_logstats_traffic_logstats_LDADD += \
@SWOC_LIBS@ \
diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc
index a58bb29981..157f2c8929 100644
--- a/src/traffic_server/InkAPI.cc
+++ b/src/traffic_server/InkAPI.cc
@@ -34,6 +34,7 @@
#include "tscore/I_Layout.h"
#include "tscore/I_Version.h"
#include "tscore/Diags.h"
+#include "api/Metrics.h"
#include "InkAPIInternal.h"
#include "Log.h"
@@ -102,10 +103,6 @@
extern AppVersionInfo appVersionInfo;
-// Globals for new librecords stats
-static int api_rsb_index;
-static RecRawStatBlock *api_rsb;
-
/** Reservation for a user arg.
*/
struct UserArg {
@@ -752,11 +749,7 @@ sdk_sanity_check_null_ptr(void const *ptr)
static TSReturnCode
sdk_sanity_check_stat_id(int id)
{
- if (id < 0 || id >= api_rsb->max_stats) {
- return TS_ERROR;
- }
-
- return TS_SUCCESS;
+ return (ts::Metrics::getInstance().valid(id) ? TS_SUCCESS : TS_ERROR);
}
static TSReturnCode
@@ -1829,18 +1822,6 @@ api_init()
lifecycle_hooks = new LifecycleAPIHooks;
global_config_cbs = new ConfigUpdateCbTable;
- int api_metrics = max_records_entries - REC_INTERNAL_RECORDS;
- if (api_metrics > 0) {
- api_rsb = RecAllocateRawStatBlock(api_metrics);
- if (nullptr == api_rsb) {
- Warning("Can't allocate API stats block");
- } else {
- Debug("sdk", "initialized SDK stats APIs with %d slots", api_metrics);
- }
- } else {
- api_rsb = nullptr;
- }
-
// Setup the version string for returning to plugins
ink_strlcpy(traffic_server_version, appVersionInfo.VersionStr, sizeof(traffic_server_version));
// Extract the elements.
@@ -7511,40 +7492,9 @@ TSCacheScan(TSCont contp, TSCacheKey key, int KB_per_second)
int
TSStatCreate(const char *the_name, TSRecordDataType the_type, TSStatPersistence persist, TSStatSync sync)
{
- int id = ink_atomic_increment(&api_rsb_index, 1);
- RecRawStatSyncCb syncer = RecRawStatSyncCount;
-
- // TODO: This only supports "int" data types at this point, since the "Raw" stats
- // interfaces only supports integers. Going forward, we could extend either the "Raw"
- // stats APIs, or make non-int use the direct (synchronous) stats APIs (slower).
- if ((sdk_sanity_check_null_ptr((void *)the_name) != TS_SUCCESS) || (sdk_sanity_check_null_ptr((void *)api_rsb) != TS_SUCCESS) ||
- (id >= api_rsb->max_stats)) {
- return TS_ERROR;
- }
-
- switch (sync) {
- case TS_STAT_SYNC_SUM:
- syncer = RecRawStatSyncSum;
- break;
- case TS_STAT_SYNC_AVG:
- syncer = RecRawStatSyncAvg;
- break;
- case TS_STAT_SYNC_TIMEAVG:
- syncer = RecRawStatSyncHrTimeAvg;
- break;
- default:
- syncer = RecRawStatSyncCount;
- break;
- }
+ int id = ts::Metrics::getInstance().newMetric(the_name);
- switch (persist) {
- case TS_STAT_PERSISTENT:
- RecRegisterRawStat(api_rsb, RECT_PLUGIN, the_name, (RecDataT)the_type, RECP_PERSISTENT, id, syncer);
- break;
- case TS_STAT_NON_PERSISTENT:
- RecRegisterRawStat(api_rsb, RECT_PLUGIN, the_name, (RecDataT)the_type, RECP_NON_PERSISTENT, id, syncer);
- break;
- default:
+ if (id == ts::Metrics::NOT_FOUND) {
return TS_ERROR;
}
@@ -7555,49 +7505,44 @@ void
TSStatIntIncrement(int id, TSMgmtInt amount)
{
sdk_assert(sdk_sanity_check_stat_id(id) == TS_SUCCESS);
- RecIncrRawStat(api_rsb, nullptr, id, amount);
+ ts::Metrics::getInstance().increment(id, amount);
}
void
TSStatIntDecrement(int id, TSMgmtInt amount)
{
- RecDecrRawStat(api_rsb, nullptr, id, amount);
+ sdk_assert(sdk_sanity_check_stat_id(id) == TS_SUCCESS);
+ ts::Metrics::getInstance().decrement(id, amount);
}
TSMgmtInt
TSStatIntGet(int id)
{
- TSMgmtInt value;
-
sdk_assert(sdk_sanity_check_stat_id(id) == TS_SUCCESS);
- RecGetGlobalRawStatSum(api_rsb, id, &value);
- return value;
+ return ts::Metrics::getInstance().get(id);
}
void
TSStatIntSet(int id, TSMgmtInt value)
{
sdk_assert(sdk_sanity_check_stat_id(id) == TS_SUCCESS);
- RecSetGlobalRawStatSum(api_rsb, id, value);
+ ts::Metrics::getInstance().set(id, value);
}
TSReturnCode
TSStatFindName(const char *name, int *idp)
{
- int id;
-
sdk_assert(sdk_sanity_check_null_ptr((void *)name) == TS_SUCCESS);
+ sdk_assert(sdk_sanity_check_null_ptr((void *)idp) == TS_SUCCESS);
- if (RecGetRecordOrderAndId(name, nullptr, &id, true, true) != REC_ERR_OKAY) {
- return TS_ERROR;
- }
+ int id = ts::Metrics::getInstance().lookup(name);
- if (RecGetGlobalRawStatPtr(api_rsb, id) == nullptr) {
+ if (id == ts::Metrics::NOT_FOUND) {
return TS_ERROR;
+ } else {
+ *idp = id;
+ return TS_SUCCESS;
}
-
- *idp = id;
- return TS_SUCCESS;
}
/************************** Tracing API ****************************/
diff --git a/src/traffic_server/Makefile.inc b/src/traffic_server/Makefile.inc
index 54469db224..e4ba7b9c0c 100644
--- a/src/traffic_server/Makefile.inc
+++ b/src/traffic_server/Makefile.inc
@@ -81,6 +81,7 @@ traffic_server_traffic_server_LDADD = \
$(top_builddir)/iocore/aio/libinkaio.a \
$(top_builddir)/src/tscore/libtscore.la \
$(top_builddir)/src/tscpp/util/libtscpputil.la \
+ $(top_builddir)/src/api/libtsapi.a \
$(top_builddir)/proxy/libproxy.a \
$(top_builddir)/iocore/net/libinknet.a \
$(top_builddir)/src/records/librecords_p.a \