Skip to content

feat(rum): let the console set the session sample rate - #30

Open
Fiona2016 wants to merge 19 commits into
publishfrom
feat/remote-sampling-configuration
Open

feat(rum): let the console set the session sample rate#30
Fiona2016 wants to merge 19 commits into
publishfrom
feat/remote-sampling-configuration

Conversation

@Fiona2016

Copy link
Copy Markdown
Collaborator

Lets an application owner change how much traffic RUM keeps without the customer
shipping a new release.

Off by default: without setRemoteConfigurationEnabled(true) the SDK makes no
extra request and behaves exactly as before.

How it works

  • Fetching follows the rhythm of the sessions that read it: once at start-up and
    once whenever a new session begins. There is no timer between sessions — a
    change can only matter at the next draw.
  • Values persisted by a previous launch apply before the network answers, so the
    first sessions after a cold start already draw with what the console last
    provided rather than with the values the app was built with.
  • A failed, timed-out or unreadable response leaves the stored values exactly as
    they were, and is retried twice (5s then 60s, jittered) before waiting for the
    next natural trigger.
  • The stored configuration is revalidated with If-None-Match, so an unchanged
    answer costs a 304 rather than a body.
  • An operator can additionally allow a refresh when the app returns to the
    foreground, gated by staleness. It is off unless turned on for the application:
    session starts spread requests across the day, while returning to the
    foreground bunches them at the moment everyone opens the app.
  • Each session records the rate it was actually drawn under and the configuration
    version it came from; its events report both, so server-side extrapolation
    lines up with the draw that kept the session.
  • RumMonitor.getRemoteConfig() returns the console's custom values, decoded.
    Nested objects and arrays come back as Map and List; a body that cannot be
    read answers as nothing published, since no rate or decision depends on it.
  • RumMonitor.setForcedSession() collects the current visitor regardless of the
    rates, for a support or debugging flow.

Only the session sample rate is delivered. Session Replay and distributed tracing
are not configured from here on this platform, so the replay sample rate stays
where the app configures it and Session Replay draws with exactly the sampler the
app was built with.

Verification

  • :features:dd-sdk-android-rum:testDebugUnitTest green
  • API surface regenerated; the only public change is getRemoteConfig() and
    setForcedSession() on RumMonitor plus the builder switch

Both sampling rates were fixed when the app called `RUM.enable()`, so
changing either one meant shipping a new release and waiting for users to
update. That is days or weeks at exactly the moments the knob is worth
having: an incident, a launch, a bill that jumped overnight.

With `setRemoteConfigurationEnabled(true)` the SDK takes the session and
session replay sample rates from the application's settings instead,
polling `/api/v2/rum/config` for them. Left off — the default — nothing is
requested and the SDK behaves exactly as before.

The rates are read where a session's fate is decided, in `renewSession`, so
a change never disturbs a session already under way: it applies from the
next one. The server can also ask for immediate activation, in which case
the session is restarted as soon as rates that actually change this client
arrive, so a new one starts under them. Restarting rather than flipping the
running session in place keeps every session a complete record of itself.

The replay rate travels to Session Replay on the message RUM already sends
it when a session is renewed, so one request drives both decisions and
there is no second store to keep in step.

Failure is always "keep collecting with what you have": nothing here can
delay initialisation, an error or timeout leaves the stored rates
untouched, and a rate the server does not send stays with the value passed
at init — a rate is never invented, least of all a zero, which would switch
off collection nobody asked to switch off.

Events keep reporting the rate their session was really drawn at rather
than the one the app was built with, so the configured sample rate on an
event stays true.
An app spends most of its life in the background, where the poll timer
cannot be trusted: the system may not run it for hours. Someone could
reopen the app and carry on under settings that were changed while it was
away.

Returning to the foreground is now its own reason to ask, subject to the
same ttl, so switching between apps does not turn into a request each
time. Rotations and activity-to-activity navigation keep the started count
above zero, so neither is mistaken for a return.

Deliberately not a method the app has to call: the apps that would never
get fresh settings are exactly the ones that never read far enough to find
such a method.

The ttl the server asked for is now remembered when the response is read
rather than around the request, so a fetch that fails keeps it instead of
falling back to ours.
The console had no honest way to tell whether a saved change had reached
anyone. Events cannot answer it: an event only exists for a session that
was kept, so at a low sample rate they describe the sampled few, and the
size of that blind spot is set by the very rate being changed.

