Why does the SDK allow changing the TraceConfig (span limits) but not the Sampler?

[bogdandrutu]

Sampler is probably the most important config part to change while debugging issues in a not sample all environment. It is at least strange that we allow changing the TraceConfig but not the Sampler.

[anuraaga]

Sampler is an interface so it's pretty easy to provide a dynamic implementation unlike span limits, which is what zpages does. If accepting supplier for it seems useful we could, but it doesn't seem as valuable.

[bogdandrutu]

The problem with providing a dynamic implementation is that it is not guaranteed that two consecutive calls will call the same sampler, not sure if that is important but wanted to point that difference. In case of a Supplier the SDK can call supplier once and the interface multiple times and kind of have a deterministic result. Not sure if important but want to discuss this

[jkwatson]

It would be very easy to add a supplier-based interface if a future need is uncovered. I think what we have is an ok API for 1.0, and easy to extend as needed.

That being said, if there's a strong desire to change the Sampler to being configured via a Supplier before 1.0, I can live with that.

[anuraaga]

I'd lean towards leaving as is and adding if we get a strong use case - for actual dynamic samplers like a remote sampler, I think the issue of consecutive calls is already a thing anyways. Don't think I see a difference conceptually between a dynamic implementation of e.g., trace-id-ratio and a remote sampler in terms of any caveats. And we definitely need to support dynamic samplers which we have one implementation of too.

[bogdandrutu]

Ok convinced about sampler. now let's move on the TraceConfig :)

[bogdandrutu]

I am a bit confused that TraceConfig is different than all of the other and also has two ways of setting it Supplier or directly the object. Maybe we can just remove the direct call for setting the TraceConfig and leave only with Supplier than users will call setTraceConfig(()-> traceConfig) instead of setTraceConfig(traceConfig) then we ensure that is always the Supplier and no order requirements in the Builder

[anuraaga]

I don't think we can lose the ability to set the result of the traceconfig builder directly as it's much more common. #2668 is an option to achieve that but the API is a bit less clear than one method for static, one for dynamic. This is a case of balancing common case vs exceptional case.