feat: custom flagging endpoint and assignment download refactoring. #2917

typotter · 2025-10-07T04:28:56Z

What does this PR do?

This PR refactors the precomputed assignments request mechanism to follow the SDK-wide Factory + Executor pattern, decoupling request construction from execution. The main changes include:

Created PrecomputedAssignmentsRequestFactory to handle request building
Renamed DefaultFlagsNetworkManager → PrecomputedAssignmentsDownloader to focus on request execution
Properly wired custom flag endpoint configuration through the entire request flow
Added comprehensive unit and integration tests

Motivation

Problem: The existing DefaultFlagsNetworkManager tightly coupled request construction (building headers, body, URL) with request execution (network calls, error handling), making it difficult to test request construction independently and inconsistent with patterns used throughout the SDK.

Goals:

Decouple request building from execution - Separate concerns for better testability and maintainability
Follow SDK patterns - Match the Factory + Executor pattern used by DataOkHttpUploader, ExposuresRequestFactory, and other features
Improve testability - Enable testing request construction without network calls and vice versa
Ensure custom endpoint support - Verify that custom flag endpoint configuration works correctly end-to-end

Architecture Changes

Before:

DefaultFlagsNetworkManager
  ├─> downloadPrecomputedFlags()
  ├─> buildHeaders()
  ├─> buildRequestBody()
  └─> executeDownloadRequest()

After (Factory + Executor Pattern):

FlagsFeature
  └─> precomputedRequestFactory (shared, stateless)

FlagsClient
  └─> PrecomputedAssignmentsDownloader(requestFactory, flagsContext)
        ├─> factory.create() → builds Request
        └─> executeDownloadRequest() → executes Request

Key Components

1. PrecomputedAssignmentsRequestFactory

Interface: Defines contract for creating OkHttp requests
DefaultPrecomputedAssignmentsRequestFactory: Implementation that builds requests with:
- URL determination via EndpointsHelper (supports custom endpoints)
- Authentication headers (client token, application ID)
- JSON-API formatted request body
- Proper error handling with logging
Scope: One instance per feature (stateless, shared across clients)

2. PrecomputedAssignmentsDownloader

Renamed from: DefaultFlagsNetworkManager
Responsibilities: Request execution only
- Gets request from factory
- Executes with OkHttp
- Handles network errors (UnknownHostException, IOException, etc.)
- Processes responses
Pattern: Mirrors DataOkHttpUploader error handling

3. EndpointsHelper

Refactored to: Static methods (object)
Methods: getFlaggingEndpoint(), buildEndpointHost()
Created internally: By request factory when building requests

Additional Notes

Pattern Consistency:
This refactoring aligns the Flags feature with the SDK's standard patterns:

Uploads: RequestFactory → DataOkHttpUploader
Exposures: ExposuresRequestFactory → DataOkHttpUploader
Precomputed Assignments (this PR): PrecomputedAssignmentsRequestFactory → PrecomputedAssignmentsDownloader

Review checklist (to be filled by reviewers)

Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
Make sure you discussed the feature or bugfix with the maintaining team in an Issue
Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

…s-enable' into typo/FFL-1112-flagging-proxy-custom-precomputed-assignments-endpoint

codecov-commenter · 2025-10-07T04:57:53Z

Codecov Report

❌ Patch coverage is 69.47368% with 58 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.71%. Comparing base (410c494) to head (60c0a84).

Files with missing lines	Patch %	Lines
...tadog/android/core/internal/NoOpInternalSdkCore.kt	0.00%	15 Missing ⚠️
...s/internal/net/PrecomputedAssignmentsDownloader.kt	63.16%	14 Missing ⚠️
...ternal/net/PrecomputedAssignmentsRequestFactory.kt	73.08%	13 Missing and 1 partial ⚠️
...ags/internal/persistence/FlagsStateDeserializer.kt	77.14%	8 Missing ⚠️
.../datadog/android/flags/featureflags/FlagsClient.kt	0.00%	6 Missing ⚠️
.../com/datadog/android/api/feature/FeatureSdkCore.kt	0.00%	1 Missing ⚠️

Additional details and impacted files

@@                     Coverage Diff                      @@
##           feature/feature-flagging    #2917      +/-   ##
============================================================
+ Coverage                     70.69%   70.71%   +0.01%     
============================================================
  Files                           834      835       +1     
  Lines                         30380    30388       +8     
  Branches                       5132     5132              
============================================================
+ Hits                          21476    21486      +10     
+ Misses                         7444     7440       -4     
- Partials                       1460     1462       +2

