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

RUMM-2169 Isolate RUM feature from Core's configuration and dependencies #879

Merged

Conversation

ncreated
Copy link
Member

@ncreated ncreated commented Jun 2, 2022

What and why?

🔨🧰 A counterpart of #877 but for the RUMFeature.

This refactoring is required to better isolate Feature modules from Core implementation. It prepares the codebase for further introduction of SDK context in V2.

How?

Common configuration is now removed from FeaturesConfiguration.RUM:

struct RUM {
    // ...

-   let common: Common
    let uploadURL: URL
    let applicationID: String
    let sessionSampler: Sampler
    // ...
}

Common dependencies are now removed from RUMFeature interface:

internal final class RUMFeature: V1FeatureInitializable {
   // ...

-   // MARK: - Dependencies
-   let sdkInitDate: Date
-   let dateProvider: DateProvider
-   let dateCorrector: DateCorrectorType
-   let appStateListener: AppStateListening
-   let userInfoProvider: UserInfoProvider
-   let networkConnectionInfoProvider: NetworkConnectionInfoProviderType
-   let carrierInfoProvider: CarrierInfoProviderType
-   let launchTimeProvider: LaunchTimeProviderType

-   let vitalCPUReader: SamplingBasedVitalReader
-   let vitalMemoryReader: SamplingBasedVitalReader
-   let vitalRefreshRateReader: ContinuousVitalReader

-   let onSessionStart: RUMSessionListener?

   // ...
}

🚚 There will be one more PR on this subject which will consolidate all TODOs and cleanup this new concept. It is only possible now, after all Features are migrated to the new flow.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes

Custom CI job configuration (optional)

  • Run unit tests
  • Run integration tests
  • Run smoke tests

@ncreated ncreated requested a review from a team as a code owner June 2, 2022 16:43
@ncreated ncreated self-assigned this Jun 2, 2022
Comment on lines -962 to -966
// MARK: - Tracking Consent

func testWhenChangingConsentValues_itUploadsOnlyAuthorizedRUMEvents() throws {
let consentProvider = ConsentProvider(initialConsent: .pending)

Copy link
Member Author

Choose a reason for hiding this comment

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

Same as in logging PR - consent aware uploads a no longer responsibility of Feature so we can't test it here.

Base automatically changed from ncreated/RUMM-2169-isolate-tracing-from-core-configuration to feature/v2 June 3, 2022 11:04
Copy link
Member

@maxep maxep left a comment

Choose a reason for hiding this comment

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

🚀

@ncreated ncreated merged commit c0fd507 into feature/v2 Jun 3, 2022
@ncreated ncreated deleted the ncreated/RUMM-2169-isolate-RUM-from-core-configuration branch June 3, 2022 12:07
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.

2 participants