[Fix] fix naming style and remove registry

Author: flesher0813

docs/source/developer-guide/add_metrics.md

@@ -3,8 +3,8 @@ UCM supports custom metrics with bidirectional updates from both Python and C++
 
 ## Architecture Overview
 The metrics consists of these components below:
-- **metrics** : Central stats registry that manages all metric lifecycle operations (registration, creation, updates, queries)
-- **observability.py** : Prometheus integration layer that handles metric exposition and multiprocess collection
+- **monitor** : Central stats registry that manages all metric lifecycle operations (registration, creation, updates, queries)
+- **observability.py** : Prometheus integration layer that handles metric exposition
 - **metrics_config.yaml** : Declarative configuration that defines which custom metrics to register and their properties
 
 ## Getting Started
@@ -31,7 +31,7 @@ prometheus:
 
   # Gauge metrics configuration
   gauges:
-    - name: "lookup_hit_rate"
+    - name: "external_lookup_hit_rate"
       documentation: "Hit rate of ucm lookup requests"
       multiprocess_mode: "livemostrecent"
 
@@ -43,15 +43,14 @@ prometheus:
 ```
 
 ### Use Monitor APIs to Update Stats
-The monitor provides a unified interface for metric operations. Note that the workflow requires registering a stats class before creating an instance.
+The monitor provides a unified interface for metric operations. Users only need to create stats and update them, while the observability component is responsible for fetching the stats and pushing them to Prometheus.
 :::::{tab-set}
 :sync-group: install
 
 ::::{tab-item} Python side interfaces
 :selected:
 :sync: py
 **Lifecycle Methods**
-- `register_istats(name, py::object)`: Register a new stats class implementation.
 - `create_stats(name)`: Create and initialize a registered stats object.
 
 **Operation Methods**
@@ -64,11 +63,8 @@ The monitor provides a unified interface for metric operations. Note that the wo
 
 **Example:** Using built-in ConnStats
 ```python
-from ucm.integration.vllm.conn_stats import ConnStats
 from ucm.shared.metrics import ucmmonitor
 
-conn_stats = ConnStats(name="ConnStats")
-ucmmonitor.register_stats("ConnStats", conn_stats) # Register stats
 ucmmonitor.create_stats("ConnStats") # Create a stats obj
 
 # Update stats
