Fix definition of composable parent threshold sampler

[anuraaga]

I noticed I completely messed up the definition for composable parent based, mistakenly converting the existing parent based as is. The new parent based is a more opinionated one with only a single delegate for root spans. It's also called parent_threshold, not parent_based.

https://opentelemetry.io/docs/specs/otel/trace/sdk/#composableparentthreshold

[jack-berg]

If I understand, ComposableParentThreshold accepts a parameter root, but since it accepts only one parameter, you're shortcutting it and just saying that whatever object is set is the root.

So in the following example, always_on is the root.

tracer_provider:
  sampler:
    composite/development:
      parent_threshold:
        always_on:

If so, I think this breaks consistency and we should opt for a more verbose, yet consistent alternative:

tracer_provider:
  sampler:
    composite/development:
      parent_threshold:
        root:
          always_on:

It does looks silly. But its possible that ComposableParentThreshold adds additional parameters in the future. Also, a user familiar with the schema might go looking for the ComposableParentThreshold type in the schema and the root parameter, and be surprised not to find it since they find types for every other named sampler.

[anuraaga]

I guess this is the same as the schema compression I did for composite/development so we'd probably need to update it too to match if going with it.

https://github.com/open-telemetry/opentelemetry-configuration/blob/main/schema/tracer_provider.yaml#L84

tracer_provider:
  sampler:
    composite/development:
      sampler:
        parent_threshold:
          root:
            always_on:

Though maybe that looks a bit too weird to even consider. If we can make a decision to simplify the types for composite/development, it may give some precedence to doing it for parent_threshold as well.

I've been erring on making the configs simpler than not since it seems easier to use that way. Generally I've had a user reading docs or looking at example yaml files in mind with the schema being an implementation detail of the yaml - a user actually looking for types in the schema didn't come to mind, but I suspect it's not really a common case.

Future additional arguments is definitely a good reason, but at the same time given how stable the Sampler APIs have been so far, I expect it to be similar for the new ones, basically no changes, even additive, after being marked stable. I lean towards keeping the yaml simpler vs speculating on that.

But I understand the arguments for including root as well and just don't lean towards it - @jack-berg if you still feel it's better though then I'll make the change.

[jack-berg]

I think we've got to go with having a root property. In addition to the arguments discussed, language implementations are going to tooling that expects SDK extension plugin types to be handled in a particular way. Changing the pattern will throw a wrench in that tooling in the few places we end up with deviations.

We have prior are inSimple{LogRecord|Span}Processors have a single property named exporter:

tracer_provider:
  processors:
    - simple:
         exporter:
           console:
logger_provider:
  processors:
    - simple:
         exporter:
           console:

I guess this is the same as the schema compression I did for composite/development so we'd probably need to update it too to match if going with it.

Maybe. The distinction between ComposableSampler and CompositeSampler at the spec level is strange. Do we need both in declarative config? Can we get away with just ComposableSampler? Mabye we can:

• Introduce .tracer_provider.composable_sampler/development property, of type ExperimentalComposableSampler
• State that either .tracer_provider.composable_sampler/development OR .tracer_provider.sampler can be specified, but specifying both is an error.
• Keep ExperimentalCompositeSampler around just to be able to reference ComposableSamplers inside of traditional samplers. Or just delete it and say we don't support this for now - if you want to use new samples, set .tracer_provider.composable_sampler/development instead.

(Aside: #397 would really help with evaluation of these types of things because we could ask PRs to include a snippet so we're all on the same page about the UX.)