feat: Graceful mode and error logging policies by typotter · Pull Request #2963 · DataDog/dd-sdk-android

typotter · 2025-10-22T23:03:49Z

What does this PR do?

Implements graceful mode error handling for the Flags SDK, providing three-tier policy-based error handling (Graceful/Error/Strict) that adapts behavior based on build type and configuration.

Key changes:

Added gracefulModeEnabled configuration parameter (defaults to true)
Implemented debug build detection using ApplicationInfo.FLAG_DEBUGGABLE
Added policy-aware error logging in FlagsFeature.logErrorWithPolicy()
Refactored NoOpFlagsClient to use policy-based logging functions
Updated client creation and retrieval to crash in strict mode for misuse scenarios

Policy behavior:

Graceful Policy (Release builds): Conditional logging through SDK logger
Error Policy (Debug + graceful enabled): Always log to System.err, non-crashing
Strict Policy (Debug + graceful disabled): Crash immediately on misuse with
IllegalStateException

Motivation

The Flags SDK needs configurable error handling to balance developer experience across
different environments:

Development/QA: Fail fast with crashes to catch configuration errors early
Staging/Dogfooding: Loud warnings without crashes for visibility without disruption
Production: Silent fallbacks for maximum stability

This aligns with OpenFeature error handling patterns and provides flexibility for teams
with different development workflows (small teams vs. large modular apps, monoliths vs.
DI-based architectures).

Related: FFL-1187

Additional Notes

Implementation highlights:

Debug Detection: Uses ApplicationInfo.FLAG_DEBUGGABLE (same pattern as
DatadogCore.isAppDebuggable()) with performance caching via @Volatile field primed
during onInitialize()
Thread Safety:
- @Volatile fields for appContext, debugBuildCache, and initialized flag
- Double-check locking pattern in getOrRegisterNewClient()
- Proper lifecycle management with cleanup in onStop()
Crash Points (Strict Mode Only):
- Duplicate client creation: FlagsFeature.getOrRegisterNewClient()
- Client not found: FlagsClient.get()
- Uses Kotlin's error() function to throw IllegalStateException
Safe Fallback: When FlagsFeature not enabled, always uses graceful behavior (can't determine policy without config)

Documentation:

Comprehensive KDoc with environment-specific guidance
Added @throws IllegalStateException documentation to build() and get() methods

Intentional deviations from spec:

Kept NoOpFlagsClient name instead of FallbackClient

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)

codecov-commenter · 2025-10-22T23:34:46Z

Codecov Report

❌ Patch coverage is 72.16495% with 27 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.88%. Comparing base (412d3f3) to head (826ac08).

Files with missing lines	Patch %	Lines
...in/kotlin/com/datadog/android/flags/FlagsClient.kt	53.49%	19 Missing and 1 partial ⚠️
...com/datadog/android/flags/internal/FlagsFeature.kt	79.41%	5 Missing and 2 partials ⚠️

Additional details and impacted files

@@                     Coverage Diff                      @@
##           feature/feature-flagging    #2963      +/-   ##
============================================================
+ Coverage                     70.84%   70.88%   +0.04%     
============================================================
  Files                           840      840              
  Lines                         30559    30603      +44     
  Branches                       5169     5176       +7     
============================================================
+ Hits                          21649    21691      +42     
- Misses                         7446     7453       +7     
+ Partials                       1464     1459       -5

Files with missing lines	Coverage Δ
...in/com/datadog/android/flags/FlagsConfiguration.kt	`92.31% <100.00%> (+1.83%)`	⬆️
.../datadog/android/flags/internal/NoOpFlagsClient.kt	`100.00% <100.00%> (+4.17%)`	⬆️
...com/datadog/android/flags/internal/FlagsFeature.kt	`83.33% <79.41%> (-5.04%)`	⬇️
...in/kotlin/com/datadog/android/flags/FlagsClient.kt	`35.29% <53.49%> (-0.28%)`	⬇️

... and 40 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-10-23T08:56:45Z

+    internal fun logErrorWithPolicy(
+        message: String,
+        level: InternalLogger.Level = InternalLogger.Level.ERROR,
+        shouldCrashInStrict: Boolean = false


This is a controversial approach, because customers may choose strict mode by mistake in the release builds and get a crash in production.

Are we doing the same in iOS SDK?

strict mode cannot be enabled in release builds - the graceful mode option is overridden by the build type below. When !isDebugBuild, we log with the internal logger only

typotter

Thanks for the review. Confirming with @ncreated on the matter of System.err

ptal

0xnm · 2025-10-24T07:50:40Z

                    { "Missing required context parameters: $missingParams" }
                )

+                val logWithPolicy: (String, InternalLogger.Level) -> Unit = { message, level ->


maybe it is worth to create typealias for (String, InternalLogger.Level) -> Unit

yes, agreed. helps to define the interaction.

0xnm · 2025-10-24T08:01:10Z

+    @Test
+    fun `M throw IllegalArgumentException W Builder() {blank name}`() {
+        // When/Then
+        org.junit.jupiter.api.assertThrows<IllegalArgumentException> {


this needs to go to import

0xnm · 2025-10-24T08:02:04Z

+        verify(mockLogger).log(
+            eq(InternalLogger.Level.ERROR),
+            eq(InternalLogger.Target.USER),
+            any(),


It is better to verify the message as well, otherwise it can be an error for something else. Same for the test below.

0xnm · 2025-10-24T08:04:42Z

        whenever(mockSdkCore.createSingleThreadExecutorService(any())) doReturn mockExecutorService

+        // Setup mockContext with default release build (flags = 0)
+        val applicationInfo = android.content.pm.ApplicationInfo().apply {


also needs to go to import

0xnm · 2025-10-24T08:10:20Z

+        )
+
+        // Then - uses android.util.Log.e which can't be easily verified in unit tests
+        // Just verify it doesn't crash and doesn't use internalLogger


you can also wrap logErrorWithPolicy above in assertNotThrows to explicitly highlight the intention.

0xnm · 2025-10-24T08:13:08Z

+     * @param level The log level for conditional logging (graceful policy)
+     * @param shouldCrashInStrict If true, crashes in strict policy
+     */
+    internal fun logErrorWithPolicy(


this method should be moved below, after the override methods

Update release build tests to include '[Datadog Flags]' prefix in log message assertions. The logErrorWithPolicy() method adds LOG_TAG as a prefix when logging through internalLogger, so tests need to check for the full formatted message. Fixed tests: - M log through internalLogger W logErrorWithPolicy() { release build } - M log through internalLogger W logErrorWithPolicy() { release build, gracefulModeEnabled false } All 217 tests passing.

typotter

Thanks for the review.
Since the feature branch now has the package renaming (pr #2966), I ended up rebuilding the change on a fresh tree.
ptal

typotter · 2025-10-27T18:05:09Z

                    { "Missing required context parameters: $missingParams" }
                )

+                val logWithPolicy: (String, InternalLogger.Level) -> Unit = { message, level ->


yes, agreed. helps to define the interaction.

typotter · 2025-10-27T18:22:47Z

+     * @param level The log level for conditional logging (graceful policy)
+     * @param shouldCrashInStrict If true, crashes in strict policy
+     */
+    internal fun logErrorWithPolicy(


typotter · 2025-10-27T18:23:52Z

+    @Test
+    fun `M throw IllegalArgumentException W Builder() {blank name}`() {
+        // When/Then
+        org.junit.jupiter.api.assertThrows<IllegalArgumentException> {


typotter · 2025-10-27T18:24:11Z

+        verify(mockLogger).log(
+            eq(InternalLogger.Level.ERROR),
+            eq(InternalLogger.Target.USER),
+            any(),


typotter · 2025-10-27T18:29:56Z

        whenever(mockSdkCore.createSingleThreadExecutorService(any())) doReturn mockExecutorService

+        // Setup mockContext with default release build (flags = 0)
+        val applicationInfo = android.content.pm.ApplicationInfo().apply {


typotter · 2025-10-27T18:34:11Z

+        verify(mockInternalLogger).log(
+            eq(InternalLogger.Level.ERROR),
+            eq(InternalLogger.Target.USER),
+            any(),


typotter · 2025-10-27T18:35:06Z

+    @Test
+    fun `M log through internalLogger W logErrorWithPolicy() { release build, gracefulModeEnabled false }`() {
+        // Given - release build should ignore gracefulModeEnabled setting
+        val releaseAppInfo = android.content.pm.ApplicationInfo().apply { flags = 0 }


typotter · 2025-10-27T18:37:27Z

+        )
+
+        // Then - uses android.util.Log.e which can't be easily verified in unit tests
+        // Just verify it doesn't crash and doesn't use internalLogger


0xnm · 2025-10-28T08:39:56Z

            featureSdkCore: FeatureSdkCore,
-            flagsFeature: FlagsFeature
+            flagsFeature: FlagsFeature,
+            name: String = DEFAULT_CLIENT_NAME


is default argument needed here, given it is already there for public API?

0xnm · 2025-10-28T08:40:27Z

                )
            } else {
-                // Create FlagsContext combining core SDK context with feature configuration
+                // Build a [DatadogFlagsClient] instance from its dependencies.


this comment is wrong

Comment is meant to apply to the whole block here.

0xnm · 2025-10-28T08:44:27Z


    @Test
-    fun `M log critical error W resolveBooleanValue()`(forge: Forge) {
+    fun `M log error W resolveBooleanValue()`(forge: Forge) {


although here and for other tests in this file it is not error, but warning

0xnm · 2025-10-28T08:44:52Z

-
 package com.datadog.android.flags.internal
-


seems like redundant edits

typotter · 2025-10-29T14:42:10Z

/merge

dd-devflow-routing-codex · 2025-10-29T14:42:14Z

View all feedbacks in Devflow UI.

2025-10-29 14:42:14 UTC ℹ️ Start processing command /merge

2025-10-29 14:42:19 UTC ℹ️ MergeQueue: pull request added to the queue

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

2025-10-29 15:51:10 UTC ❌ MergeQueue: The checks failed on this merge request

Tests failed on this commit 7320516:

dd-gitlab/test-pyramid:legacy-integration-instrumented-min-api

What to do next?

Investigate the failures and when ready, re-add your pull request to the queue!
If your PR checks are green, try to rebase/merge. It might be because the CI run is a bit old.
Any question, go check the FAQ.

typotter · 2025-10-29T16:27:02Z

/merge

dd-devflow-routing-codex · 2025-10-29T16:27:06Z

View all feedbacks in Devflow UI.

2025-10-29 16:27:05 UTC ℹ️ Start processing command /merge

2025-10-29 16:27:11 UTC ℹ️ MergeQueue: pull request added to the queue

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

2025-10-29 17:42:20 UTC ❌ MergeQueue: The checks failed on this merge request

Tests failed on this commit a0a6f64:

dd-gitlab/test-pyramid:legacy-integration-instrumented-latest-api

What to do next?

Investigate the failures and when ready, re-add your pull request to the queue!
If your PR checks are green, try to rebase/merge. It might be because the CI run is a bit old.
Any question, go check the FAQ.

typotter · 2025-10-29T17:55:06Z

/merge

dd-devflow-routing-codex · 2025-10-29T17:55:10Z

View all feedbacks in Devflow UI.

2025-10-29 17:55:10 UTC ℹ️ Start processing command /merge

2025-10-29 17:55:15 UTC ℹ️ MergeQueue: pull request added to the queue

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

2025-10-29 18:53:55 UTC ℹ️ MergeQueue: This merge request was merged

typotter commented Oct 22, 2025

View reviewed changes

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

typotter force-pushed the typo/FFL-1187-graceful-mode branch from 25de984 to b672f0f Compare October 22, 2025 23:10

typotter marked this pull request as ready for review October 22, 2025 23:11

typotter requested review from a team as code owners October 22, 2025 23:11

typotter requested review from jonathanmos and ncreated October 22, 2025 23:11

0xnm reviewed Oct 23, 2025

View reviewed changes

jonathanmos reviewed Oct 23, 2025

View reviewed changes

Comment thread ...ures/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/internal/FlagsFeature.kt Outdated

Comment thread features/dd-sdk-android-flags/src/main/kotlin/com/datadog/android/flags/FlagsConfiguration.kt

typotter force-pushed the typo/FFL-1187-graceful-mode branch from e0b4add to 304213a Compare October 23, 2025 14:20

typotter commented Oct 23, 2025

View reviewed changes

typotter requested review from 0xnm and jonathanmos October 23, 2025 14:26

typotter assigned 0xnm Oct 23, 2025

0xnm requested changes Oct 24, 2025

View reviewed changes

typotter added 5 commits October 24, 2025 14:52

Hard rebase - Graceful Mode

de97dd7

type alias, comments, small refactor to get()

a2b4d16

log missing config params through policy and crash in strict

12af94a

tidy code, improve tests

2e986bb

typotter force-pushed the typo/FFL-1187-graceful-mode branch from cfe311d to 2e986bb Compare October 27, 2025 18:40

typotter commented Oct 27, 2025

View reviewed changes

typotter requested a review from 0xnm October 27, 2025 21:33

0xnm previously approved these changes Oct 28, 2025

View reviewed changes

address review comments - test names, defaults, formatting

826ac08

typotter dismissed 0xnm’s stale review via 826ac08 October 29, 2025 05:09

typotter requested a review from 0xnm October 29, 2025 05:13

0xnm approved these changes Oct 29, 2025

View reviewed changes

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

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

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

dd-mergequeue Bot merged commit 6868ce4 into feature/feature-flagging Oct 29, 2025
27 checks passed

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

dd-mergequeue Bot deleted the typo/FFL-1187-graceful-mode branch October 29, 2025 18:53

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

This was referenced Nov 4, 2025

Flags Feature #2979

Closed

Flags Feature #2982

Merged

Uh oh!

Conversation

typotter commented Oct 22, 2025

What does this PR do?

Motivation

Additional Notes

Review checklist (to be filled by reviewers)

Uh oh!

Uh oh!

codecov-commenter commented Oct 22, 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!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

typotter left a comment

Choose a reason for hiding this comment

Uh oh!

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!

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!

Uh oh!

Choose a reason for hiding this comment

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

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!

codecov-commenter commented Oct 22, 2025 •

edited

Loading

typotter Oct 23, 2025 •

edited

Loading

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

edited

Loading

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

edited

Loading

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

edited

Loading