SamplerProvider: add a Provider for configuring Samplers

[tsloughter]

The SamplerProvider is used by the TracerProvider to get the Sampler
to use based on the SamplerProvider's configuration for choosing
a Sampler by InstrumentationScope name and/or version.

I put the SamplerProvider only in the SDK but maybe it should be defined in the API?

Changes

Adds the SamplerProvider as an interface for getting samplers in the TracerProvider.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[tsloughter]

I've encountered another use case.

I have a database client instrumented and want to give the user flexibility on if spans are started for a query. The current solution has been an option on the query function the user can set to enable/disable span creation.

The issue there is now if a user is using the library not for a project they control but to create a library they then too have to make options the user can configure to ultimately pass to the client, and so on.

The Sampler Provider wouldn't in itself be able to make per-query span samplers, at least in the database client use-case I have, since the Scope would be per-connection pool, not per-query. But it would make it possible to use Samplers specific to the database client. Then I can provide the user with configurable Samplers from the database client library to make it easy for them to setup.

Technically it is possible to currently have a sampler that can be written to not sample based on something like the query string, but that would be a Sampler run for any Span in the system. Any sampler helper from the database client library must be able to compose with other samplers. The complexity vs just being able to have a Sampler specific to the database client scope feels like enough to warrant a solution like the SamplerProvider.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[tsloughter]

Updated to support an argument to ShouldSample for those that a SamplerProvider would not be able to be done in a backwards compatible way.

[PeterF778]

This seems like duplication of functionality. Once we have this optional argument, we do not need the notion of SamplerProvider at all, as we can provide our own Sampler which will delegate the sampling decision to different samplers based on the InstrumentationScope.

[tsloughter]

It requires the user provide a sampler that does that delegation The point of the SamplerProvider is the user is able to instead configure that Tracer X use always_off or some ratio, rather than having to write a sampler themselves that does this work.

Implementations that prefer the argument to Sampler instead of a SamplerProvider could provide that delegation sampler of course and essentially implement the same resulting end user experience I suppose.

[PeterF778]

For consistency and completeness, the section on Span creation (https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#sdk-span-creation) needs a new point between steps 1 and 2 which would say that if the SDK supports SamplerProvider then it needs to query it to obtain the Sampler.

[PeterF778]

I feel there's a potential conflict between the old Sampler and the new SamplerProvider. Configurable SDKs will support setting a custom Sampler for TraceProvider, and they will also need to support setting or configuring SamplerProvider. This may lead to user confusion: which will take effect if she first configures SamplerProvider and then sets her own Sampler?

[tsloughter]

@PeterF778 yea, to be backwards compatible existing implementations will need to still accept a Sampler and create a simple SamplerProvider, that just returns that Sampler, from it.

Implementations would have to document how their sampler configuration works, esp since some will have a SamplerProvider (and must decide if it takes precedence over a configured Sampler -- I think it should) and some a Scope passed to the Sample call.

[jmacd]

I think we should leave this option out; the reason for a SamplerProvider is that we do not want this to happen, right?

[tsloughter]

The reason to add it is to keep backwards compatibility. My understanding from the discussion in the spec meeting was that some implementations will find it hard to impossible to move to a SamplerProvider and keep backwards compatibility.

[jmacd]

Can we add something to say that:

• If a single Sampler is configured and not a SamplerProvider, then the SDK MUST use a SamplerProvider that returns the singleton Sampler.
• If a Sampler is not configured, the default rule given above applies.
  The default sampler value ParentBased(root=AlwaysOn) is stated somewhere else, right? Can we consolidate the specs so that ParentBased(root=AlwaysOn) appears only here?

[tsloughter]

Sorry, I thought I replied tot his. But I think the desire for some to not add a SamplerProvider means not adding something like a MUST wrapping a single Sampler in a Provider.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.