respond to comments

Signed-off-by: Tim Li <[email protected]>

Author: timl3136

cadence/_internal/visibility/__init__.py

@@ -12,7 +12,7 @@
 from grpc.aio import Channel, ClientInterceptor, secure_channel, insecure_channel
 from cadence.api.v1.service_workflow_pb2_grpc import WorkflowAPIStub
 from cadence.data_converter import DataConverter, DefaultDataConverter
-from cadence._internal.visibility.metrics import MetricsEmitter, NoOpMetricsEmitter
+from cadence.metrics import MetricsEmitter, NoOpMetricsEmitter
 
 
 class ClientOptions(TypedDict, total=False):

cadence/client.py

@@ -0,0 +1,12 @@
+"""Metrics collection components for Cadence client."""
+
+from .metrics import MetricsEmitter, NoOpMetricsEmitter, MetricType
+from .prometheus import PrometheusMetrics, PrometheusConfig
+
+__all__ = [
+    "MetricsEmitter",
+    "NoOpMetricsEmitter",
+    "MetricType",
+    "PrometheusMetrics", 
+    "PrometheusConfig",
+]

cadence/metrics/__init__.py

@@ -2,7 +2,6 @@
 
 import logging
 from dataclasses import dataclass, field
-from enum import Enum
 from typing import Dict, Optional
 
 from prometheus_client import (  # type: ignore[import-not-found]
@@ -14,7 +13,7 @@
     generate_latest,
 )
 
-from cadence._internal.visibility.metrics import MetricsEmitter
+from .metrics import MetricsEmitter
 
 
 logger = logging.getLogger(__name__)
@@ -24,9 +23,6 @@
 class PrometheusConfig:
     """Configuration for Prometheus metrics."""
 
-    # Metric name prefix
-    metric_prefix: str = "cadence_"
-
     # Default labels to apply to all metrics
     default_labels: Dict[str, str] = field(default_factory=dict)
 
@@ -47,8 +43,8 @@ def __init__(self, config: Optional[PrometheusConfig] = None):
         self._histograms: Dict[str, Histogram] = {}
 
     def _get_metric_name(self, name: str) -> str:
-        """Get the full metric name with prefix."""
-        return f"{self.config.metric_prefix}{name}"
+        """Get the metric name."""
+        return name
 
     def _merge_labels(self, labels: Optional[Dict[str, str]]) -> Dict[str, str]:
         """Merge provided labels with default labels."""
@@ -169,30 +165,3 @@ def get_metrics_text(self) -> str:
         except Exception as e:
             logger.error(f"Failed to generate metrics text: {e}")
             return ""
-
-
-# Default Cadence metrics names
-class CadenceMetrics(Enum):
-    """Standard Cadence client metrics."""
-
-    # Workflow metrics
-    WORKFLOW_STARTED_TOTAL = "workflow_started_total"
-    WORKFLOW_COMPLETED_TOTAL = "workflow_completed_total"
-    WORKFLOW_FAILED_TOTAL = "workflow_failed_total"
-    WORKFLOW_DURATION_SECONDS = "workflow_duration_seconds"
-
-    # Activity metrics
-    ACTIVITY_STARTED_TOTAL = "activity_started_total"
-    ACTIVITY_COMPLETED_TOTAL = "activity_completed_total"
-    ACTIVITY_FAILED_TOTAL = "activity_failed_total"
-    ACTIVITY_DURATION_SECONDS = "activity_duration_seconds"
-
-    # Worker metrics
-    WORKER_TASK_POLLS_TOTAL = "worker_task_polls_total"
-    WORKER_TASK_POLL_ERRORS_TOTAL = "worker_task_poll_errors_total"
-    WORKER_ACTIVE_TASKS = "worker_active_tasks"
-
-    # Client metrics
-    CLIENT_REQUESTS_TOTAL = "client_requests_total"
-    CLIENT_REQUEST_DURATION_SECONDS = "client_request_duration_seconds"
-    CLIENT_REQUEST_ERRORS_TOTAL = "client_request_errors_total"

cadence/_internal/visibility/metrics.py renamed to cadence/metrics/metrics.py

@@ -3,7 +3,7 @@
 from unittest.mock import Mock
 
 
