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-886 setUserInfo(..., extraInfo) added #315

Merged
merged 2 commits into from
Nov 26, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions Datadog/Example/Debugging/DebugRUMViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,12 @@ class DebugRUMViewController: UIViewController {

/// Creates an instance of `UIViewController` subclass with a given name.
private func createUIViewControllerSubclassInstance(named viewControllerClassName: String) -> UIViewController {
let theClass: AnyClass = objc_allocateClassPair(UIViewController.classForCoder(), viewControllerClassName, 0)!
objc_registerClassPair(theClass)
let theClass: AnyClass = NSClassFromString(viewControllerClassName) ?? {
let cls: AnyClass
cls = objc_allocateClassPair(UIViewController.classForCoder(), viewControllerClassName, 0)!
objc_registerClassPair(cls)
return cls
}()
return theClass.alloc() as! UIViewController
}

Expand Down
2 changes: 1 addition & 1 deletion Datadog/Example/ExampleAppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class ExampleAppDelegate: UIResponder, UIApplicationDelegate {
)

// Set user information
Datadog.setUserInfo(id: "abcd-1234", name: "foo", email: "foo@example.com")
Datadog.setUserInfo(id: "abcd-1234", name: "foo", email: "foo@example.com", extraInfo: ["key-extraUserInfo": "value-extraUserInfo"])

// Create Logger
logger = Logger.builder
Expand Down
7 changes: 4 additions & 3 deletions Sources/Datadog/Core/Attributes/UserInfoProvider.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ internal class UserInfoProvider {
/// `UserInfo` can be mutated by any user thread with `Datadog.setUserInfo(id:name:email:)` - at the same
/// time it might be accessed by different queues running in the SDK.
private let queue = DispatchQueue(label: "com.datadoghq.user-info-provider", qos: .userInteractive)
private var current = UserInfo(id: nil, name: nil, email: nil)
private var _value = UserInfo(id: nil, name: nil, email: nil, extraInfo: [:])

var value: UserInfo {
set { queue.async { self.current = newValue } }
get { queue.sync { self.current } }
set { queue.async { self._value = newValue } }
get { queue.sync { self._value } }
}
}

Expand All @@ -25,4 +25,5 @@ internal struct UserInfo {
let id: String?
let name: String?
let email: String?
let extraInfo: [AttributeKey: AttributeValue]
}
10 changes: 8 additions & 2 deletions Sources/Datadog/Datadog.swift
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,15 @@ public class Datadog {
public static func setUserInfo(
Copy link
Member

Choose a reason for hiding this comment

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

This public API is missing the comment.

id: String? = nil,
name: String? = nil,
email: String? = nil
email: String? = nil,
extraInfo: [AttributeKey: AttributeValue] = [:]
) {
instance?.userInfoProvider.value = UserInfo(id: id, name: name, email: email)
instance?.userInfoProvider.value = UserInfo(
id: id,
name: name,
email: email,
extraInfo: extraInfo
)
}

// MARK: - Internal
Expand Down
10 changes: 8 additions & 2 deletions Sources/Datadog/Logging/Log/LogEncoder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,19 @@ internal struct LogEncoder {
// Encode attributes...
var attributesContainer = encoder.container(keyedBy: DynamicCodingKey.self)

// ... first, user attributes ...
// 1. user info attributes
try log.userInfo.extraInfo.forEach {
let key = DynamicCodingKey("usr.\($0)")
try attributesContainer.encode(EncodableValue($1), forKey: key)
}

// 2. user attributes
let encodableUserAttributes = Dictionary(
uniqueKeysWithValues: log.attributes.userAttributes.map { name, value in (name, EncodableValue(value)) }
)
try encodableUserAttributes.forEach { try attributesContainer.encode($0.value, forKey: DynamicCodingKey($0.key)) }

// ... then, internal attributes:
// 3. internal attributes
if let internalAttributes = log.attributes.internalAttributes {
let encodableInternalAttributes = Dictionary(
uniqueKeysWithValues: internalAttributes.map { name, value in (name, EncodableValue(value)) }
Expand Down
14 changes: 12 additions & 2 deletions Sources/Datadog/RUM/RUMEvent/RUMEventBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,18 @@

import Foundation

internal struct RUMEventBuilder {
internal class RUMEventBuilder {
let userInfoProvider: UserInfoProvider

init(userInfoProvider: UserInfoProvider) {
self.userInfoProvider = userInfoProvider
}

func createRUMEvent<DM: RUMDataModel>(with model: DM, attributes: [String: Encodable]) -> RUMEvent<DM> {
return RUMEvent(model: model, attributes: attributes)
return RUMEvent(
model: model,
attributes: attributes,
userInfoAttributes: userInfoProvider.value.extraInfo
)
}
}
4 changes: 4 additions & 0 deletions Sources/Datadog/RUM/RUMEvent/RUMEventEncoder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ internal struct RUMEvent<DM: RUMDataModel>: Encodable {

/// Custom attributes set by the user
let attributes: [String: Encodable]
let userInfoAttributes: [String: Encodable]

func encode(to encoder: Encoder) throws {
try RUMEventEncoder().encode(self, to: encoder)
Expand All @@ -36,6 +37,9 @@ internal struct RUMEventEncoder {
try event.attributes.forEach { attributeName, attributeValue in
try attributesContainer.encode(EncodableValue(attributeValue), forKey: DynamicCodingKey("context.\(attributeName)"))
}
try event.userInfoAttributes.forEach { attributeName, attributeValue in
try attributesContainer.encode(EncodableValue(attributeValue), forKey: DynamicCodingKey("context.usr.\(attributeName)"))
}

// Encode `RUMDataModel`
try event.model.encode(to: encoder)
Expand Down
2 changes: 1 addition & 1 deletion Sources/Datadog/RUMMonitor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public class RUMMonitor: DDRUMMonitor, RUMCommandSubscriber {
networkConnectionInfoProvider: rumFeature.networkConnectionInfoProvider,
carrierInfoProvider: rumFeature.carrierInfoProvider
),
eventBuilder: RUMEventBuilder(),
eventBuilder: RUMEventBuilder(userInfoProvider: rumFeature.userInfoProvider),
eventOutput: RUMEventFileOutput(
fileWriter: rumFeature.storage.writer
),
Expand Down
8 changes: 3 additions & 5 deletions Sources/Datadog/Tracing/Span/SpanBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,12 @@ internal struct SpanBuilder {
func createSpan(from ddspan: DDSpan, finishTime: Date) -> Span {
let tagsReducer = SpanTagsReducer(spanTags: ddspan.tags, logFields: ddspan.logFields)

var jsonStringEncodedTags: [String: JSONStringEncodableValue] = [:]

// First, add baggage items as tags...
var jsonStringEncodedTags = [String: JSONStringEncodableValue]()
// 1. add baggage items as tags
for (itemKey, itemValue) in ddspan.ddContext.baggageItems.all {
jsonStringEncodedTags[itemKey] = JSONStringEncodableValue(itemValue, encodedUsing: tagsJSONEncoder)
}

// ... then, add regular tags
// 2. add regular tags
for (tagName, tagValue) in tagsReducer.reducedSpanTags {
jsonStringEncodedTags[tagName] = JSONStringEncodableValue(tagValue, encodedUsing: tagsJSONEncoder)
}
Expand Down
3 changes: 3 additions & 0 deletions Sources/Datadog/Tracing/Span/SpanEncoder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -198,5 +198,8 @@ internal struct SpanEncoder {
let metaKey = "meta.\($0.key)"
try container.encode($0.value, forKey: DynamicCodingKey(metaKey))
}
try span.userInfo.extraInfo.forEach {
try container.encode(EncodableValue($1), forKey: DynamicCodingKey("meta.usr.\($0)"))
Copy link
Member

Choose a reason for hiding this comment

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

This leads to all spans being rejected by Intake if the user extra attribute is not String, e.g.:

Datadog.setUserInfo(
    id: "abcd-1234",
    name: "foo",
    email: "foo@example.com",
    extraInfo: [
        "is-registered": true
    ]
)

As stated in this method comment:

// NOTE: RUMM-299 only string values are supported for `meta.*` attributes

Whatever we encode on meta.* key must be serialised to String representation. We should use JSONStringEncodableValue in SpanBuilder as it is done for span.tags.

}
}
}
8 changes: 4 additions & 4 deletions Tests/DatadogTests/Datadog/LoggerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -186,16 +186,16 @@ class LoggerTests: XCTestCase {
Datadog.setUserInfo(id: "abc-123", name: "Foo")
logger.debug("message with user `id` and `name`")

Datadog.setUserInfo(id: "abc-123", name: "Foo", email: "foo@example.com")
logger.debug("message with user `id`, `name` and `email`")
Datadog.setUserInfo(id: "abc-123", name: "Foo", email: "foo@example.com", extraInfo: ["extra_key": "extra_value"])
Copy link
Member

Choose a reason for hiding this comment

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

Because extraInfo encoding depends on the feature (Logging and RUM supports multiple Encodable types, but Tracing encodes all values as String), it seems better to mix few types in this test:

extraInfo: ["extra_string": "string_value", "extra_int": 42, "extra_bool": true]

and document the expected encoding result in test assertions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i can't pass different types in a dict to assertUserInfo method below
i will need to use matcher.assertValue(forKey:value:)

logger.debug("message with user `id`, `name`, `email` and `extraInfo`")

Datadog.setUserInfo(id: nil, name: nil, email: nil)
logger.debug("message with no user info")

let logMatchers = try LoggingFeature.waitAndReturnLogMatchers(count: 4)
logMatchers[0].assertUserInfo(equals: nil)
logMatchers[1].assertUserInfo(equals: (id: "abc-123", name: "Foo", email: nil))
logMatchers[2].assertUserInfo(equals: (id: "abc-123", name: "Foo", email: "foo@example.com"))
logMatchers[1].assertUserInfo(equals: (id: "abc-123", name: "Foo", email: nil, extraInfo: nil))
logMatchers[2].assertUserInfo(equals: (id: "abc-123", name: "Foo", email: "foo@example.com", extraInfo: ["extra_key": "extra_value"]))
logMatchers[3].assertUserInfo(equals: nil)
}

Expand Down
2 changes: 1 addition & 1 deletion Tests/DatadogTests/Datadog/Mocks/CoreMocks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ extension UserInfo {
}

static func mockEmpty() -> UserInfo {
return UserInfo(id: nil, name: nil, email: nil)
return UserInfo(id: nil, name: nil, email: nil, extraInfo: [:])
}
}

Expand Down
4 changes: 2 additions & 2 deletions Tests/DatadogTests/Datadog/Mocks/RUMFeatureMocks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ struct RUMDataModelMock: RUMDataModel, Equatable {

extension RUMEventBuilder {
static func mockAny() -> RUMEventBuilder {
return RUMEventBuilder()
return RUMEventBuilder(userInfoProvider: UserInfoProvider.mockAny())
}
}

Expand Down Expand Up @@ -323,7 +323,7 @@ extension RUMScopeDependencies {
networkConnectionInfoProvider: NetworkConnectionInfoProviderMock(networkConnectionInfo: nil),
carrierInfoProvider: CarrierInfoProviderMock(carrierInfo: nil)
),
eventBuilder: RUMEventBuilder = RUMEventBuilder(),
eventBuilder: RUMEventBuilder = RUMEventBuilder(userInfoProvider: UserInfoProvider.mockAny()),
eventOutput: RUMEventOutput = RUMEventOutputMock(),
rumUUIDGenerator: RUMUUIDGenerator = DefaultRUMUUIDGenerator()
) -> RUMScopeDependencies {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import XCTest

class RUMEventBuilderTests: XCTestCase {
func testItBuildsRUMEvent() {
let builder = RUMEventBuilder()
let builder = RUMEventBuilder(userInfoProvider: UserInfoProvider.mockAny())
let event = builder.createRUMEvent(
with: RUMDataModelMock(attribute: "foo"),
attributes: ["foo": "bar", "fizz": "buzz"]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@ class RUMUserInfoProviderTests: XCTestCase {
}

func testWhenUserInfoIsAvailable_itReturnsRUMUserInfo() {
userInfoProvider.value = UserInfo(id: "abc-123", name: nil, email: nil)
userInfoProvider.value = UserInfo(id: "abc-123", name: nil, email: nil, extraInfo: [:])
XCTAssertEqual(rumUserInfoProvider.current, RUMDataUSR(id: "abc-123", name: nil, email: nil))

userInfoProvider.value = UserInfo(id: "abc-123", name: "Foo", email: nil)
userInfoProvider.value = UserInfo(id: "abc-123", name: "Foo", email: nil, extraInfo: [:])
XCTAssertEqual(rumUserInfoProvider.current, RUMDataUSR(id: "abc-123", name: "Foo", email: nil))

userInfoProvider.value = UserInfo(id: "abc-123", name: "Foo", email: "foo@bar.com")
userInfoProvider.value = UserInfo(id: "abc-123", name: "Foo", email: "foo@bar.com", extraInfo: [:])
XCTAssertEqual(rumUserInfoProvider.current, RUMDataUSR(id: "abc-123", name: "Foo", email: "foo@bar.com"))

userInfoProvider.value = UserInfo(id: "abc-123", name: "Foo", email: "foo@bar.com", extraInfo: [:])
XCTAssertEqual(rumUserInfoProvider.current, RUMDataUSR(id: "abc-123", name: "Foo", email: "foo@bar.com"))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class RUMEventFileOutputTests: XCTestCase {
func testItWritesRUMEventToFileAsJSON() throws {
let fileCreationDateProvider = RelativeDateProvider(startingFrom: .mockDecember15th2019At10AMUTC())
let queue = DispatchQueue(label: "com.datadohq.testItWritesRUMEventToFileAsJSON")
let builder = RUMEventBuilder()
let builder = RUMEventBuilder(userInfoProvider: UserInfoProvider.mockAny())
let output = RUMEventFileOutput(
fileWriter: FileWriter(
dataFormat: RUMFeature.dataFormat,
Expand Down
10 changes: 9 additions & 1 deletion Tests/DatadogTests/Datadog/RUMMonitorTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -445,7 +445,12 @@ class RUMMonitorTests: XCTestCase {
directory: temporaryDirectory,
dependencies: .mockWith(
userInfoProvider: .mockWith(
userInfo: UserInfo(id: "abc-123", name: "Foo", email: "foo@bar.com")
userInfo: UserInfo(
id: "abc-123",
name: "Foo",
email: "foo@bar.com",
extraInfo: ["extra_key": "extra_value"]
Copy link
Member

Choose a reason for hiding this comment

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

Because extraInfo encoding depends on the feature (Logging and RUM supports multiple Encodable types, but Tracing encodes all values as String), it seems better to mix few types in this test:

extraInfo: ["extra_string": "string_value", "extra_int": 42, "extra_bool": true]

and document the expected encoding result in test assertions.

)
)
)
)
Expand All @@ -465,6 +470,9 @@ class RUMMonitorTests: XCTestCase {

let rumEventMatchers = try RUMFeature.waitAndReturnRUMEventMatchers(count: 11)
let expectedUserInfo = RUMDataUSR(id: "abc-123", name: "Foo", email: "foo@bar.com")
rumEventMatchers.forEach { event in
event.jsonMatcher.assertValue(forKey: "context.usr.extra_key", equals: "extra_value")
}
try rumEventMatchers.forEachRUMEvent(ofType: RUMDataAction.self) { action in
XCTAssertEqual(action.usr, expectedUserInfo)
}
Expand Down
6 changes: 4 additions & 2 deletions Tests/DatadogTests/Datadog/TracerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -335,8 +335,8 @@ class TracerTests: XCTestCase {
Datadog.setUserInfo(id: "abc-123", name: "Foo")
tracer.startSpan(operationName: "span with user `id` and `name`").finish()

Datadog.setUserInfo(id: "abc-123", name: "Foo", email: "foo@example.com")
tracer.startSpan(operationName: "span with user `id`, `name` and `email`").finish()
Datadog.setUserInfo(id: "abc-123", name: "Foo", email: "foo@example.com", extraInfo: ["extra_key": "extra_value"])
Copy link
Member

Choose a reason for hiding this comment

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

Because extraInfo encoding depends on the feature (Logging and RUM supports multiple Encodable types, but Tracing encodes all values as String), it seems better to mix few types in this test:

extraInfo: ["extra_string": "string_value", "extra_int": 42, "extra_bool": true]

and document the expected encoding result in test assertions.

tracer.startSpan(operationName: "span with user `id`, `name`, `email` and `extraInfo`").finish()

Datadog.setUserInfo(id: nil, name: nil, email: nil)
tracer.startSpan(operationName: "span with no user info").finish()
Expand All @@ -353,10 +353,12 @@ class TracerTests: XCTestCase {
XCTAssertEqual(try spanMatchers[2].meta.userID(), "abc-123")
XCTAssertEqual(try spanMatchers[2].meta.userName(), "Foo")
XCTAssertEqual(try spanMatchers[2].meta.userEmail(), "foo@example.com")
XCTAssertEqual(try spanMatchers[2].meta.custom(keyPath: "meta.usr.extra_key"), "extra_value")

XCTAssertNil(try? spanMatchers[3].meta.userID())
XCTAssertNil(try? spanMatchers[3].meta.userName())
XCTAssertNil(try? spanMatchers[3].meta.userEmail())
XCTAssertNil(try? spanMatchers[3].meta.custom(keyPath: "meta.usr.extra_key"))
}

// MARK: - Sending carrier info
Expand Down
5 changes: 4 additions & 1 deletion Tests/DatadogTests/Matchers/LogMatcher.swift
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ internal class LogMatcher: JSONDataMatcher {
assertValue(forKey: JSONKey.message, equals: message, file: file, line: line)
}

func assertUserInfo(equals userInfo: (id: String?, name: String?, email: String?)?, file: StaticString = #file, line: UInt = #line) {
func assertUserInfo(equals userInfo: (id: String?, name: String?, email: String?, extraInfo: [String: String]?)?, file: StaticString = #file, line: UInt = #line) {
if let id = userInfo?.id {
assertValue(forKey: JSONKey.userId, equals: id, file: file, line: line)
} else {
Expand All @@ -128,6 +128,9 @@ internal class LogMatcher: JSONDataMatcher {
} else {
assertNoValue(forKey: JSONKey.userEmail, file: file, line: line)
}
userInfo?.extraInfo?.forEach {
assertValue(forKey: "usr.\($0)", equals: $1)
}
}

func assertAttributes(equal attributes: [String: Any], file: StaticString = #file, line: UInt = #line) {
Expand Down