changeset 6222:96c7a44a595b

new metrics orthanc_available_dicom_threads + fix orthanc_rest_api_active_requests
author Alain Mazy <am@orthanc.team>
date Mon, 07 Jul 2025 17:12:55 +0200
parents add00150107b
children da6430b05c8d
files NEWS OrthancFramework/Sources/DicomNetworking/DicomServer.cpp OrthancFramework/Sources/DicomNetworking/DicomServer.h OrthancFramework/Sources/MetricsRegistry.cpp OrthancFramework/Sources/MetricsRegistry.h OrthancFramework/Sources/MultiThreading/RunnableWorkersPool.cpp OrthancFramework/Sources/MultiThreading/RunnableWorkersPool.h OrthancFramework/UnitTestsSources/FrameworkTests.cpp OrthancServer/Plugins/Samples/MultitenantDicom/MultitenantDicomServer.cpp OrthancServer/Plugins/Samples/MultitenantDicom/MultitenantDicomServer.h OrthancServer/Plugins/Samples/MultitenantDicom/OrthancFrameworkDependencies.cpp OrthancServer/Resources/RunCppCheck.sh OrthancServer/Sources/main.cpp
diffstat 13 files changed, 208 insertions(+), 22 deletions(-) [+]
line wrap: on
line diff
--- a/NEWS	Fri Jul 04 15:33:46 2025 +0200
+++ b/NEWS	Mon Jul 07 17:12:55 2025 +0200
@@ -10,8 +10,12 @@
 --------
 
 * When creating a job, you can now add a "UserData" field in the payload.
-  This data will travel along with the job and will be available in the 
+  This data travels along with the job and is available in the 
   /jobs/{jobId} route.
+* New metrics "orthanc_available_dicom_threads" that displays the minimum
+  number of DICOM Threads that were available during the last 10 seconds.
+* Fixed the "orthanc_rest_api_active_requests" metrix that was not 
+  reset after 10 seconds.
 
 
 Plugin SDK
--- a/OrthancFramework/Sources/DicomNetworking/DicomServer.cpp	Fri Jul 04 15:33:46 2025 +0200
+++ b/OrthancFramework/Sources/DicomNetworking/DicomServer.cpp	Mon Jul 07 17:12:55 2025 +0200
@@ -89,7 +89,7 @@
   }
 
 
-  DicomServer::DicomServer() : 
+  DicomServer::DicomServer(MetricsRegistry& metricsRegistry) : 
     pimpl_(new PImpl),
     checkCalledAet_(true),
     aet_("ANY-SCP"),
@@ -105,6 +105,7 @@
     worklistRequestHandlerFactory_(NULL),
     storageCommitmentFactory_(NULL),
     applicationEntityFilter_(NULL),
+    metricsRegistry_(metricsRegistry),
     useDicomTls_(false),
     maximumPduLength_(ASC_DEFAULTMAXPDU),
     remoteCertificateRequired_(true),
@@ -432,7 +433,7 @@
 
     CLOG(INFO, DICOM) << "The embedded DICOM server will use " << threadsCount_ << " threads";
 
-    pimpl_->workers_.reset(new RunnableWorkersPool(threadsCount_, "DICOM-"));
+    pimpl_->workers_.reset(new RunnableWorkersPool(threadsCount_, "DICOM-", metricsRegistry_, "orthanc_available_dicom_threads"));
     pimpl_->thread_ = boost::thread(ServerThread, this, maximumPduLength_, useDicomTls_);
   }
 
