Skip to content

add device id to feature flag context by default #867

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ public JSONObject getRequestBodyAsJson() throws JSONException {
private static class MockFeatureFlagDelegate implements FeatureFlagDelegate {
MPConfig configToReturn;
String distinctIdToReturn = TEST_DISTINCT_ID;
String anonymousIdToReturn = "test_anonymous_id";
String tokenToReturn = TEST_TOKEN;
List<TrackCall> trackCalls = new ArrayList<>();
CountDownLatch trackCalledLatch; // Optional: for tests waiting for track
Expand Down Expand Up @@ -101,6 +102,11 @@ public String getDistinctId() {
return distinctIdToReturn;
}

@Override
public String getAnonymousId() {
return anonymousIdToReturn;
}

@Override
public void track(String eventName, JSONObject properties) {
MPLog.v("FeatureFlagManagerTest", "MockDelegate.track called: " + eventName);
Expand Down Expand Up @@ -453,6 +459,18 @@ public void testTracking_getVariantSync_calledOnce() throws InterruptedException
assertEquals("$experiment_started", call.eventName);
assertEquals("track_flag_sync", call.properties.getString("Experiment name"));
assertEquals("v_track_sync", call.properties.getString("Variant name"));
assertEquals("feature_flag", call.properties.getString("$experiment_type"));

// Verify that timing properties are included
assertTrue("timeLastFetched should be present", call.properties.has("timeLastFetched"));
assertTrue("fetchLatencyMs should be present", call.properties.has("fetchLatencyMs"));

// Verify that timing values are reasonable
long timeLastFetched = call.properties.getLong("timeLastFetched");
long fetchLatencyMs = call.properties.getLong("fetchLatencyMs");
assertTrue("timeLastFetched should be positive", timeLastFetched > 0);
assertTrue("fetchLatencyMs should be non-negative", fetchLatencyMs >= 0);
assertTrue("fetchLatencyMs should be reasonable (< 5 seconds)", fetchLatencyMs < 5000);
}

@Test
Expand Down Expand Up @@ -940,6 +958,11 @@ public void testRequestBodyConstruction_performFetchRequest() throws Interrupted
assertEquals("Context should contain correct distinct_id",
"test_user_123", requestContext.getString("distinct_id"));

// Verify device_id is in the context
assertTrue("Context should contain device_id", requestContext.has("device_id"));
assertEquals("Context should contain correct device_id",
"test_anonymous_id", requestContext.getString("device_id"));

// Verify the context contains the expected properties from FlagsConfig
assertEquals("Context should contain $os", "Android", requestContext.getString("$os"));
assertEquals("Context should contain $os_version", "13", requestContext.getString("$os_version"));
Expand All @@ -962,8 +985,9 @@ public void testRequestBodyConstruction_withNullContext() throws InterruptedExce
// Setup with flags enabled but null context
setupFlagsConfig(true, null);

// Set distinct ID
// Set distinct ID and anonymous ID
mMockDelegate.distinctIdToReturn = "user_456";
mMockDelegate.anonymousIdToReturn = "device_789";

// Create response
Map<String, MixpanelFlagVariant> serverFlags = new HashMap<>();
Expand All @@ -987,9 +1011,13 @@ public void testRequestBodyConstruction_withNullContext() throws InterruptedExce
// Verify distinct_id is in context
assertEquals("Context should contain correct distinct_id", "user_456", requestContext.getString("distinct_id"));

// When FlagsConfig context is null, the context object should only contain distinct_id
assertEquals("Context should only contain distinct_id when FlagsConfig context is null",
1, requestContext.length());
// Verify device_id is in context
assertTrue("Context should contain device_id", requestContext.has("device_id"));
assertEquals("Context should contain correct device_id", "device_789", requestContext.getString("device_id"));

// When FlagsConfig context is null, the context object should only contain distinct_id and device_id
assertEquals("Context should only contain distinct_id and device_id when FlagsConfig context is null",
2, requestContext.length());
}

@Test
Expand All @@ -999,6 +1027,7 @@ public void testRequestBodyConstruction_withEmptyDistinctId() throws Interrupted

// Set empty distinct ID
mMockDelegate.distinctIdToReturn = "";
mMockDelegate.anonymousIdToReturn = "device_empty_test";

// Create response
Map<String, MixpanelFlagVariant> serverFlags = new HashMap<>();
Expand All @@ -1022,6 +1051,10 @@ public void testRequestBodyConstruction_withEmptyDistinctId() throws Interrupted
// Verify distinct_id is included in context even when empty
assertTrue("Context should contain distinct_id field", requestContext.has("distinct_id"));
assertEquals("Context should contain empty distinct_id", "", requestContext.getString("distinct_id"));

// Verify device_id is included in context
assertTrue("Context should contain device_id field", requestContext.has("device_id"));
assertEquals("Context should contain correct device_id", "device_empty_test", requestContext.getString("device_id"));
}

