diff options
author | Arne Juul <arnej@yahoo-inc.com> | 2017-12-04 11:10:55 +0000 |
---|---|---|
committer | Arne Juul <arnej@yahoo-inc.com> | 2017-12-05 13:11:20 +0000 |
commit | 70b757e310533b6ba97d53a6a13733b6ee480243 (patch) | |
tree | 8d30309e26734fbd61ca82542332902501193c2f | |
parent | fcbd742e57abb3c03ec9de18309840479e5d6dc0 (diff) |
update metrics library after review
* move ticker kill
* use relaxed JSON in test
* rewrite loops in MockTick
* no need for atomics when protected by mutex
* add alive() to Tick API
* hook up total metrics
* add some documentation comments
24 files changed, 182 insertions, 55 deletions
diff --git a/staging_vespalib/src/tests/metrics/mock_tick.cpp b/staging_vespalib/src/tests/metrics/mock_tick.cpp index e78b7c4644f..2178b5364e9 100644 --- a/staging_vespalib/src/tests/metrics/mock_tick.cpp +++ b/staging_vespalib/src/tests/metrics/mock_tick.cpp @@ -3,43 +3,49 @@ namespace vespalib::metrics { -const std::chrono::seconds oneSec{1}; - TimeStamp MockTick::next(TimeStamp prev) { std::unique_lock<std::mutex> locker(_lock); _prevValue = prev; - while (_runFlag) { - if (_provided) { - _blocked.store(false); - _provided.store(false); - return _nextValue; - } - _blocked.store(true); - _blockedCond.notify_all(); - auto r = _providedCond.wait_for(locker, oneSec); - (void)r; + _blocked = true; + _blockedCond.notify_all(); + while (_runFlag && !_provided) { + _providedCond.wait(locker); + } + _blocked = false; + if (_provided) { + _provided = false; + return _nextValue; + } else { + // killed + return TimeStamp(0); } - return TimeStamp(0); } void MockTick::kill() { std::unique_lock<std::mutex> locker(_lock); - _runFlag.store(false); + _runFlag = false; _blockedCond.notify_all(); _providedCond.notify_all(); } +bool +MockTick::alive() +{ + std::unique_lock<std::mutex> locker(_lock); + return _runFlag; +} + void MockTick::provide(TimeStamp value) { std::unique_lock<std::mutex> locker(_lock); _nextValue = value; - _blocked.store(false); - _provided.store(true); + _blocked = false; + _provided = true; _providedCond.notify_all(); } @@ -47,14 +53,15 @@ TimeStamp MockTick::waitUntilBlocked() { std::unique_lock<std::mutex> locker(_lock); - while (_runFlag) { - if (_blocked) { - return _prevValue; - } - auto r = _blockedCond.wait_for(locker, oneSec); - (void)r; + while (_runFlag && !_blocked) { + _blockedCond.wait(locker); + } + if (_blocked) { + return _prevValue; + } else { + // killed + return TimeStamp(0); } - return TimeStamp(0); } MockTick::MockTick() diff --git a/staging_vespalib/src/tests/metrics/mock_tick.h b/staging_vespalib/src/tests/metrics/mock_tick.h index 2f69f69fc91..e0336260880 100644 --- a/staging_vespalib/src/tests/metrics/mock_tick.h +++ b/staging_vespalib/src/tests/metrics/mock_tick.h @@ -12,9 +12,9 @@ namespace vespalib::metrics { class MockTick : public Tick { private: std::mutex _lock; - std::atomic<bool> _runFlag; - std::atomic<bool> _provided; - std::atomic<bool> _blocked; + bool _runFlag; + bool _provided; + bool _blocked; std::condition_variable _providedCond; std::condition_variable _blockedCond; TimeStamp _nextValue; @@ -23,6 +23,7 @@ public: MockTick(); TimeStamp next(TimeStamp prev) override; void kill() override; + bool alive() override; void provide(TimeStamp value); TimeStamp waitUntilBlocked(); @@ -33,6 +34,7 @@ struct TickProxy : Tick { TickProxy(std::shared_ptr<Tick> tick_in) : tick(std::move(tick_in)) {} TimeStamp next(TimeStamp prev) override { return tick->next(prev); } void kill() override { tick->kill(); } + bool alive() override { return tick->alive(); } }; } // namespace vespalib::metrics diff --git a/staging_vespalib/src/tests/metrics/simple_metrics_test.cpp b/staging_vespalib/src/tests/metrics/simple_metrics_test.cpp index 80030cb89b2..463a285e7b3 100644 --- a/staging_vespalib/src/tests/metrics/simple_metrics_test.cpp +++ b/staging_vespalib/src/tests/metrics/simple_metrics_test.cpp @@ -90,24 +90,24 @@ fprintf(stderr, "bad json b\n"); void check_json(const vespalib::string &actual) { vespalib::string expect = "{" - " \"snapshot\": { \"from\": 1, \"to\": 4 }," - " \"values\": [ { \"name\": \"foo\"," - " \"values\": { \"count\": 17, \"rate\": 4.85714 }" + " snapshot: { from: 1, to: 4 }," + " values: [ { name: 'foo'," + " values: { count: 17, rate: 4.85714 }" " }, {" - " \"name\": \"foo\"," - " \"dimensions\": { \"chain\": \"default\", \"documenttype\": \"music\", \"thread\": \"0\" }," - " \"values\": { \"count\": 4, \"rate\": 1.14286 }" + " name: 'foo'," + " dimensions: { chain: 'default', documenttype: 'music', thread: '0' }," + " values: { count: 4, rate: 1.14286 }" " }, {" - " \"name\": \"bar\"," - " \"values\": { \"count\": 4, \"rate\": 1.14286, \"average\": 42, \"min\": 41, \"max\": 43, \"last\": 42 }" + " name: 'bar'," + " values: { count: 4, rate: 1.14286, average: 42, min: 41, max: 43, last: 42 }" " }, {" - " \"name\": \"bar\"," - " \"dimensions\": { \"chain\": \"vespa\", \"documenttype\": \"blogpost\", \"thread\": \"1\" }," - " \"values\": { \"count\": 1, \"rate\": 0.285714, \"average\": 14, \"min\": 14, \"max\": 14, \"last\": 14 }" + " name: 'bar'," + " dimensions: { chain: 'vespa', documenttype: 'blogpost', thread: '1' }," + " values: { count: 1, rate: 0.285714, average: 14, min: 14, max: 14, last: 14 }" " }, {" - " \"name\": \"bar\"," - " \"dimensions\": { \"chain\": \"vespa\", \"documenttype\": \"blogpost\", \"thread\": \"2\" }," - " \"values\": { \"count\": 1, \"rate\": 0.285714, \"average\": 11, \"min\": 11, \"max\": 11, \"last\": 11 }" + " name: 'bar'," + " dimensions: { chain: 'vespa', documenttype: 'blogpost', thread: '2' }," + " values: { count: 1, rate: 0.285714, average: 11, min: 11, max: 11, last: 11 }" " } ]" "}"; EXPECT_TRUE(compare_json(expect, actual)); diff --git a/staging_vespalib/src/vespa/vespalib/metrics/clock.h b/staging_vespalib/src/vespa/vespalib/metrics/clock.h index d46c691b648..a5c29cf8579 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/clock.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/clock.h @@ -23,6 +23,7 @@ struct Tick { virtual TimeStamp next(TimeStamp prev) = 0; TimeStamp first() { return next(TimeStamp(0.0)); } virtual void kill() = 0; + virtual bool alive() = 0; virtual ~Tick() {} }; diff --git a/staging_vespalib/src/vespa/vespalib/metrics/counter.h b/staging_vespalib/src/vespa/vespalib/metrics/counter.h index 1af61638d6b..36a25adda2d 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/counter.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/counter.h @@ -12,6 +12,9 @@ class MetricsManager; class CounterAggregator; +/** + * Represents a counter metric that can be incremented. + **/ class Counter { std::shared_ptr<MetricsManager> _manager; MetricName _id; @@ -30,8 +33,14 @@ public: void add(Point p) { add(1, p); } void add(size_t count) const { add(count, Point::empty); } + /** + * Increment the counter. + * @param count the amount to increment by (default 1) + * @param p the point representing labels for this increment (default empty) + **/ void add(size_t count, Point p) const; + // internal struct Increment { MetricIdentifier idx; size_t value; diff --git a/staging_vespalib/src/vespa/vespalib/metrics/gauge.h b/staging_vespalib/src/vespa/vespalib/metrics/gauge.h index 5f15a573b73..fa3f826f2b8 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/gauge.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/gauge.h @@ -11,6 +11,9 @@ namespace metrics { class MetricsManager; class GaugeAggregator; +/** + * Represents a gauge metric that can be measured. + **/ class Gauge { private: std::shared_ptr<MetricsManager> _manager; @@ -20,8 +23,14 @@ public: : _manager(std::move(m)), _id(id) {} + /** + * Provide a sample for the gauge. + * @param value the measurement for this sample + * @param p the point representing labels for this sample (default empty) + **/ void sample(double value, Point p = Point::empty) const; + // internal struct Measurement { MetricIdentifier idx; double value; diff --git a/staging_vespalib/src/vespa/vespalib/metrics/json_formatter.h b/staging_vespalib/src/vespa/vespalib/metrics/json_formatter.h index c1dc5302c1a..672b71766ae 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/json_formatter.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/json_formatter.h @@ -9,6 +9,10 @@ namespace vespalib { namespace metrics { +/** + * utility for converting a snapshot to JSON format + * (which can be inserted into /state/v1/metrics page). + **/ class JsonFormatter { private: diff --git a/staging_vespalib/src/vespa/vespalib/metrics/metric_identifier.h b/staging_vespalib/src/vespa/vespalib/metrics/metric_identifier.h index 020b44dc4b7..0fc5af41a81 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/metric_identifier.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/metric_identifier.h @@ -8,6 +8,7 @@ namespace vespalib { namespace metrics { +// internal struct MetricIdentifier { const MetricName _name; const Point _point; diff --git a/staging_vespalib/src/vespa/vespalib/metrics/metric_types.h b/staging_vespalib/src/vespa/vespalib/metrics/metric_types.h index 5bda0230bd4..5c764dcc74e 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/metric_types.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/metric_types.h @@ -8,6 +8,7 @@ namespace vespalib { namespace metrics { +// internal class for typechecking class MetricTypes { static const char *_typeNames[]; public: diff --git a/staging_vespalib/src/vespa/vespalib/metrics/metrics_manager.h b/staging_vespalib/src/vespa/vespalib/metrics/metrics_manager.h index 2742252ed44..c86e5a315dc 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/metrics_manager.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/metrics_manager.h @@ -14,31 +14,72 @@ #include "dimension.h" #include "label.h" -namespace vespalib { -namespace metrics { +namespace vespalib::metrics { +/** + * Interface for a Metrics manager, for creating metrics + * and for fetching snapshots. + **/ class MetricsManager : public std::enable_shared_from_this<MetricsManager> { public: virtual ~MetricsManager() {} - virtual Counter counter(const vespalib::string &name) = 0; // get or create - virtual Gauge gauge (const vespalib::string &name) = 0; // get or create + /** + * Get or create a counter metric. + * @param name the name of the metric. + **/ + virtual Counter counter(const vespalib::string &name) = 0; + /** + * Get or create a gauge metric. + * @param name the name of the metric. + **/ + virtual Gauge gauge(const vespalib::string &name) = 0; + + /** + * Get or create a dimension for labeling metrics. + * @param name the name of the dimension. + **/ virtual Dimension dimension(const vespalib::string &name) = 0; // get or create + + /** + * Get or create a label. + * @param value the label value. + **/ virtual Label label(const vespalib::string &value) = 0; // get or create + + /** + * Create a PointBuilder for labeling metrics. + **/ PointBuilder pointBuilder() { return PointBuilder(shared_from_this()); } - virtual PointBuilder pointBuilder(Point from) = 0; - virtual Point pointFrom(PointMap::BackingMap map) = 0; + /** + * Create a PointBuilder for labeling metrics, starting with + * an Point of already existing dimension/label pairs, which can + * then be added to or changed. + * @param from provide a Point to start from. + * + **/ + virtual PointBuilder pointBuilder(Point from) = 0; + /** + * Create a snapshot of sampled metrics (usually for the last minute). + **/ virtual Snapshot snapshot() = 0; + + /** + * Create a snapshot of all sampled metrics the manager has seen. + **/ virtual Snapshot totalSnapshot() = 0; + // for use from PointBuilder only + virtual Point pointFrom(PointMap::BackingMap map) = 0; + // for use from Counter only virtual void add(Counter::Increment inc) = 0; @@ -48,4 +89,3 @@ public: } // namespace vespalib::metrics -} // namespace vespalib diff --git a/staging_vespalib/src/vespa/vespalib/metrics/name_collection.h b/staging_vespalib/src/vespa/vespalib/metrics/name_collection.h index 6fd25552d6e..566fd2a3997 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/name_collection.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/name_collection.h @@ -9,6 +9,7 @@ namespace vespalib { namespace metrics { +// internal class NameCollection { private: using Map = std::map<vespalib::string, size_t>; diff --git a/staging_vespalib/src/vespa/vespalib/metrics/point_builder.h b/staging_vespalib/src/vespa/vespalib/metrics/point_builder.h index 84d4c7fb569..83b804228dd 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/point_builder.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/point_builder.h @@ -12,21 +12,42 @@ namespace metrics { class MetricsManager; +/** + * Build a Point for labeling metrics + **/ class PointBuilder { private: std::shared_ptr<MetricsManager> _owner; PointMap::BackingMap _map; public: + // for use from MetricsManager PointBuilder(std::shared_ptr<MetricsManager> m); PointBuilder(std::shared_ptr<MetricsManager> m, const PointMap::BackingMap &from); ~PointBuilder() {} + /** + * Bind a dimension to a label. + * Overwrites any label already bound to that dimension. + **/ PointBuilder &&bind(Dimension dimension, Label label) &&; + + /** + * Bind a dimension to a label. + * Convenience method that converts the label value. + **/ PointBuilder &&bind(Dimension dimension, LabelValue label) &&; + + /** + * Bind a dimension to a label. + * Convenience method that converts both the dimension name and the label value. + **/ PointBuilder &&bind(DimensionName dimension, LabelValue label) &&; + /** make a Point from the builder */ Point build(); + + /** make a Point from the builder */ operator Point () &&; }; diff --git a/staging_vespalib/src/vespa/vespalib/metrics/point_map.h b/staging_vespalib/src/vespa/vespalib/metrics/point_map.h index 2810aa1aa9d..2ed50a85842 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/point_map.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/point_map.h @@ -8,6 +8,7 @@ namespace vespalib { namespace metrics { +// internal class PointMap { public: using BackingMap = std::map<Dimension, Label>; diff --git a/staging_vespalib/src/vespa/vespalib/metrics/point_map_collection.h b/staging_vespalib/src/vespa/vespalib/metrics/point_map_collection.h index 83891e7fa0c..ba301ff3f06 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/point_map_collection.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/point_map_collection.h @@ -9,6 +9,7 @@ namespace vespalib { namespace metrics { +// internal class PointMapCollection { private: using PointMapMap = std::map<PointMap, size_t>; diff --git a/staging_vespalib/src/vespa/vespalib/metrics/producer.cpp b/staging_vespalib/src/vespa/vespalib/metrics/producer.cpp index a848628a4c0..ae8d6d76eac 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/producer.cpp +++ b/staging_vespalib/src/vespa/vespalib/metrics/producer.cpp @@ -22,8 +22,9 @@ Producer::getMetrics(const vespalib::string &) vespalib::string Producer::getTotalMetrics(const vespalib::string &) { - // not implemented - return ""; + Snapshot snap = _manager->totalSnapshot(); + JsonFormatter fmt(snap); + return fmt.asString(); } diff --git a/staging_vespalib/src/vespa/vespalib/metrics/producer.h b/staging_vespalib/src/vespa/vespalib/metrics/producer.h index c1a9cae86ed..387d94379b7 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/producer.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/producer.h @@ -9,6 +9,9 @@ namespace metrics { class MetricsManager; +/** + * Utility class for wiring a MetricsManager into a StateApi. + **/ class Producer : public vespalib::MetricsProducer { private: std::shared_ptr<MetricsManager> _manager; diff --git a/staging_vespalib/src/vespa/vespalib/metrics/simple_metrics.h b/staging_vespalib/src/vespa/vespalib/metrics/simple_metrics.h index 826a43f6713..6e6cff55b1f 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/simple_metrics.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/simple_metrics.h @@ -5,3 +5,19 @@ #include <chrono> #include <memory> #include <vespa/vespalib/stllike/string.h> + +#include "clock.h" +#include "counter.h" +#include "dimension.h" +#include "dummy_metrics_manager.h" +#include "gauge.h" +#include "json_formatter.h" +#include "label.h" +#include "metric_identifier.h" +#include "metric_name.h" +#include "metrics_manager.h" +#include "point_builder.h" +#include "point.h" +#include "producer.h" +#include "simple_metrics_manager.h" +#include "snapshots.h" diff --git a/staging_vespalib/src/vespa/vespalib/metrics/simple_metrics_manager.cpp b/staging_vespalib/src/vespa/vespalib/metrics/simple_metrics_manager.cpp index 90c9ef0eab6..b9b3415a0e3 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/simple_metrics_manager.cpp +++ b/staging_vespalib/src/vespa/vespalib/metrics/simple_metrics_manager.cpp @@ -25,7 +25,6 @@ SimpleMetricsManager::SimpleMetricsManager(const SimpleManagerConfig &config, _firstBucket(0), _maxBuckets(config.sliding_window_seconds), _totalsBucket(0, _startTime, _startTime), - _runFlag(true), _thread(&SimpleMetricsManager::tickerLoop, this) { if (_maxBuckets < 1) _maxBuckets = 1; @@ -35,7 +34,6 @@ SimpleMetricsManager::SimpleMetricsManager(const SimpleManagerConfig &config, SimpleMetricsManager::~SimpleMetricsManager() { - _tickSupplier->kill(); stopThread(); } @@ -199,16 +197,18 @@ SimpleMetricsManager::pointFrom(PointMap::BackingMap map) void SimpleMetricsManager::tickerLoop() { - while (_runFlag) { + while (_tickSupplier->alive()) { TimeStamp now = _tickSupplier->next(_curTime); - tick(now); + if (_tickSupplier->alive()) { + tick(now); + } } } void SimpleMetricsManager::stopThread() { - _runFlag.store(false); + _tickSupplier->kill(); _thread.join(); } diff --git a/staging_vespalib/src/vespa/vespalib/metrics/simple_metrics_manager.h b/staging_vespalib/src/vespa/vespalib/metrics/simple_metrics_manager.h index aa2efba49be..693b3f65ec6 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/simple_metrics_manager.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/simple_metrics_manager.h @@ -57,7 +57,6 @@ private: size_t _maxBuckets; Bucket _totalsBucket; - std::atomic<bool> _runFlag; std::thread _thread; void tickerLoop(); void stopThread(); diff --git a/staging_vespalib/src/vespa/vespalib/metrics/simple_tick.cpp b/staging_vespalib/src/vespa/vespalib/metrics/simple_tick.cpp index c71655ff7d0..0bcfc1ea272 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/simple_tick.cpp +++ b/staging_vespalib/src/vespa/vespalib/metrics/simple_tick.cpp @@ -46,4 +46,11 @@ SimpleTick::kill() _cond.notify_all(); } +bool +SimpleTick::alive() +{ + std::unique_lock<std::mutex> locker(_lock); + return _runFlag; +} + } // namespace vespalib::metrics diff --git a/staging_vespalib/src/vespa/vespalib/metrics/simple_tick.h b/staging_vespalib/src/vespa/vespalib/metrics/simple_tick.h index 653944ea257..959dd6b4d98 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/simple_tick.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/simple_tick.h @@ -9,6 +9,7 @@ namespace vespalib::metrics { +// internal class SimpleTick : public Tick { private: std::mutex _lock; @@ -18,6 +19,7 @@ public: SimpleTick(); TimeStamp next(TimeStamp prev) override; void kill() override; + bool alive() override; }; } // namespace vespalib::metrics diff --git a/staging_vespalib/src/vespa/vespalib/metrics/stable_store.h b/staging_vespalib/src/vespa/vespalib/metrics/stable_store.h index ca9db21a2d8..b62e0f2a94e 100644 --- a/staging_vespalib/src/vespa/vespalib/metrics/stable_store.h +++ b/staging_vespalib/src/vespa/vespalib/metrics/stable_store.h @@ -8,6 +8,7 @@ namespace vespalib { +/** metrics-internal utility class */ template <typename T> class StableStore { diff --git a/staging_vespalib/src/vespa/vespalib/metrics/ticker_thread.cpp b/staging_vespalib/src/vespa/vespalib/metrics/ticker_thread.cpp deleted file mode 100644 index e69de29bb2d..00000000000 --- a/staging_vespalib/src/vespa/vespalib/metrics/ticker_thread.cpp +++ /dev/null diff --git a/staging_vespalib/src/vespa/vespalib/metrics/ticker_thread.h b/staging_vespalib/src/vespa/vespalib/metrics/ticker_thread.h deleted file mode 100644 index e69de29bb2d..00000000000 --- a/staging_vespalib/src/vespa/vespalib/metrics/ticker_thread.h +++ /dev/null |