# HG changeset patch # User Sebastien Jodogne # Date 1687881309 -7200 # Node ID b376abae664aa6e50a8cd199aa6e014f0e0c871b # Parent dd9795dc380d4609d6308522c6979f6ff554e75d Metrics can be stored either as floating-point numbers, or as integers diff -r dd9795dc380d -r b376abae664a NEWS --- a/NEWS Tue Jun 27 15:56:04 2023 +0200 +++ b/NEWS Tue Jun 27 17:55:09 2023 +0200 @@ -31,7 +31,7 @@ (https://discourse.orthanc-server.org/t/issue-with-deleting-incoming-dicoms-when-maximumstoragesize-is-reached/3510) * When deleting a resource, its parents LastUpdate metadata are now updated * Reduced the memory usage when downloading archives when "ZipLoaderThreads" > 0 -* Metrics are now stored as integers instead of floats to avoid precision loss in increments +* Metrics can be stored either as floating-point numbers, or as integers * Upgraded dependencies for static builds: - boost 1.82.0 diff -r dd9795dc380d -r b376abae664a OrthancFramework/Sources/FileStorage/StorageAccessor.cpp --- a/OrthancFramework/Sources/FileStorage/StorageAccessor.cpp Tue Jun 27 15:56:04 2023 +0200 +++ b/OrthancFramework/Sources/FileStorage/StorageAccessor.cpp Tue Jun 27 17:55:09 2023 +0200 @@ -125,7 +125,7 @@ if (metrics_ != NULL) { - metrics_->IncrementValue(METRICS_WRITTEN_BYTES, size); + metrics_->IncrementIntegerValue(METRICS_WRITTEN_BYTES, size); } if (cache_ != NULL) @@ -165,7 +165,7 @@ if (metrics_ != NULL) { - metrics_->IncrementValue(METRICS_WRITTEN_BYTES, compressed.size()); + metrics_->IncrementIntegerValue(METRICS_WRITTEN_BYTES, compressed.size()); } if (cache_ != NULL) @@ -211,7 +211,7 @@ if (metrics_ != NULL) { - metrics_->IncrementValue(METRICS_READ_BYTES, buffer->GetSize()); + metrics_->IncrementIntegerValue(METRICS_READ_BYTES, buffer->GetSize()); } buffer->MoveToString(content); @@ -232,7 +232,7 @@ if (metrics_ != NULL) { - metrics_->IncrementValue(METRICS_READ_BYTES, compressed->GetSize()); + metrics_->IncrementIntegerValue(METRICS_READ_BYTES, compressed->GetSize()); } zlib.Uncompress(content, compressed->GetData(), compressed->GetSize()); @@ -271,7 +271,7 @@ if (metrics_ != NULL) { - metrics_->IncrementValue(METRICS_READ_BYTES, buffer->GetSize()); + metrics_->IncrementIntegerValue(METRICS_READ_BYTES, buffer->GetSize()); } buffer->MoveToString(content); @@ -317,7 +317,7 @@ if (metrics_ != NULL) { - metrics_->IncrementValue(METRICS_READ_BYTES, buffer->GetSize()); + metrics_->IncrementIntegerValue(METRICS_READ_BYTES, buffer->GetSize()); } buffer->MoveToString(target); diff -r dd9795dc380d -r b376abae664a OrthancFramework/Sources/MetricsRegistry.cpp --- a/OrthancFramework/Sources/MetricsRegistry.cpp Tue Jun 27 15:56:04 2023 +0200 +++ b/OrthancFramework/Sources/MetricsRegistry.cpp Tue Jun 27 17:55:09 2023 +0200 @@ -28,6 +28,8 @@ #include "Compatibility.h" #include "OrthancException.h" +#include + namespace Orthanc { static const boost::posix_time::ptime GetNow() @@ -35,135 +37,277 @@ return boost::posix_time::microsec_clock::universal_time(); } + namespace + { + template + class TimestampedValue : public boost::noncopyable + { + private: + boost::posix_time::ptime time_; + bool hasValue_; + T value_; + + void SetValue(const T& value, + const boost::posix_time::ptime& now) + { + hasValue_ = true; + value_ = value; + time_ = now; + } + + bool IsLargerOverPeriod(const T& value, + int duration, + const boost::posix_time::ptime& now) const + { + if (hasValue_) + { + return (value > value_ || + (now - time_).total_seconds() > duration /* old value has expired */); + } + else + { + return true; // No value yet + } + } + + bool IsSmallerOverPeriod(const T& value, + int duration, + const boost::posix_time::ptime& now) const + { + if (hasValue_) + { + return (value < value_ || + (now - time_).total_seconds() > duration /* old value has expired */); + } + else + { + return true; // No value yet + } + } + + public: + explicit TimestampedValue() : + hasValue_(false), + value_(0) + { + } + + void Update(const T& value, + const MetricsUpdatePolicy& policy) + { + const boost::posix_time::ptime now = GetNow(); + + switch (policy) + { + case MetricsUpdatePolicy_Directly: + SetValue(value, now); + break; + + case MetricsUpdatePolicy_MaxOver10Seconds: + if (IsLargerOverPeriod(value, 10, now)) + { + SetValue(value, now); + } + break; + + case MetricsUpdatePolicy_MaxOver1Minute: + if (IsLargerOverPeriod(value, 60, now)) + { + SetValue(value, now); + } + break; + + case MetricsUpdatePolicy_MinOver10Seconds: + if (IsSmallerOverPeriod(value, 10, now)) + { + SetValue(value, now); + } + break; + + case MetricsUpdatePolicy_MinOver1Minute: + if (IsSmallerOverPeriod(value, 60, now)) + { + SetValue(value, now); + } + break; + + default: + throw OrthancException(ErrorCode_NotImplemented); + } + } + + void Increment(const T& delta) + { + if (hasValue_) + { + value_ += delta; + } + else + { + value_ = delta; + } + } + + bool HasValue() const + { + return hasValue_; + } + + const boost::posix_time::ptime& GetTime() const + { + if (hasValue_) + { + return time_; + } + else + { + throw OrthancException(ErrorCode_BadSequenceOfCalls); + } + } + + const T& GetValue() const + { + if (hasValue_) + { + return value_; + } + else + { + throw OrthancException(ErrorCode_BadSequenceOfCalls); + } + } + }; + } + + class MetricsRegistry::Item : public boost::noncopyable { private: - MetricsUpdate update_; - boost::posix_time::ptime time_; - bool hasValue_; - int64_t value_; + MetricsUpdatePolicy policy_; - void SetValue(int64_t value, - const boost::posix_time::ptime& now) - { - hasValue_ = true; - value_ = value; - time_ = now; - } - - bool IsLargerOverPeriod(int64_t value, - int duration, - const boost::posix_time::ptime& now) const + public: + Item(MetricsUpdatePolicy policy) : + policy_(policy) { - if (hasValue_) - { - return (value > value_ || - (now - time_).total_seconds() > duration /* old value has expired */); - } - else - { - return true; // No value yet - } } - - bool IsSmallerOverPeriod(int64_t value, - int duration, - const boost::posix_time::ptime& now) const - { - if (hasValue_) - { - return (value < value_ || - (now - time_).total_seconds() > duration /* old value has expired */); - } - else - { - return true; // No value yet - } - } - - public: - explicit Item(MetricsUpdate update) : - update_(update), - hasValue_(false), - value_(0) + + virtual ~Item() { } - MetricsUpdate GetUpdate() const + MetricsUpdatePolicy GetPolicy() const { - return update_; + return policy_; + } + + virtual void UpdateFloat(float value) = 0; + + virtual void UpdateInteger(int64_t value) = 0; + + virtual void IncrementInteger(int64_t delta) = 0; + + virtual MetricsDataType GetDataType() const = 0; + + virtual bool HasValue() const = 0; + + virtual const boost::posix_time::ptime& GetTime() const = 0; + + virtual std::string FormatValue() const = 0; + }; + + + class MetricsRegistry::FloatItem : public Item + { + private: + TimestampedValue value_; + + public: + FloatItem(MetricsUpdatePolicy policy) : + Item(policy) + { + } + + virtual void UpdateFloat(float value) ORTHANC_OVERRIDE + { + value_.Update(value, GetPolicy()); + } + + virtual void UpdateInteger(int64_t value) ORTHANC_OVERRIDE + { + value_.Update(static_cast(value), GetPolicy()); + } + + virtual void IncrementInteger(int64_t delta) ORTHANC_OVERRIDE + { + value_.Increment(static_cast(delta)); + } + + virtual MetricsDataType GetDataType() const ORTHANC_OVERRIDE + { + return MetricsDataType_Float; + } + + virtual bool HasValue() const ORTHANC_OVERRIDE + { + return value_.HasValue(); } - void Update(int64_t value) + virtual const boost::posix_time::ptime& GetTime() const ORTHANC_OVERRIDE { - const boost::posix_time::ptime now = GetNow(); - - switch (update_) - { - case MetricsUpdate_Directly: - SetValue(value, now); - break; - - case MetricsUpdate_MaxOver10Seconds: - if (IsLargerOverPeriod(value, 10, now)) - { - SetValue(value, now); - } - break; + return value_.GetTime(); + } + + virtual std::string FormatValue() const ORTHANC_OVERRIDE + { + return boost::lexical_cast(value_.GetValue()); + } + }; - case MetricsUpdate_MaxOver1Minute: - if (IsLargerOverPeriod(value, 60, now)) - { - SetValue(value, now); - } - break; + + class MetricsRegistry::IntegerItem : public Item + { + private: + TimestampedValue value_; - case MetricsUpdate_MinOver10Seconds: - if (IsSmallerOverPeriod(value, 10, now)) - { - SetValue(value, now); - } - break; - - case MetricsUpdate_MinOver1Minute: - if (IsSmallerOverPeriod(value, 60, now)) - { - SetValue(value, now); - } - break; - - default: - throw OrthancException(ErrorCode_NotImplemented); - } + public: + IntegerItem(MetricsUpdatePolicy policy) : + Item(policy) + { + } + + virtual void UpdateFloat(float value) ORTHANC_OVERRIDE + { + value_.Update(boost::math::llround(value), GetPolicy()); } - bool HasValue() const + virtual void UpdateInteger(int64_t value) ORTHANC_OVERRIDE { - return hasValue_; + value_.Update(value, GetPolicy()); + } + + virtual void IncrementInteger(int64_t delta) ORTHANC_OVERRIDE + { + value_.Increment(delta); } - const boost::posix_time::ptime& GetTime() const + virtual MetricsDataType GetDataType() const ORTHANC_OVERRIDE { - if (hasValue_) - { - return time_; - } - else - { - throw OrthancException(ErrorCode_BadSequenceOfCalls); - } + return MetricsDataType_Integer; } - int64_t GetValue() const + virtual bool HasValue() const ORTHANC_OVERRIDE + { + return value_.HasValue(); + } + + virtual const boost::posix_time::ptime& GetTime() const ORTHANC_OVERRIDE { - if (hasValue_) - { - return value_; - } - else - { - throw OrthancException(ErrorCode_BadSequenceOfCalls); - } + return value_.GetTime(); + } + + virtual std::string FormatValue() const ORTHANC_OVERRIDE + { + return boost::lexical_cast(value_.GetValue()); } }; @@ -191,39 +335,46 @@ void MetricsRegistry::Register(const std::string& name, - MetricsUpdate update) + MetricsUpdatePolicy policy, + MetricsDataType type) { boost::mutex::scoped_lock lock(mutex_); - Content::iterator found = content_.find(name); - - if (found == content_.end()) + if (content_.find(name) != content_.end()) { - content_[name] = new Item(update); + throw OrthancException(ErrorCode_BadSequenceOfCalls, "Cannot register twice the same metrics: " + name); } else { - assert(found->second != NULL); - - // This metrics already exists: Only recreate it if there is a - // mismatch in the type of metrics - if (found->second->GetUpdate() != update) - { - delete found->second; - found->second = new Item(update); - } - } + GetItemInternal(name, policy, type); + } } MetricsRegistry::Item& MetricsRegistry::GetItemInternal(const std::string& name, - MetricsUpdate update) + MetricsUpdatePolicy policy, + MetricsDataType type) { Content::iterator found = content_.find(name); if (found == content_.end()) { - Item* item = new Item(update); + Item* item = NULL; + + switch (type) + { + case MetricsDataType_Float: + item = new FloatItem(policy); + break; + + case MetricsDataType_Integer: + item = new IntegerItem(policy); + break; + + default: + throw OrthancException(ErrorCode_ParameterOutOfRange); + } + content_[name] = item; return *item; } @@ -240,45 +391,49 @@ } - void MetricsRegistry::SetValue(const std::string &name, - int64_t value, - MetricsUpdate update) + void MetricsRegistry::SetFloatValue(const std::string& name, + float value, + MetricsUpdatePolicy policy) { // Inlining to avoid loosing time if metrics are disabled if (enabled_) { boost::mutex::scoped_lock lock(mutex_); - GetItemInternal(name, update).Update(value); + GetItemInternal(name, policy, MetricsDataType_Float).UpdateFloat(value); } } - + - void MetricsRegistry::IncrementValue(const std::string &name, - int64_t delta) + void MetricsRegistry::SetIntegerValue(const std::string &name, + int64_t value, + MetricsUpdatePolicy policy) { // Inlining to avoid loosing time if metrics are disabled if (enabled_) { boost::mutex::scoped_lock lock(mutex_); - Item& item = GetItemInternal(name, MetricsUpdate_Directly); - - if (item.HasValue()) - { - item.Update(item.GetValue() + delta); - } - else - { - item.Update(delta); - } + GetItemInternal(name, policy, MetricsDataType_Integer).UpdateInteger(value); } } - MetricsUpdate MetricsRegistry::GetMetricsUpdate(const std::string& name) + void MetricsRegistry::IncrementIntegerValue(const std::string &name, + int64_t delta) + { + // Inlining to avoid loosing time if metrics are disabled + if (enabled_) + { + boost::mutex::scoped_lock lock(mutex_); + GetItemInternal(name, MetricsUpdatePolicy_Directly, MetricsDataType_Integer).IncrementInteger(delta); + } + } + + + MetricsUpdatePolicy MetricsRegistry::GetUpdatePolicy(const std::string& metrics) { boost::mutex::scoped_lock lock(mutex_); - Content::const_iterator found = content_.find(name); + Content::const_iterator found = content_.find(metrics); if (found == content_.end()) { @@ -287,7 +442,25 @@ else { assert(found->second != NULL); - return found->second->GetUpdate(); + return found->second->GetPolicy(); + } + } + + + MetricsDataType MetricsRegistry::GetDataType(const std::string& metrics) + { + boost::mutex::scoped_lock lock(mutex_); + + Content::const_iterator found = content_.find(metrics); + + if (found == content_.end()) + { + throw OrthancException(ErrorCode_InexistentItem); + } + else + { + assert(found->second != NULL); + return found->second->GetDataType(); } } @@ -318,7 +491,7 @@ boost::posix_time::time_duration diff = it->second->GetTime() - EPOCH; std::string line = (it->first + " " + - boost::lexical_cast(it->second->GetValue()) + " " + + it->second->FormatValue() + " " + boost::lexical_cast(diff.total_milliseconds()) + "\n"); buffer.AddChunk(line); @@ -331,7 +504,7 @@ MetricsRegistry::SharedMetrics::SharedMetrics(MetricsRegistry ®istry, const std::string &name, - MetricsUpdate update) : + MetricsUpdatePolicy policy) : registry_(registry), name_(name), value_(0) @@ -342,7 +515,7 @@ { boost::mutex::scoped_lock lock(mutex_); value_ += delta; - registry_.SetValue(name_, value_); + registry_.SetIntegerValue(name_, value_); } @@ -376,7 +549,7 @@ const std::string &name) : registry_(registry), name_(name), - update_(MetricsUpdate_MaxOver10Seconds) + policy_(MetricsUpdatePolicy_MaxOver10Seconds) { Start(); } @@ -384,10 +557,10 @@ MetricsRegistry::Timer::Timer(MetricsRegistry ®istry, const std::string &name, - MetricsUpdate update) : + MetricsUpdatePolicy policy) : registry_(registry), name_(name), - update_(update) + policy_(policy) { Start(); } @@ -398,8 +571,7 @@ if (active_) { boost::posix_time::time_duration diff = GetNow() - start_; - registry_.SetValue( - name_, static_cast(diff.total_milliseconds()), update_); + registry_.SetIntegerValue(name_, static_cast(diff.total_milliseconds()), policy_); } } } diff -r dd9795dc380d -r b376abae664a OrthancFramework/Sources/MetricsRegistry.h --- a/OrthancFramework/Sources/MetricsRegistry.h Tue Jun 27 15:56:04 2023 +0200 +++ b/OrthancFramework/Sources/MetricsRegistry.h Tue Jun 27 17:55:09 2023 +0200 @@ -39,19 +39,27 @@ namespace Orthanc { - enum MetricsUpdate + enum MetricsUpdatePolicy { - MetricsUpdate_Directly, - MetricsUpdate_MaxOver10Seconds, - MetricsUpdate_MaxOver1Minute, - MetricsUpdate_MinOver10Seconds, - MetricsUpdate_MinOver1Minute + MetricsUpdatePolicy_Directly, + MetricsUpdatePolicy_MaxOver10Seconds, + MetricsUpdatePolicy_MaxOver1Minute, + MetricsUpdatePolicy_MinOver10Seconds, + MetricsUpdatePolicy_MinOver1Minute + }; + + enum MetricsDataType + { + MetricsDataType_Float, + MetricsDataType_Integer }; class ORTHANC_PUBLIC MetricsRegistry : public boost::noncopyable { private: class Item; + class FloatItem; + class IntegerItem; typedef std::map Content; @@ -61,7 +69,8 @@ // The mutex must be locked Item& GetItemInternal(const std::string& name, - MetricsUpdate update); + MetricsUpdatePolicy policy, + MetricsDataType type); public: MetricsRegistry(); @@ -73,22 +82,35 @@ void SetEnabled(bool enabled); void Register(const std::string& name, - MetricsUpdate update); + MetricsUpdatePolicy policy, + MetricsDataType type); - void SetValue(const std::string& name, - int64_t value, - MetricsUpdate update); + void SetFloatValue(const std::string& name, + float value, + MetricsUpdatePolicy policy /* only used if this is a new metrics */); + + void SetFloatValue(const std::string& name, + float value) + { + SetFloatValue(name, value, MetricsUpdatePolicy_Directly); + } - void SetValue(const std::string& name, - int64_t value) + void SetIntegerValue(const std::string& name, + int64_t value, + MetricsUpdatePolicy policy /* only used if this is a new metrics */); + + void SetIntegerValue(const std::string& name, + int64_t value) { - SetValue(name, value, MetricsUpdate_Directly); + SetIntegerValue(name, value, MetricsUpdatePolicy_Directly); } + + void IncrementIntegerValue(const std::string& name, + int64_t delta); - void IncrementValue(const std::string& name, - int64_t delta); + MetricsUpdatePolicy GetUpdatePolicy(const std::string& metrics); - MetricsUpdate GetMetricsUpdate(const std::string& name); + MetricsDataType GetDataType(const std::string& metrics); // https://prometheus.io/docs/instrumenting/exposition_formats/#text-based-format void ExportPrometheusText(std::string& s); @@ -105,7 +127,7 @@ public: SharedMetrics(MetricsRegistry& registry, const std::string& name, - MetricsUpdate update); + MetricsUpdatePolicy policy); void Add(int64_t delta); }; @@ -128,7 +150,7 @@ private: MetricsRegistry& registry_; std::string name_; - MetricsUpdate update_; + MetricsUpdatePolicy policy_; bool active_; boost::posix_time::ptime start_; @@ -140,7 +162,7 @@ Timer(MetricsRegistry& registry, const std::string& name, - MetricsUpdate update); + MetricsUpdatePolicy policy); ~Timer(); }; diff -r dd9795dc380d -r b376abae664a OrthancFramework/UnitTestsSources/FrameworkTests.cpp --- a/OrthancFramework/UnitTestsSources/FrameworkTests.cpp Tue Jun 27 15:56:04 2023 +0200 +++ b/OrthancFramework/UnitTestsSources/FrameworkTests.cpp Tue Jun 27 17:55:09 2023 +0200 @@ -1312,7 +1312,7 @@ { MetricsRegistry m; m.SetEnabled(false); - m.SetValue("hello.world", 42); + m.SetIntegerValue("hello.world", 42); std::string s; m.ExportPrometheusText(s); @@ -1321,7 +1321,7 @@ { MetricsRegistry m; - m.Register("hello.world", MetricsUpdate_Directly); + m.Register("hello.world", MetricsUpdatePolicy_Directly, MetricsDataType_Integer); std::string s; m.ExportPrometheusText(s); @@ -1330,9 +1330,9 @@ { MetricsRegistry m; - m.SetValue("hello.world", -42); - ASSERT_EQ(MetricsUpdate_Directly, m.GetMetricsUpdate("hello.world")); - ASSERT_THROW(m.GetMetricsUpdate("nope"), OrthancException); + m.SetIntegerValue("hello.world", -42); + ASSERT_EQ(MetricsUpdatePolicy_Directly, m.GetUpdatePolicy("hello.world")); + ASSERT_THROW(m.GetUpdatePolicy("nope"), OrthancException); std::string s; m.ExportPrometheusText(s); @@ -1346,27 +1346,27 @@ { MetricsRegistry m; - m.Register("hello.max", MetricsUpdate_MaxOver10Seconds); - m.SetValue("hello.max", 10); - m.SetValue("hello.max", 20); - m.SetValue("hello.max", -10); - m.SetValue("hello.max", 5); + m.Register("hello.max", MetricsUpdatePolicy_MaxOver10Seconds, MetricsDataType_Integer); + m.SetIntegerValue("hello.max", 10); + m.SetIntegerValue("hello.max", 20); + m.SetIntegerValue("hello.max", -10); + m.SetIntegerValue("hello.max", 5); - m.Register("hello.min", MetricsUpdate_MinOver10Seconds); - m.SetValue("hello.min", 10); - m.SetValue("hello.min", 20); - m.SetValue("hello.min", -10); - m.SetValue("hello.min", 5); + m.Register("hello.min", MetricsUpdatePolicy_MinOver10Seconds, MetricsDataType_Integer); + m.SetIntegerValue("hello.min", 10); + m.SetIntegerValue("hello.min", 20); + m.SetIntegerValue("hello.min", -10); + m.SetIntegerValue("hello.min", 5); - m.Register("hello.directly", MetricsUpdate_Directly); - m.SetValue("hello.directly", 10); - m.SetValue("hello.directly", 20); - m.SetValue("hello.directly", -10); - m.SetValue("hello.directly", 5); + m.Register("hello.directly", MetricsUpdatePolicy_Directly, MetricsDataType_Integer); + m.SetIntegerValue("hello.directly", 10); + m.SetIntegerValue("hello.directly", 20); + m.SetIntegerValue("hello.directly", -10); + m.SetIntegerValue("hello.directly", 5); - ASSERT_EQ(MetricsUpdate_MaxOver10Seconds, m.GetMetricsUpdate("hello.max")); - ASSERT_EQ(MetricsUpdate_MinOver10Seconds, m.GetMetricsUpdate("hello.min")); - ASSERT_EQ(MetricsUpdate_Directly, m.GetMetricsUpdate("hello.directly")); + ASSERT_EQ(MetricsUpdatePolicy_MaxOver10Seconds, m.GetUpdatePolicy("hello.max")); + ASSERT_EQ(MetricsUpdatePolicy_MinOver10Seconds, m.GetUpdatePolicy("hello.min")); + ASSERT_EQ(MetricsUpdatePolicy_Directly, m.GetUpdatePolicy("hello.directly")); std::string s; m.ExportPrometheusText(s); @@ -1392,19 +1392,19 @@ { MetricsRegistry m; - m.SetValue("a", 10); - m.SetValue("b", 10, MetricsUpdate_MinOver10Seconds); + m.SetIntegerValue("a", 10); + m.SetIntegerValue("b", 10, MetricsUpdatePolicy_MinOver10Seconds); - m.Register("c", MetricsUpdate_MaxOver10Seconds); - m.SetValue("c", 10, MetricsUpdate_MinOver10Seconds); + m.Register("c", MetricsUpdatePolicy_MaxOver10Seconds, MetricsDataType_Integer); + m.SetIntegerValue("c", 10, MetricsUpdatePolicy_MinOver10Seconds); - m.Register("d", MetricsUpdate_MaxOver10Seconds); - m.Register("d", MetricsUpdate_Directly); + m.Register("d", MetricsUpdatePolicy_MaxOver10Seconds, MetricsDataType_Integer); + ASSERT_THROW(m.Register("d", MetricsUpdatePolicy_Directly, MetricsDataType_Integer), OrthancException); - ASSERT_EQ(MetricsUpdate_Directly, m.GetMetricsUpdate("a")); - ASSERT_EQ(MetricsUpdate_MinOver10Seconds, m.GetMetricsUpdate("b")); - ASSERT_EQ(MetricsUpdate_MaxOver10Seconds, m.GetMetricsUpdate("c")); - ASSERT_EQ(MetricsUpdate_Directly, m.GetMetricsUpdate("d")); + ASSERT_EQ(MetricsUpdatePolicy_Directly, m.GetUpdatePolicy("a")); + ASSERT_EQ(MetricsUpdatePolicy_MinOver10Seconds, m.GetUpdatePolicy("b")); + ASSERT_EQ(MetricsUpdatePolicy_MaxOver10Seconds, m.GetUpdatePolicy("c")); + ASSERT_EQ(MetricsUpdatePolicy_MaxOver10Seconds, m.GetUpdatePolicy("d")); } { @@ -1412,11 +1412,47 @@ { MetricsRegistry::Timer t1(m, "a"); - MetricsRegistry::Timer t2(m, "b", MetricsUpdate_MinOver10Seconds); + MetricsRegistry::Timer t2(m, "b", MetricsUpdatePolicy_MinOver10Seconds); } - ASSERT_EQ(MetricsUpdate_MaxOver10Seconds, m.GetMetricsUpdate("a")); - ASSERT_EQ(MetricsUpdate_MinOver10Seconds, m.GetMetricsUpdate("b")); + ASSERT_EQ(MetricsUpdatePolicy_MaxOver10Seconds, m.GetUpdatePolicy("a")); + ASSERT_EQ(MetricsUpdatePolicy_MinOver10Seconds, m.GetUpdatePolicy("b")); + } + + { + MetricsRegistry m; + m.Register("c", MetricsUpdatePolicy_MaxOver10Seconds, MetricsDataType_Integer); + m.SetFloatValue("c", 100, MetricsUpdatePolicy_MinOver10Seconds); + + ASSERT_EQ(MetricsUpdatePolicy_MaxOver10Seconds, m.GetUpdatePolicy("c")); + ASSERT_EQ(MetricsDataType_Integer, m.GetDataType("c")); + } + + { + MetricsRegistry m; + m.Register("c", MetricsUpdatePolicy_MaxOver10Seconds, MetricsDataType_Float); + m.SetIntegerValue("c", 100, MetricsUpdatePolicy_MinOver10Seconds); + + ASSERT_EQ(MetricsUpdatePolicy_MaxOver10Seconds, m.GetUpdatePolicy("c")); + ASSERT_EQ(MetricsDataType_Float, m.GetDataType("c")); + } + + { + MetricsRegistry m; + m.SetIntegerValue("c", 100, MetricsUpdatePolicy_MinOver10Seconds); + m.SetFloatValue("c", 101, MetricsUpdatePolicy_MaxOver10Seconds); + + ASSERT_EQ(MetricsUpdatePolicy_MinOver10Seconds, m.GetUpdatePolicy("c")); + ASSERT_EQ(MetricsDataType_Integer, m.GetDataType("c")); + } + + { + MetricsRegistry m; + m.SetIntegerValue("c", 100); + m.SetFloatValue("c", 101, MetricsUpdatePolicy_MaxOver10Seconds); + + ASSERT_EQ(MetricsUpdatePolicy_Directly, m.GetUpdatePolicy("c")); + ASSERT_EQ(MetricsDataType_Integer, m.GetDataType("c")); } } #endif diff -r dd9795dc380d -r b376abae664a OrthancServer/Plugins/Engine/OrthancPlugins.cpp --- a/OrthancServer/Plugins/Engine/OrthancPlugins.cpp Tue Jun 27 15:56:04 2023 +0200 +++ b/OrthancServer/Plugins/Engine/OrthancPlugins.cpp Tue Jun 27 17:55:09 2023 +0200 @@ -5299,8 +5299,7 @@ { PImpl::ServerContextLock lock(*pimpl_); - lock.GetContext().GetMetricsRegistry().SetValue(p.name, boost::math::llround(p.value), - Plugins::Convert(p.type)); + lock.GetContext().GetMetricsRegistry().SetFloatValue(p.name, p.value, Plugins::Convert(p.type)); } return true; @@ -5313,7 +5312,7 @@ { PImpl::ServerContextLock lock(*pimpl_); - lock.GetContext().GetMetricsRegistry().SetValue(p.name, p.value, Plugins::Convert(p.type)); + lock.GetContext().GetMetricsRegistry().SetIntegerValue(p.name, p.value, Plugins::Convert(p.type)); } return true; diff -r dd9795dc380d -r b376abae664a OrthancServer/Plugins/Engine/PluginsEnumerations.cpp --- a/OrthancServer/Plugins/Engine/PluginsEnumerations.cpp Tue Jun 27 15:56:04 2023 +0200 +++ b/OrthancServer/Plugins/Engine/PluginsEnumerations.cpp Tue Jun 27 17:55:09 2023 +0200 @@ -580,15 +580,15 @@ } - MetricsUpdate Convert(OrthancPluginMetricsType type) + MetricsUpdatePolicy Convert(OrthancPluginMetricsType type) { switch (type) { case OrthancPluginMetricsType_Default: - return MetricsUpdate_Directly; + return MetricsUpdatePolicy_Directly; case OrthancPluginMetricsType_Timer: - return MetricsUpdate_MaxOver10Seconds; + return MetricsUpdatePolicy_MaxOver10Seconds; default: throw OrthancException(ErrorCode_ParameterOutOfRange); diff -r dd9795dc380d -r b376abae664a OrthancServer/Plugins/Engine/PluginsEnumerations.h --- a/OrthancServer/Plugins/Engine/PluginsEnumerations.h Tue Jun 27 15:56:04 2023 +0200 +++ b/OrthancServer/Plugins/Engine/PluginsEnumerations.h Tue Jun 27 17:55:09 2023 +0200 @@ -73,7 +73,7 @@ StorageCommitmentFailureReason Convert(OrthancPluginStorageCommitmentFailureReason reason); - MetricsUpdate Convert(OrthancPluginMetricsType type); + MetricsUpdatePolicy Convert(OrthancPluginMetricsType type); } } diff -r dd9795dc380d -r b376abae664a OrthancServer/Sources/OrthancRestApi/OrthancRestApi.cpp --- a/OrthancServer/Sources/OrthancRestApi/OrthancRestApi.cpp Tue Jun 27 15:56:04 2023 +0200 +++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestApi.cpp Tue Jun 27 17:55:09 2023 +0200 @@ -259,7 +259,7 @@ resetRequestReceived_(false), activeRequests_(context.GetMetricsRegistry(), "orthanc_rest_api_active_requests", - MetricsUpdate_MaxOver10Seconds) + MetricsUpdatePolicy_MaxOver10Seconds) { RegisterSystem(orthancExplorerEnabled); diff -r dd9795dc380d -r b376abae664a OrthancServer/Sources/OrthancRestApi/OrthancRestSystem.cpp --- a/OrthancServer/Sources/OrthancRestApi/OrthancRestSystem.cpp Tue Jun 27 15:56:04 2023 +0200 +++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestSystem.cpp Tue Jun 27 17:55:09 2023 +0200 @@ -32,7 +32,6 @@ #include "../ServerContext.h" #include -#include namespace Orthanc @@ -914,19 +913,19 @@ context.GetIndex().GetLastChange(lastChange); MetricsRegistry& registry = context.GetMetricsRegistry(); - registry.SetValue("orthanc_disk_size_mb", boost::math::llround(static_cast(diskSize) / MEGA_BYTES)); - registry.SetValue("orthanc_uncompressed_size_mb", boost::math::llround(static_cast(diskSize) / MEGA_BYTES)); - registry.SetValue("orthanc_count_patients", static_cast(countPatients)); - registry.SetValue("orthanc_count_studies", static_cast(countStudies)); - registry.SetValue("orthanc_count_series", static_cast(countSeries)); - registry.SetValue("orthanc_count_instances", static_cast(countInstances)); - registry.SetValue("orthanc_jobs_pending", jobsPending); - registry.SetValue("orthanc_jobs_running", jobsRunning); - registry.SetValue("orthanc_jobs_completed", jobsSuccess + jobsFailed); - registry.SetValue("orthanc_jobs_success", jobsSuccess); - registry.SetValue("orthanc_jobs_failed", jobsFailed); - registry.SetValue("orthanc_up_time_s", serverUpTime); - registry.SetValue("orthanc_last_change", lastChange["Last"].asInt64()); + registry.SetFloatValue("orthanc_disk_size_mb", static_cast(diskSize) / MEGA_BYTES); + registry.SetFloatValue("orthanc_uncompressed_size_mb", static_cast(diskSize) / MEGA_BYTES); + registry.SetIntegerValue("orthanc_count_patients", static_cast(countPatients)); + registry.SetIntegerValue("orthanc_count_studies", static_cast(countStudies)); + registry.SetIntegerValue("orthanc_count_series", static_cast(countSeries)); + registry.SetIntegerValue("orthanc_count_instances", static_cast(countInstances)); + registry.SetIntegerValue("orthanc_jobs_pending", jobsPending); + registry.SetIntegerValue("orthanc_jobs_running", jobsRunning); + registry.SetIntegerValue("orthanc_jobs_completed", jobsSuccess + jobsFailed); + registry.SetIntegerValue("orthanc_jobs_success", jobsSuccess); + registry.SetIntegerValue("orthanc_jobs_failed", jobsFailed); + registry.SetIntegerValue("orthanc_up_time_s", serverUpTime); + registry.SetIntegerValue("orthanc_last_change", lastChange["Last"].asInt64()); std::string s; registry.ExportPrometheusText(s); diff -r dd9795dc380d -r b376abae664a OrthancServer/Sources/ServerContext.cpp --- a/OrthancServer/Sources/ServerContext.cpp Tue Jun 27 15:56:04 2023 +0200 +++ b/OrthancServer/Sources/ServerContext.cpp Tue Jun 27 17:55:09 2023 +0200 @@ -46,7 +46,6 @@ #include "ServerToolbox.h" #include "StorageCommitmentReports.h" -#include #include #include @@ -298,9 +297,9 @@ void ServerContext::PublishDicomCacheMetrics() { - metricsRegistry_->SetValue("orthanc_dicom_cache_size", - boost::math::llround(static_cast(dicomCache_.GetCurrentSize()) / static_cast(1024 * 1024))); - metricsRegistry_->SetValue("orthanc_dicom_cache_count", dicomCache_.GetNumberOfItems()); + metricsRegistry_->SetFloatValue("orthanc_dicom_cache_size", + static_cast(dicomCache_.GetCurrentSize()) / static_cast(1024 * 1024)); + metricsRegistry_->SetIntegerValue("orthanc_dicom_cache_count", dicomCache_.GetNumberOfItems()); } diff -r dd9795dc380d -r b376abae664a TODO --- a/TODO Tue Jun 27 15:56:04 2023 +0200 +++ b/TODO Tue Jun 27 17:55:09 2023 +0200 @@ -37,9 +37,6 @@ * Add configurations to enable/disable warnings: - Modifying an instance while keeping its original SOPInstanceUID: This should be avoided! - Modifying a study while keeping its original StudyInstanceUID: This should be avoided! -* In Orthanc <= 1.12.0, all the metrics were floating-point. - Since Orthanc >= 1.12.1, all the metrics are integer. - => Add a data type in class MetricsRegistry. ============================