@Test
Expand Down Expand Up @@ -1114,9 +1147,11 @@ public void testFlagsConfigContextUsage_emptyContext() throws InterruptedExcepti
JSONObject requestBody = capturedRequest.getRequestBodyAsJson();
JSONObject requestContext = requestBody.getJSONObject("context");

// Context should only contain distinct_id when initial context is empty
assertEquals("Context should only contain distinct_id", 1, requestContext.length());
// Context should only contain distinct_id and device_id when initial context is empty
assertEquals("Context should only contain distinct_id and device_id", 2, requestContext.length());
assertEquals("distinct_id should be present", "test_user", requestContext.getString("distinct_id"));
assertTrue("device_id should be present", requestContext.has("device_id"));
assertEquals("device_id should be present", "test_anonymous_id", requestContext.getString("device_id"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
interface FeatureFlagDelegate {
MPConfig getMPConfig();
String getDistinctId();
String getAnonymousId();
void track(String eventName, JSONObject properties);
String getToken();
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ class FeatureFlagManager implements MixpanelAPI.Flags {
private final Set<String> mTrackedFlags = new HashSet<>();
private boolean mIsFetching = false;
private List<FlagCompletionCallback<Boolean>> mFetchCompletionCallbacks = new ArrayList<>();
// Track last fetch time and latency for $experiment_started event
private volatile long mTimeLastFetched = 0L;
private volatile long mFetchLatencyMs = 0L;
// ---

// Message codes for Handler
Expand Down Expand Up @@ -430,6 +433,7 @@ private void _fetchFlagsIfNeeded(@Nullable FlagCompletionCallback<Boolean> compl
* back to the mHandler thread via MSG_COMPLETE_FETCH.
*/
private void _performFetchRequest() {
long fetchStart = System.currentTimeMillis();
MPLog.v(LOGTAG, "Performing fetch request on thread: " + Thread.currentThread().getName());
boolean success = false;
JSONObject responseJson = null; // To hold parsed successful response
Expand All @@ -444,6 +448,7 @@ private void _performFetchRequest() {

final MPConfig config = delegate.getMPConfig();
final String distinctId = delegate.getDistinctId();
final String deviceId = delegate.getAnonymousId();

if (distinctId == null) {
MPLog.w(LOGTAG, "Distinct ID is null. Cannot fetch flags.");
Expand All @@ -456,6 +461,9 @@ private void _performFetchRequest() {
// 1. Build Request Body JSON
JSONObject contextJson = new JSONObject(mFlagsConfig.context.toString());
contextJson.put("distinct_id", distinctId);
if (deviceId != null) {
contextJson.put("device_id", deviceId);
}
JSONObject requestJson = new JSONObject();
requestJson.put("context", contextJson);
String requestJsonString = requestJson.toString();
Expand Down Expand Up @@ -527,6 +535,9 @@ private void _performFetchRequest() {
MPLog.e(LOGTAG, errorMessage, e);
}

long fetchEnd = System.currentTimeMillis();
mTimeLastFetched = fetchEnd;
mFetchLatencyMs = fetchEnd - fetchStart;
// 6. Post result back to Handler thread
postResultToHandler(success, responseJson, errorMessage);
}
Expand Down Expand Up @@ -625,6 +636,8 @@ private void _performTrackingDelegateCall(String flagName, MixpanelFlagVariant v
properties.put("Experiment name", flagName);
properties.put("Variant name", variant.key); // Use the variant key
properties.put("$experiment_type", "feature_flag");
properties.put("timeLastFetched", mTimeLastFetched);
properties.put("fetchLatencyMs", mFetchLatencyMs);
} catch (JSONException e) {
MPLog.e(LOGTAG, "Failed to create JSON properties for $experiment_started event", e);
return; // Don't track if properties failed
Expand Down
Loading