sentry-native icon indicating copy to clipboard operation
sentry-native copied to clipboard

Add Metrics API

Open tustanivsky opened this issue 9 months ago • 7 comments

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

tustanivsky avatar May 06 '24 11:05 tustanivsky

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     

codecov[bot] avatar May 29 '24 09:05 codecov[bot]

@tustanivsky, do you already understand why the Windows tests time out?

supervacuus avatar Jun 03 '24 12:06 supervacuus

@tustanivsky, do you already understand why the Windows tests time out?

Not really, things work as expected on Windows when running tests locally.

tustanivsky avatar Jun 03 '24 12:06 tustanivsky

@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. 🤞

supervacuus avatar Jun 03 '24 13:06 supervacuus

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.

cleptric avatar Jun 03 '24 14:06 cleptric

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.

supervacuus avatar Jun 03 '24 14:06 supervacuus

@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

supervacuus avatar Jun 03 '24 14:06 supervacuus