🚩RUM-1755 Add config overrides for debug launch arguments

[fuzzybinary]

Adds two launch arguments:
DD_DEBUG sets sampling to 100%, verbosity to debug, and upload frequency to frequent
DD_DEBUG_RUM enables Datadog.debugRUM

All of these settings are changed during initialization to override whatever configuration the user set

• Feature or bugfix MUST have appropriate tests (unit, integration)
• Make sure each commit and the PR mention the Issue number or JIRA reference

[maxep]

This looks great 🚀 well done with the configuration mutation and the console warning!
I have a suggestion re. the MobileDevice singleton, LMKWYT!

[maxep]

I think we can simplify this and avoid having to use a second singleton definition by creating a convenience init(), like:

    convenience init() {
        #if !targetEnvironment(simulator)
        // Real device
        self.init(
            uiDevice: UIDevice.current,
            processInfo: ProcessInfo.processInfo,
            notificationCenter: .default
        )
        #else
        // iOS Simulator - battery monitoring doesn't work on Simulator, so return "always OK" value
        self.init(
            model: UIDevice.current.model,
            osName: UIDevice.current.systemName,
            osVersion: UIDevice.current.systemVersion,
            processInfo: ProcessInfo.processInfo,
            enableBatteryStatusMonitoring: {},
            resetBatteryStatusMonitoring: {},
            currentBatteryStatus: { BatteryStatus(state: .full, level: 1, isLowPowerModeEnabled: false) }
        )
        #endif
    }

    /// Returns current mobile device.
    static var current = MobileDevice()

    #if DD_SDK_COMPILED_FOR_TESTING
    static func clearForTesting() {
        current = MobileDevice()
    }
    #endif

Also the clearForTesting might not be even necessary if we do:

func testSomething() {
    MobileDevice.current = .mock()
    defer { MobileDevice.current = .init() }

    ...
}

[fuzzybinary]

That's a good call. I was originally thinking ok making set only visible to testing, but that wasn't really possible so your solution ends up being a lot cleaner given what's possible.

[fuzzybinary]

@maxep Updated with your suggestions.

[ncreated]

Good contribution 👍, although I wonder if we couldn't achieve this in a simpler way. Here we propose a few new concepts like MobileDevice singleton or Datadog.Configuration mutability and I'm not sure if adding a debug utility is enough reason for this. I left separate comments, I wonder what's your take on these topics 🙂🙌.

Also, please leverage our GH PR template for next PRs 🚀 (what/how sections), as it's part of our contribution guide and really helps visiting past topics in the future 🙏.

[ncreated]

I wonder if we have enough reasons for making MobileDevice a singleton. Our existing architecture highly leverages dependency injection for keeping minimal number of different instances and MobileDevice is already managed as single dependency created once and passed downstream to all modules through FeaturesCommonDependencies. Changing it to singleton comes with additional cost:

• we need to manage its global state in tests, which can cause flakiness (if we want to go this path, it requires at least adding an integrity check for MobileDevice as we do with other globals);
• as we're planning to support multiple SDK instances in V2, we'd need to refactor this anyway (to use DI).

That said, I'd rather vote on keeping MobileDevice an instance-based component. To avoid confusion, we could even remove MobileDevice.current API and only allow MobileDevice(). I think the .current is a leftover after series of refactors we've been doing to this class recently.

[fuzzybinary]

👍 I'm largely against singletons as well. I'll look for a way to remove it cleanly in this PR if we want. I was mostly looking to disturb as little as possible for this particular PR.

[ncreated]

I think that mobileDevice.processInfo.arguments.contains(_:) reveals the Inappropriate Intimacy problem. From MobileDevice class comment, I disagree that it should describe launch arguments:

/// Describes current mobile device.
internal class MobileDevice {

The intention for MobileDevice is to provide our own abstraction for accessing specifics of user device, including model name, OS version or battery information. I think "process launch arguments" (and/or ENVs) are different class of problem and rather deserve separate component (at simplest case, direct use of ProcessInfo would be IMO more cohesive and less coupled). WDYT @buranmert @maxep @fuzzybinary ?

[ncreated]

Very good idea with these prints 👌💪

[ncreated]

Instead of introducing mutability to Datadog.Configuration (so, a new concept), I think we could leverage existing FeaturesConfiguration and its config resolution skill:

extension FeaturesConfiguration {
    /// Builds and validates the configuration for all features.
    ///
    /// It takes two types received from the user: `Datadog.Configuration` and `AppContext` and blends them together
    /// with resolving defaults and ensuring the configuration consistency.
    ///
    /// Throws an error on invalid user input, i.e. broken custom URL.
    /// Prints a warning if configuration is inconsistent, i.e. RUM is enabled, but RUM Application ID was not specified.
    init(configuration: Datadog.Configuration, appContext: AppContext) throws {

The idea behind FeaturesConfiguration is to take different sorts of configuration:

• user-specified Datadog.Configuration,
• AppContext inferred from system

and create a single representation of config for all features. This includes operations like defaults resolution and validation. How about introducing third source of configuration to this init() and let it take process info into account as well?

[fuzzybinary]

@ncreated Thinking about the best way to approach this, I'm thinking putting ProcessInfo into AppContext might actually be the simplest / best approach here that allows us use the system supplied default or inject a fake ProcessInfo without cluttering the public API.

Thoughts?

[ncreated]

@ncreated Thinking about the best way to approach this, I'm thinking putting ProcessInfo into AppContext might actually be the simplest / best approach here that allows us use the system supplied default or inject a fake ProcessInfo without cluttering the public API.

Thoughts?

@fuzzybinary I think this is really good idea 👌! Extending "app context" with "launch arguments" sounds conceptually correct, and if thinking of multiple SDK instances - each would have it's own context, so that will be easy to scope this feature to the right instance of Datadog 👍.

[fuzzybinary]

@ncreated Made the changes, and removed MobileDevice.current -- as you pointed out MobileDevice.init() essentially did the same thing. I also moved the flag overrides into FeaturesConfiguration for most of the places that was possible.

I did leave ProcessInfo as the argument to AppContext, as there might be other items in ProcessInfo we could leverage in the future, like environment. Let me know what you think.

[ncreated]

Looks 💯🚀! And really well done in testing 👍.

I'm adding needs-doc label, so we can track this for future documentation work (usually we update docs later, after release).