@@ -85,7 +81,6 @@ See more detailed example in [test case](https://github.com/ModelEngine-Group/un
 ::::{tab-item} C++ side interfaces
 :sync: cc
 **Lifecycle Methods**
-- `RegistStats(std::string name, Creator creator)`: Register a new stats class implementation.
 - `CreateStats(const std::string& name)`: Create and initialize a registered stats object.
 
 **Operation Methods**
@@ -102,10 +97,8 @@ UCM supports custom metrics by following steps:
    ```c++
     target_link_libraries(xxxstore PUBLIC storeinfra monitor_static)
     ```
-- Step 2: Inheriting from the IStats class to implement custom stats classes
-- Step 3: Register stats class to monitor
-- Step 4: Create stats object in monitor
-- Step 5: Update or get stats info using operation methods
+- Step 2: Create stats object using function **CreateStats**
+- Step 3: Update using function **UpdateStats**
 
 See more detailed example in [test case](https://github.com/ModelEngine-Group/unified-cache-management/tree/develop/ucm/shared/test/case).
 

ucm/integration/vllm/conn_stats.py

@@ -17,7 +17,6 @@
 from vllm.platforms import current_platform
 from vllm.v1.core.sched.output import SchedulerOutput
 
-from ucm.integration.vllm.conn_stats import ConnStats
 from ucm.logger import init_logger
 from ucm.observability import UCMStatsLogger
 from ucm.shared.metrics import ucmmonitor
@@ -173,8 +172,6 @@ def __init__(self, vllm_config: "VllmConfig", role: KVConnectorRole):
 
         self.metrics_config = self.launch_config.get("metrics_config_path", "")
         if self.metrics_config:
-            conn_stats = ConnStats(name="ConnStats")
-            ucmmonitor.register_stats("ConnStats", conn_stats)
             ucmmonitor.create_stats("ConnStats")
             self.stats_logger = UCMStatsLogger(
                 vllm_config.model_config.served_model_name,

ucm/integration/vllm/ucm_connector.py

@@ -21,22 +21,17 @@
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 # SOFTWARE.
 #
-
+# Be impressed by https://github.com/LMCache/LMCache/blob/dev/lmcache/observability.py
 
 import os
 import threading
 import time
 from dataclasses import dataclass
-from pathlib import Path
-from typing import Any, Dict, List, Optional, Union
+from typing import Any, List
 
 import prometheus_client
 import yaml
 
-# Third Party
-from prometheus_client import REGISTRY
-from vllm.distributed.parallel_state import get_world_group
-
 from ucm.logger import init_logger
 from ucm.shared.metrics import ucmmonitor
 
@@ -56,7 +51,7 @@ class PrometheusLogger:
     _counter_cls = prometheus_client.Counter
     _histogram_cls = prometheus_client.Histogram
 
-    def __init__(self, metadata: UCMEngineMetadata, config: Dict[str, Any]):
+    def __init__(self, metadata: UCMEngineMetadata, config: dict[str, Any]):
         # Ensure PROMETHEUS_MULTIPROC_DIR is set before any metric registration
         prometheus_config = config.get("prometheus", {})
         multiproc_dir = prometheus_config.get("multiproc_dir", "/vllm-workspace")
@@ -74,7 +69,7 @@ def __init__(self, metadata: UCMEngineMetadata, config: Dict[str, Any]):
         self._init_metrics_from_config(labelnames, prometheus_config)
 
     def _init_metrics_from_config(
-        self, labelnames: List[str], prometheus_config: Dict[str, Any]
+        self, labelnames: List[str], prometheus_config: dict[str, Any]
     ):
         """Initialize metrics based on configuration"""
         enabled = prometheus_config.get("enabled_metrics", {})
@@ -85,7 +80,7 @@ def _init_metrics_from_config(
 
         # Store metric mapping: metric_name -> (metric_type, attribute_name, stats_field_name)
         # This mapping will be used in log_prometheus to dynamically log metrics
-        self.metric_mappings: Dict[str, Dict[str, str]] = {}
+        self.metric_mappings: dict[str, dict[str, str]] = {}
 
         # Initialize counters
         if enabled.get("counters", True):
@@ -172,23 +167,23 @@ def _init_metrics_from_config(
                         "attr": attr_name,
                     }
 
-    def _log_gauge(self, gauge, data: Union[int, float]) -> None:
+    def _set_gauge(self, gauge, data: List) -> None:
         # Convenience function for logging to gauge.
+        if not data:
+            return
         gauge.labels(**self.labels).set(data)
 
-    def _log_counter(self, counter, data: Union[int, float]) -> None:
+    def _inc_counter(self, counter, data: List) -> None:
         # Convenience function for logging to counter.
         # Prevent ValueError from negative increment
-        if data < 0:
-            return
-        counter.labels(**self.labels).inc(data)
+        counter.labels(**self.labels).inc(sum(data))
 
-    def _log_histogram(self, histogram, data: Union[List[int], List[float]]) -> None:
+    def _observe_histogram(self, histogram, data: List) -> None:
         # Convenience function for logging to histogram.
         for value in data:
             histogram.labels(**self.labels).observe(value)
 
-    def log_prometheus(self, stats: Any):
+    def update_stats(self, stats: dict[str, List]):
         """Log metrics to Prometheus based on configuration file"""
         # Dynamically log metrics based on what's configured in YAML
         for stat_name, value in stats.items():
@@ -202,17 +197,14 @@ def log_prometheus(self, stats: Any):
 
                 # Log based on metric type
                 if metric_type == "counter":
-                    self._log_counter(metric_obj, value)
+                    self._set_gauge(metric_obj, value)
                 elif metric_type == "gauge":
-                    self._log_gauge(metric_obj, value)
+                    self._inc_counter(metric_obj, value)
                 elif metric_type == "histogram":
                     # Histograms expect a list
-                    if not isinstance(value, list):
-                        if value:
-                            value = [value]
-                        else:
-                            value = []
-                    self._log_histogram(metric_obj, value)
+                    self._observe_histogram(metric_obj, value)
+                else:
+                    logger.error(f"Not found metric type for {stat_name}")
             except Exception:
                 logger.debug(f"Failed to log metric {stat_name}")
 
@@ -223,40 +215,6 @@ def _metadata_to_labels(metadata: UCMEngineMetadata):
             "worker_id": metadata.worker_id,
         }
 
-    _instance = None
-
-    @staticmethod
-    def GetOrCreate(
-        metadata: UCMEngineMetadata,
-        config_path: str = "",
-    ) -> "PrometheusLogger":
-        if PrometheusLogger._instance is None:
-            PrometheusLogger._instance = PrometheusLogger(metadata, config_path)
-        # assert PrometheusLogger._instance.metadata == metadata, \
-        #    "PrometheusLogger instance already created with different metadata"
-        if PrometheusLogger._instance.metadata != metadata:
-            logger.error(
-                "PrometheusLogger instance already created with"
-                "different metadata. This should not happen except "
-                "in test"
-            )
-        return PrometheusLogger._instance
-
-    @staticmethod
-    def GetInstance() -> "PrometheusLogger":
-        assert (
-            PrometheusLogger._instance is not None
-        ), "PrometheusLogger instance not created yet"
-        return PrometheusLogger._instance
-
-    @staticmethod
-    def GetInstanceOrNone() -> Optional["PrometheusLogger"]:
-        """
-        Returns the singleton instance of PrometheusLogger if it exists,
-        otherwise returns None.
-        """
-        return PrometheusLogger._instance
-
 
 class UCMStatsLogger:
     def __init__(self, model_name: str, rank: int, config_path: str = ""):
@@ -267,13 +225,13 @@ def __init__(self, model_name: str, rank: int, config_path: str = ""):
         # Load configuration
         config = self._load_config(config_path)
         self.log_interval = config.get("log_interval", 10)
-        self.prometheus_logger = PrometheusLogger.GetOrCreate(self.metadata, config)
+        self.prometheus_logger = PrometheusLogger(self.metadata, config)
         self.is_running = True
 
         self.thread = threading.Thread(target=self.log_worker, daemon=True)
         self.thread.start()
 
-    def _load_config(self, config_path: str) -> Dict[str, Any]:
+    def _load_config(self, config_path: str) -> dict[str, Any]:
         """Load configuration from YAML file"""
         try:
             with open(config_path, "r") as f:
@@ -293,11 +251,16 @@ def _load_config(self, config_path: str) -> Dict[str, Any]:
 
     def log_worker(self):
         while self.is_running:
-            # Use UCMStatsMonitor.get_states_and_clear() from external import
             stats = ucmmonitor.get_all_stats_and_clear().data
-            self.prometheus_logger.log_prometheus(stats)
+            self.prometheus_logger.update_stats(stats)
             time.sleep(self.log_interval)
 
     def shutdown(self):
         self.is_running = False
         self.thread.join()
+
+    def __del__(self):
+        try:
+            self.shutdown()
+        except Exception:
+            pass

ucm/observability.py

@@ -24,11 +24,6 @@
 #include "stats_monitor_api.h"
 namespace UC::Metrics {
 
-void RegistStats(std::string name, Creator creator)
-{
-    StatsRegistry::GetInstance().RegisterStats(name, creator);
-}
-
 void CreateStats(const std::string& name) { StatsMonitor::GetInstance().CreateStats(name); }
 
 void UpdateStats(const std::string& name, const std::unordered_map<std::string, double>& params)

ucm/shared/metrics/cc/api/stats_monitor_api.cc

@@ -23,8 +23,6 @@
  * */
 #ifndef UNIFIEDCACHE_MONITOR_API_H
 #define UNIFIEDCACHE_MONITOR_API_H
-#include <string>
-#include <unordered_map>
 #include "stats_monitor.h"
 
 namespace UC::Metrics {
@@ -33,7 +31,6 @@ struct StatsResult {
     std::unordered_map<std::string, std::vector<double>> data;
 };
 
-void RegistStats(std::string name, Creator creator);
 void CreateStats(const std::string& name);
 void UpdateStats(const std::string& name, const std::unordered_map<std::string, double>& params);
 void ResetStats(const std::string& name);

ucm/shared/metrics/cc/api/stats_monitor_api.h

@@ -21,24 +21,29 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  * */
-#ifndef UNIFIEDCACHE_ISTATS_H
-#define UNIFIEDCACHE_ISTATS_H
+#ifndef UNIFIEDCACHE_STATS_H
+#define UNIFIEDCACHE_STATS_H
 
-#include <functional>
-#include <memory>
 #include <string>
 #include <unordered_map>
 #include <vector>
 
 namespace UC::Metrics {
 
-class IStats {
+class Stats {
 public:
-    virtual ~IStats() = default;
-    virtual std::string Name() const = 0;
-    virtual void Update(const std::unordered_map<std::string, double>& params) = 0;
-    virtual void Reset() = 0;
-    virtual std::unordered_map<std::string, std::vector<double>> Data() = 0;
+    explicit Stats(const std::string& name) : name_(name) {}
+    std::string Name() { return name_; }
+    void Update(const std::unordered_map<std::string, double>& params)
+    {
+        for (const auto& [key, val] : params) { data_[key].push_back(val); }
+    }
+    void Reset() { data_.clear(); }
+    std::unordered_map<std::string, std::vector<double>> Data() { return data_; }
+
+private:
+    std::string name_;
+    std::unordered_map<std::string, std::vector<double>> data_;
 };
 
 } // namespace UC::Metrics