changeset 5326:fbe857e942cd

store metrics as integers instead of floats to avoid precision loss in increments
author Sebastien Jodogne <s.jodogne@gmail.com>
date Sun, 25 Jun 2023 18:28:49 +0200
parents 9c00e832985f
children f01c06f75d67
files NEWS OrthancFramework/Sources/MetricsRegistry.cpp OrthancFramework/Sources/MetricsRegistry.h OrthancFramework/UnitTestsSources/FrameworkTests.cpp OrthancServer/Plugins/Engine/OrthancPlugins.cpp OrthancServer/Plugins/Engine/PluginsEnumerations.cpp OrthancServer/Plugins/Engine/PluginsEnumerations.h OrthancServer/Plugins/Include/orthanc/OrthancCPlugin.h OrthancServer/Sources/OrthancRestApi/OrthancRestSystem.cpp OrthancServer/Sources/ServerContext.cpp TODO
diffstat 11 files changed, 117 insertions(+), 53 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Sun Jun 25 17:49:34 2023 +0200
+++ b/NEWS	Sun Jun 25 18:28:49 2023 +0200
@@ -12,6 +12,7 @@
 -------
 
 * Added "OrthancPluginLoadDicomInstance()" to load DICOM instances from the database
+* Added "OrthancPluginSetMetricsIntegerValue()" to track metrics with integer values
 
 Maintenance
 -----------
