Skip to content

Commit 43aac5d

Browse files
committed
change naming and combine metricsLogic
Signed-off-by: Tim Li <ltim@uber.com>
1 parent f125401 commit 43aac5d

File tree

6 files changed

+91
-188
lines changed

6 files changed

+91
-188
lines changed

.cursorrules

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,15 @@ pip install -e ".[dev]"
3535
- Use `uv run python scripts/dev.py` for development tasks
3636

3737
## Code Quality
38+
- **ALWAYS run linter and type checker after making code changes**
3839
- Run linter with auto-fix: `uv tool run ruff check --fix`
3940
- Run type checking: `uv tool run mypy cadence/`
40-
- Always run both commands before committing code changes
4141
- Use `uv tool run ruff check --fix && uv tool run mypy cadence/` to run both together
42+
- **Standard workflow**: Make changes → Run linter → Run type checker → Commit
43+
44+
## Development Workflow
45+
1. Make code changes
46+
2. Run `uv tool run ruff check --fix` (fixes formatting and linting issues)
47+
3. Run `uv tool run mypy cadence/` (checks type safety)
48+
4. Run `uv run python -m pytest` (run tests)
49+
5. Commit changes
Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
"""Visibility and metrics collection components for Cadence client."""
22

3-
from .metrics import MetricsRegistry, get_default_registry
3+
from .metrics import MetricsHandler, NoOpMetricsHandler, get_default_handler
44
from .prometheus import PrometheusMetrics, PrometheusConfig
55

66
__all__ = [
7-
"MetricsRegistry",
8-
"get_default_registry",
7+
"MetricsHandler",
8+
"NoOpMetricsHandler",
9+
"get_default_handler",
910
"PrometheusMetrics",
1011
"PrometheusConfig",
1112
]

cadence/_internal/visibility/metrics.py

Lines changed: 16 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import logging
44
from enum import Enum
5-
from typing import Dict, Optional, Protocol, Set
5+
from typing import Dict, Optional, Protocol
66

77
logger = logging.getLogger(__name__)
88

@@ -16,7 +16,7 @@ class MetricType(Enum):
1616
SUMMARY = "summary"
1717

1818

19-
class MetricsCollector(Protocol):
19+
class MetricsHandler(Protocol):
2020
"""Protocol for metrics collection backends."""
2121

