[part 8] refactor!(telemetry): rework metrics API#206
Conversation
BenchmarksBenchmark execution time: 2025-04-21 15:20:12 Comparing candidate commit c6e60dc in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics. |
dmehala
force-pushed
the
dmehala/part-8-metrics-usage
branch
3 times, most recently
from
6bb0603 to
8484803
Compare
Redesigned the metrics API to eliminate the need for pre-registering a list of metrics. Previously, the API relied on a pull-based approach where metrics had to be registered during the construction of the telemetry module. The module would then periodically retrieve values from the registered metrics. While functional, this design was cumbersome—especially for metrics involving dynamic tags. This refactoring introduces a push-based mechanism. Calls to `rate`, `counter`, or `distribution` now directly send data to the telemetry module, removing the need for upfront registration. This approach simplifies usage and improves flexibility.
dmehala
force-pushed
the
dmehala/part-8-metrics-usage
branch
from
8484803 to
2f4dd5f
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #206 +/- ##
==========================================
- Coverage 89.67% 87.09% -2.58%
==========================================
Files 80 80
Lines 4705 5083 +378
==========================================
+ Hits 4219 4427 +208
- Misses 486 656 +170 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| namespace datadog::telemetry { | ||
|
|
||
| // This uses a reference_wrapper so references to internal metric values can |
There was a problem hiding this comment.
nit: not a reference_wrapper anymore
| if (auto distributions_series = encode_distributions(distributions); | ||
| !distributions.empty()) { |
There was a problem hiding this comment.
nit: if I'm understanding correctly, !distributions.empty() will be checked after encode_distributions(distributions) has been run. Is there a reason to not check for emptiness first, and then encode them if necessary?
There was a problem hiding this comment.
In the end what matters is the JSON payload. In the unlikely event that encode_distributions returns an empty JSON even if distributions is not empty, this approach ensures that the empty payload is not added. While such a scenario is unlikely, it offers a better guarantee IMO. That said, whether it's written this way or as you suggested, the outcome is effectively the same
Co-authored-by: pablomartinezbernardo <134320516+pablomartinezbernardo@users.noreply.github.com>
4 participants
Description
Redesigned the metrics API to eliminate the need for pre-registering a list of metrics.
Previously, the API relied on a pull-based approach where metrics had to be registered during the construction of the telemetry. The module would then periodically retrieve values from the registered metrics. While functional, this design was cumbersome - especially for metrics involving dynamic tags.
This refactoring introduces a push-based mechanism. Calls to
rate,counter, ordistributionnow directly send data to the telemetry instance, removing the need for upfront registration.This approach simplifies usage and improves flexibility.