Files with missing lines	Coverage Δ
...n/com/datadog/android/core/internal/CoreFeature.kt	`84.84% <100.00%> (+0.36%)`	⬆️
...n/com/datadog/android/core/internal/DatadogCore.kt	`78.26% <100.00%> (+0.07%)`	⬆️
...in/com/datadog/android/flags/FlagsConfiguration.kt	`100.00% <100.00%> (ø)`
...ureflags/internal/evaluation/EvaluationsManager.kt	`89.29% <100.00%> (ø)`
.../flags/featureflags/internal/model/FlagsContext.kt	`100.00% <100.00%> (ø)`
.../android/flags/internal/ExposureEventsProcessor.kt	`100.00% <100.00%> (ø)`
...com/datadog/android/flags/internal/FlagsFeature.kt	`88.37% <100.00%> (+0.87%)`	⬆️
...adog/android/flags/internal/model/ExposureEvent.kt	`100.00% <100.00%> (ø)`
...roid/flags/internal/net/ExposuresRequestFactory.kt	`100.00% <100.00%> (ø)`
...lags/internal/storage/ExposureEventRecordWriter.kt	`100.00% <100.00%> (ø)`
... and 6 more

... and 36 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

...c/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactory.kt

aleksandr-gringauz · 2025-10-07T09:50:02Z

...tadog/android/flags/featureflags/internal/repository/net/PrecomputedAssignmentsDownloader.kt