-from cadence._internal.visibility.metrics import (
+from cadence.metrics import (
     MetricsEmitter,
     MetricType,
     NoOpMetricsEmitter,

cadence/_internal/visibility/prometheus.py renamed to cadence/metrics/prometheus.py

@@ -2,10 +2,9 @@
 
 from unittest.mock import Mock, patch
 
-from cadence._internal.visibility.prometheus import (
+from cadence.metrics import (
     PrometheusMetrics,
     PrometheusConfig,
-    CadenceMetrics,
 )
 
 
@@ -15,7 +14,6 @@ class TestPrometheusConfig:
     def test_default_config(self):
         """Test default configuration values."""
         config = PrometheusConfig()
-        assert config.metric_prefix == "cadence_"
         assert config.default_labels == {}
         assert config.registry is None
 
@@ -25,11 +23,9 @@ def test_custom_config(self):
 
         registry = CollectorRegistry()
         config = PrometheusConfig(
-            metric_prefix="my_",
             default_labels={"env": "test"},
             registry=registry
         )
-        assert config.metric_prefix == "my_"
         assert config.default_labels == {"env": "test"}
         assert config.registry is registry
 
@@ -40,7 +36,6 @@ class TestPrometheusMetrics:
     def test_init_with_default_config(self):
         """Test initialization with default config."""
         metrics = PrometheusMetrics()
-        assert metrics.config.metric_prefix == "cadence_"
         assert metrics.registry is not None
 
     def test_init_with_custom_config(self):
@@ -49,15 +44,13 @@ def test_init_with_custom_config(self):
 
         registry = CollectorRegistry()
         config = PrometheusConfig(
-            metric_prefix="custom_",
             default_labels={"service": "test"},
             registry=registry
         )
         metrics = PrometheusMetrics(config)
-        assert metrics.config.metric_prefix == "custom_"
         assert metrics.registry is registry
 
-    @patch('cadence._internal.visibility.prometheus.Counter')
+    @patch('cadence.metrics.prometheus.Counter')
     def test_counter_metric(self, mock_counter_class):
         """Test counter metric creation and usage."""
         mock_counter = Mock()
@@ -71,7 +64,7 @@ def test_counter_metric(self, mock_counter_class):
         mock_counter.labels.assert_called_once_with(label="value")
         mock_counter.labels.return_value.inc.assert_called_once_with(5)
 
-    @patch('cadence._internal.visibility.prometheus.Gauge')
+    @patch('cadence.metrics.prometheus.Gauge')
     def test_gauge_metric(self, mock_gauge_class):
         """Test gauge metric creation and usage."""
         mock_gauge = Mock()
@@ -85,7 +78,7 @@ def test_gauge_metric(self, mock_gauge_class):
         mock_gauge.labels.assert_called_once_with(env="prod")
         mock_gauge.labels.return_value.set.assert_called_once_with(42.5)
 
-    @patch('cadence._internal.visibility.prometheus.Histogram')
+    @patch('cadence.metrics.prometheus.Histogram')
     def test_histogram_metric(self, mock_histogram_class):
         """Test histogram metric creation and usage."""
         mock_histogram = Mock()
@@ -101,12 +94,11 @@ def test_histogram_metric(self, mock_histogram_class):
 
 
     def test_metric_name_generation(self):
-        """Test metric name generation with prefix."""
-        config = PrometheusConfig(metric_prefix="my_app_")
-        metrics = PrometheusMetrics(config)
+        """Test metric name generation."""
+        metrics = PrometheusMetrics()
 
         metric_name = metrics._get_metric_name("test_metric")
-        assert metric_name == "my_app_test_metric"
+        assert metric_name == "test_metric"
 
     def test_label_merging(self):
         """Test label merging with default labels."""
@@ -124,7 +116,7 @@ def test_label_merging(self):
         merged_none = metrics._merge_labels(None)
         assert merged_none == {"service": "cadence", "version": "1.0"}
 
-    @patch('cadence._internal.visibility.prometheus.generate_latest')
+    @patch('cadence.metrics.prometheus.generate_latest')
     def test_get_metrics_text(self, mock_generate_latest):
         """Test getting metrics in text format."""
         mock_generate_latest.return_value = b"# HELP test_metric Test metric\n# TYPE test_metric counter\ntest_metric 1.0\n"
@@ -153,32 +145,3 @@ def test_error_handling_in_gauge(self):
             metrics.gauge("test_gauge", 1.0)
             # Should not raise, just log error
 
-
-class TestCadenceMetrics:
-    """Test cases for CadenceMetrics enum."""
-
-    def test_workflow_metrics(self):
-        """Test workflow metric names."""
-        assert CadenceMetrics.WORKFLOW_STARTED_TOTAL.value == "workflow_started_total"
-        assert CadenceMetrics.WORKFLOW_COMPLETED_TOTAL.value == "workflow_completed_total"
-        assert CadenceMetrics.WORKFLOW_FAILED_TOTAL.value == "workflow_failed_total"
-        assert CadenceMetrics.WORKFLOW_DURATION_SECONDS.value == "workflow_duration_seconds"
-
-    def test_activity_metrics(self):
-        """Test activity metric names."""
-        assert CadenceMetrics.ACTIVITY_STARTED_TOTAL.value == "activity_started_total"
-        assert CadenceMetrics.ACTIVITY_COMPLETED_TOTAL.value == "activity_completed_total"
-        assert CadenceMetrics.ACTIVITY_FAILED_TOTAL.value == "activity_failed_total"
-        assert CadenceMetrics.ACTIVITY_DURATION_SECONDS.value == "activity_duration_seconds"
-
-    def test_worker_metrics(self):
-        """Test worker metric names."""
-        assert CadenceMetrics.WORKER_TASK_POLLS_TOTAL.value == "worker_task_polls_total"
-        assert CadenceMetrics.WORKER_TASK_POLL_ERRORS_TOTAL.value == "worker_task_poll_errors_total"
-        assert CadenceMetrics.WORKER_ACTIVE_TASKS.value == "worker_active_tasks"
-
-    def test_client_metrics(self):
-        """Test client metric names."""
-        assert CadenceMetrics.CLIENT_REQUESTS_TOTAL.value == "client_requests_total"
-        assert CadenceMetrics.CLIENT_REQUEST_DURATION_SECONDS.value == "client_request_duration_seconds"
-        assert CadenceMetrics.CLIENT_REQUEST_ERRORS_TOTAL.value == "client_request_errors_total"