--- a/OrthancFramework/Sources/DicomNetworking/DicomServer.h	Fri Jul 04 15:33:46 2025 +0200
+++ b/OrthancFramework/Sources/DicomNetworking/DicomServer.h	Mon Jul 07 17:12:55 2025 +0200
@@ -47,6 +47,8 @@
 
 namespace Orthanc
 {
+  class MetricsRegistry;
+
   class DicomServer : public boost::noncopyable
   {
   public:
@@ -83,6 +85,7 @@
     IWorklistRequestHandlerFactory* worklistRequestHandlerFactory_;
     IStorageCommitmentRequestHandlerFactory* storageCommitmentFactory_;
     IApplicationEntityFilter* applicationEntityFilter_;
+    MetricsRegistry& metricsRegistry_;
 
     // New in Orthanc 1.9.0 for DICOM TLS
     bool         useDicomTls_;
@@ -100,7 +103,7 @@
                              bool useDicomTls);
 
   public:
-    DicomServer();
+    explicit DicomServer(MetricsRegistry& metricsRegistry);
 
     ~DicomServer();
 
--- a/OrthancFramework/Sources/MetricsRegistry.cpp	Fri Jul 04 15:33:46 2025 +0200
+++ b/OrthancFramework/Sources/MetricsRegistry.cpp	Mon Jul 07 17:12:55 2025 +0200
@@ -47,6 +47,8 @@
       boost::posix_time::ptime  time_;
       bool                      hasValue_;
       T                         value_;
+      bool                      hasNextValue_;  // for min and max values over period, we need to store the next value
+      T                         nextValue_;
 
       void SetValue(const T& value,
                     const boost::posix_time::ptime& now)
@@ -89,8 +91,25 @@
     public:
       explicit TimestampedValue() :
         hasValue_(false),
-        value_(0)
+        value_(0),
+        hasNextValue_(false),
+        nextValue_(0)
+      {
+      }
+
+      int GetPeriodDuration(const MetricsUpdatePolicy& policy)
       {
+        switch (policy)
+        {
+          case MetricsUpdatePolicy_MaxOver10Seconds:
+          case MetricsUpdatePolicy_MinOver10Seconds:
+            return 10;
+          case MetricsUpdatePolicy_MaxOver1Minute:
+          case MetricsUpdatePolicy_MinOver1Minute:
+            return 60;
+          default:
+            throw OrthancException(ErrorCode_InternalError);
+        }
       }
 
       void Update(const T& value,
@@ -105,30 +124,54 @@
             break;
           
           case MetricsUpdatePolicy_MaxOver10Seconds:
-            if (IsLargerOverPeriod(value, 10, now))
+          case MetricsUpdatePolicy_MaxOver1Minute:
+            if (IsLargerOverPeriod(value, GetPeriodDuration(policy), now))
             {
               SetValue(value, now);
             }
-            break;
-
-          case MetricsUpdatePolicy_MaxOver1Minute:
-            if (IsLargerOverPeriod(value, 60, now))
+            else
             {
-              SetValue(value, now);
+              hasNextValue_ = true;
+              nextValue_ = value;
             }
             break;
 
           case MetricsUpdatePolicy_MinOver10Seconds:
-            if (IsSmallerOverPeriod(value, 10, now))
+          case MetricsUpdatePolicy_MinOver1Minute:
+            if (IsSmallerOverPeriod(value, GetPeriodDuration(policy), now))
             {
               SetValue(value, now);
             }
+            else
+            {
+              hasNextValue_ = true;
+              nextValue_ = value;
+            }
             break;
+          default:
+            throw OrthancException(ErrorCode_NotImplemented);
+        }
+      }
 
+      void Refresh(const MetricsUpdatePolicy& policy)
+      {
+        const boost::posix_time::ptime now = GetNow();
+
+        switch (policy)
+        {
+          case MetricsUpdatePolicy_Directly:
+            // nothing to do
+            break;
+          
+          case MetricsUpdatePolicy_MaxOver10Seconds:
+          case MetricsUpdatePolicy_MinOver10Seconds:
+          case MetricsUpdatePolicy_MaxOver1Minute:
           case MetricsUpdatePolicy_MinOver1Minute:
-            if (IsSmallerOverPeriod(value, 60, now))
+            // if the min/max value is older than the period, get the latest value
+            if ((now - time_).total_seconds() > GetPeriodDuration(policy) /* old value has expired */ && hasNextValue_)
             {
-              SetValue(value, now);
+              SetValue(nextValue_, now);
+              hasNextValue_ = false;
             }
             break;
 
@@ -217,6 +260,8 @@
     virtual const boost::posix_time::ptime& GetTime() const = 0;
     
     virtual std::string FormatValue() const = 0;
+
+    virtual void Refresh() = 0;
   };
 
   
@@ -265,6 +310,12 @@
     {
       return boost::lexical_cast<std::string>(value_.GetValue());
     }
+
+    virtual void Refresh() ORTHANC_OVERRIDE
+    {
+      value_.Refresh(GetPolicy());
+    }
+
   };
 
   
@@ -313,6 +364,11 @@
     {
       return boost::lexical_cast<std::string>(value_.GetValue());
     }
+
+    virtual void Refresh() ORTHANC_OVERRIDE
+    {
+      value_.Refresh(GetPolicy());
+    }
   };
 
 