+    }
+
+    private fun setupOkHttpClient() {
+        callFactory = OkHttpCallFactory {


Maybe this is not a topic for this PR, but I would like to discuss it anyway.

Up until now we've had only one OkHttpClient in the whole SDK here.

If you create OkHttpClient the way you do here it will create its own thread pool and connection pool which is most likely is a waste of resources for no reason. See this doc for example.

I suggest we do something like this to share the underlying thread pool and connection pool between different OkHttpClients in our SDK.

This also has the benefit that we can add some common settings for all clients (like cache). Not sure that we need it in this particular case though.

WDYT?

I think that's a great idea, and thank you for bringing it up here!

Great!

One more question. Do you think that OkHttpClient for the flags feature should have the same configuration as the one we already had in the SDK (link)? Or it should be different?

It of course depends on the backend. As far as I see, the host for flags will be different than for the rest of the SDK (this).

No strong opinion from me here.

It should have the same configuration given Flags are supposed to work in GovCloud as well, which puts restrictions on the set of TLS suites allowed for the FIPS compliance.

just to note, Flags is not currently supported in GovCloud, (the intent is for it to be, but we've not scoped a GovCloud deploy yet).

I think it's useful to use the same existing configuration and just allow certain overrides in the FlagsClient on parameters like timeout and number of retries.
We're not speccing any customization in the API at current, so I think copying the existing configuration makes the most sense, especially given it is already hardened

(I'll update my code here)

aleksandr-gringauz · 2025-10-07T09:55:07Z

...tadog/android/flags/featureflags/internal/repository/net/PrecomputedAssignmentsDownloader.kt

+    private val requestFactory: PrecomputedAssignmentsRequestFactory
+) : FlagsNetworkManager {
+
+    internal lateinit var callFactory: OkHttpCallFactory


internal lateinit var callFactory: OkHttpCallFactory -> private val callFactory: OkHttpCallFactory

aleksandr-gringauz · 2025-10-08T08:08:26Z

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/NoOpInternalSdkCore.kt

    }

+    override fun createOkHttpCallFactory(block: okhttp3.OkHttpClient.Builder.() -> Unit): okhttp3.Call.Factory {
+        return okhttp3.Call.Factory { request ->


This isn't actually a "no-op" implementation. It will create a real OkHttpClient. You have to create a NoOpCallFactory.

Here it is:

object NoOpCallFactory: Call.Factory { object NoOpCall: Call { override fun cancel() { } override fun clone(): Call { return this } override fun enqueue(responseCallback: Callback) { } override fun execute(): Response { return Response.Builder().build() } override fun isCanceled(): Boolean { return false } override fun isExecuted(): Boolean { return false } override fun request(): Request { return Request.Builder().build() } override fun timeout(): Timeout { return Timeout.NONE } } override fun newCall(request: Request): Call { return NoOpCall } }

thanks!
Would it make sense for the noop call factory to return the request it was created with?

Would it make sense for the noop call factory to return the request it was created with?

yes, this is better, indeed

aleksandr-gringauz · 2025-10-08T09:49:28Z

...ain/kotlin/com/datadog/android/flags/featureflags/internal/repository/net/EndpointsHelper.kt

    )

    private fun createSiteConfigObject(dc: String? = null, tld: String? = null): JSONObject = try {
        JSONObject().apply {


Why are we using JSON here? You can create something like data class SiteConfig(val dc: String, val tld: String).

And siteConfig can become Map<DatadogSite, SiteConfig>.

agree with you here. changed

actually, #2922 is an even cleaner approach

...tadog/android/flags/featureflags/internal/repository/net/PrecomputedAssignmentsDownloader.kt

0xnm

lgtm, but I left some non-blocking suggestions

0xnm · 2025-10-15T07:54:53Z

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/DatadogCore.kt

    }

+    /** @inheritDoc */
+    override fun createOkHttpCallFactory(block: okhttp3.OkHttpClient.Builder.() -> Unit): okhttp3.Call.Factory {


minor: okhttp3 should go to import

Suggested change

override fun createOkHttpCallFactory(block: okhttp3.OkHttpClient.Builder.() -> Unit): okhttp3.Call.Factory {

override fun createOkHttpCallFactory(block: OkHttpClient.Builder.() -> Unit): Call.Factory {

dd-sdk-android-core/src/test/kotlin/com/datadog/android/core/internal/CoreFeatureTest.kt

0xnm · 2025-10-15T07:59:35Z

...s/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/featureflags/FlagsClient.kt

                }

-                val flagsNetworkManager = DefaultFlagsNetworkManager(
+                val callFactory = sdkCore.createOkHttpCallFactory {}


minor: maybe it is worth to use default lambda argument in the createOkHttpCallFactory signature? Otherwise such call with empty lambda looks a bit strange.

added to the interface. empty () certainly looks more normal here, thx

0xnm · 2025-10-15T08:04:48Z

...res/dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/FlagsConfigurationTest.kt

-        builder.trackExposures(fakeTrackExposuresState)
+        builder.trackExposures(
+            fakeTrackExposuresState
+        ).useCustomExposureEndpoint(fakeCustomExposureEndpoint).useCustomFlagEndpoint(fakeCustomFlagEndpoint)


minor: seems like useCustomFlagEndpoint call can go on another line. Formatting is off in this call chain.

0xnm · 2025-10-15T08:07:29Z

.../dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt

    fun `set up`() {
        whenever(mockSdkCore.internalLogger) doReturn mockInternalLogger
        whenever(mockSdkCore.createSingleThreadExecutorService(any())) doReturn mockExecutorService
+//        whenever(mockSdkCore.getDatadogContext()) doReturn mockDatadogContext


seems like this line should be removed and there is no actual usage of mockDatadogContext above?

...c/test/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsDownloaderTest.kt

0xnm · 2025-10-15T08:13:15Z

...st/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactoryTest.kt

+        val request = testedFactory.create(context, flagsContext)
+
+        // Then
+        assertThat(request).isNotNull


it actually still the same, the suggested change is not applied here and in the tests below

typotter

Thanks again for the review. Sorry for missing that handful of fixes.
The last commit to address comments requires another stamp. PTAL

typotter · 2025-10-16T06:45:39Z

.../dd-sdk-android-flags/src/test/kotlin/com/datadog/android/flags/internal/FlagsFeatureTest.kt

    fun `set up`() {
        whenever(mockSdkCore.internalLogger) doReturn mockInternalLogger
        whenever(mockSdkCore.createSingleThreadExecutorService(any())) doReturn mockExecutorService
+//        whenever(mockSdkCore.getDatadogContext()) doReturn mockDatadogContext


typotter · 2025-10-16T06:49:10Z

...st/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactoryTest.kt

+        val request = testedFactory.create(context, flagsContext)
+
+        // Then
+        assertThat(request).isNotNull


typotter · 2025-10-16T06:49:46Z

dd-sdk-android-core/src/main/kotlin/com/datadog/android/core/internal/DatadogCore.kt

    }

+    /** @inheritDoc */
+    override fun createOkHttpCallFactory(block: okhttp3.OkHttpClient.Builder.() -> Unit): okhttp3.Call.Factory {


typotter · 2025-10-16T06:52:47Z

...s/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/featureflags/FlagsClient.kt

                }

-                val flagsNetworkManager = DefaultFlagsNetworkManager(
+                val callFactory = sdkCore.createOkHttpCallFactory {}


added to the interface. empty () certainly looks more normal here, thx

typotter · 2025-10-16T06:55:28Z

...-android-flags/src/main/kotlin/com/datadog/android/flags/internal/ExposureEventsProcessor.kt

fixed; I'll connect w/Jonathan on verifying my ktlint config

typotter · 2025-10-16T07:34:08Z

/merge

dd-devflow-routing-codex · 2025-10-16T07:34:12Z

View all feedbacks in Devflow UI.

2025-10-16 07:34:11 UTC ℹ️ Start processing command /merge

2025-10-16 07:34:17 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
It will be added to the queue as soon as checks pass and/or get approvals.
Note: if you pushed new commits since the last approval, you may need additional approval.
You can remove it from the waiting list with /remove command.

2025-10-16 08:49:07 UTC ℹ️ MergeQueue: merge request added to the queue

The expected merge time in feature/feature-flagging is approximately 1h (p90).

2025-10-16 09:50:18 UTC ℹ️ MergeQueue: This merge request was merged

typotter added 5 commits October 6, 2025 20:04

lint

c132e95

Merge branch 'typo/FFL-1117-pass-default-client-configuration-to-flag…

a6d4119

…s-enable' into typo/FFL-1112-flagging-proxy-custom-precomputed-assignments-endpoint

Request Factory and refactor EndpointsHelper

ecc4928

Rename net manager to downloader

8f576a2

new classes and integration test

105f2f7

typotter changed the base branch from develop to typo/FFL-1117-pass-default-client-configuration-to-flags-enable October 7, 2025 04:29

tidy

b0a722f

typotter commented Oct 7, 2025

View reviewed changes

...c/main/kotlin/com/datadog/android/flags/internal/net/PrecomputedAssignmentsRequestFactory.kt Show resolved Hide resolved

update comments

f687fa3

typotter marked this pull request as ready for review October 7, 2025 05:05

typotter requested review from a team as code owners October 7, 2025 05:05

aleksandr-gringauz reviewed Oct 7, 2025

View reviewed changes

typotter changed the title ~~feat: custom flagging endpoint and assignment download refacotring.~~ feat: custom flagging endpoint and assignment download refactoring. Oct 7, 2025

typotter added 3 commits October 7, 2025 15:07

feat: add shared pool for okHttp call factory

99dbfc9

merge

96fe3a0

renaming

b4f552b

typotter force-pushed the typo/FFL-1112-flagging-proxy-custom-precomputed-assignments-endpoint branch from f34c5b8 to b4f552b Compare October 8, 2025 05:42

improve tests

7d3e217

typotter requested a review from aleksandr-gringauz October 8, 2025 06:09

aleksandr-gringauz reviewed Oct 8, 2025

View reviewed changes

...tadog/android/flags/featureflags/internal/repository/net/PrecomputedAssignmentsDownloader.kt Outdated Show resolved Hide resolved

typotter requested a review from jonathanmos October 8, 2025 14:42

typotter added 3 commits October 8, 2025 09:52

Allow okHTTP builder methods

171ea2b

tests and noop call factory

eb76dfc

simplify endpoints helper

d6939d6

typotter mentioned this pull request Oct 8, 2025

Flags.enable and FlagsClient APIs #2900

Merged

3 tasks

move HttpClient builder defaults to lazy shared builder

9aab403

typotter requested review from 0xnm, aleksandr-gringauz and jonathanmos October 15, 2025 06:12

aleksandr-gringauz previously approved these changes Oct 15, 2025

View reviewed changes

0xnm previously approved these changes Oct 15, 2025

View reviewed changes

comments

5d66111

typotter dismissed stale reviews from 0xnm and aleksandr-gringauz via 5d66111 October 16, 2025 05:51

typotter added 2 commits October 16, 2025 00:43

test specific log string

0a45f17

revert formatting

d35ba81

typotter commented Oct 16, 2025

View reviewed changes

typotter requested review from 0xnm and aleksandr-gringauz October 16, 2025 06:59

0xnm previously approved these changes Oct 16, 2025

View reviewed changes

lint

459d407

typotter dismissed 0xnm’s stale review via 459d407 October 16, 2025 07:34

dd-devflow bot added the mergequeue-status: waiting label Oct 16, 2025

merge feature/feature-flagging

60c0a84

0xnm approved these changes Oct 16, 2025

View reviewed changes

dd-devflow bot added mergequeue-status: queued mergequeue-status: in_progress and removed mergequeue-status: waiting mergequeue-status: queued labels Oct 16, 2025

dd-mergequeue bot merged commit a70a89e into feature/feature-flagging Oct 16, 2025
26 checks passed

dd-mergequeue bot deleted the typo/FFL-1112-flagging-proxy-custom-precomputed-assignments-endpoint branch October 16, 2025 09:50

dd-devflow bot added mergequeue-status: done and removed mergequeue-status: in_progress labels Oct 16, 2025

	override fun createOkHttpCallFactory(block: okhttp3.OkHttpClient.Builder.() -> Unit): okhttp3.Call.Factory {
	override fun createOkHttpCallFactory(block: OkHttpClient.Builder.() -> Unit): Call.Factory {

feat: custom flagging endpoint and assignment download refactoring. #2917

feat: custom flagging endpoint and assignment download refactoring. #2917

Uh oh!

Conversation

typotter commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Architecture Changes

Key Components

1. PrecomputedAssignmentsRequestFactory

2. PrecomputedAssignmentsDownloader

3. EndpointsHelper

Additional Notes

Review checklist (to be filled by reviewers)

Uh oh!

codecov-commenter commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

0xnm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

typotter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

typotter commented Oct 7, 2025 •

edited

Loading

codecov-commenter commented Oct 7, 2025 •

edited

Loading

dd-devflow-routing-codex bot commented Oct 16, 2025 •

edited

Loading