RUMM-2169 Isolate Logging feature from Core's configuration and dependencies #877
Conversation
and use SDK context instead.
…re configuration
which means we're no longer testing Core as part of `LoggingFeature` tests. Instead, the mock is installed early on the writting layer, and no-ops are used for batches orchestration and upload.
because these behaviours should be tested as part of Core tests. Whats more: - the tracking consent logic is already covered in `FeatureStorage` tests, - the upload condition logic is for now covered in `RUMMonitor` and `Tracer` tests, but will be covered in `FeatureUpload` once all features are migrated in RUMM-2169.
| // MARK: - Tracking Consent | ||
|
|
||
| func testWhenChangingConsentValues_itUploadsOnlyAuthorizedLogs() throws { | ||
| let consentProvider = ConsentProvider(initialConsent: .pending) |
There was a problem hiding this comment.
I'm removing this test, because in V2 tracking consent will be fully managed by the Core module and transparent to Feature modules. We already have it covered in FeatureStorage tests.
I'm doing it now, because since this PR we're no longer running full Core backend in part of Logger tests. Both FeatureStorage and FeatureUpload are mocked, so we don't have their consent logic available.
| // MARK: - Sending logs with different network and battery conditions | ||
|
|
||
| func testGivenBadBatteryConditions_itDoesNotTryToSendLogs() throws { |
There was a problem hiding this comment.
I'm removing these 2 tests, because we're no longer running full Core backend in Logger tests and the behaviour of conditional uploads is part of the Core. This needs to be tested separately as FeatureUpload tests. I plan to add such tests after migrating both Tracing and RUM in similar fashion.
| /// Waits until given number of events is written and returns data for these events. | ||
| /// Passing no `timeout` will result with picking the recommended timeout for unit tests. | ||
| func waitAndReturnEventsData(count: UInt, timeout: TimeInterval? = nil, file: StaticString = #file, line: UInt = #line) -> [Data] { |
There was a problem hiding this comment.
This is copied from DataUploadWorkerMock as InMemoryWriter is aimed to replace it entirely after Tracing and RUM are migrated in next PRs.
| @@ -9,6 +9,11 @@ import Foundation | |||
|
|
|||
| internal final class DatadogCoreMock: Flushable { | |||
| private var v1Features: [String: Any] = [:] | |||
| private var v1Context: DatadogV1Context? | |||
There was a problem hiding this comment.
Can this be internal so we can mutate the context during a test case? it could then be reset to nil or .mockAny() on flush, WDYT?
There was a problem hiding this comment.
Good idea 👌, it helps a lot in some repetitive tests like LoggerTests where we need to dance with the core each time.
Changed ✅, however, I left the init(v1Context:) too, as in some other tests the inline initialisation comes handy.
ncreated
deleted the
ncreated/RUMM-2169-isolate-logging-from-core-configuration
branch
What and why?
🔨🧰 This PR makes
LoggingFeatureno longer depend onCoreConfigurationandCoreDependencies.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?
There are two main changes and everything else is mainly their consequence:
FeaturesConfiguration.Loggingstruct Logging { - let common: Common let uploadURL: URL let logEventMapper: LogEventMapper? }LoggingFeatureinterfaceinternal final class LoggingFeature: V1FeatureInitializable { // ... - // MARK: - Dependencies - - let dateProvider: DateProvider - let dateCorrector: DateCorrectorType - let userInfoProvider: UserInfoProvider - let networkConnectionInfoProvider: NetworkConnectionInfoProviderType - let carrierInfoProvider: CarrierInfoProviderType // ... }In V2 both, "common configuration" and "common dependencies" will belong to the Core module and their values will be passed through SDK context.
🎁 I made a bonus change, which should help us in transitioning to V2-like Features testing. The way we now create
LoggingFeaturein tests does no longer depend on dummyDatadogCoreinstance. Instead, the feature is instantiated with mockedFeatureStorageandFeatureUpload:FeatureUploadis now 100% no-op as we won't need any uploads in V2 Features tests;FeatureStorageuses newInMemoryWriter, which records events in-memory and returns them back for assertions.This bonus change is only scoped to configuration update in
LoggingFeatureMocksand addition ofInMemoryWriter. It's transparent for everything else.Review checklist
Custom CI job configuration (optional)