API: Sampling Decision may need to update tracestate, not only attributes

[SergeyKanzhelev]

No description provided.

[tedsuo]

@SergeyKanzhelev can you show an example of the API you would like, or the requirements it needs . to have?

Do we want the sampler to modify TraceState directly? Or return it?

(Personally, I think this should be done under the hood, at least if there is only "one right way" to do this.)

[SergeyKanzhelev]

Typical example of such need is when sampler need to read and set if missing some tracestate key/value pair that indicates details of sampling decision. Like a sampling rate that was used by this sampler at this point of time.

Currently the decision sampler returns contain a set of attributes associated with the decision. However, those attributes would not be propagated down the downstream services.

  Decision shouldSample(
      @Nullable SpanContext parentContext,
      @Nullable Boolean hasRemoteParent,
      TraceId traceId,
      SpanId spanId,
      String name,
      List<Span> parentLinks);

  interface Decision {
    /** Return sampling decision whether span should be sampled or not. */
    boolean isSampled();

    /** Return tags which will be attached to the span.
     * @return attributes added to span. These attributes should be added to the span only for root
     *     span or when sampling decision {@link #isSampled()} changes from false to true. */
    Map<String, AttributeValue> attributes();
  }

Perhaps Decisions may be extended to also return a new Tracestate class. That tracestate is easy to construct as incoming Tracestate can be used in the TracestateBuilder. And if not null - it should replace Tracestate in SpanContext.

  interface Decision {
    /** Return sampling decision whether span should be sampled or not. */
    boolean isSampled();

    /** Return tags which will be attached to the span.
     * @return attributes added to span. These attributes should be added to the span only for root
     *     span or when sampling decision {@link #isSampled()} changes from false to true. */
    Map<String, AttributeValue> attributes();

    /** If not null - should be used to overwrite the Tracestate in the SpanContext. **/
    Tracestate tracestate()
  }

[SergeyKanzhelev]

@bogdandrutu Any opinions on this?

I also want to add that the approach of moving the Tracestate manipulation to separate callback (which is also a common request) has a problem that now sampler that needs Tracestate manipulation will need to exchange information with that callback. And there actually no means to carry any information from one sampler to that callback except, maybe, attributes of a span (and a intermittent call to toSpanData to get that attribute value). Which will complicate a lot of logic

[jmacd]

Background question: why should the sampling decision output attributes?
In looking at the prototype Java SDK, I see that there are TraceOptions which include the "is_sampled" bit. I feel the sampling rate belongs there. are TraceOptions serialized into Tracestate?

[SergeyKanzhelev]

examples of attribute may be algo used to make a decision and sampling priority. Sampling rate is not a common-enough concept to make it strongly typed or include into TraceOptions. However I believe it's a common scenario to put it into Tracestate.

TraceOptions are serialized into trace-flags: https://w3c.github.io/trace-context/#trace-flags

[jmacd]

I have a terminology question. I was under the impression that "Attribute" referred to properties of a span, and that "Tags" referred to contextual properties that would propagate along with trace context. In this thread, I believe we're talking about context tags which a sampling algorithm would output for propagation.

I feel somehow that context tags should be reserved for applications to use, not for tracing systems to propagate internal state. I see the sense in allowing some vendor-specific state in Tracestate, but I worry that we're imagining something generally useful, where all we really need is one floating-point number alongside "is_sampled" to propagate a sampling rate, for systems that use fixed-probability sampling. Sampling algorithms may get quite complex, but I feel that for the most part sampling decisions should be recorded along with span data, not propagated via context.

That said, there are plenty of reasons to allow tracing systems to make decisions about propagating attributes from one node to another to facilitate distributed trace joins (e.g., see Pivot Tracing). In that sense, we should disentangle these things:

1. tracing systems sometimes want to know sampling rates downstream
2. tracing systems sometimes want to inject tags for themselves to use downstream in sampling decisions.

I'd suggest that an optional sampling-rate field in trace-flags or Tracestate will address (1), and an independent-of-sampling mechanism to allow tracing implementations to decide on the contents of a correlation context will address (2). A tracing system could want (2) even when it's not sampling.

[SergeyKanzhelev]

this issue is specifically about sampler and how sampler will propagate it's decision and extra arguments to downstream components. Correlation Context will be reserved for apps, so Tracestate is a native place for it.

[jmacd]

As I understand it, Tracestate contains Entries of key:value attributes that permit the sampling implementation to propagate its decision and extra arguments to downstream components. Your proposal above allows the sampling implementation to the span ("Return tags which will be attached to the span."). That's the part I'm confused about, since I believe Span attributes should be reserved for apps (like Correlation context).

[jmacd]

In other words, I now see that the sampling decision should have a chance to modify Tracestate. But if the sampling decision intends to record any key-value data, I think it should do so in the Tracestate entries.

[rochdev]

Just wanted to mention that we have a constraint with client sampling to not do allocations when a trace will be dropped. The way we do it right now is that we take the decision as early as possible (before the root span is even created) and then use a no-op span for any span that is a part of the that trace. The no-op span is a singleton that is reused for all dropped traces. Then propagation is done from the no-op span which will always propagate a decision to drop.

I'm not familiar with TraceState so hopefully I'm not deviating from the scope of this issue.

[lizthegrey]

#180 is now salient, so cross-linking it.

I'm okay with using Tracestate rather than TraceOptions but I feel uncomfortable with the status quo of not propagating the info at all right now.

[jmacd]

Closing this as it is discussed under open-telemetry/oteps#79

[Oberon00]

@jmacd To me it looks like the only relation of this issue to open-telemetry/oteps#79 is that both are in the area of sampling...

Closing this issue probably still makes sense because it seems to be stale. @lizthegrey what do you think? I think this was related to https://www.honeycomb.io/blog/dynamic-sampling-by-example/