Skip to content
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

feat: Add Metrics API #3791

Merged
merged 16 commits into from
Mar 27, 2024
Merged

feat: Add Metrics API #3791

merged 16 commits into from
Mar 27, 2024

Conversation

philipphofmann
Copy link
Member

@philipphofmann philipphofmann commented Mar 26, 2024

📜 Description

Add beta implementation of MetricsAPI.

💡 Motivation and Context

#3631

Docs PR getsentry/sentry-docs#9554.

💚 How did you test it?

Unit tests and simulator.

📝 Checklist

You have to check all boxes before merging:

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Copy link

github-actions bot commented Mar 26, 2024

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 9aed2e3

Copy link

codecov bot commented Mar 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.511%. Comparing base (69d2789) to head (9aed2e3).

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3791       +/-   ##
=============================================
+ Coverage   89.473%   89.511%   +0.038%     
=============================================
  Files          557       559        +2     
  Lines        60247     60542      +295     
  Branches     21669     21747       +78     
=============================================
+ Hits         53905     54192      +287     
- Misses        5306      5426      +120     
+ Partials      1036       924      -112     
Files Coverage Δ
Sources/Sentry/SentryHub.m 98.611% <100.000%> (+0.091%) ⬆️
Sources/Sentry/SentryNSDataUtils.m 95.652% <100.000%> (+0.414%) ⬆️
Sources/Sentry/SentryOptions.m 97.703% <100.000%> (+0.039%) ⬆️
Sources/Sentry/SentrySDK.m 90.681% <100.000%> (+0.101%) ⬆️
Sources/Sentry/SentrySpan.m 98.522% <100.000%> (+0.068%) ⬆️
Sources/Sentry/SentryTransaction.m 87.654% <100.000%> (+0.987%) ⬆️
...urces/Swift/Metrics/BucketsMetricsAggregator.swift 100.000% <100.000%> (ø)
Sources/Swift/Metrics/LocalMetricsAggregator.swift 100.000% <100.000%> (ø)
Sources/Swift/Metrics/SentryMetricsAPI.swift 100.000% <100.000%> (ø)
Sources/Swift/Metrics/SetMetric.swift 100.000% <100.000%> (ø)
... and 10 more

... and 46 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69d2789...9aed2e3. Read the comment docs.

Copy link
Contributor

@brustolin brustolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

I have just one suggestion.

Sources/Sentry/SentryHub.m Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Mar 26, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1234.25 ms 1252.13 ms 17.88 ms
Size 21.58 KiB 571.85 KiB 550.27 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
01a28a9 1237.24 ms 1253.24 ms 16.00 ms
60bfc91 1220.39 ms 1250.89 ms 30.50 ms
8aba9c4 1236.94 ms 1248.29 ms 11.35 ms
72c8d84 1257.34 ms 1276.96 ms 19.62 ms
e90aa6d 1277.38 ms 1303.18 ms 25.81 ms
84fb4d9 1212.45 ms 1223.39 ms 10.94 ms
98a8c16 1206.40 ms 1232.14 ms 25.74 ms
ecd9ecd 1191.76 ms 1216.92 ms 25.16 ms
7cd187e 1243.56 ms 1250.20 ms 6.64 ms
2b55154 1231.36 ms 1247.82 ms 16.46 ms

App size

Revision Plain With Sentry Diff
01a28a9 22.85 KiB 405.39 KiB 382.54 KiB
60bfc91 20.76 KiB 434.94 KiB 414.18 KiB
8aba9c4 21.58 KiB 544.72 KiB 523.14 KiB
72c8d84 22.85 KiB 408.88 KiB 386.03 KiB
e90aa6d 21.58 KiB 418.82 KiB 397.24 KiB
84fb4d9 22.84 KiB 402.56 KiB 379.72 KiB
98a8c16 20.76 KiB 431.00 KiB 410.24 KiB
ecd9ecd 20.76 KiB 420.23 KiB 399.47 KiB
7cd187e 20.76 KiB 401.65 KiB 380.89 KiB
2b55154 22.84 KiB 402.19 KiB 379.34 KiB

Previous results on branch: feat/metrics

Startup times

Revision Plain With Sentry Diff
3303cb5 1230.40 ms 1235.86 ms 5.46 ms

App size

Revision Plain With Sentry Diff
3303cb5 21.58 KiB 571.42 KiB 549.84 KiB

The MetricsAPI for adding a set should accept a string, which gets
converted to a CRC32 hash instead of an integer.
@philipphofmann philipphofmann merged commit 8cd6cfa into main Mar 27, 2024
69 of 70 checks passed
@philipphofmann philipphofmann deleted the feat/metrics branch March 27, 2024 08:28
threema-matteo pushed a commit to threema-ch/sentry-cocoa that referenced this pull request May 21, 2024
Add beta implementation of MetricsAPI.

Fixes getsentryGH-3631
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.

None yet

2 participants