@@ -494,6 +550,8 @@
       {
         boost::posix_time::time_duration diff = it->second->GetTime() - EPOCH;
 
+        it->second->Refresh();
+
         std::string line = (it->first + " " +
                             it->second->FormatValue() + " " + 
                             boost::lexical_cast<std::string>(diff.total_milliseconds()) + "\n");
@@ -513,6 +571,7 @@
     name_(name),
     value_(0)
   {
+    registry_.Register(name, policy, MetricsDataType_Integer);
   }
 
   void MetricsRegistry::SharedMetrics::Add(int64_t delta)
@@ -535,6 +594,18 @@
   }
 
 
+  MetricsRegistry::AvailableResourcesDecounter::AvailableResourcesDecounter(MetricsRegistry::SharedMetrics &metrics) :
+    metrics_(metrics)
+  {
+    metrics_.Add(-1);
+  }
+
+  MetricsRegistry::AvailableResourcesDecounter::~AvailableResourcesDecounter()
+  {
+    metrics_.Add(1);
+  }
+
+
   void  MetricsRegistry::Timer::Start()
   {
     if (registry_.IsEnabled())
--- a/OrthancFramework/Sources/MetricsRegistry.h	Fri Jul 04 15:33:46 2025 +0200
+++ b/OrthancFramework/Sources/MetricsRegistry.h	Mon Jul 07 17:12:55 2025 +0200
@@ -146,6 +146,17 @@
     };
 
 
