Skip to content

Conversation

@josecelano
Copy link
Member

@josecelano josecelano commented Jun 17, 2025

Overhaul stats: Refactor AggregateValue in metrics package. Use the primitive type (u64 or f64) according to the mathematical aggregate function result type.

… 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.
@josecelano josecelano self-assigned this Jun 17, 2025
@josecelano josecelano requested a review from da2ce7 June 17, 2025 08:09
@josecelano josecelano linked an issue Jun 17, 2025 that may be closed by this pull request
@josecelano
Copy link
Member Author

ACK 7df7d36

@codecov
Copy link

codecov bot commented Jun 17, 2025

Codecov Report

Attention: Patch coverage is 97.22222% with 2 lines in your changes missing coverage. Please review.

Project coverage is 84.59%. Comparing base (70fd119) to head (7df7d36).
Report is 6 commits behind head on develop.

Files with missing lines Patch % Lines
...ges/metrics/src/metric_collection/aggregate/sum.rs 83.33% 2 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@josecelano josecelano merged commit 1ad19e7 into torrust:develop Jun 17, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Overhaul stats: Refactor AggregateValue in metrics package

1 participant