chore: Refactor FlagsProvider and FlagsClient into one by typotter · Pull Request #2899 · DataDog/dd-sdk-android

typotter · 2025-09-25T21:25:31Z

Flags Feature - FlagsClient

Feature Flagging mobile SDK spike
stacked on #2896
FFL-1116

What does this PR do?

Refactors the WIP dd-sdk-android-flags package to consolidate FlagsProvider functionality into FlagsClient using an interface + companion object pattern. This provides cleaner static access (FlagsClient.instance()) while maintaining instance-based flag operations, aligning with mobile-api-spec.md requirements.

Motivation

The existing dual-class pattern (FlagsClient object + FlagsProvider interface) was confusing and didn't align with mobile-api-spec.md. The spec requires static access for client instances but instance methods for flag operations. This refactoring simplifies the API to a single FlagsClient class that provides both patterns cleanly and adheres to the agreed upon spec for mobile flags SDKs

Additional Notes

Out of scope

Creating/getting clients by name is tracked in FFL-1118

API Target

val client = FlagsClient.instance()
client.resolveBooleanValue("flag", false)

Key Changes Made

Consolidated Classes: Eliminated FlagsProvider separate class
Enhanced Interface: Added companion object with static registry to FlagsClient
Simplified API: Single FlagsClient.instance() entry point
Updated Tests: Rewrote tests to use interface + companion pattern
Documentation: Added comprehensive KDoc for all methods and companion object

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)

…d improve docs

codecov-commenter · 2025-09-25T22:01:54Z

Codecov Report

❌ Patch coverage is 79.48718% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.59%. Comparing base (f2fed1b) to head (b9406f1).
⚠️ Report is 305 commits behind head on feature/feature-flagging.

Files with missing lines	Patch %	Lines
.../datadog/android/flags/featureflags/FlagsClient.kt	71.43%	5 Missing and 3 partials ⚠️
.../flags/featureflags/internal/DatadogFlagsClient.kt	82.86%	1 Missing and 5 partials ⚠️
...src/main/kotlin/com/datadog/android/flags/Flags.kt	87.50%	1 Missing ⚠️
...oid/flags/featureflags/internal/NoOpFlagsClient.kt	85.71%	1 Missing ⚠️

Additional details and impacted files

@@                     Coverage Diff                      @@
##           feature/feature-flagging    #2899      +/-   ##
============================================================
- Coverage                     70.75%   70.59%   -0.16%     
============================================================
  Files                           826      826              
  Lines                         30078    30087       +9     
  Branches                       5009     5124     +115     
============================================================
- Hits                          21279    21238      -41     
- Misses                         7408     7410       +2     
- Partials                       1391     1439      +48

Files with missing lines	Coverage Δ
...ureflags/internal/evaluation/EvaluationsManager.kt	`89.29% <ø> (ø)`
...src/main/kotlin/com/datadog/android/flags/Flags.kt	`88.68% <87.50%> (-1.12%)`	⬇️
...oid/flags/featureflags/internal/NoOpFlagsClient.kt	`85.71% <85.71%> (ø)`
.../flags/featureflags/internal/DatadogFlagsClient.kt	`82.86% <82.86%> (ø)`
.../datadog/android/flags/featureflags/FlagsClient.kt	`71.43% <71.43%> (-2.76%)`	⬇️

... and 94 files with indirect coverage changes

🚀 New features to boost your workflow:

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

0xnm · 2025-09-26T10:48:49Z

