open-telemetry / opentelemetry-configuration

JSON Schema definitions for OpenTelemetry file configuration
Apache License 2.0
40 stars 17 forks source link

custom samplers #102

Open brettmc opened 4 months ago

brettmc commented 4 months ago

The file-configuration schema allows for extra properties in the sampler, allowing me to define a sampler other than the ones defined in spec.

I'm in the process of implementing a rule-based sampler, based on Java's implementation. I foresee a collision in the future where Java SIG will also want to add and configure a rule-based sampler, and it probably won't have the same schema.

How can we support the configuration of custom/non-spec-defined samplers such that we can coexist with future samplers?

For reference, an example of our custom rule-based sampler configuration:

tracer_provider:
  sampler:
    rule_based:
      rule_sets:
        - rules:
            - attribute: {key: is.important, pattern: ~false~ }
          delegate:
            always_off: {}
        - rules:
            - link: { sampled: true }
          delegate:
            always_on: {}
        - rules:
            - span_name: { pattern: ~foobar~ }
          delegate:
            always_off: {}
        - rules:
            - span_kind: { kind: SERVER }
            - attribute: { key: url.path, pattern: ~^/health$~ }
          delegate:
            always_off: {}
      fallback:
        always_on: {}

The sampler runs through rules until it finds a match, and then delegates the decision to the configured sampler for that rule. If no match, fall back to a different sampler.

marcalff commented 4 months ago

In my understanding, having implemented this for opentelemetry-cpp:

For a native sampler (always_on), the schema is fixed, and the list of samplers known comes from the schema.

For a custom sampler (rule_based), there is no schema enforced. The list of custom samplers comes from what component is registered:

Is your concern that the name "rule_based" may collide later, if a native sampler of that name is added ?

I think the issue will not be with the schema, it will happen sooner, when the registration fails, because a sampler of that name already exists.

The type and name comprise a unique key. Register MUST return an error if it is called multiple times with the same type and name combination.

The spec is unclear for built-in components, since it is unclear if native (built-in) components are "pre registered" or not, but I would expect registration colliding with native names to fail.

marcalff commented 4 months ago

So the question becomes, how to share a namespace between native and custom components.

brettmc commented 4 months ago

Is your concern that the name "rule_based" may collide later, if a native sampler of that name is added ?

Not really, I think in that scenario we would manage the transition from the custom to native..

My main concern is in a setup like the otel demo app, if we wanted to use one configuration file to configure multiple services. rule_based might mean something different to Java, or might not be implemented at all.

I think that right now, a single file-based configuration could be applied to any SIG and would work. But the use of a custom sampler in one or more SIGs/services would mean we might need multiple config files. I'm not sure whether it was ever an intention that one config file could be used to configure all SIGs, but I assumed it was.

edit: I hadn't noticed the example about merging multiple configurations, and that might actually resolve my concerns above...

jack-berg commented 4 months ago

How can we support the configuration of custom/non-spec-defined samplers such that we can coexist with future samplers?

Short of informal coordination across language sigs, I don't think we can avoid collisions until someone successfully drives defining a rule based sampler at the spec level. IMO, its a obvious missing piece. I just don't have capacity to own it.

brettmc commented 4 months ago

until someone successfully drives defining a rule based sampler at the spec level...

I submitted an initial PR for a rule-based sampler, but it looks like OTEP-250 encompasses this. If that's going to get up, presumably a spec change will follow which will provide for a rule-based sampler.

I think the safest approach for me is to give my sampler a key like contrib_rule_based that won't conflict with a future rule-based sampler.