-
-
Notifications
You must be signed in to change notification settings - Fork 372
feat(metrics): Add integration with installation by SDK #6956
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: main
Are you sure you want to change the base?
Conversation
|
Tests/SentryTests/Integrations/Metrics/SentryMetricsIntegrationTests.swift
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6956 +/- ##
=============================================
+ Coverage 84.933% 85.115% +0.182%
=============================================
Files 457 461 +4
Lines 27624 28043 +419
Branches 12144 12339 +195
=============================================
+ Hits 23462 23869 +407
+ Misses 4119 3916 -203
- Partials 43 258 +215
... and 40 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Tests/SentryTests/Integrations/Metrics/SentryMetricsIntegrationTests.swift
Show resolved
Hide resolved
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.
✅ Bugbot reviewed your changes and found no bugs!
a5056fb to
1210307
Compare
- Introduced new test file `SentryItemBatcherTests.swift` to validate the behavior of the `SentryItemBatcher`. - Added tests for various scenarios including item addition, buffer size limits, timeout handling, and attribute enrichment. - Updated project configuration to include the new test file in the build settings.
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, when only merging this to main as pointed out in #6956 (comment).
…lprime/metrics-bootstrap
|
After reconsideration and learnings while working on #6957 I decided that I will move this feature to be marked as "experimental" first to keep some leeway to react if the public API using Swift Language features such as ExpressibleByInt is not working as expected. |
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 pull request introduces a new metrics integration to the Sentry SDK for Swift, allowing users to enable metrics collection and reporting via the SDK. The changes include the addition of the
enableMetricsoption, implementation of the metrics integration, updates to the SDK configuration, and new tests to verify the integration. The project structure is also updated to include the new integration and its tests.Metrics Integration Feature:
enableMetricsoption toSentryOptions, allowing users to enable or disable metrics collection. [1] [2]MetricsIntegration, which is installed whenenableMetricsis set totrue. [1] [2]enableMetricsoption. [1] [2]💡 Motivation and Context
Closes #6954
💚 How did you test it?
enableLogs📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.