+    class ORTHANC_PUBLIC AvailableResourcesDecounter : public boost::noncopyable
+    {
+    private:
+      SharedMetrics&   metrics_;
+
+    public:
+      explicit AvailableResourcesDecounter(SharedMetrics& metrics);
+
+      ~AvailableResourcesDecounter();
+    };
+
     class ORTHANC_PUBLIC Timer : public boost::noncopyable
     {
     private:
--- a/OrthancFramework/Sources/MultiThreading/RunnableWorkersPool.cpp	Fri Jul 04 15:33:46 2025 +0200
+++ b/OrthancFramework/Sources/MultiThreading/RunnableWorkersPool.cpp	Mon Jul 07 17:12:55 2025 +0200
@@ -29,6 +29,8 @@
 #include "../Compatibility.h"
 #include "../OrthancException.h"
 #include "../Logging.h"
+#include "../MetricsRegistry.h"
+
 
 namespace Orthanc
 {
@@ -41,6 +43,7 @@
       SharedMessageQueue&   queue_;
       boost::thread         thread_;
       std::string           name_;
+      MetricsRegistry::SharedMetrics& availableWorkers_;
  
       static void WorkerThread(Worker* that)
       {
@@ -51,8 +54,11 @@
           try
           {
             std::unique_ptr<IDynamicObject>  obj(that->queue_.Dequeue(100));
+            
             if (obj.get() != NULL)
             {
+              MetricsRegistry::AvailableResourcesDecounter counter(that->availableWorkers_);
+
               IRunnableBySteps& runnable = *dynamic_cast<IRunnableBySteps*>(obj.get());
               
               bool wishToContinue = runnable.Step();
@@ -86,10 +92,12 @@
     public:
       Worker(const bool& globalContinue,
              SharedMessageQueue& queue,
-             const std::string& name) : 
+             const std::string& name,
+             MetricsRegistry::SharedMetrics& availableWorkers) : 
         continue_(globalContinue),
         queue_(queue),
-        name_(name)
+        name_(name),
+        availableWorkers_(availableWorkers)
       {
         thread_ = boost::thread(WorkerThread, this);
       }
@@ -107,11 +115,23 @@
     bool                  continue_;
     std::vector<Worker*>  workers_;
     SharedMessageQueue    queue_;
+    MetricsRegistry::SharedMetrics  availableWorkers_;
+
+  public:
+    PImpl(MetricsRegistry& metricsRegistry, const char* availableWorkersMetricsName) :
+      continue_(false),
+      availableWorkers_(metricsRegistry, availableWorkersMetricsName, MetricsUpdatePolicy_MinOver10Seconds)
+    {
+    }
   };
 
 
 
-  RunnableWorkersPool::RunnableWorkersPool(size_t countWorkers, const std::string& name) : pimpl_(new PImpl)
+  RunnableWorkersPool::RunnableWorkersPool(size_t countWorkers, 
+                                           const std::string& name, 
+                                           MetricsRegistry& metricsRegistry, 
+                                           const char* availableWorkersMetricsName) : 
+    pimpl_(new PImpl(metricsRegistry, availableWorkersMetricsName))
   {
     pimpl_->continue_ = true;
 
@@ -121,11 +141,12 @@
     }
 
     pimpl_->workers_.resize(countWorkers);
+    pimpl_->availableWorkers_.Add(countWorkers); // mark all workers as available
 
     for (size_t i = 0; i < countWorkers; i++)
     {
       std::string workerName = name + boost::lexical_cast<std::string>(i);
-      pimpl_->workers_[i] = new PImpl::Worker(pimpl_->continue_, pimpl_->queue_, workerName);
+      pimpl_->workers_[i] = new PImpl::Worker(pimpl_->continue_, pimpl_->queue_, workerName, pimpl_->availableWorkers_);
     }
   }
 
--- a/OrthancFramework/Sources/MultiThreading/RunnableWorkersPool.h	Fri Jul 04 15:33:46 2025 +0200
+++ b/OrthancFramework/Sources/MultiThreading/RunnableWorkersPool.h	Mon Jul 07 17:12:55 2025 +0200
@@ -30,6 +30,8 @@
 
 namespace Orthanc
 {
+  class MetricsRegistry;
+
   class RunnableWorkersPool : public boost::noncopyable
   {
   private:
@@ -39,7 +41,7 @@
     void Stop();
 
   public:
-    explicit RunnableWorkersPool(size_t countWorkers, const std::string& name);
+    explicit RunnableWorkersPool(size_t countWorkers, const std::string& name, MetricsRegistry& metricsRegistry, const char* availableWorkersMetricsName);
 
     ~RunnableWorkersPool();
 
--- a/OrthancFramework/UnitTestsSources/FrameworkTests.cpp	Fri Jul 04 15:33:46 2025 +0200
+++ b/OrthancFramework/UnitTestsSources/FrameworkTests.cpp	Mon Jul 07 17:12:55 2025 +0200
@@ -49,6 +49,7 @@
 #endif
 
 #include <ctype.h>
+#include <boost/thread.hpp>
 
 
 using namespace Orthanc;
@@ -1425,6 +1426,25 @@
 
 
 #if ORTHANC_SANDBOXED != 1
+
+void GetValuesDico(std::map<std::string, std::string>& values, MetricsRegistry& m)
+{
+  values.clear();
+
+  std::string s;
+  m.ExportPrometheusText(s);
+
+  std::vector<std::string> t;
+  Toolbox::TokenizeString(t, s, '\n');
+ 
+  for (size_t i = 0; i < t.size() - 1; i++)
+  {
+    std::vector<std::string> v;
+    Toolbox::TokenizeString(v, t[i], ' ');
+    values[v[0]] = v[1];
+  }
+}
+
 TEST(MetricsRegistry, Basic)
 {
   {
@@ -1572,6 +1592,56 @@
     ASSERT_EQ(MetricsUpdatePolicy_Directly, m.GetUpdatePolicy("c"));
     ASSERT_EQ(MetricsDataType_Integer, m.GetDataType("c"));
   }
+
+  {
+    std::map<std::string, std::string> values;
+
+    MetricsRegistry mr;
+
+    {
+      MetricsRegistry::SharedMetrics max10(mr, "shared_max10", MetricsUpdatePolicy_MaxOver10Seconds);
+    
+      {
+        MetricsRegistry::ActiveCounter c1(max10);
+        MetricsRegistry::ActiveCounter c2(max10);
+        GetValuesDico(values, mr);
+        ASSERT_EQ("2", values["shared_max10"]);
+      }
+
+      GetValuesDico(values, mr);
+      ASSERT_EQ("2", values["shared_max10"]);
+
+      // // Uncomment to test max values going back to latest values after expiration of the 10 seconds period
+      // boost::this_thread::sleep(boost::posix_time::milliseconds(12000));
+
+      // GetValuesDico(values, mr);
+      // ASSERT_EQ("0", values["shared_max10"]);
+    }
+
+    {
+      MetricsRegistry::SharedMetrics min10(mr, "shared_min10", MetricsUpdatePolicy_MinOver10Seconds);
+      min10.Add(10);
+
+      GetValuesDico(values, mr);
+      ASSERT_EQ("10", values["shared_min10"]);
+
+      {
+        MetricsRegistry::AvailableResourcesDecounter c1(min10);
+        MetricsRegistry::AvailableResourcesDecounter c2(min10);
+        GetValuesDico(values, mr);
+        ASSERT_EQ("8", values["shared_min10"]);
+      }
+
+      GetValuesDico(values, mr);
+      ASSERT_EQ("8", values["shared_min10"]);
+
+      // // Uncomment to test min values going back to latest values after expiration of the 10 seconds period
+      // boost::this_thread::sleep(boost::posix_time::milliseconds(12000));
+
+      // GetValuesDico(values, mr);
+      // ASSERT_EQ("10", values["shared_min10"]);
+    }
+  }
 }
 #endif
 
--- a/OrthancServer/Plugins/Samples/MultitenantDicom/MultitenantDicomServer.cpp	Fri Jul 04 15:33:46 2025 +0200
+++ b/OrthancServer/Plugins/Samples/MultitenantDicom/MultitenantDicomServer.cpp	Mon Jul 07 17:12:55 2025 +0200
@@ -93,7 +93,7 @@
     labelsStoreLevels_.insert(Orthanc::ResourceType_Instance);
   }
   
-  server_.reset(new Orthanc::DicomServer);
+  server_.reset(new Orthanc::DicomServer(dummyMetricsRegistry_));
 
   {
     OrthancPlugins::OrthancConfiguration globalConfig;
--- a/OrthancServer/Plugins/Samples/MultitenantDicom/MultitenantDicomServer.h	Fri Jul 04 15:33:46 2025 +0200
+++ b/OrthancServer/Plugins/Samples/MultitenantDicom/MultitenantDicomServer.h	Mon Jul 07 17:12:55 2025 +0200
@@ -27,6 +27,7 @@
 #include "PluginEnumerations.h"
 
 #include "../../../../OrthancFramework/Sources/DicomNetworking/DicomServer.h"
+#include "../../../../OrthancFramework/Sources/MetricsRegistry.h"
 
 #include <boost/thread/mutex.hpp>
 
@@ -59,6 +60,7 @@
   bool                                   isStrictAet_;
   DicomFilter                            filter_;
   std::unique_ptr<Orthanc::DicomServer>  server_;
+  Orthanc::MetricsRegistry               dummyMetricsRegistry_;
 
 public:
   explicit MultitenantDicomServer(const Json::Value& serverConfig);
--- a/OrthancServer/Plugins/Samples/MultitenantDicom/OrthancFrameworkDependencies.cpp	Fri Jul 04 15:33:46 2025 +0200
+++ b/OrthancServer/Plugins/Samples/MultitenantDicom/OrthancFrameworkDependencies.cpp	Mon Jul 07 17:12:55 2025 +0200
@@ -77,6 +77,7 @@
 #include "../../../../OrthancFramework/Sources/Images/PngReader.cpp"
 #include "../../../../OrthancFramework/Sources/Images/PngWriter.cpp"
 #include "../../../../OrthancFramework/Sources/Logging.cpp"
+#include "../../../../OrthancFramework/Sources/MetricsRegistry.cpp"
 #include "../../../../OrthancFramework/Sources/MultiThreading/RunnableWorkersPool.cpp"
 #include "../../../../OrthancFramework/Sources/MultiThreading/SharedMessageQueue.cpp"
 #include "../../../../OrthancFramework/Sources/OrthancException.cpp"
--- a/OrthancServer/Resources/RunCppCheck.sh	Fri Jul 04 15:33:46 2025 +0200
+++ b/OrthancServer/Resources/RunCppCheck.sh	Mon Jul 07 17:12:55 2025 +0200
@@ -36,7 +36,7 @@
 assertWithSideEffect:../../OrthancServer/Plugins/Engine/OrthancPluginDatabase.cpp:1026
 assertWithSideEffect:../../OrthancServer/Sources/Database/Compatibility/DatabaseLookup.cpp:292
 assertWithSideEffect:../../OrthancServer/Sources/Database/Compatibility/DatabaseLookup.cpp:391
-assertWithSideEffect:../../OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp:3058
+assertWithSideEffect:../../OrthancServer/Sources/Database/StatelessDatabaseOperations.cpp:3066
 assertWithSideEffect:../../OrthancServer/Sources/ServerJobs/ResourceModificationJob.cpp:286
 assertWithSideEffect:../../OrthancFramework/Sources/DicomNetworking/Internals/CommandDispatcher.cpp:454
 EOF
--- a/OrthancServer/Sources/main.cpp	Fri Jul 04 15:33:46 2025 +0200
+++ b/OrthancServer/Sources/main.cpp	Mon Jul 07 17:12:55 2025 +0200
@@ -1274,7 +1274,7 @@
     ModalitiesFromConfiguration modalities;
   
     // Setup the DICOM server  
-    DicomServer dicomServer;
+    DicomServer dicomServer(context.GetMetricsRegistry());
     dicomServer.SetRemoteModalities(modalities);
     dicomServer.SetStoreRequestHandlerFactory(serverFactory);
     dicomServer.SetMoveRequestHandlerFactory(serverFactory);