Global object for configuration

[toumorokoshi]

Right now, we are heading toward singletons that are created per API to set / retrieve singletons.

For example, tracer has it's own tracer.tracer(), with that pattern.

Having the APIs loosely coupled with an explicit composition object requires the SDK / API consumer to be aware of all singletons, and set them themselves. If one is missed, one could end up with no emission of tracers / metrics and would require them to understand the whole OTel API and part of the code to debug.

We should consider a global object that has a method that requires configuration of all required APIs. This would ensure that the consumer has a quick reference that will ensure that the SDK will operate as desired.

[Oberon00]

This should be the commit I meant https://github.com/dynatrace-oss-contrib/opentelemetry-python/blob/1d5c219232545ceb694db49c938eac294410f721/opentelemetry-api/opentelemetry/backend.py (not sure why that file is outside src; see also #15 & #29)

[toumorokoshi]

I'd like to sketch the approach here, in case people have opinions before I start writing code.

My approach at this point I think will expose the following APIs and be used as such:

# retrieves the opentelemetry global instance.
# we could also go with opentelemetry.globals() as suggested by Christian 
opentelemetry.instance() 
opentelemetry.instance().tracer  # these could be properties with appropriate setters / getters
opentelemetry.instance().propagator  # or httptextformatter
opentelemetry.instance().meter

# configures the global instance. This provides a simple way
# to see all configurable parameters and to set them.
# alternatively opentelemetry.instance().configure()
opentelemetry.configure(
    tracer=Tracer(),
    httptextformatter=TraceStateHTTPTextFormatter(),
    span_exporter=SpanExporter()
)

[Oberon00]

Is that configure a convenience SDK function? Because there is no concept of exporters on the API level.

Actually, I'd prefer something that allows step-by-step configuratio, maybe something like I suggested in #45, with finish_configuration or initialize APIs, because it would allow something like:

# Configure the global instance to use tracer, httptextformatter, ... of my.tracing.only.vendor
my.tracing.only.vendor.setup()

 # Configure the the global instance to use meter from other vendor
my.metric.vendor.setup()
# Configure any remaining instances to no-op, invoke factory functions, ...
opentelemetry.loader.finish_configuration() 

[Oberon00]

BTW does anyone remember why we originally removed the single global object/dict from #29?

EDIT: I think I found the discussion: #29 (comment) A comment by @c24t triggered that.

[carlosalberto]

If one is missed, one could end up with no emission of tracers / metrics and would require them to understand the whole OTel API and part of the code to debug.

Btw, not all vendors support metrics, and there may be a scenario where you only want metrics with a Prometheus exporter, and the default no-op tracing, for example.

[carlosalberto]

opentelemetry.instance()

I don't like the intermediate instance() call at all. We can store in any way we want the the internal/global objects, but having the user fetching this instance all the time seems like boilerplate we could avoid.

[toumorokoshi]

Regarding instance: we could remove it, and have top-level apis that look like:

# retrieves the opentelemetry global instance.
# we could also go with opentelemetry.globals() as suggested by Christian 
opentelemetry.tracer() 
opentelemetry.propagator()
opentelemetry.meter()

It wouldn't be that hard to discover, just need proper documentation on top-level modules. There's a good argument too that if we show singleton references in the top level module, it's a quick reference to see what is and isn't configurable. @carlosalberto would that work?

[toumorokoshi]

Regarding configure: I think no matter what we do, vendors can always provide a simple aggregate function to configure OpenTelemetry however they like.

I don't think #45 addresses my concern of making it clear what is or isn't configurable, or should be configured, although I see how it will help ensure configuration is deferred to first access so that people have a chance to configure globals before they're accessed.

I think I'll drop the configure() method in my proposal if it's controversial. This can also be accomplished with proper documentation and examples, which I think the README and example-app can provide.

[toumorokoshi]

@Oberon00 I guess if this goes through preferences on the right solution to #45? I'd probably vote for a setter, with an implicit loader that matches the behavior of the current loader by default.

[carlosalberto]

Slightly related to #144

[Oberon00]

@toumorokoshi The problem with a setter is that everyone will use it to set the SDK and then if you want to override that you need to change the source code to call another setter or monkeypatching it.

[toumorokoshi]

Sorry @Oberon00, I'm not clear on what you mean by "change the source code to call another setter". Do you have an example to clarify?

[Oberon00]

Say I'm a PaaS vendor and have my own AwesomeSDK that implements the OTel API. Now I have a customer that deploys an application that uses OpenTelemetry. In the startup code, the application uses opentelemetry.setTracer(opentelemetry.sdk.trace.Tracer()) to enable tracing. However, to see the traces in AwesomeUI, the AwesomeSDK must be used. With the setter, probably the solution would be to change the application's source code to call the setter with another AwesomeSDK when running in the PaaS, like:

if os.getenv('AWESOME_PAAS_ENV'):
  awesomeutil.setup() # Calls setTracer
else:
  opentelemetry.setTracer(opentelemetry.sdk.trace.Tracer()) # Or sdk.setup()

With the current loader, the PaaS vendor can just set the OPENTELEMETRY_PYTHON_IMPLEMENTATION_TRACER environment variable and the set_preferred_tracer_implementation(lambda _: opentelemetry.sdk.trace.Tracer()) call will be overriden without even constructing a SDK tracer.
In theory, we could define the setters to be no-ops in case such an environment variable is set, but that would be very weird and unexpected IMHO.

[Oberon00]

Related #144 .

[toumorokoshi]

@ocelotl has a WIP prototype he'll be sending out in a PR over the next week.