change api #1133

SabrinaTardio · 2024-12-16T16:22:07Z

Please review the release process for BrowserServicesKit here.

Required:

Task/Issue URL: https://app.asana.com/0/1204186595873227/1208964427775425/f
iOS PR: duckduckgo/iOS#3732
macOS PR: duckduckgo/macos-browser#3670
What kind of version bump will this require?: Minor

Optional:

Tech Design URL:
CC:

Description: Changes the name of the original fireExperimentPixel func and adds a new function where the framework does not manages the counting

Steps to test this PR:

Run the app and check that the following pixel is sent: experiment_enroll_experimenttestaa_control (or treatment) with the expected parameters
Perform 4 searches and foreground the app 4 times check that only one experiment pixel is fired: experiment_metrics_experimenttestaa_blue ["pixelSource": "browser-dmg", "metric": "search", "enrollmentDate": "2024-12-04", "value": "1", "conversionWindowDays": "0”]
Move the system date to the following day and perform 4 searches and 4 foregorunding, only 2 pixels should be fired: experiment_metrics_experimenttestaa_control ["pixelSource": "browser-dmg", "metric": "search", "enrollmentDate": "2024-12-05", "value": "1", "conversionWindowDays": “1”] experiment_metrics_experimenttestaa_control ["pixelSource": "browser-dmg", "metric": “ app_use", "enrollmentDate": "2024-12-05", "value": "1", "conversionWindowDays": “1”]
Try the same up to day 4
On day 5 do the same and in this case you should see multiple pixels: 👾[Unique By Name And Parameters-Fired] experiment_metrics_experimenttestaa_control ["conversionWindowDays": "5", "value": "1", "enrollmentDate": "2024-12-04", "metric": "search", "pixelSource": "browser-dmg"] 👾[Unique By Name And Parameters-Fired] experiment_metrics_experimenttestaa_control ["pixelSource": "browser-dmg", "conversionWindowDays": "5-7", "enrollmentDate": "2024-12-04", "value": "1", "metric": "search"] 👾[Unique By Name And Parameters-Fired] experiment_metrics_experimenttestaa_control ["pixelSource": "browser-dmg", "value": "1", "conversionWindowDays": "5", "metric": "app_use", "enrollmentDate": "2024-12-04"] 👾[Unique By Name And Parameters-Fired] experiment_metrics_experimenttestaa_control ["metric": "app_use", "conversionWindowDays": "5-7", "pixelSource": "browser-dmg", "enrollmentDate": "2024-12-04", "value": "1"] 👾[Unique By Name And Parameters-Fired] experiment_metrics_experimenttestaa_control ["value": "4", "pixelSource": "browser-dmg", "conversionWindowDays": "5-7", "metric": "search", "enrollmentDate": "2024-12-04"] 👾[Unique By Name And Parameters-Fired] experiment_metrics_experimenttestaa_control ["value": "4", "enrollmentDate": "2024-12-04", "metric": "app_use", "pixelSource": "browser-dmg", "conversionWindowDays": "5-7”]

test use of new func

In ContinueSetUpModel import PixelExprimentKit, actionCount var and set it to 0 add the following code on line 172 (default browser action)

actionCount += 1
PixelKit.fireExperimentPixel(for: ExperimentTestSubfeatures.experimentTestAA.rawValue, metric: "Provola", conversionWindowDays: 0...10, value: String(actionCount))

Check that every time you click the button you see the experiment pixel sent with every time the updated value

OS Testing:

Internal references:

Software Engineering Expectations
Technical Design Template

SabrinaTardio · 2024-12-17T08:57:49Z

Sources/PixelExperimentKit/PixelExperimentKit.swift

+    /// 1. Validates if the experiment is active.
+    /// 2. Ensures the user is within the specified conversion window.
+    /// 3. Tracks actions performed and sends the pixel once the target value is reached (if applicable).
+    public static func fireExperimentPixelWhenReachingNumberOfCalls(for subfeatureID: SubfeatureID,


If you have any better name suggestion do let me know!

I think we could include "threshold" in the name, e.g:

fireExperimentPixelOnValueThreshold or fireExperimentPixelfThresholdReached

aataraxiaa

~~Thanks for the quick progress on this @SabrinaTardio. Left a few comments and tested this with my branch. The changes allow us to send pixels when the value param has changed. However, the API now also allows us to send the same pixel when the value is unchanged. I believe we still need to track the pixel that was sent so that we can ensure we don't send repeat pixels. ~~

EDIT: Nevermind the above, missed the uniqueness being handled in pixelkit

aataraxiaa · 2024-12-17T10:36:42Z

Sources/PixelExperimentKit/PixelExperimentKit.swift

+        // Define event
+        let event = event(for: subfeatureID, experimentData: experimentData, conversionWindowDays: conversionWindowDays, metric: metric, value: value)
+
+        // Send unique by name and parameter pixel if within conversion window
+        let isInWindow = isUserInConversionWindow(conversionWindowDays, enrollmentDate: experimentData.enrollmentDate)
+        if isInWindow {
+            ExperimentConfig.fireFunction(event, .uniqueByNameAndParameters, false)
+        }


Minor: We could avoid the creation of the event and return early by moving the conversion window check up:

Suggested change

// Define event

let event = event(for: subfeatureID, experimentData: experimentData, conversionWindowDays: conversionWindowDays, metric: metric, value: value)

// Send unique by name and parameter pixel if within conversion window

let isInWindow = isUserInConversionWindow(conversionWindowDays, enrollmentDate: experimentData.enrollmentDate)

if isInWindow {

ExperimentConfig.fireFunction(event, .uniqueByNameAndParameters, false)

}

// Check if within conversion window

guard isUserInConversionWindow(conversionWindowDays, enrollmentDate: experimentData.enrollmentDate) else { return }

// Define event

let event = event(for: subfeatureID, experimentData: experimentData, conversionWindowDays: conversionWindowDays, metric: metric, value: value)

ExperimentConfig.fireFunction(event, .uniqueByNameAndParameters, false)

aataraxiaa · 2024-12-17T10:39:30Z

Sources/PixelExperimentKit/PixelExperimentKit.swift

+    /// 1. Validates if the experiment is active.
+    /// 2. Ensures the user is within the specified conversion window.
+    /// 3. Tracks actions performed and sends the pixel once the target value is reached (if applicable).
+    public static func fireExperimentPixelWhenReachingNumberOfCalls(for subfeatureID: SubfeatureID,


I think we could include "threshold" in the name, e.g:

fireExperimentPixelOnValueThreshold or fireExperimentPixelfThresholdReached

aataraxiaa · 2024-12-17T10:46:16Z

Sources/PixelExperimentKit/PixelExperimentKit.swift

@@ -82,7 +82,7 @@ extension PixelKit {
    /// This function:
    /// 1. Validates if the experiment is active.
    /// 2. Ensures the user is within the specified conversion window.
-    /// 3. Tracks actions performed and sends the pixel once the target value is reached (if applicable).
+    /// 3. Sends the pixel if not sent before (unique by name and parameter)


Should we not be storing the pixel that was sent in this method so that we can ensure pixels are unique? (i.e not sent again if they were sent before). When the value being sent is unchanged, this method fires the same pixel on subsequent calls.

Nevermind the above, missed the uniqueness being handled in pixelkit

aataraxiaa

Thanks @SabrinaTardio, works great! 🥳

* main: (24 commits) change api (#1133) Ensure authToken is present before calling refreshAuthTokenIfNeeded Add 'locale' to report broken site params Add 'locale' to report broken site params Ensure authToken is present before calling refreshAuthTokenIfNeeded Privacy Pro Free Trials - Models and API (#1120) remove MaliciousSiteProtectionSubfeature (#1131) Malware protection 6: Malware integration (#1099) Add underlying error to PrivacyStatsError (#1130) Initial changes for compilation time tracking. (#1111) iOS System level credential provider (#1127) Increase ratio of complete form saves (#1124) Add PrivacyStatsError.failedToClearPrivacyStats (#1128) Update autofill to 16.0.0 (#1122) Update local network routing (#1117) Update RMF version matching to ignore build number (#1118) Fix threading issue between using a Semaphore and async/await (#1115) add experiment test fake feature (#1119) Bump Tests/BrowserServicesKitTests/Resources/privacy-reference-tests from `a603ff9` to `6133e7d` (#1109) experiment default metrics pixels (#1107) ...

SabrinaTardio added 3 commits December 16, 2024 17:19

change api

590f416

fix linting

511f3a8

remove commented out code

f303cfa

SabrinaTardio commented Dec 17, 2024

View reviewed changes

SabrinaTardio marked this pull request as ready for review December 17, 2024 09:02

SabrinaTardio requested a review from aataraxiaa December 17, 2024 09:02

update some comments

e5c0ba3

aataraxiaa requested changes Dec 17, 2024

View reviewed changes

aataraxiaa approved these changes Dec 17, 2024

View reviewed changes

SabrinaTardio added 2 commits December 17, 2024 12:25

address comments

9aa81e8

fix linting

6b67ffb

SabrinaTardio merged commit b71ed70 into main Dec 17, 2024
7 checks passed

SabrinaTardio deleted the sabrina/change-experiment-api branch December 17, 2024 11:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change api #1133

change api #1133

SabrinaTardio commented Dec 16, 2024 •

edited

Loading

SabrinaTardio Dec 17, 2024

aataraxiaa Dec 17, 2024

aataraxiaa left a comment •

edited

Loading

aataraxiaa Dec 17, 2024

aataraxiaa Dec 17, 2024

aataraxiaa Dec 17, 2024

aataraxiaa Dec 17, 2024

aataraxiaa left a comment

change api #1133

change api #1133

Conversation

SabrinaTardio commented Dec 16, 2024 • edited Loading

Internal references:

SabrinaTardio Dec 17, 2024

Choose a reason for hiding this comment

aataraxiaa Dec 17, 2024

Choose a reason for hiding this comment

aataraxiaa left a comment • edited Loading

Choose a reason for hiding this comment

aataraxiaa Dec 17, 2024

Choose a reason for hiding this comment

aataraxiaa Dec 17, 2024

Choose a reason for hiding this comment

aataraxiaa Dec 17, 2024

Choose a reason for hiding this comment

aataraxiaa Dec 17, 2024

Choose a reason for hiding this comment

aataraxiaa left a comment

Choose a reason for hiding this comment

SabrinaTardio commented Dec 16, 2024 •

edited

Loading

aataraxiaa left a comment •

edited

Loading