CompositeSampler configuration

[anuraaga]

There are currently Development level specs for a CompositeSampler which composes ComposableSamplers while handling trace state.

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

4 of them are the composable version of existing samplers, AlwaysOn, AlwaysOff, Probability, ParentBased. The impression I get, though I could be wrong, is the long term goal is to transparently swap the existing samplers with the composable ones - they behave similarly while additionally sending trace state so are an additive change.

Until then, I propose a global flag on sampler itself, composite: true. If set, the sampler is CompositeSampler and any configuration below uses the composable version rather than the old version.

For composite-only samplers like ComposableRuleBased, probably we could make it an error to define that without defining composite: true, alternatively we could implicitly enable composite if it's present but an error seems clearer.

[jack-berg]

Today you specify a sampler like:

tracer_provider:
  sampler: ...

We could evolve this to be something like:

tracer_provider:
  composite_sampler/development: ...

And have descriptive comments for .tracer_provider indicating that either sampler or composite_sampler/development may be set, but not both.

[Nevay]

The composite sampler could be added as tracer_provider.sampler.composite sampler type instead of adding a new tracer_provider.composite_sampler node:

tracer_provider:
  sampler:
    composite/development:
      delegate:
        parent_threshold:
          root:
            probability:
              ratio: 0.0001

Example JSON schema

{
    "$id": "https://opentelemetry.io/otelconfig/tracer_provider.json",
    "$schema": "https://json-schema.org/draft/2020-12/schema",
    "type": "object",
    "additionalProperties": false,
    "properties": {
        "processors": {},
        "limits": {},
        "sampler": {
            "$ref": "#/$defs/Sampler"
        },
        "tracer_configurator/development": {}
    },
    "$defs": {
        "Sampler": {
            "type": "object",
            "additionalProperties": {
                "type": ["object", "null"]
            },
            "minProperties": 1,
            "maxProperties": 1,
            "properties": {
                "always_off": {},
                "always_on": {},
                "jaeger_remote": {},
                "parent_based": {},
                "trace_id_ratio_based": {},
                "composite/development": {
                    "$ref": "#/$defs/ExperimentalCompositeSampler"
                }
            }
        },
        "ExperimentalCompositeSampler": {
            "type": ["object"],
            "additionalProperties": false,
            "properties": {
                "delegate": {
                    "$ref": "#/$defs/ExperimentalComposableSampler"
                }
            }
        },
        "ExperimentalComposableSampler": {
            "type": "object",
            "additionalProperties": {
                "type": ["object", "null"]
            },
            "minProperties": 1,
            "maxProperties": 1,
            "properties": {
                "always_off": {
                    "$ref": "#/$defs/ExperimentalComposableAlwaysOffSampler"
                },
                "always_on": {
                    "$ref": "#/$defs/ExperimentalComposableAlwaysOnSampler"
                },
                "probability": {
                    "$ref": "#/$defs/ExperimentalComposableProbabilitySampler"
                },
                "parent_threshold": {
                    "$ref": "#/$defs/ExperimentalComposableParentThresholdSampler"
                },
                "rule_based": {},
                "annotating": {}
            }
        },
        "ExperimentalComposableAlwaysOffSampler": {
            "type": ["object", "null"],
            "additionalProperties": false
        },
        "ExperimentalComposableAlwaysOnSampler": {
            "type": ["object", "null"],
            "additionalProperties": false
        },
        "ExperimentalComposableProbabilitySampler": {
            "type": ["object", "null"],
            "additionalProperties": false,
            "properties": {
                "ratio": {
                    "type": ["number", "null"]
                }
            }
        },
        "ExperimentalComposableParentThresholdSampler": {
            "type": ["object", "null"],
            "additionalProperties": false,
            "properties": {
                "root": {
                    "$ref": "#/$defs/ExperimentalComposableSampler"
                }
            }
        }
    }
}

This would also allow using sampler decorators like the newly proposed AlwaysRecordSampler with composable samplers:

tracer_provider:
  sampler:
    always_record:
      root:
        composite/development: ...

---

to transparently swap the existing samplers with the composable ones

I can only speak for the PHP implementation: the composite.delegate part could be made optional in the future by automatically registering an additional ComponentProvider<Sampler> for every ComponentProvider<ComposableSampler>.

Having different JSON schema types for Sampler and ComposableSampler seems preferable as not all Samplers might be implemented/implementable as ComposableSampler.

[anuraaga]

Thanks @Nevay - actually at first I was convinced by @jack-berg 's comment but then remembered why I came up with my proposal on the first place. I think it affects both ideas.

Basically

The impression I get, though I could be wrong, is the long term goal is to transparently swap the existing samplers with the composable ones - they behave similarly while additionally sending trace state so are an additive change.

I think the intent should be to add a way to switch to the composite samplers without changing the general structure - all existing configs can be "upgraded" to composite, while it also enables some new samplers. Eventually any change from this issue should be a no-op once the new samplers are the default stable samplers.

I think @jack-berg 's idea is still closer to this but a boolean still feels simpler somehow, it could of course be prefixed with experimental if needed. What do you all think?

[jack-berg]

all existing configs can be "upgraded" to composite,

Is that true? Can every Sampler be modeled as a CompositeSampler? I know the built-in options all have equivalents, but when you set .tracer_provider.sampler, you're not limited to built-in options. You can also reference custom samplers through the ComponentProvider mechanism.

@anuraaga just to clarify your proposal. I would be able to write config like:

tracer_provider:
  sampler:
    composite: true
    composable_rule_based:
       # ... omitted for brevity

And the composite boolean flag is telling the implementation whether the sampler is a composite variety or regular?

If this is correct, then this approach conflicts with how we model SDK extension plugin interfaces. Currently, every type which represents a SDK extension plugin interface has a schema which:

• Is type object
• Has minProperties = 1
• Has maxProperties = 1
• Has additionalProperties = [object, null]

See SpanExporter for example.

The single property key is a reference to the name of the SDK extension plugin interface, which may be built-in or the name of a custom implementation. The object value associated with this key is passed to the corresponding ComponentProvider as config to initialize it.

So adding a composite boolean key breaks the exactly-one property assertion, making it easier for users to right invalid schemas (i.e. one with only composite: true and no other sampler key / value pair), and slightly harder for implementations.

Not the end of the world and at the end of the day, I want us to have a schema that makes has the best UX in the long term. Maybe a boolean helps get us there.

[anuraaga]

You can also reference custom samplers through the ComponentProvider mechanism.

Thanks for the reminder, you're right. I think this means the boolean proposal doesn't work well.

So @Nevay's proposal which does model the SDK structures better (CompositeSampler is a Sampler) seems good to me. One question is whether we need the delegate: field, I think CompositeSampler is always just a wrapper around a single delegate and maybe that can be collapsed down.