This is an automated email from the ASF dual-hosted git repository. cmcfarlen pushed a commit to branch NewAPIMetricsPOC in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit e36897821358c4f93f317bd541719ae7a0838601 Author: Leif Hedstrom <zw...@apache.org> AuthorDate: Thu Jul 6 15:42:50 2023 -0600 Adding some comments, because you will ask --- include/api/Metrics.h | 6 +++++- src/api/Metrics.cc | 5 ++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/include/api/Metrics.h b/include/api/Metrics.h index 0ab82324b8..1c6a7e95a3 100644 --- a/include/api/Metrics.h +++ b/include/api/Metrics.h @@ -40,7 +40,7 @@ class Metrics { private: using self_type = Metrics; - using IdType = int32_t; + 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: @@ -69,6 +69,10 @@ public: // Singleton static Metrics &getInstance(); + // Yes, we don't return objects here, but rather ID's and atomic's directly. We could + // make these objects, but that's a lot of object instances to manage, and right now the + // ID in the containers is very small and sufficient to work with. But agreed, it's not + // very C++-like (but compatible with old librecords ATS code!). IdType newMetric(const std::string_view name); IdType lookup(const std::string_view name); AtomicType *lookup(IdType id) const; diff --git a/src/api/Metrics.cc b/src/api/Metrics.cc index c87239b61a..fc500df46a 100644 --- a/src/api/Metrics.cc +++ b/src/api/Metrics.cc @@ -97,6 +97,9 @@ Metrics::lookup(IdType id) const return &(atomics[std::get<1>(idx)]); } +// ToDo: This is probably not great, and likely we should have some better +// way exposing iterators over the Metrics. That avoids this ugly dependency +// between librecords and this code. void Metrics::recordsDump(RecDumpEntryCb callback, void *edata) { @@ -123,7 +126,7 @@ Metrics::recordsDump(RecDumpEntryCb callback, void *edata) 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); + callback(RECT_PLUGIN, edata, 1, std::get<0>(names[j]).c_str(), TS_RECORDDATATYPE_INT, &datum); } } }