zwoop commented on code in PR #10694: URL: https://github.com/apache/trafficserver/pull/10694#discussion_r1384383644
########## include/api/Metrics.h: ########## @@ -289,10 +280,126 @@ class Metrics mutable std::mutex _mutex; LookupTable _lookups; - MetricBlobs _blobs; + BlobStorage _blobs; uint16_t _cur_blob = 0; uint16_t _cur_off = 0; +public: + // These are sort of factory classes, using the Metrics singleton for all storage etc. + class Gauge + { + public: + using self_type = Gauge; + using SpanType = Metrics::SpanType; + + class AtomicType : public Metrics::AtomicType + { + }; + + static Metrics::IdType + create(const std::string_view name) + { + auto &instance = Metrics::instance(); + + return instance._create(name); + } + + static AtomicType * + createPtr(const std::string_view name) + { + auto &instance = Metrics::instance(); + + return reinterpret_cast<AtomicType *>(instance.lookup(instance._create(name))); + } + + static Metrics::Gauge::SpanType + createSpan(size_t size, IdType *id = nullptr) + { + auto &instance = Metrics::instance(); + + return instance._createSpan(size, id); + } + + static void + increment(AtomicType *metric, uint64_t val = 1) + { + ink_assert(metric); + metric->_value.fetch_add(val, MEMORY_ORDER); + } + + static void + decrement(AtomicType *metric, uint64_t val = 1) + { + ink_assert(metric); + metric->_value.fetch_sub(val, MEMORY_ORDER); + } + + static int64_t + load(AtomicType *metric) + { + ink_assert(metric); + return metric->_value.load(); + } + + static void + store(AtomicType *metric, int64_t val) + { + ink_assert(metric); + return metric->_value.store(val); + } + + }; // class Gauge + + class Counter + { + public: + using self_type = Counter; + using SpanType = Metrics::SpanType; + + class AtomicType : public Metrics::AtomicType + { + }; + + static Metrics::IdType + create(const std::string_view name) + { + auto &instance = Metrics::instance(); + + return instance._create(name); + } + + static AtomicType * + createPtr(const std::string_view name) + { + auto &instance = Metrics::instance(); + + return reinterpret_cast<AtomicType *>(instance.lookup(instance._create(name))); + } + + static Metrics::Counter::SpanType + createSpan(size_t size, IdType *id = nullptr) + { + auto &instance = Metrics::instance(); + + return instance._createSpan(size, id); + } + + static void + increment(AtomicType *metric, uint64_t val = 1) + { + ink_assert(metric); + metric->_value.fetch_add(val, MEMORY_ORDER); + } + + static int64_t + load(AtomicType *metric) + { + ink_assert(metric); + return metric->_value.load(); + } + + }; // class Counter Review Comment: Well, I'll probably argue that if you need to do clear(), you are using this wrong, and it should be a gauge. Counters are supposed to always increase, randomly setting it to 0 (or any value) is wrong, and why I even bothered adding this difference of Counters vs Gauges. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@trafficserver.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org