-
Notifications
You must be signed in to change notification settings - Fork 49
Overhaul stats: Refactor AggregateValue in metrics package
#1585
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
josecelano
merged 5 commits into
torrust:develop
from
josecelano:1580-overhaul-stats-refactor-aggregatevalue-in-metrics-package
Jun 17, 2025
Merged
Overhaul stats: Refactor AggregateValue in metrics package
#1585
josecelano
merged 5 commits into
torrust:develop
from
josecelano:1580-overhaul-stats-refactor-aggregatevalue-in-metrics-package
Jun 17, 2025
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… types for mathematically correct return types - Replace AggregateValue return type with associated Output type in Sum trait - Counter metrics now return u64 (preserving integer precision) - Gauge metrics now return f64 (avoiding unnecessary wrapper type) - Update all test cases to expect primitive types instead of AggregateValue - Convert primitive results to AggregateValue at collection level for backward compatibility - Use proper floating-point comparison in gauge tests with epsilon tolerance This change ensures each aggregate function returns the mathematically appropriate type while maintaining API compatibility for metric collections.
…level Sum trait - Convert collection Sum trait from fixed return type to associated Output type - MetricKindCollection<Counter> now returns Option<u64> preserving integer precision - MetricKindCollection<Gauge> now returns Option<f64> for direct float access - MetricCollection maintains Option<AggregateValue> for backward compatibility - Simplify implementation by directly delegating to metric-level sum methods - Remove intermediate conversions in metric kind collections This completes the associated types pattern across both metric-level and collection-level Sum traits, allowing each implementation to return the most mathematically appropriate type while maintaining API compatibility.
…rn primitive types from aggregates - Remove AggregateValue struct and its entire module from metrics package - Simplify Sum trait in metric collections to return Option<f64> directly - Update MetricKindCollection implementations to cast counter values to f64 - Remove AggregateValue dependencies from http-tracker-core, udp-tracker-core, and udp-tracker-server - Eliminate unnecessary wrapper overhead in aggregate operations - Maintain backward compatibility by converting all aggregate results to f64 This change completes the metrics package refactoring by removing the generic AggregateValue wrapper that added no value when aggregate functions can return mathematically appropriate primitive types directly.
…ates into submodules - Move metric_collection/aggregate.rs to aggregate/sum.rs submodule - Create proper module structure for aggregate operations - Update import paths in http-tracker-core, udp-tracker-core, and udp-tracker-server - Change imports from `aggregate::Sum` to `aggregate::sum::Sum` - Maintain the same Sum trait functionality with cleaner module organization This reorganization prepares for potential future aggregate operations beyond just sum while keeping the existing Sum trait API intact.
…amples - Add detailed overview and key features section - Include quick start guide with practical usage examples - Document architecture with core components and type system - Add comprehensive development guide with building, testing, and coverage - Include performance considerations and compatibility notes - Add contributing guidelines and related projects - Transform from basic description to full developer documentation - Update cSpell.json with new technical terms (println, serde) This provides much better onboarding for developers and users of the metrics library.
|
ACK 7df7d36 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1585 +/- ##
===========================================
- Coverage 84.71% 84.59% -0.13%
===========================================
Files 288 287 -1
Lines 21498 21382 -116
Branches 21498 21382 -116
===========================================
- Hits 18212 18088 -124
- Misses 2966 2979 +13
+ Partials 320 315 -5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overhaul stats: Refactor
AggregateValueinmetricspackage. Use the primitive type (u64 or f64) according to the mathematical aggregate function result type.