@@ -22,8 +23,9 @@
 * Modality worklists plugin: allow searching on private tags (exact match only)
 * Fix orphan files remaining in storage when working with MaximumStorageSize
   (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.
+* 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
 * Upgraded dependencies for static builds:
   - boost 1.82.0
 
--- a/OrthancFramework/Sources/MetricsRegistry.cpp	Sun Jun 25 17:49:34 2023 +0200
+++ b/OrthancFramework/Sources/MetricsRegistry.cpp	Sun Jun 25 18:28:49 2023 +0200
@@ -41,9 +41,9 @@
     MetricsType               type_;
     boost::posix_time::ptime  time_;
     bool                      hasValue_;
-    float                     value_;
+    int64_t                   value_;
     
-    void Touch(float value,
+    void Touch(int64_t value,
                const boost::posix_time::ptime& now)
     {
       hasValue_ = true;
@@ -51,12 +51,12 @@
       time_ = now;
     }
 
-    void Touch(float value)
+    void Touch(int64_t value)
     {
       Touch(value, GetNow());
     }
 
-    void UpdateMax(float value,
+    void UpdateMax(int64_t value,
                    int duration)
     {
       if (hasValue_)
@@ -75,7 +75,7 @@
       }
     }
     
-    void UpdateMin(float value,
+    void UpdateMin(int64_t value,
                    int duration)
     {
       if (hasValue_)
@@ -107,7 +107,7 @@
       return type_;
     }
 
-    void Update(float value)
+    void Update(int64_t value)
     {
       switch (type_)
       {
@@ -153,7 +153,7 @@
       }
     }
 
-    float GetValue() const
+    int64_t GetValue() const
     {
       if (hasValue_)
       {
@@ -240,7 +240,7 @@
 
 
   void MetricsRegistry::SetValue(const std::string &name,
-                                 float value,
+                                 int64_t value,
                                  MetricsType type)
   {
     // Inlining to avoid loosing time if metrics are disabled
@@ -253,7 +253,7 @@
 
 
   void MetricsRegistry::IncrementValue(const std::string &name,
-                                       float delta)
+                                       int64_t delta)
   {
     // Inlining to avoid loosing time if metrics are disabled
     if (enabled_)
@@ -337,7 +337,7 @@
   {
   }
 
-  void MetricsRegistry::SharedMetrics::Add(float delta)
+  void MetricsRegistry::SharedMetrics::Add(int64_t delta)
   {
     boost::mutex::scoped_lock lock(mutex_);
     value_ += delta;
@@ -398,7 +398,7 @@
     {
       boost::posix_time::time_duration diff = GetNow() - start_;
       registry_.SetValue(
-            name_, static_cast<float>(diff.total_milliseconds()), type_);
+            name_, static_cast<int64_t>(diff.total_milliseconds()), type_);
     }
   }
 }
--- a/OrthancFramework/Sources/MetricsRegistry.h	Sun Jun 25 17:49:34 2023 +0200
+++ b/OrthancFramework/Sources/MetricsRegistry.h	Sun Jun 25 18:28:49 2023 +0200
@@ -35,6 +35,7 @@
 
 #include <boost/thread/mutex.hpp>
 #include <boost/date_time/posix_time/posix_time.hpp>
+#include <stdint.h>
 
 namespace Orthanc
 {
@@ -75,17 +76,17 @@
                   MetricsType type);
 
     void SetValue(const std::string& name,
-                  float value,
+                  int64_t value,
                   MetricsType type);
     
     void SetValue(const std::string& name,
-                  float value)
+                  int64_t value)
     {
       SetValue(name, value, MetricsType_Default);
     }
 
     void IncrementValue(const std::string& name,
-                        float delta);
+                        int64_t delta);
 
     MetricsType GetMetricsType(const std::string& name);
 
@@ -99,14 +100,14 @@
       boost::mutex      mutex_;
       MetricsRegistry&  registry_;
       std::string       name_;
-      float             value_;
+      int64_t           value_;
 
     public:
       SharedMetrics(MetricsRegistry& registry,
                     const std::string& name,
                     MetricsType type);
 
-      void Add(float delta);
+      void Add(int64_t delta);
     };
 
 
--- a/OrthancFramework/UnitTestsSources/FrameworkTests.cpp	Sun Jun 25 17:49:34 2023 +0200
+++ b/OrthancFramework/UnitTestsSources/FrameworkTests.cpp	Sun Jun 25 18:28:49 2023 +0200
@@ -1311,7 +1311,7 @@
   {
     MetricsRegistry m;
     m.SetEnabled(false);
-    m.SetValue("hello.world", 42.5f);
+    m.SetValue("hello.world", 42);
     
     std::string s;
     m.ExportPrometheusText(s);
@@ -1329,7 +1329,7 @@
 
   {
     MetricsRegistry m;
-    m.SetValue("hello.world", 42.5f);
+    m.SetValue("hello.world", -42);
     ASSERT_EQ(MetricsType_Default, m.GetMetricsType("hello.world"));
     ASSERT_THROW(m.GetMetricsType("nope"), OrthancException);
     
@@ -1339,7 +1339,7 @@
     std::vector<std::string> t;
     Toolbox::TokenizeString(t, s, '\n');
     ASSERT_EQ(2u, t.size());
-    ASSERT_EQ("hello.world 42.5 ", t[0].substr(0, 17));
+    ASSERT_EQ("hello.world -42 ", t[0].substr(0, 16));
     ASSERT_TRUE(t[1].empty());
   }
 
--- a/OrthancServer/Plugins/Engine/OrthancPlugins.cpp	Sun Jun 25 17:49:34 2023 +0200
+++ b/OrthancServer/Plugins/Engine/OrthancPlugins.cpp	Sun Jun 25 18:28:49 2023 +0200
@@ -5280,24 +5280,23 @@
         const _OrthancPluginSetMetricsValue& p =
           *reinterpret_cast<const _OrthancPluginSetMetricsValue*>(parameters);
 
-        MetricsType type;
-        switch (p.type)
-        {
-          case OrthancPluginMetricsType_Default:
-            type = MetricsType_Default;
-            break;
-
-          case OrthancPluginMetricsType_Timer:
-            type = MetricsType_MaxOver10Seconds;
-            break;
-
-          default:
-            throw OrthancException(ErrorCode_ParameterOutOfRange);
-        }
-        
         {
           PImpl::ServerContextLock lock(*pimpl_);
-          lock.GetContext().GetMetricsRegistry().SetValue(p.name, p.value, type);
+          lock.GetContext().GetMetricsRegistry().SetValue(p.name, boost::math::llround(p.value),
+                                                          Plugins::Convert(p.type));
+        }
+
+        return true;
+      }
+
+      case _OrthancPluginService_SetMetricsIntegerValue:
+      {
+        const _OrthancPluginSetMetricsIntegerValue& p =
+          *reinterpret_cast<const _OrthancPluginSetMetricsIntegerValue*>(parameters);
+
+        {
+          PImpl::ServerContextLock lock(*pimpl_);
+          lock.GetContext().GetMetricsRegistry().SetValue(p.name, p.value, Plugins::Convert(p.type));
         }
 
         return true;
--- a/OrthancServer/Plugins/Engine/PluginsEnumerations.cpp	Sun Jun 25 17:49:34 2023 +0200
+++ b/OrthancServer/Plugins/Engine/PluginsEnumerations.cpp	Sun Jun 25 18:28:49 2023 +0200
@@ -578,5 +578,21 @@
           throw OrthancException(ErrorCode_ParameterOutOfRange);
       }
     }
