refactor: device attributes using data pipeline #434

mrehan27 · 2023-12-13T08:06:50Z

helps: https://github.com/customerio/issues/issues/11609

Changes

Removed unused initialize from DataPipeline
Updated DataPipelineImplementation to set device attributes using analytics
Removed device attributes and token implementation from CustomerIOImplementation

github-actions · 2023-12-13T08:07:08Z

Sample app builds 📱

Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.

CocoaPods-FCM: Build failed. See CI job logs to determine the issue and try re-building.
APN-UIKit: rehan/cdp-register-token (1702660730)

mrehan27 · 2023-12-13T08:08:21Z

Sources/DataPipeline/DataPipeline.swift

-    public static func initialize(
-        writeKey: String,
-        configure configureHandler: ((inout DataPipelineConfigOptions) -> Void)? = nil
-    ) -> CustomerIOInstance {


Removed this as we are now using DataPipeline.initialize(moduleConfig: DataPipelineConfigOptions)

mrehan27 · 2023-12-13T08:11:27Z

Sources/DataPipeline/DataPipelineImplementation.swift

+    private func initialize() {
+        if let token = globalDataStore.pushDeviceToken {
+            // if the device token exists, pass it to the plugin to ensure device attributes are updated with each request
+            setDeviceToken(token)


Wasn't able to test complete flow because Journey implementation is not yet complete. But testing with assumptions and looking at code, it looks like segment is storing token only in memory, so we apparently need to pass it on every app launch.

Sources/DataPipeline/DataPipelineImplementation.swift

mrehan27 · 2023-12-13T08:19:15Z

Sources/Tracking/CustomerIOImplementation.swift

-     Adds device default and custom attributes and registers device token.
-     */
-    // TODO: Segment doesn't provide this method by default needs to get added
-    private func addDeviceAttributes(deviceToken: String, customAttributes: [String: Any] = [:]) {


With DataPipelineImplementation handling all this stuff, we may completely remove CustomerIOImplementation, but doing it in phases so it is easier for us to make sure we aren't ignoring anything unknowingly.

Update: We will be keeping CustomerIOImplementation as it will help us keep the same experience for existing users without impacting customer coming from Segment.

Shahroz16 · 2023-12-13T15:00:20Z

Sources/DataPipeline/DataPipelineImplementation.swift

+    }
+
+    private func initialize() {
+        if let token = globalDataStore.pushDeviceToken {


we might not need do it anymore with journeys inclusion

Discussed this on call, will decide after discussing more with team. Looks good for now.

Sources/DataPipeline/DataPipelineImplementation.swift

mrehan27 · 2023-12-13T16:25:24Z

Sources/DataPipeline/plugins/DeviceAttributes.swift

@@ -6,24 +6,31 @@ class DeviceAttributes: Plugin {
    public let type = PluginType.before
    public weak var analytics: Analytics?

+    public var token: String?


We are now not using segment plugin for token and relying on our plugin instead

Sources/Tracking/CustomerIOImplementation.swift

mrehan27 · 2023-12-14T12:46:13Z

Makefile

@@ -6,6 +6,7 @@ SHELL = /bin/sh
 #   imports - Import statements to be at the top of the generated files in case the file needs classes from other modules. Split by `-` (example: `imports=Cio-Foo-Bar`) 
 generate:
 	./binny sourcery --sources Sources/Common --templates Sources/Templates --output Sources/Common/autogenerated 
+	./binny sourcery --sources Sources/DataPipeline --templates Sources/Templates --output Sources/DataPipeline/autogenerated --args imports=CioInternalCommon


Added this for DeviceAttributesProvider

mrehan27 · 2023-12-14T12:50:30Z

Sources/DataPipeline/DataPipelineConfigOptions.swift

@@ -57,4 +57,7 @@ public struct DataPipelineConfigOptions {
    public var flushQueue: DispatchQueue = .init(label: "com.segment.operatingModeQueue", qos: .utility)
    public var operatingMode: OperatingMode = .asynchronous
    public var trackApplicationLifecycleEvents: Bool = true
+
+    /// Configuration options for users to easily add available plugins
+    public var autoTrackDeviceAttributes: Bool = true


QUESTION

This option might be new for analytics users, but same for Customer.io users. And analytics would still be adding some attributes, should we stop them too or remove this config completely?

Could you ask this in the slack channel? I want to make sure it doesn't go unanswered.

mrehan27 · 2023-12-14T12:53:21Z

Sources/DataPipeline/Util/DeviceAttributesProvider.swift

            onComplete([:])
            return
        }

        var deviceAttributes = [
-            "cio_sdk_version": getSdkVersionAttribute(),
+            "cio_sdk_version": deviceInfo.sdkVersion,


Because wrappers will have their own analytics implementation, I think we can simply refer sdkVersion now

mrehan27 · 2023-12-14T12:57:11Z

Sources/Tracking/CustomerIOImplementation.swift

-        guard let identifier = profileStore.identifier else {
-            logger.info("no profile identified, so not registering device token to a profile")
-            return
-        }
-        if identifier.isBlankOrEmpty() {
-            logger.error("profile cannot be identified: Identifier is empty, so not registering device token to a profile")
-            return
-        }
-
-        // OS name might not be available if running on non-apple product. We currently only support iOS for the SDK
-        // and iOS should always be non-nil. Though, we are consolidating all Apple platforms under iOS but this check
-        // is
-        // required to prevent SDK execution for unsupported OS.
-        if deviceInfo.osName == nil {
-            logger.info("SDK being executed from unsupported OS. Ignoring request to register push token.")
-            return
-        }


We are no longer blocking token calls on these checks because

Device token can now be attached before profile is identified

analytics doesn't prevent unknown OS names from registering token, so to keep analytics user unaffected, we are no longer blocking this. However, if needed for we can keep this check in CustomerIOImplementation.swift with slight changes

mrehan27 · 2023-12-14T12:58:29Z

Sources/Tracking/CustomerIOImplementation.swift

-        guard let existingDeviceToken = globalDataStore.pushDeviceToken else {
-            logger.info("no device token exists so ignoring request to delete")
-            return // no device token to delete, ignore request
-        }
-        // Do not delete push token from device storage. The token is valid
-        // once given to SDK. We need it for future profile identifications.
-
-        guard let identifiedProfileId = profileStore.identifier else {
-            logger.info("no profile identified so not removing device token from profile")
-            return // no profile to delete token from, ignore request
-        }


Same as above, no checks because analytics doesn't do that either. And customers may still want to reset data for anonymous users.

mrehan27 · 2023-12-15T17:19:49Z

Sources/DataPipeline/DataPipelineImplementation.swift

+        deviceAttributesProvider.getDefaultDeviceAttributes { defaultDeviceAttributes in
+            let deviceAttributes: [String: Any] = defaultDeviceAttributes
+                .mergeWith([
+                    "last_used": self.dateUtil.now


Removed platform as it causes issues on server side, we no longer need it

mrehan27 · 2023-12-15T17:27:35Z

Device and metric events json have been verified. Merging this as the changes are required on other branches for testing and code changes.

levibostian · 2023-12-20T16:51:31Z

Sources/DataPipeline/DataPipelineImplementation.swift

+        if let existingDeviceToken = globalDataStore.pushDeviceToken {
+            // if the device token exists, pass it to the plugin and ensure device attributes are updated
+            addDeviceAttributes(token: existingDeviceToken)
+        }


I believe that this logic can be removed once we have the eventbus implemented and the CDP module begins listening to register device token events from the eventbus.

Correct, @Shahroz16?

If this is true, could we make a TODO or something to document this change?

I think we will need to see the use-case as its using a plugin, i probably transforming data on basis of event.

levibostian · 2023-12-20T16:56:40Z

Sources/DataPipeline/plugins/DeviceAttributes.swift

+        guard var workingEvent = event,
+              var context = workingEvent.context?.dictionaryValue
+        else { return event }


What is this doing? From reading it, I don't understand what this code is for?

transforming the data, if you look into the payload expected by CDP and generated by SDK that might help in understand it more.

Raw Event already has a json, which has context and other json object in it. This plugin updates the values of context json object.

levibostian · 2023-12-20T16:57:59Z

Sources/DataPipeline/plugins/DeviceAttributes.swift

            }
+        } catch {
+            analytics?.reportInternalError(error)


Since this file is code that we wrote, I was expecting to use the CIO logger, logger.error(). Could that work better?

mrehan27 added 6 commits December 13, 2023 12:40

added util classes to shared graph

2a11e5d

removed unnecessary method

01ab20d

passed on calls to data pipeline

f4df178

registered loken on launch

1a36da6

updated notes

5274236

updated merge attributes

e8a1dac

mrehan27 self-assigned this Dec 13, 2023

mrehan27 commented Dec 13, 2023

View reviewed changes

mrehan27 requested a review from a team December 13, 2023 08:20

mrehan27 marked this pull request as ready for review December 13, 2023 08:20

mrehan27 added 4 commits December 13, 2023 16:12

refectored methods

feb0c0d

formatted code

3be34e2

removed deleteDeviceToken cio implementation

9cd94f9

updated device attributes with token

c03ce6e

Shahroz16 reviewed Dec 13, 2023

View reviewed changes

Sources/DataPipeline/DataPipelineImplementation.swift Outdated Show resolved Hide resolved

Shahroz16 reviewed Dec 13, 2023

View reviewed changes

Sources/DataPipeline/DataPipelineImplementation.swift Outdated Show resolved Hide resolved

mrehan27 added 4 commits December 13, 2023 21:10

updated attributes plugin with token

086322b

fixed delete device token

2afef5e

register token updates

f7f159c

log message updates

fea6295

mrehan27 commented Dec 13, 2023

View reviewed changes

mrehan27 added 5 commits December 14, 2023 17:09

removed old token implementation

c4f6bac

updated makefile

a3222bb

DeviceAttributesProvider fixes

e3c948c

auto gen files

418e0aa

removed os check

c1ce9d5

mrehan27 commented Dec 14, 2023

View reviewed changes

mrehan27 requested a review from Shahroz16 December 14, 2023 14:50

mrehan27 added 8 commits December 15, 2023 16:59

refactor: profile attributes using data pipeline (#435)

798c01d

more fixes

3b2f934

updated prop names

baccd57

customerio destination plugin

fde696c

sequence fix

16fecf2

added docs

5e29108

reverted attribute names

eb8143d

added todos

9152503

mrehan27 commented Dec 15, 2023

View reviewed changes

mrehan27 merged commit 4ff70b0 into main-replica-for-cdp Dec 15, 2023

mrehan27 deleted the rehan/cdp-register-token branch December 15, 2023 17:27

levibostian reviewed Dec 20, 2023

View reviewed changes

Ahmed-PublicCIO pushed a commit to Ahmed-PublicCIO/customerio-ios that referenced this pull request Dec 21, 2023

refactor: device attributes using data pipeline (customerio#434)

e65d2d4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: device attributes using data pipeline #434

refactor: device attributes using data pipeline #434

mrehan27 commented Dec 13, 2023

github-actions bot commented Dec 13, 2023 •

edited

Loading

mrehan27 Dec 13, 2023

mrehan27 Dec 13, 2023

mrehan27 Dec 13, 2023

mrehan27 Dec 13, 2023

Shahroz16 Dec 13, 2023

mrehan27 Dec 13, 2023

mrehan27 Dec 13, 2023

mrehan27 Dec 14, 2023

mrehan27 Dec 14, 2023

levibostian Dec 20, 2023

mrehan27 Dec 14, 2023

mrehan27 Dec 14, 2023

mrehan27 Dec 14, 2023

mrehan27 Dec 15, 2023

mrehan27 commented Dec 15, 2023

levibostian Dec 20, 2023

levibostian Dec 20, 2023

Shahroz16 Dec 20, 2023

levibostian Dec 20, 2023

Shahroz16 Dec 20, 2023

levibostian Dec 20, 2023

refactor: device attributes using data pipeline #434

refactor: device attributes using data pipeline #434

Conversation

mrehan27 commented Dec 13, 2023

Changes

github-actions bot commented Dec 13, 2023 • edited Loading

Sample app builds 📱

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrehan27 commented Dec 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 13, 2023 •

edited

Loading