The version each response carried is now stored alongside the rates and
sent back on the next request — the one request every client makes,
whether or not its session was kept. It is kept even when the response
carried no rates, which is what 'remote configuration is off, use your own
settings' looks like, so the console can still see the app is up to date
with the change that turned them off.
Asking again when the app returned to the foreground was unconditional.
The poll spreads requests across the ttl; returning to the foreground does
the opposite, bunching them at the moment everyone opens the app — the same
shape as a release herd, and the ttl throttle bounds the rate rather than
the shape.

It now happens only when the configuration says so, which is off by
default.
setForcedSession() on RumMonitor is the escape hatch for "collect this
user now": the application knows who needs debugging (its own
allow-list, a support flow), the SDK only provides the switch. The
session restarts so the forced draw applies from a clean session - RUM
cannot flip the replay decision of one already under way - and the
renewal message tells Session Replay to skip its own draw, so a forced
session always comes out with replay. Calling again while the forced
session runs is a no-op, and the forced state lasts for the process
lifetime, so the application decides on each app start whether to call
again.
The console can publish a small bag of application-defined JSON values
alongside the sampling settings; the SDK persists it with them and
hands it to the host application verbatim through
RumMonitor.getRemoteConfig(), as the raw JSON object string, never
interpreting it. What a value means is entirely up to the application's
own code - a debug allow-list to pair with setForcedSession(), a
feature toggle. The bag is persisted like the rates, so what one launch
fetched answers immediately on the next; when the kill switch turns
remote configuration off, the bag goes with it. A custom-only change
never restarts a session - immediate activation keeps comparing rates
alone.
…tion

The channel no longer carries only sampling rates - the console's
contract grew a trace sample rate and a replay privacy level - so
everything internal that called it "sampling" takes the broader name:
the controller, the store and its preferences file, and every symbol
wired through the feature, the monitor and the scopes. The public init
option is untouched (setRemoteConfigurationEnabled already said it),
as is the endpoint and every behaviour; this commit only moves names.
The ttl poll timer is gone. A fetch now happens at start-up and after
every session draw - the one rhythm a change can matter on, since the
draw for the running session has already happened and the response
lands in storage for the next one, which is exactly the next-session
semantics the console promises. The server's ttl field stays, but only
bounds staleness for the (server-gated) foreground refresh; a polling
mode may come back later and the protocol field is reserved for it.

A failed fetch is retried quickly (5s), then patiently (60s), then not
at all until the next natural trigger: two extra requests per outage
per client, so a fleet can never turn an endpoint incident into a
storm. Each delay is spread by +/-20% so recovering clients do not all
return at the same moment. A new trigger cancels a waiting retry and
re-arms the backoff, and a failure never clears the stored values.

Wiring note: sessions created by startNewSession (after the first) now
also receive the remote configuration store, which the earlier wiring
had only given to the very first session scope.
The server answers a conditional request with 304 when nothing
changed, so the SDK now stores the ETag beside the configuration it
validated and echoes it back as If-None-Match. The validator belongs
to that stored answer specifically - the body varies per caller
context - so it lives in the same store and is kept even by the kill
switch, whose answer is what the next revalidation stands on. A 304
counts as a success: nothing to apply, no retry owed, and the
staleness bookkeeping moves on.

The store key now covers everything that can change the answer: the
storage format version (a prefix, bumped on format change rather than
on SDK upgrade), the endpoint host, the RUM application id, the
service, the environment and the app version. It deliberately still
leaves out the SDK version, which would throw the cache away on every
upgrade. The SDK version goes on the request instead, as sdk_version,
for the server's future targeting.

Store persistence gains its first unit tests (round-trip across
instances, omitted knobs forgotten, kill switch keeps the version,
storage unavailable falls back to init) via an in-memory
SharedPreferences.
…events

Events used to carry the init sampling rate even when the console's
settings decided the draw, skewing server-side extrapolation. Each
session draw now records a DrawnConfiguration - the rates it actually
used (the console's where it set them, the init values where it did
not) and the remote settings version they came from - married to the
session id and kept in storage next to the settings cache, so a stale
record is inert rather than wrong. View events report the drawn rates
in _dd.configuration, now also populating the existing
session_replay_sample_rate field, and carry rc_version naming the
settings version so an audit can recover the exact configuration from
the version history (0 when none was ever fetched; the field is a
FlashCat addition to the view schema - our intake reads it, others
ignore it).

The drawn replay rate falls back to what Session Replay publishes
about its own configuration, since the console-side rate lives on the
RUM feature. Sessions drawn without remote configuration report
nothing new - for them the init values are the drawn values.

Also covers the previously untested remote read at session renewal and
the replay rate riding the session-renewed bus message.
The field landed with the event change; the generated surface files did not go
with it, so the api-surface check would have failed on the next run for a change
that was already made.