2222
def counter(
@@ -44,8 +44,8 @@ def histogram(
4444
...
4545

4646

47-
class NoOpMetricsCollector:
48-
"""No-op metrics collector that discards all metrics."""
47+
class NoOpMetricsHandler:
48+
"""No-op metrics handler that discards all metrics."""
4949

5050
def counter(
5151
self, key: str, n: int = 1, tags: Optional[Dict[str, str]] = None
@@ -68,77 +68,19 @@ def histogram(
6868
pass
6969

7070

71-
class MetricsRegistry:
72-
"""Registry for managing metrics collection in the Cadence client."""
71+
# Global default handler
72+
_default_handler: Optional[MetricsHandler] = None
7373

74-
def __init__(self, collector: Optional[MetricsCollector] = None):
75-
self._collector = collector or NoOpMetricsCollector()
76-
self._registered_metrics: Set[str] = set()
7774

78-
def set_collector(self, collector: MetricsCollector) -> None:
79-
"""Set the metrics collector backend."""
80-
self._collector = collector
81-
logger.info(f"Metrics collector set to {type(collector).__name__}")
75+
def get_default_handler() -> MetricsHandler:
76+
"""Get the default global metrics handler."""
77+
global _default_handler
78+
if _default_handler is None:
79+
_default_handler = NoOpMetricsHandler()
80+
return _default_handler
8281

83-
def register_metric(self, name: str, metric_type: MetricType) -> None:
84-
"""Register a metric with the registry."""
85-
if name in self._registered_metrics:
86-
logger.warning(f"Metric {name} already registered")
87-
return
8882

89-
self._registered_metrics.add(name)
90-
logger.debug(f"Registered {metric_type.value} metric: {name}")
91-
92-
def counter(
93-
self, key: str, n: int = 1, tags: Optional[Dict[str, str]] = None
94-
) -> None:
95-
"""Send a counter metric."""
96-
try:
97-
self._collector.counter(key, n, tags)
98-
except Exception as e:
99-
logger.error(f"Failed to send counter {key}: {e}")
100-
101-
def gauge(
102-
self, key: str, value: float, tags: Optional[Dict[str, str]] = None
103-
) -> None:
104-
"""Send a gauge metric."""
105-
try:
106-
self._collector.gauge(key, value, tags)
107-
except Exception as e:
108-
logger.error(f"Failed to send gauge {key}: {e}")
109-
110-
def timer(
111-
self, key: str, duration: float, tags: Optional[Dict[str, str]] = None
112-
) -> None:
113-
"""Send a timer metric."""
114-
try:
115-
self._collector.timer(key, duration, tags)
116-
except Exception as e:
117-
logger.error(f"Failed to send timer {key}: {e}")
118-
119-
def histogram(
120-
self, key: str, value: float, tags: Optional[Dict[str, str]] = None
121-
) -> None:
122-
"""Send a histogram metric."""
123-
try:
124-
self._collector.histogram(key, value, tags)
125-
except Exception as e:
126-
logger.error(f"Failed to send histogram {key}: {e}")
127-
128-
129-
# Global default registry
130-
_default_registry: Optional[MetricsRegistry] = None
131-
132-
133-
def get_default_registry() -> MetricsRegistry:
134-
"""Get the default global metrics registry."""
135-
global _default_registry
136-
if _default_registry is None:
137-
_default_registry = MetricsRegistry()
138-
return _default_registry
139-
140-
141-
def set_default_registry(registry: MetricsRegistry) -> None:
142-
"""Set the default global metrics registry."""
143-
global _default_registry
144-
_default_registry = registry
83+
def set_default_handler(handler: MetricsHandler) -> None:
84+
"""Set the default global metrics handler."""
85+
global _default_handler
86+
_default_handler = handler

cadence/_internal/visibility/prometheus.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -146,10 +146,10 @@ def _get_or_create_summary(
146146

147147
return self._summaries[metric_name]
148148

149-
def count(
149+
def counter(
150150
self, key: str, n: int = 1, tags: Optional[Dict[str, str]] = None
151151
) -> None:
152-
"""Send a count metric (aligned with M3.count)."""
152+
"""Send a counter metric."""
153153
try:
154154
counter = self._get_or_create_counter(key, tags)
155155
merged_tags = self._merge_labels(tags)
@@ -160,12 +160,12 @@ def count(
160160
counter.inc(n)
161161

162162
except Exception as e:
163-
logger.error(f"Failed to send count {key}: {e}")
163+
logger.error(f"Failed to send counter {key}: {e}")
164164

165165
def gauge(
166166
self, key: str, value: float, tags: Optional[Dict[str, str]] = None
167167
) -> None:
168-
"""Send a gauge metric (aligned with M3.gauge)."""
168+
"""Send a gauge metric."""
169169
try:
170170
gauge = self._get_or_create_gauge(key, tags)
171171
merged_tags = self._merge_labels(tags)
@@ -178,10 +178,10 @@ def gauge(
178178
except Exception as e:
179179
logger.error(f"Failed to send gauge {key}: {e}")
180180

181-
def timing(
181+
def timer(
182182
self, key: str, duration: float, tags: Optional[Dict[str, str]] = None
183183
) -> None:
184-
"""Send a timing metric (aligned with M3.timing) - implemented as histogram."""
184+
"""Send a timer metric - implemented as histogram."""
185185
try:
186186
histogram = self._get_or_create_histogram(key, tags)
187187
merged_tags = self._merge_labels(tags)
@@ -192,12 +192,12 @@ def timing(
192192
histogram.observe(duration)
193193

194194
except Exception as e:
195-
logger.error(f"Failed to send timing {key}: {e}")
195+
logger.error(f"Failed to send timer {key}: {e}")
196196

197197
def histogram(
198198
self, key: str, value: float, tags: Optional[Dict[str, str]] = None
199199
) -> None:
200-
"""Send a histogram metric (aligned with M3.histogram)."""
200+
"""Send a histogram metric."""
201201
try:
202202
histogram = self._get_or_create_histogram(key, tags)
203203
merged_tags = self._merge_labels(tags)

tests/cadence/_internal/visibility/test_metrics.py

Lines changed: 44 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -4,126 +4,78 @@
44

55

66
from cadence._internal.visibility.metrics import (
7-
MetricsRegistry,
8-
MetricType,
9-
NoOpMetricsCollector,
10-
get_default_registry,
11-
set_default_registry,
7+
MetricsHandler,
8+
NoOpMetricsHandler,
9+
get_default_handler,
10+
set_default_handler,
1211
)
1312

1413

15-
class TestMetricsRegistry:
16-
"""Test cases for MetricsRegistry."""
14+
class TestMetricsHandler:
15+
"""Test cases for MetricsHandler protocol."""
1716

18-
def test_registry_with_no_collector(self):
19-
"""Test registry with default no-op collector."""
20-
registry = MetricsRegistry()
17+
def test_noop_handler(self):
18+
"""Test no-op handler doesn't raise exceptions."""
19+
handler = NoOpMetricsHandler()
2120

2221
# Should not raise any exceptions
23-
registry.counter("test_counter", 1)
24-
registry.gauge("test_gauge", 42.0)
25-
registry.histogram("test_histogram", 0.5)
26-
registry.timer("test_timing", 1.5)
22+
handler.counter("test_counter", 1)
23+
handler.gauge("test_gauge", 42.0)
24+
handler.histogram("test_histogram", 0.5)
25+
handler.timer("test_timing", 1.5)
2726

28-
def test_registry_with_mock_collector(self):
29-
"""Test registry with mock collector."""
30-
mock_collector = Mock()
31-
registry = MetricsRegistry(mock_collector)
27+
def test_mock_handler(self):
28+
"""Test mock handler implementation."""
29+
mock_handler = Mock(spec=MetricsHandler)
3230

3331
# Test counter
34-
registry.counter("test_counter", 2, {"label": "value"})
35-
mock_collector.counter.assert_called_once_with(
32+
mock_handler.counter("test_counter", 2, {"label": "value"})
33+
mock_handler.counter.assert_called_once_with(
3634
"test_counter", 2, {"label": "value"}
3735
)
3836

3937
# Test gauge
40-
registry.gauge("test_gauge", 100.0, {"env": "test"})
41-
mock_collector.gauge.assert_called_once_with(
38+
mock_handler.gauge("test_gauge", 100.0, {"env": "test"})
39+
mock_handler.gauge.assert_called_once_with(
4240
"test_gauge", 100.0, {"env": "test"}
4341
)
4442

4543
# Test timer
46-
registry.timer("test_timing", 0.75)
47-
mock_collector.timer.assert_called_once_with(
48-
"test_timing", 0.75, None
44+
mock_handler.timer("test_timing", 0.75, {"tag": "value"})
45+
mock_handler.timer.assert_called_once_with(
46+
"test_timing", 0.75, {"tag": "value"}
4947
)
5048

5149
# Test histogram
52-
registry.histogram("test_histogram", 2.5)
53-
mock_collector.histogram.assert_called_once_with(
54-
"test_histogram", 2.5, None
50+
mock_handler.histogram("test_histogram", 2.5, {"env": "prod"})
51+
mock_handler.histogram.assert_called_once_with(
52+
"test_histogram", 2.5, {"env": "prod"}
5553
)
5654

57-
def test_set_collector(self):
58-
"""Test setting a new collector."""
59-
registry = MetricsRegistry()
60-
mock_collector = Mock()
6155

62-
registry.set_collector(mock_collector)
63-
registry.counter("test", 1)
56+
class TestDefaultHandler:
57+
"""Test cases for default handler management."""
6458

65-
mock_collector.counter.assert_called_once_with("test", 1, None)
66-
67-
def test_register_metric(self):
68-
"""Test metric registration."""
69-
registry = MetricsRegistry()
70-
71-
registry.register_metric("test_counter", MetricType.COUNTER)
72-
registry.register_metric("test_gauge", MetricType.GAUGE)
73-
74-
# Registering the same metric twice should not raise an error
75-
registry.register_metric("test_counter", MetricType.COUNTER)
76-
77-
def test_collector_exception_handling(self):
78-
"""Test that collector exceptions are handled gracefully."""
79-
mock_collector = Mock()
80-
mock_collector.counter.side_effect = Exception("Test exception")
81-
82-
registry = MetricsRegistry(mock_collector)
83-
84-
# Should not raise exception, but log error
85-
registry.counter("test", 1)
86-
87-
mock_collector.counter.assert_called_once()
88-
89-
90-
class TestNoOpMetricsCollector:
91-
"""Test cases for NoOpMetricsCollector."""
92-
93-
def test_no_op_collector(self):
94-
"""Test that no-op collector doesn't raise exceptions."""
95-
collector = NoOpMetricsCollector()
96-
97-
# Should not raise any exceptions
98-
collector.counter("test", 1, {"label": "value"})
99-
collector.gauge("test", 42.0)
100-
collector.histogram("test", 0.5, {"env": "test"})
101-
collector.timer("test", 1.5)
102-
103-
104-
class TestDefaultRegistry:
105-
"""Test cases for default registry management."""
106-
107-
def test_get_default_registry(self):
108-
"""Test getting the default registry."""
109-
registry = get_default_registry()
110-
assert isinstance(registry, MetricsRegistry)
59+
def test_get_default_handler(self):
60+
"""Test getting the default handler."""
61+
handler = get_default_handler()
62+
assert isinstance(handler, NoOpMetricsHandler)
11163

11264
# Should return the same instance
113-
registry2 = get_default_registry()
114-
assert registry is registry2
65+
handler2 = get_default_handler()
66+
assert handler is handler2
11567

116-
def test_set_default_registry(self):
117-
"""Test setting a custom default registry."""
118-
original_registry = get_default_registry()
119-
custom_registry = MetricsRegistry()
68+
def test_set_default_handler(self):
69+
"""Test setting a custom default handler."""
70+
original_handler = get_default_handler()
71+
custom_handler = NoOpMetricsHandler()
12072

121-
set_default_registry(custom_registry)
73+
set_default_handler(custom_handler)
12274

123-
# Should return the custom registry
124-
current_registry = get_default_registry()
125-
assert current_registry is custom_registry
126-
assert current_registry is not original_registry
75+
# Should return the custom handler
76+
current_handler = get_default_handler()
77+
assert current_handler is custom_handler
78+
assert current_handler is not original_handler
12779

12880
# Restore original for other tests
129-
set_default_registry(original_registry)
81+
set_default_handler(original_handler)

0 commit comments

Comments
 (0)