RUM-6866 Allow typed Sampler

[xgouchet]

What does this PR do?

Preparing for a new implementation of a sampler for the TracingInterceptor, add a type to the sampler to allow adapting the sampling decision to the event being sampled.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 71.42857% with 8 lines in your changes missing coverage. Please review.

Project coverage is 70.31%. Comparing base (a7c4f31) to head (808f3d7).
Report is 5 commits behind head on develop.

Files with missing lines	Patch %	Lines
...n/com/datadog/android/okhttp/DatadogInterceptor.kt	0.00%	3 Missing ⚠️
...datadog/android/okhttp/trace/TracingInterceptor.kt	62.50%	3 Missing ⚠️
...g/android/log/internal/logger/DatadogLogHandler.kt	66.67%	0 Missing and 1 partial ⚠️
...ndroid/telemetry/internal/TelemetryEventHandler.kt	80.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #2385      +/-   ##
===========================================
+ Coverage    70.28%   70.31%   +0.02%     
===========================================
  Files          744      744              
  Lines        27733    27694      -39     
  Branches      4631     4621      -10     
===========================================
- Hits         19492    19471      -21     
+ Misses        6953     6947       -6     
+ Partials      1288     1276      -12     

Files with missing lines	Coverage Δ
.../android/core/internal/logger/SdkInternalLogger.kt	86.17% <100.00%> (-3.08%)	⬇️
.../datadog/android/core/sampling/RateBasedSampler.kt	86.96% <100.00%> (-8.70%)	⬇️
...oid/sessionreplay/internal/SessionReplayFeature.kt	98.74% <100.00%> (ø)
...id/webview/internal/log/WebViewLogEventConsumer.kt	82.43% <100.00%> (+2.70%)	⬆️
...g/android/log/internal/logger/DatadogLogHandler.kt	77.12% <66.67%> (ø)
...ndroid/telemetry/internal/TelemetryEventHandler.kt	88.02% <80.00%> (+0.76%)	⬆️
...n/com/datadog/android/okhttp/DatadogInterceptor.kt	58.86% <0.00%> (-1.82%)	⬇️
...datadog/android/okhttp/trace/TracingInterceptor.kt	77.02% <62.50%> (+0.36%)	⬆️

... and 29 files with indirect coverage changes

[0xnm]

to be reverted

[xgouchet]

👍

[0xnm]

this is potentially a binary-incompatible change, because public constructors of DatadogInterceptor and TracingInterceptor (and builder methods since recently) take Sampler instance which can be some customer-defined Sampler rather than default RateBasedSampler.

Is there a way to avoid breaking it?

[xgouchet]

I hesitated to do it, as indeed it's technically a breaking change on a public interface. That being said, looking at our code base, the interface is used in:

• the internal class TelemetryEventHandler constructor, but the sampler is built internally based on a sample rate provided in the configuration
• the internal class SessionReplayFeature constructor, but the sampler is built internally based on a sample rate provided in the configuration
• the internal class DatadogLogHandler constructor, but the sampler is built internally based on a sample rate provided in the configuration
• an internal property of the internal class WebViewLogEventConsumer, built internally based on a sample rate provided in the configuration
• in the DatadogInterceptor and TracingInterceptor deprecated constructors; they were replaced by a Builder. The builder only accepts a Sampler, which is the only place we could have ab issue for customer, which I guess is acceptable. I'll also add a builder method to set the sampling rate as I feel it's missing.

[mariusc83]

🎯 suggestion: ‏ What about a keeping the builder method that accepts the Sampler and wrapping this old Sampler that the customer is passing to your own Sampler<Unit> ? In this case it will not be a breaking change anymore. And we could deprecate that method for later.

[ambushwork]

will it make sense to make the API be like fun sample(T? = null): Boolean instead of fun sample(T): Boolean?

[0xnm]

I believe we probably don't want to allow nullable types in the interface definition. But probably even if we do so, it would be still binary incompatible change, because then customer implementing custom Sampler still need to update the method signature.

My main concern is indeed in the interceptors usage (other places are pretty much internal), but if we ultimately cannot avoid this and we need to progress on this topic, I'm fine with this minor breaking change.

[mariusc83]

🎯 suggestion: ‏ Like I suggested in my previous comment maybe we can keep also the previous method and wrap the customer old Sampler implementation in our own Sampler<Span> which delegates to the customer. Not sure but I think is doable. We can also Deprecate the old method.

[0xnm]

I believe we probably don't want to allow nullable types in the interface definition. But probably even if we do so, it would be still binary incompatible change, because then customer implementing custom Sampler still need to update the method signature.

My main concern is indeed in the interceptors usage (other places are pretty much internal), but if we ultimately cannot avoid this and we need to progress on this topic, I'm fine with this minor breaking change.