trace sdk: add samplers

[iRevive]

Reference	Link
Spec	https://opentelemetry.io/docs/specs/otel/trace/sdk/#sampler
Java implementation	Sampler.java ParentBasedSampler.java TraceIdRatioBasedSampler.java SamplingResult.java

---

I started decoupling #325 into smaller changes to simplify the review process.

[iRevive]

Should we use Either instead?

def create(ratio: Double): Either[IllegalArgumentException, Sampler] =

[NthPortal]

I opine that we should not—it makes the API much more annoying to use if you'r using it correctly. at some point, the user does just need to read the docs

[iRevive]

A temporary solution. It can be removed once the SpanContext is updated.

got tricked by a link into only reviewing a single commit

[iRevive]

Java SDK uses a default method in the interface: https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/samplers/SamplingResult.java#L134

Unfortunately, this approach does not work if we want to keep the trait sealed.

I've added TraceStateUpdater instead.

[armanbilge]

Unfortunately, this approach does not work if we want to keep the trait sealed.

Dumb question: why? I'm having trouble seeing the difference.

[iRevive]

According to the spec, the SamplingResult could be defined as:

sealed trait SamplingResult {
  def decision: SamplingDecision
  def attributes: Attributes
  def updatedTraceState: TraceState
}

But with this design, we cannot use constant sampling results (e.g. SamplingResult.Drop) anymore, because we always need to return the parent's trace state. So, we need to create a new sampling result on every call of Sampler.shouldSample.

Since only a few samplers in the contrib module (e.g. ConsistentSampler) provide a custom implementation of the getUpdatedTraceState Java SDK team decided to go their way, I assume.

---

We can still use a method, but encoding will be cumbersome:

sealed trait SamplingResult {
  ...
  def updateTraceState(state: TraceState): TraceState
}

def apply(
    decision: SamplingDecision,
    attributes: Attributes,
    updateState: TraceState => TraceState
): SamplingResult = ...

When we use a function instead of a TraceStateUpdater ADT, it's more complex to:

1. Implement a proper hashing
2. Informative toString
3. Could be less intuitive

[iRevive]

I've updated the PR (merged changes from the main, primarily). May I ask for yet another review, please?
@armanbilge @NthPortal

[NthPortal]

is this correct? ratio == 0.5 should yield 0 not Long.MaxValue/2, shouldn't it?

[NthPortal]

I'm struggling very hard to figure out how to do the maths without resorting to BigDecimal

[iRevive]

The input ratio must be between 0 and 1.0. 0.1 means we sample only 10% of the incoming spans, and 1.0 means 100%.

The ratio-based sampler should be deterministic, so under the hood, we extract a long value from the trace id: https://github.com/typelevel/otel4s/pull/355/files#diff-fc234a9eef7868569981b776843221255b54d7835308e46da41ab99eb4bc315eR48.

The logic is the following: the span is marked as 'sampled' if the extracted long value is lower than the upper bound limit (ratio * Long.MaxValue).

[NthPortal]

let me try to clarify.

the Long value is used to represent a sequence of bits and is in that sense "unsigned", but when doing maths on it, it's signed. so it can have any value from Long.MinValue to Long.MaxValue. even if the input ratio is 0.0, the current implementation will still sample any values in Long.MinValue until 0L, which is 50% not 0%

[iRevive]

The math.abs is used here, so the distribution of values from 0 to Long.MaxValue.

https://github.com/typelevel/otel4s/pull/355/files#diff-fc234a9eef7868569981b776843221255b54d7835308e46da41ab99eb4bc315eR54

[NthPortal]

ah, I see. oops

[NthPortal]

edge case: math.abs(Long.MinValue) == Long.MinValue

[iRevive]

Valid point. If I understand correctly, this rare case shouldn't affect the sampling ratio much.

[NthPortal]

except it will violate the expectations of a ratio of 0.0

[iRevive]

hm, I didn't think about it that way. I made an issue for myself #404