-
-
Notifications
You must be signed in to change notification settings - Fork 372
feat(metrics): Add implementation for metrics envelope item #6960
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
base: philprime/metrics-bootstrap
Are you sure you want to change the base?
feat(metrics): Add implementation for metrics envelope item #6960
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## philprime/metrics-bootstrap #6960 +/- ##
=================================================================
- Coverage 85.115% 84.990% -0.125%
=================================================================
Files 461 461
Lines 28043 27730 -313
Branches 12339 12190 -149
=================================================================
- Hits 23869 23568 -301
- Misses 3916 4123 +207
+ Partials 258 39 -219
... and 44 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
a5056fb to
1210307
Compare
02c3573 to
303d533
Compare
|
@sentry review |
Tests/SentryTests/Integrations/Metrics/MetricBatcherTests.swift
Outdated
Show resolved
Hide resolved
Tests/SentryTests/Integrations/Metrics/MetricBatcherTests.swift
Outdated
Show resolved
Hide resolved
|
@sentry review |
Tests/SentryTests/Integrations/Metrics/SentryMetricsIntegrationTests.swift
Show resolved
Hide resolved
Tests/SentryTests/Integrations/Metrics/MetricBatcherTests.swift
Outdated
Show resolved
Hide resolved
philipphofmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a couple of comments. Thanks for doing this.
Tests/SentryTests/Integrations/Metrics/SentryMetricsBatcherTests.swift
Outdated
Show resolved
Hide resolved
Tests/SentryTests/Integrations/Metrics/SentryMetricsBatcherTests.swift
Outdated
Show resolved
Hide resolved
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 2d018e1 | 1227.67 ms | 1245.79 ms | 18.12 ms |
| c17fb00 | 1213.18 ms | 1244.28 ms | 31.10 ms |
| 1fc93d0 | 1231.06 ms | 1261.37 ms | 30.31 ms |
| b0e9352 | 1229.50 ms | 1265.81 ms | 36.31 ms |
| 40a71d5 | 1221.24 ms | 1263.70 ms | 42.46 ms |
| 9ee5dae | 1205.02 ms | 1237.27 ms | 32.25 ms |
| 94a1efd | 1212.39 ms | 1250.19 ms | 37.80 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 2d018e1 | 24.14 KiB | 1.03 MiB | 1.01 MiB |
| c17fb00 | 24.14 KiB | 1.03 MiB | 1.00 MiB |
| 1fc93d0 | 24.14 KiB | 1.03 MiB | 1.00 MiB |
| b0e9352 | 24.14 KiB | 1.03 MiB | 1.01 MiB |
| 40a71d5 | 24.14 KiB | 1.03 MiB | 1.01 MiB |
| 9ee5dae | 24.14 KiB | 1.03 MiB | 1.01 MiB |
| 94a1efd | 24.14 KiB | 1.03 MiB | 1.00 MiB |
Previous results on branch: philprime/metrics-envelope
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 63535b8 | 1228.64 ms | 1255.27 ms | 26.63 ms |
| 34e473a | 1195.53 ms | 1222.53 ms | 27.00 ms |
| 94c57e8 | 1190.73 ms | 1211.46 ms | 20.72 ms |
| f1ee7f4 | 1221.29 ms | 1248.16 ms | 26.87 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 63535b8 | 24.14 KiB | 1.04 MiB | 1.01 MiB |
| 34e473a | 24.14 KiB | 1.04 MiB | 1.01 MiB |
| 94c57e8 | 24.14 KiB | 1.04 MiB | 1.01 MiB |
| f1ee7f4 | 24.14 KiB | 1.04 MiB | 1.01 MiB |
…umentation, and improve metric handling in tests.
philipphofmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
Tests/SentryTests/Integrations/Metrics/SentryMetricsBatcherTests.swift
Outdated
Show resolved
Hide resolved
Tests/SentryTests/Integrations/Metrics/SentryMetricsIntegrationTests.swift
Outdated
Show resolved
Hide resolved
philipphofmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, LGTM.
|
Thanks @philipphofmann, I'll keep this PR open and ready-for-merge until #6957 is ready to be merged too (blocked by #7077) so I merge all three PRs directly after another. |
This PR is part of a merge-chain and should be merged one-by-one into
mainas soon as all of them are ready to be merged:📜 Description
This PR implements the metrics envelope item functionality for the Sentry Cocoa SDK. It adds the core infrastructure for capturing, batching, and sending metrics to Sentry.
Key Changes
SentryMetric Protocol: Added a new
SentryMetricclass that represents a metric entry with support for:SentryMetricBatcher: Implemented a batcher that:
beforeSendMetriccallback for filtering/modifying metricsMetricsIntegration Updates: Enhanced the integration to:
addMetric(_:scope:)API for adding metricsClient Support: Added
captureMetricsData(_:with:)method toSentryClientthat:trace_metricapplication/vnd.sentry.items.trace-metric+jsonData Category Support: Added
trace_metricdata category to:SentryDataCategoryenumSentryDataCategoryMapperfor mapping envelope item types to categoriesSentryEnvelopeItemTypefor the envelope item type constantOptions: Added
beforeSendMetriccallback option for filtering/modifying metrics before sendingTesting
Added comprehensive test coverage:
captureMetricsDataenvelope creation💡 Motivation and Context
Closes #6948
Closes #6951
Closes #6952
💚 How did you test it?
SentryMetriccovering encoding, attribute management, and metric typesSentryMetricBatchercovering batching, flushing, timeout behavior, attribute enrichment, and edge casesMetricsIntegrationandSentryClientmetrics capture📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.