+
+
+    MetricsType Convert(OrthancPluginMetricsType type)
+    {
+      switch (type)
+      {
+        case OrthancPluginMetricsType_Default:
+          return MetricsType_Default;
+
+        case OrthancPluginMetricsType_Timer:
+          return MetricsType_MaxOver10Seconds;
+
+        default:
+          throw OrthancException(ErrorCode_ParameterOutOfRange);
+      }
+    }
   }
 }
--- a/OrthancServer/Plugins/Engine/PluginsEnumerations.h	Sun Jun 25 17:49:34 2023 +0200
+++ b/OrthancServer/Plugins/Engine/PluginsEnumerations.h	Sun Jun 25 18:28:49 2023 +0200
@@ -31,6 +31,7 @@
  * "orthanc-databases" project.
  **/
 
+#include "../../../OrthancFramework/Sources/MetricsRegistry.h"
 #include "../../Sources/Search/DatabaseConstraint.h"
 #include "../../Sources/ServerEnumerations.h"
 #include "../Include/orthanc/OrthancCPlugin.h"
@@ -71,6 +72,8 @@
     JobStepCode Convert(OrthancPluginJobStepStatus step);
 
     StorageCommitmentFailureReason Convert(OrthancPluginStorageCommitmentFailureReason reason);
+
+    MetricsType Convert(OrthancPluginMetricsType type);
   }
 }
 
--- a/OrthancServer/Plugins/Include/orthanc/OrthancCPlugin.h	Sun Jun 25 17:49:34 2023 +0200
+++ b/OrthancServer/Plugins/Include/orthanc/OrthancCPlugin.h	Sun Jun 25 18:28:49 2023 +0200
@@ -447,6 +447,7 @@
     _OrthancPluginService_CreateMemoryBuffer64 = 40, /* New in Orthanc 1.9.0 */
     _OrthancPluginService_CreateDicom2 = 41,         /* New in Orthanc 1.9.0 */
     _OrthancPluginService_GetDatabaseServerIdentifier = 42,         /* New in Orthanc 1.11.1 */
+    _OrthancPluginService_SetMetricsIntegerValue = 43,              /* New in Orthanc 1.12.1 */
 
     /* Registration of callbacks */
     _OrthancPluginService_RegisterRestCallback = 1000,
@@ -1746,7 +1747,8 @@
    * Signature of a callback function that is called by Orthanc
    * whenever a monitoring tool (such as Prometheus) asks the current
    * values of the metrics. This callback gives the plugin a chance to
-   * update its metrics, by calling OrthancPluginSetMetricsValue().
+   * update its metrics, by calling OrthancPluginSetMetricsValue() or
+   * OrthancPluginSetMetricsIntegerValue().
    * This is typically useful for metrics that are expensive to
    * acquire.
    * 
@@ -7078,11 +7080,12 @@
   } _OrthancPluginSetMetricsValue;
 
   /**
-   * @brief Set the value of a metrics.
-   *
-   * This function sets the value of a metrics to monitor the behavior
-   * of the plugin through tools such as Prometheus. The values of all
-   * the metrics are stored within the Orthanc context.
+   * @brief Set the value of a floating-point metrics.
+   *
+   * This function sets the value of a floating-point metrics to
+   * monitor the behavior of the plugin through tools such as
+   * Prometheus. The values of all the metrics are stored within the
+   * Orthanc context.
    * 
    * @param context The Orthanc plugin context, as received by OrthancPluginInitialize().
    * @param name The name of the metrics to be set.
@@ -7090,6 +7093,7 @@
    * @param type The type of the metrics. This parameter is only taken into consideration
    * the first time this metrics is set.
    * @ingroup Toolbox
+   * @see OrthancPluginSetMetricsIntegerValue()
    **/
   ORTHANC_PLUGIN_INLINE void OrthancPluginSetMetricsValue(
     OrthancPluginContext*     context,
@@ -7115,7 +7119,8 @@
    * @brief Register a callback to refresh the metrics.
    *
    * This function registers a callback to refresh the metrics. The
-   * callback must make calls to OrthancPluginSetMetricsValue().
+   * callback must make calls to OrthancPluginSetMetricsValue() or
+   * OrthancPluginSetMetricsIntegerValue().
    *
    * @param context The Orthanc plugin context, as received by OrthancPluginInitialize().
    * @param callback The callback function to handle the refresh.
@@ -9293,6 +9298,42 @@
   }
 
 
