Extend exporter interface to include initializer (__init__) or introduce a new exporter factory interface

[owais]

Recently we merged support for 100% automatic setup of tracing pipeline and instrumentations in #1036

This let's users specify the span exporter to use as a CLI arg or an environment variable. However, the code that sets up the pipeline needs to have specialized knowledge about exporters in order to initialize them properly.

For example, we have code that conditionally passes the service name as an argument to exporter's initializer method depending on the exporter name here: https://github.com/open-telemetry/opentelemetry-python/blob/master/opentelemetry-instrumentation/src/opentelemetry/instrumentation/auto_instrumentation/components.py#L77-L81

Most exporters in our repo other than OTLP and DataDog have the similar initializer method signature but this is not by design. As a result, the Datadog exporter is not even supported by the instrument command today as it's signature is considerably different from the rest. While we do support plug-able exporters via the entry points system, 3rd party exporter authors must make sure their exporter's init method signature is the same as other exporters in our repo (excluding OTLP and DD).

I propose we formalize this by including the __init__ method in SDK's SpanExporter interface. This should allow us to remove all special knowledge from the instrument command and clarify the requirements to implement a compatible exporter. As far as the instrument command is concerned all it needs is for all exporters to accept a service_name argument. OTLP does not accept it as it is recommended to provide the service name as a resource attribute instead. DataDog accepts a service argument which does the same thing as service_name. Requiring all exporters to accept a service_name keyword argument would solve the problem for instrument command but perhaps we can go a step further and try to unify other common arguments as well.

We could define an ExporterOptions interface/object that must be passed into an exporter. It could look something like:

class ExporterOptions:
   service_name: str
   endpoint: str # otlp and cencus' endpoint, zipkin's url, jaeger agent_host_name,agent_port,collector_endpoint, DDs agent_url

We can later add more common settings like TLS, common gRPC/HTTP config, auth config, headers, etc. Individual exporters can subclass ExporterOptions and add exporter specific config. For example,

class ZipkinExporterOptions(ExporterOptions):
    transport_format: Union[TRANSPORT_FORMAT_JSON, TRANSPORT_FORMAT_PROTOBUF, None]

# usage
zipkin_exporter = ZipkinSpanExporter(ZipkinExporterOptions(service_name="", endpoint="", transport_format=TRANSPORT_FORMAT_JSON)

Exporters that don't need the service_name or any other common attributes can ignore those fields.

[lzchen]

Having to import and then instantiate another class is another point of contention for the customer. It will also break a lot of customer behaviour that they are used to with exporters for Azure that used Opencensus before. I also don't think we want to enforce what the signature for vendor exporters to look like just to get a feature (that is not defined in the specs) to work.

For Azure, we are utilizing **kwargs in the constructor of the exporters, so users can pass in whatever they want. I think we should let the user pass in whatever they want without assuming anything about the signature of the exporters, and for us to attempt to construct the exporter with these parameters, and if it fails on initialize then it is simply a configuration error.

Regardless, I think the original behaviour of auto-instrumentation was meant to initialize everything with the default behaviour (instrumentations initialized with default tracer_provider, no span_callback or name_callback). Now that we are configuring exporters, we must have the ability to add configuration to auto-instrumentation and it seems the command line is getting cluttered. Perhaps we should utilize a config file based initialization instead?

[owais]

That makes sense as well. We do support some level of configuration using environment variables today for almost all exporters. That's how users are supposed to configure export endpoints today for example. Adding support for OTEL_SERVICE_NAME environment var would be good enough to resolve this specific instance of the issue. Perhaps we should wait till the spec clarifies how to set service name: open-telemetry/opentelemetry-specification#709

[lzchen]

Yeah, we will probably have a specific SIG dedicated to just talking about what we will be providing for auto-instrumentation or configuration.

[github-actions]

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