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:07 UTC
[trafficserver] branch NewAPIMetricsPOC created (now e550beb625)
This is an automated email from the ASF dual-hosted git repository.
zwoop pushed a change to branch NewAPIMetricsPOC
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
at e550beb625 Initial cut at a new Metrics API
This branch includes the following new commits:
new e550beb625 Initial cut at a new Metrics API
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[trafficserver] 01/01: Initial cut at a new Metrics API
Posted by zw...@apache.org.
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 \