sentry-native
sentry-native copied to clipboard
Add Metrics API
This PR introduces the Metrics API for sentry-native
.
Current progress:
- [x] API for creating, enriching and capturing metrics
- [x] Background metrics aggregation
- [x] statsd-encoding for envelope items
- [x] Periodical metrics flushing
- [x] Normalization for envelope items
Related to #953
Codecov Report
Attention: Patch coverage is 86.35607%
with 82 lines
in your changes are missing coverage. Please review.
Project coverage is 84.09%. Comparing base (
235f837
) to head (d65b93a
). Report is 5 commits behind head on master.
:exclamation: Current head d65b93a differs from pull request most recent head c52e3c7
Please upload reports for the commit c52e3c7 to get more accurate results.
Additional details and impacted files
@@ Coverage Diff @@
## master #985 +/- ##
==========================================
- Coverage 86.53% 84.09% -2.44%
==========================================
Files 40 55 +15
Lines 3736 6044 +2308
Branches 0 1315 +1315
==========================================
+ Hits 3233 5083 +1850
- Misses 503 841 +338
- Partials 0 120 +120
@tustanivsky, do you already understand why the Windows tests time out?
@tustanivsky, do you already understand why the Windows tests time out?
Not really, things work as expected on Windows when running tests locally.
@tustanivsky, do you already understand why the Windows tests time out?
Not really, things work as expected on Windows when running tests locally.
I think it happens because a debug assertion failed, and we are stuck in a dialog on the CI runner. Will investigate.
Since it is also stuck in metics_name_sanitize
, it might hint at some sanitizer assumptions that also affect the Android build. 🤞
As we're fundamentally shifting how Metrics will work at Sentry, I suggest putting this PR on hold for now. At least until we know how the next steps look like.
As we're fundamentally shifting how Metrics will work at Sentry, I suggest putting this PR on hold for now. At least until we know how the next steps look like,
Thanks for the heads up, @cleptric.
@tustanivsky, I would approve this PR in its current state. At least I don't see any blockers for a merge. But given the news in https://github.com/getsentry/sentry-native/pull/985#issuecomment-2145328051, I will keep merging blocked.
When this gets unblocked, I would appreciate it if @markushi could also give this a quick review so we have the perspective of another metrics implementer.
CC: @kahest