Only the RUM surface. The session-replay-noop surface is also stale in the tree,
but it was stale before this branch and its drift is upstream Session Replay
API, not ours to carry in here.
Session Replay is not supported on native yet, so a replay rate the console
could set had nothing to act on here. Only the session sample rate is
delivered; the replay rate stays where the app configures it.

It is removed from the stored values, from the draw record events carry, and
from the bus message RUM sends Session Replay on renewal, so Session Replay
draws with exactly the sampler the app was built with. The forced-session flag
on that message is unaffected.
Web and iOS hand the application a dictionary; returning the raw JSON string
here made the same console value cost an extra parser on Android alone. Nested
objects and arrays come back as Map and List, and a body that cannot be read
answers as nothing published — no rate or decision depends on this bag.

Storage keeps the raw JSON, which has no reason to understand it.
`RemoteConfigValues.isEmpty()` had no caller anywhere: it meant something while
several knobs were delivered, and says nothing now that only the session sample
rate is. `readRate` took the field name from its single call site, and the retry
schedule was visible outside the file that is its only reader.
Reading a configuration response had two ways to go wrong, and neither
was handled.

A body that is not JSON threw out of the fetch. The parse ran inside a
try that only catches IOException and IllegalStateException, so a
JSONException escaped the whole method: the in-flight flag was never
cleared, every later trigger returned early, and remote configuration
stopped for the lifetime of the process with no log and no retry. A
captive portal answering 200 with a login page is enough to cause it,
and nothing checks the content type.

A body written to a newer contract was read field by field and applied.
The server states the shape it wrote in `schema_version`; a reader that
guesses instead of checking is exactly what that field exists to
prevent, and only code already on the device can refuse - a check added
in a later SDK would be ignored by the very clients it needs to protect.

`apply()` now reports one of three outcomes instead of throwing:

  APPLIED             the body was read and its values are stored
  UNREADABLE          not a configuration at all - ask again
  UNSUPPORTED_SCHEMA  a contract this SDK does not read - refused whole

Only UNREADABLE is retried. A schema we do not know is an answer, not a
failure: asking again would fetch the same refusal, so a server-side
schema bump cannot turn a fleet into a retry storm. Nothing from a
refused body reaches storage, not even the fields that happened to
parse, and the values already in use keep applying either way.
`setBeforeSampling` is consulted synchronously every time a new session
is about to be drawn, with the rate that would apply and the console's
custom values. Return a rate to override it, or null to leave it alone.
It runs after the console's rate on purpose: an allow-list is only
useful if it can keep collecting a visitor the console's rate would
drop.

Anything unusable - a null, a rate outside 0..100, a throw - leaves the
incoming rate alone. A mistake in the host application must never take a
customer's collection down with it.

Two behaviours are corrected to match the iOS and HarmonyOS SDKs, so one
console setting means one thing everywhere:

- setForcedSession() no longer restarts a session that is already being
  collected. RUM cannot retro-collect what a running session already
  dropped, so cutting it in two gained nothing; only an uncollected
  session is now replaced.
- rc_version is omitted rather than sent as 0 before the first
  configuration arrives, which is the shape the other platforms send.

Imports touched by the remote-configuration work are also sorted to the
layout .editorconfig declares.
The configuration a session was drawn under is held in memory and travels to
the view scopes that report it, which is all it is for. It was also written to
shared preferences on every session renewal, and nothing ever read it back: a
session does not survive the process here, so there is nothing for a stored
record to be restored into. What is left is a disk write per renewal and a JSON
codec kept alive to serve it.

The record itself, and everything that reports from it, is unchanged.
A response with no schema stamp at all was refused as a shape this SDK cannot
read, because the absent-value sentinel was compared against the supported
version like any other number. A body without a stamp is, by construction, the
shape that existed before the stamp did — which is the shape this reader was
written against. Refusing it switches remote configuration silently off against
a server that merely predates the field, and nothing says so: the refusal takes
the same path as a body we genuinely cannot read, so there is no error to
notice.

Only a stamp that is present and unrecognised is a refusal now, which is what
the web SDK already did. The two no longer disagree about the same response.
Removing the record's persistence left two of its three fields with no reader
at all: the rate the draw used travels down the scope chain as sampleRate and
is what every event already reports, and the session id was only ever the
validity check for the storage that is gone. Keeping them would be two records
of one fact, and one record of nothing.

The stamp check is also made strict. optInt would quietly turn the string "1"
into 1 and accept a body that iOS and HarmonyOS refuse, and a field whose whole
purpose is that every reader agrees about the same response cannot be the one
place they disagree. A stamp that is present but not a number is refused; an
explicit null reads as no stamp at all, which is what the other two do.
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.

1 participant