-
Notifications
You must be signed in to change notification settings - Fork 135
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-1213 Allow additional configuration when initializing the SDK #456
Conversation
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.
Nice 👌. I left one important change request.
Additionally, we could pass this value to the FeaturesConfiguration.Common
so it will become visible to all features. Or we can do it later, when implementing the first additional attribute.
@@ -277,6 +277,11 @@ public class DDConfigurationBuilder: NSObject { | |||
_ = sdkBuilder.set(uploadFrequency: uploadFrequency.swiftType) | |||
} | |||
|
|||
@objc | |||
public func set(additionalConfiguration: [String: Any]) { | |||
_ = sdkBuilder.set(additionalConfiguration: castAttributesToSwift(additionalConfiguration)) |
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.
We shouldn't and we can't use castAttributesToSwift()
in this case. The castAttributesToSwift()
wraps Any
value into AnyEncodable
wrapper - so it changes its identity. That said, if we pass "_dd.foo": "bar"
from Obj-c, the downstream assumption in Swift SDK will fail:
// This will fail, as `additionalConfigurations["_dd.foo"]` is `AnyEncodable`, not `String`:
let bar = additionalConfigurations["_dd.foo"] as? String
Instead, we should just pass the additionalConfiguration
as it is:
_ = sdkBuilder.set(additionalConfiguration: additionalConfiguration)
so the above assumption can pass.
The castAttributesToSwift()
is only used for capturing serializable values (log / span / rum attributes). In their case, we don't make the as?
assumption on their identity - we just encode them using encoding implementation from AnyEncodable
.
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.
👌
…nfiguration RUMM-1213 Allow additional configuration when initializing the SDK
…nfiguration RUMM-1213 Allow additional configuration when initializing the SDK
…nfiguration RUMM-1213 Allow additional configuration when initializing the SDK (cherry picked from commit 8eb7385)
…nfiguration RUMM-1213 Allow additional configuration when initializing the SDK (cherry picked from commit 8eb7385)
What and why?
Let customer set additional configuration as a Dictionary. This will be used to configure internal settings of the SDK