+         * in production code. It delegates to [clear] to remove all client registrations.
+         */
+        @Suppress("unused")
+        private fun reset() {


is it used anywhere? seems like redundant method if it delegates to another one anyway.

good catch; no longer needed

still there, can be removed

Co-authored-by: Nikita Ogorodnikov <[email protected]>

typotter

thanks for the review. PTAL

typotter · 2025-10-01T19:52:39Z

+         * in production code. It delegates to [clear] to remove all client registrations.
+         */
+        @Suppress("unused")
+        private fun reset() {


good catch; no longer needed

…client-client-client-manager-or-eliminate

typotter · 2025-10-03T17:30:08Z

/merge

dd-devflow-routing-codex · 2025-10-03T17:30:12Z

View all feedbacks in Devflow UI.

2025-10-03 17:30:12 UTC ℹ️ Start processing command /merge

2025-10-03 17:30:17 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in typo/fixes is approximately 0s (p90).

2025-10-03 17:39:09 UTC ⚠️ MergeQueue: This merge request build was cancelled

[email protected] cancelled this merge request build

dd-devflow-routing-codex · 2025-10-03T18:06:38Z

View all feedbacks in Devflow UI.

2025-10-03 18:06:37 UTC ℹ️ Start processing command /merge

2025-10-03 18:06:44 UTC ℹ️ MergeQueue: waiting for PR to be ready

This merge request is not mergeable yet, because of pending checks/missing approvals. 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-03 19:17:41 UTC ⚠️ MergeQueue: This merge request was unqueued

[email protected] unqueued this merge request

…name-provider-client-client-client-manager-or-eliminate

typotter · 2025-10-03T19:17:33Z

/remove

dd-devflow-routing-codex · 2025-10-03T19:17:36Z

View all feedbacks in Devflow UI.

2025-10-03 19:17:36 UTC ℹ️ Start processing command /remove

2025-10-03 19:17:39 UTC ℹ️ Devflow: /remove

jonathanmos

LGTM on the whole - I left some minor comments

jonathanmos · 2025-10-05T07:24:14Z

+                { "Missing required configuration parameters: $missingParams" }
            )

            return null


Do we prefer null here or a NoOpFlagsClient? Also, after this we could probably remove the TODO on line 82

null here is fine; we are dropping this create-on-enable routine anyway and will guarantee a non-null Client from the create/get methods

jonathanmos · 2025-10-05T07:31:29Z

+     * Must contain a valid targeting key; invalid contexts are logged and ignored.
+     */
+    override fun setContext(context: EvaluationContext) {
+        // Validate targeting key is not blank


nit: redundant comment

0xnm · 2025-10-06T07:40:42Z

+         * in production code. It delegates to [clear] to remove all client registrations.
+         */
+        @Suppress("unused")
+        private fun reset() {


still there, can be removed

0xnm · 2025-10-06T07:41:59Z

+     */
+    override fun resolveBooleanValue(flagKey: String, defaultValue: Boolean): Boolean {
+        val precomputedFlag = flagsRepository.getPrecomputedFlag(flagKey)
+        return precomputedFlag?.variationValue?.toBooleanStrictOrNull() ?: defaultValue


do we want to use toBooleanStrictOrNull here or just toBooleanOrNull. The doc for the former is here https://kotlinlang.org/api/core/kotlin-stdlib/kotlin.text/to-boolean-strict-or-null.html.

hm. I think toBooleanOrNull makes more sense here, thanks.
Jonathan makes this change in #2895

Co-authored-by: Nikita Ogorodnikov <[email protected]> Co-authored-by: Jonathan Moskovich <[email protected]>

typotter · 2025-10-07T16:24:02Z

/merge

dd-devflow-routing-codex · 2025-10-07T16:24:05Z

View all feedbacks in Devflow UI.

2025-10-07 16:24:05 UTC ℹ️ Start processing command /merge

2025-10-07 16:24:09 UTC ℹ️ MergeQueue: pull request added to the queue

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

2025-10-07 17:27:10 UTC ℹ️ MergeQueue: This merge request was merged

typotter added 2 commits September 25, 2025 14:21

Rename FlagsProvider-> Flags Client, FlagsClient -> FlagsClientMap an…

1b191c5

…d improve docs

chore: combine FlagsClientMap and FlagsClient

22c2685

typotter changed the title ~~Typo/ffl 1116 swap classname provider client client client manager or eliminate~~ chore: Refactor FlagsProvider and FlagsClient into one Sep 25, 2025

typotter marked this pull request as ready for review September 25, 2025 21:45

typotter requested review from a team as code owners September 25, 2025 21:45

doesNotThrow

ac465bf

lint fix

36b577e

typotter requested review from 0xnm and jonathanmos September 26, 2025 04:59

typotter mentioned this pull request Sep 26, 2025

Flags.enable and FlagsClient APIs #2900

Merged

3 tasks

0xnm requested changes Sep 26, 2025

View reviewed changes

typotter mentioned this pull request Oct 1, 2025

fix and improve tests, context management #2896

Merged

3 tasks

typotter commented Oct 1, 2025

View reviewed changes

typotter and others added 5 commits October 1, 2025 13:50

Apply suggestions from code review

f240dbd

Co-authored-by: Nikita Ogorodnikov <[email protected]>

merge fixes branch

316e402

fix tests and api after merge

73754f6

rename to resolveDouble

7a02e7b

comments and renames

e3c1633

typotter commented Oct 1, 2025

View reviewed changes

typotter requested a review from 0xnm October 1, 2025 20:27

typotter added 2 commits October 2, 2025 10:47

merge latest fixes

1cad52e

merge artifact

a520259

0xnm previously approved these changes Oct 3, 2025

View reviewed changes

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

typotter added 3 commits October 3, 2025 10:26

Merge branch 'typo/fixes' into typo/FFL-1116-swap-classname-provider-…

48ab284

…client-client-client-manager-or-eliminate

tweaks

ba53b33

Remove FlagsProvider, update tests

407f324

dd-devflow Bot added mergequeue-status: waiting and removed mergequeue-status: removed labels Oct 3, 2025

Merge branch 'feature/feature-flagging' into typo/FFL-1116-swap-class…

aaa5f94

…name-provider-client-client-client-manager-or-eliminate

dd-devflow Bot added mergequeue-status: removed and removed mergequeue-status: waiting labels Oct 3, 2025

jonathanmos previously approved these changes Oct 5, 2025

View reviewed changes

0xnm previously approved these changes Oct 6, 2025

View reviewed changes

Apply suggestions from code review

666fbad

Co-authored-by: Nikita Ogorodnikov <[email protected]> Co-authored-by: Jonathan Moskovich <[email protected]>

typotter dismissed stale reviews from 0xnm and jonathanmos via 666fbad October 6, 2025 17:31

address comments

b9406f1

typotter requested review from 0xnm and jonathanmos October 6, 2025 17:57

0xnm approved these changes Oct 7, 2025

View reviewed changes

dd-devflow Bot added mergequeue-status: queued mergequeue-status: in_progress and removed mergequeue-status: removed mergequeue-status: queued labels Oct 7, 2025

dd-mergequeue Bot merged commit b641740 into feature/feature-flagging Oct 7, 2025
26 checks passed

dd-devflow Bot removed the mergequeue-status: in_progress label Oct 7, 2025

dd-mergequeue Bot deleted the typo/FFL-1116-swap-classname-provider-client-client-client-manager-or-eliminate branch October 7, 2025 17:27

dd-devflow Bot added the mergequeue-status: done label Oct 7, 2025

This was referenced Nov 4, 2025

Flags Feature #2979

Closed

Flags Feature #2982

Merged

0xnm restored the typo/FFL-1116-swap-classname-provider-client-client-client-manager-or-eliminate branch March 26, 2026 10:48

Uh oh!

Conversation

typotter commented Sep 25, 2025 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Flags Feature - FlagsClient

What does this PR do?

Motivation

Additional Notes

Out of scope

API Target

Key Changes Made

Review checklist (to be filled by reviewers)

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

typotter left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

typotter commented Oct 3, 2025

Uh oh!

dd-devflow-routing-codex Bot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dd-devflow-routing-codex Bot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

typotter commented Oct 3, 2025

Uh oh!

dd-devflow-routing-codex Bot commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonathanmos 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

typotter commented Sep 25, 2025 •

edited by atlassian Bot

Loading

codecov-commenter commented Sep 25, 2025 •

edited

Loading

dd-devflow-routing-codex Bot commented Oct 3, 2025 •

edited

Loading

dd-devflow-routing-codex Bot commented Oct 3, 2025 •

edited

Loading

dd-devflow-routing-codex Bot commented Oct 3, 2025 •

edited

Loading

dd-devflow-routing-codex Bot commented Oct 7, 2025 •

edited

Loading