+  typedef struct
+  {
+    const char*               name;
+    int64_t                   value;
+    OrthancPluginMetricsType  type;
+  } _OrthancPluginSetMetricsIntegerValue;
+
+  /**
+   * @brief Set the value of an integer metrics.
+   *
+   * This function sets the value of an integer metrics to monitor the
+   * behavior of the plugin through tools such as Prometheus. The
+   * values of all the metrics are stored within the Orthanc context.
+   * 
+   * @param context The Orthanc plugin context, as received by OrthancPluginInitialize().
+   * @param name The name of the metrics to be set.
+   * @param value The value of the metrics.
+   * @param type The type of the metrics. This parameter is only taken into consideration
+   * the first time this metrics is set.
+   * @ingroup Toolbox
+   * @see OrthancPluginSetMetricsValue()
+   **/
+  ORTHANC_PLUGIN_INLINE void OrthancPluginSetMetricsIntegerValue(
+    OrthancPluginContext*     context,
+    const char*               name,
+    int64_t                   value,
+    OrthancPluginMetricsType  type)
+  {
+    _OrthancPluginSetMetricsIntegerValue params;
+    params.name = name;
+    params.value = value;
+    params.type = type;
+    context->InvokeService(context, _OrthancPluginService_SetMetricsIntegerValue, &params);
+  }
+
+
 #ifdef  __cplusplus
 }
 #endif
--- a/OrthancServer/Sources/OrthancRestApi/OrthancRestSystem.cpp	Sun Jun 25 17:49:34 2023 +0200
+++ b/OrthancServer/Sources/OrthancRestApi/OrthancRestSystem.cpp	Sun Jun 25 18:28:49 2023 +0200
@@ -913,12 +913,12 @@
     context.GetIndex().GetLastChange(lastChange);
 
     MetricsRegistry& registry = context.GetMetricsRegistry();
-    registry.SetValue("orthanc_disk_size_mb", static_cast<float>(diskSize) / MEGA_BYTES);
-    registry.SetValue("orthanc_uncompressed_size_mb", static_cast<float>(diskSize) / MEGA_BYTES);
-    registry.SetValue("orthanc_count_patients", static_cast<unsigned int>(countPatients));
-    registry.SetValue("orthanc_count_studies", static_cast<unsigned int>(countStudies));
-    registry.SetValue("orthanc_count_series", static_cast<unsigned int>(countSeries));
-    registry.SetValue("orthanc_count_instances", static_cast<unsigned int>(countInstances));
+    registry.SetValue("orthanc_disk_size_mb", boost::math::llround(static_cast<float>(diskSize) / MEGA_BYTES));
+    registry.SetValue("orthanc_uncompressed_size_mb", boost::math::llround(static_cast<float>(diskSize) / MEGA_BYTES));
+    registry.SetValue("orthanc_count_patients", static_cast<int64_t>(countPatients));
+    registry.SetValue("orthanc_count_studies", static_cast<int64_t>(countStudies));
+    registry.SetValue("orthanc_count_series", static_cast<int64_t>(countSeries));
+    registry.SetValue("orthanc_count_instances", static_cast<int64_t>(countInstances));
     registry.SetValue("orthanc_jobs_pending", jobsPending);
     registry.SetValue("orthanc_jobs_running", jobsRunning);
     registry.SetValue("orthanc_jobs_completed", jobsSuccess + jobsFailed);
--- a/OrthancServer/Sources/ServerContext.cpp	Sun Jun 25 17:49:34 2023 +0200
+++ b/OrthancServer/Sources/ServerContext.cpp	Sun Jun 25 18:28:49 2023 +0200
@@ -298,9 +298,8 @@
   void ServerContext::PublishDicomCacheMetrics()
   {
     metricsRegistry_->SetValue("orthanc_dicom_cache_size",
-                               static_cast<float>(dicomCache_.GetCurrentSize()) / static_cast<float>(1024 * 1024));
-    metricsRegistry_->SetValue("orthanc_dicom_cache_count",
-                               static_cast<float>(dicomCache_.GetNumberOfItems()));
+                               boost::math::llround(static_cast<float>(dicomCache_.GetCurrentSize()) / static_cast<float>(1024 * 1024)));
+    metricsRegistry_->SetValue("orthanc_dicom_cache_count", dicomCache_.GetNumberOfItems());
   }
 
 
--- a/TODO	Sun Jun 25 17:49:34 2023 +0200
+++ b/TODO	Sun Jun 25 18:28:49 2023 +0200
@@ -37,6 +37,9 @@
 * 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.
 
 
 ============================