refactor: metrics instrumentation framework typing and structure #12717
Conversation
|
Currently, only patch changes are allowed on master. Your PR labels (semver: minor) indicate that it cannot be merged into the master at this time. |
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class LabeledCounterPayload: |
There was a problem hiding this comment.
note: LabeledCounter has its own payload as it's diverging from the base Counter (e.g. it includes labels, which the base does not). Also, once schema versioning is introduced, it will likely apply only to LabeledCounter, as the base Counter payload should remain stable and not require schema changes (still TBD)
Test Results - Alternative Providers987 tests 584 ✅ 23m 10s ⏱️ Results for commit e3bd187. ♻️ This comment has been updated with latest results. |
Test Results (amd64) - Integration, Bootstrap 5 files 5 suites 2h 21m 36s ⏱️ Results for commit e3bd187. ♻️ This comment has been updated with latest results. |
9429212 to
e3bd187
Compare
Motivation
follow-up to PR #12386
The refactoring simplifies the class hierarchy and removes the overloaded factory pattern, which is increasingly becoming a code smell, as Counter and LabeledCounter evolve with diverging behavior and only partially overlapping interfaces.
The previous design made it increasingly difficult to enforce both static and dynamic typing guarantees, leading to ambiguity around the correct usage of the Counter and weakening the overall abstraction. The new structure clearly separates responsibilities, improves type safety at both runtime and development time, and lays a more solid foundation for extensibility.
Additionally, the monolithic
metrics.pymodule will be split into smaller, focused modules.Testing
pytest tests/unit/utils/analytics/test_metrics.pyTODO
note: the corresponding updates to the classes using the Counter in the ext package have been addressed in PR #4638