fix(rum): report the running SDK, not a literal, in the config request - #32
Open
Fiona2016 wants to merge 2 commits into
Open
fix(rum): report the running SDK, not a literal, in the config request#32Fiona2016 wants to merge 2 commits into
Fiona2016 wants to merge 2 commits into
Conversation
The remote sampling configuration request hardcoded `sdk=android`. A cross-platform wrapper sets `context.source` to its own name (`react-native`, `flutter`, ...) and the native SDK already stamps that value on every event it sends, so the configuration request was the one place that disagreed: a configuration rule targeting the wrapper could never match the app running it. The same request already reported the wrapper's SDK version, which made a single request describe itself as two different SDKs. `buildConfigUrl()` now takes the source and the caller passes `context.source`, which it already had in hand. A native app is unaffected — `source` falls back to "android" when no wrapper sets it. Note for rollout: an app embedding a wrapper now sends the wrapper's name, so a rule written against `sdk=android` to reach such an app stops matching and has to be retargeted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The remote sampling configuration request hardcoded
sdk=android:append("&sdk=android")buildConfigUrl()now takes the source, and the caller passescontext.source— which it already had in hand at the call site.Why
context.sourceis the field that already answers "which SDK is actually running": a cross-platform wrapper sets it to its own name, and it falls back to"android"for a native app. The native SDK already stamps that value on every RUM event it sends — the configuration request was the one place that disagreed.Two consequences of the literal:
android.Reading
sourcesettles both, and a native app is unaffected.Rollout note
This changes what an app embedding a wrapper reports. A configuration rule written against
sdk=androidin order to reach such an app stops matching once the app picks up this version and has to be retargeted. Worth checking existing rules before releasing.Tests
buildConfigUrl()tests now passsource = "android"and still expectsdk=android.M report the wrapper rather than the native SDK W buildConfigUrl()covers the wrapper case and asserts the name and version agree../gradlew :features:dd-sdk-android-rum:testReleaseUnitTest --tests "*RemoteConfigControllerTest*"— 78 tests, 0 skipped, 0 failures, 0 errors.