Skip to content

RUM-10314: Read feature context only when explicitly requested by the caller#2716

Merged
0xnm merged 1 commit into
nogorodnikov/rum-8170/feature-context-syncfrom
nogorodnikov/rum-10314/read-feature-context-only-when-requested
Jun 12, 2025
Merged

RUM-10314: Read feature context only when explicitly requested by the caller#2716
0xnm merged 1 commit into
nogorodnikov/rum-8170/feature-context-syncfrom
nogorodnikov/rum-10314/read-feature-context-only-when-requested

Conversation

@0xnm

@0xnm 0xnm commented Jun 11, 2025

Copy link
Copy Markdown
Member

What does this PR do?

We now have a possibility to update feature context from multiple threads, which imposes the need to use write/read locks.

This means that if withWithContext, withContext, etc. methods are made which provide DatadogContext in the callback, if there is any feature context update is ongoing, we need to wait for its completion (since write lock is acquired, making it impossible to read until unlocked) to put the right value.

However, caller may not need the particular feature context, so this wait may be for nothing.

This PR adds an additional argument withFeatureContext to the methods mentioned above which is used to explicitly indicate what feature contexts caller is interested in. This makes API a bit more verbose, but at the same time we avoid the possible redundant wait time.

That change won't be needed if all feature updates/reads would happen on the context processing thread, but it is not the case.

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)

@0xnm
0xnm requested review from a team as code owners June 11, 2025 09:24
Base automatically changed from nogorodnikov/rum-386/process-feature-context-on-context-thread to nogorodnikov/rum-8170/feature-context-sync June 11, 2025 09:29
@codecov-commenter

codecov-commenter commented Jun 11, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 56.29139% with 66 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.91%. Comparing base (bb9a9a8) to head (4b15242).
⚠️ Report is 126 commits behind head on nogorodnikov/rum-8170/feature-context-sync.

Files with missing lines Patch % Lines
...tadog/android/core/internal/NoOpContextProvider.kt 0.00% 38 Missing ⚠️
...g/android/log/internal/logger/DatadogLogHandler.kt 25.00% 12 Missing and 3 partials ⚠️
...in/com/datadog/android/api/feature/FeatureScope.kt 33.33% 2 Missing ⚠️
...n/com/datadog/android/core/internal/DatadogCore.kt 0.00% 2 Missing ⚠️
...og/android/core/internal/DatadogContextProvider.kt 97.56% 0 Missing and 1 partial ⚠️
...tadog/android/core/internal/NoOpInternalSdkCore.kt 0.00% 1 Missing ⚠️
...in/com/datadog/android/log/internal/LogsFeature.kt 80.00% 0 Missing and 1 partial ⚠️
.../android/rum/internal/monitor/DatadogRumMonitor.kt 94.12% 0 Missing and 1 partial ⚠️
...ndroid/telemetry/internal/TelemetryEventHandler.kt 50.00% 0 Missing and 1 partial ⚠️
...y/internal/storage/SessionReplayResourcesWriter.kt 50.00% 0 Missing and 1 partial ⚠️
... and 3 more
Additional details and impacted files
@@                              Coverage Diff                               @@
##           nogorodnikov/rum-8170/feature-context-sync    #2716      +/-   ##
==============================================================================
+ Coverage                                       69.90%   69.91%   +0.02%     
==============================================================================
  Files                                             817      819       +2     
  Lines                                           30721    30752      +31     
  Branches                                         5149     5157       +8     
==============================================================================
+ Hits                                            21473    21500      +27     
+ Misses                                           7808     7798      -10     
- Partials                                         1440     1454      +14     
Files with missing lines Coverage Δ
...kotlin/com/datadog/android/core/InternalSdkCore.kt 100.00% <100.00%> (ø)
...in/com/datadog/android/core/internal/SdkFeature.kt 91.82% <100.00%> (ø)
...g/android/core/internal/data/upload/DataFlusher.kt 100.00% <100.00%> (ø)
...id/core/internal/data/upload/DataUploadRunnable.kt 100.00% <100.00%> (ø)
...g/android/rum/internal/DatadogLateCrashReporter.kt 86.19% <100.00%> (+2.30%) ⬆️
...oid/sessionreplay/internal/SessionReplayFeature.kt 98.84% <100.00%> (+0.58%) ⬆️
.../android/trace/opentelemetry/OtelTracerProvider.kt 95.52% <100.00%> (+0.03%) ⬆️
.../kotlin/com/datadog/android/trace/AndroidTracer.kt 96.34% <100.00%> (+0.09%) ⬆️
...og/android/core/internal/DatadogContextProvider.kt 97.73% <97.56%> (ø)
...tadog/android/core/internal/NoOpInternalSdkCore.kt 8.86% <0.00%> (ø)
... and 11 more

... and 33 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
0xnm force-pushed the nogorodnikov/rum-10314/read-feature-context-only-when-requested branch from ee09fa5 to 8c53e5f Compare June 11, 2025 10:19
@0xnm
0xnm force-pushed the nogorodnikov/rum-10314/read-feature-context-only-when-requested branch from 8c53e5f to 4b15242 Compare June 12, 2025 08:48
@0xnm
0xnm requested a review from ambushwork June 12, 2025 08:48
@0xnm
0xnm merged commit 59b6870 into nogorodnikov/rum-8170/feature-context-sync Jun 12, 2025
24 checks passed
@0xnm
0xnm deleted the nogorodnikov/rum-10314/read-feature-context-only-when-requested branch June 12, 2025 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants