Guard API interfaces that should not be directly implemented by end-users

[MrAlias]

As discussed in a recent specification SIG meeting the situation where an additional method will be added to a public API. How this translates into our implementation of the specification is that a method would need to be added to an exporter interface. This type of change is not backwards compatible, if a user implemented the original interface they would fail to compile when they upgrade because they would not implement the new interface.

This is a known limitation and there are two approaches to addressing this.

1. Define a new interface with the new method, and then wherever the old interface is used, dynamically check whether the provided type is the older type or the newer type.
2. Avoid this class of problem entirely.
   • Prefer to return concrete types
   • Add an unexported method to interfaces that should not intend for users to implement.

Inventory of exported interfaces

Here are the exported interfaces that are not in internal, thrid_party, or metrics packages:

Interface	Plan to use new interface	Make a concrete type	Add private method
"otlp/exporters/otlp/otlphttp".Option			X
"otlp/exporters/otlp".ProtocolDriver	X
"otlp/exporters/stdout".Option			X
"otlp/propagation".TextMapCarrier	X
"otlp/propagation".TextMapPropagator	X
"otel".ErrorHandler	X
"otlp/bridge/opentracing/migration".DeferredContextSetupTracerExtension	X
"otlp/bridge/opentracing/migration".OverrideTracerSpanExtension	X
"otlp/oteltest".Option			X
"otlp/trace".Span	X
"otlp/trace".Tracer	X
"otlp/trace".TracerProvider	X
"otlp/trace".TracerOption			X
"otlp/trace".SpanOption			X
"otlp/trace".EventOption			X
"otlp/trace".LifeCycleOption			X
"otlp/trace".InstrumentationOption			X
"otel/sdk/resource".Option			X
"otel/sdk/resource".Detector	X
"otel/sdk/export/trace".SpanExporter	X
"otel/sdk/trace".Sampler	X
"otel/sdk/trace".ParentBasedSamplerOption			X
"otel/sdk/trace".SpanProcessor	X
"otel/sdk/trace".IDGenerator	X
"otel/sdk/trace".ReadOnlySpan			X
"otel/sdk/trace".ReadWriteSpan			X

Tasks

Add a private() method and appropriate documentation, i.e.

    // A private method to prevent users implementing the
    // interface and so future additions to it will not
    // violate compatibility.
    private()

to the following interfaces:

• "otlp/exporters/otlp/otlphttp".Option
• "otlp/exporters/stdout".Option
• "otlp/oteltest".Option
• "otlp/trace".TracerOption
• "otlp/trace".SpanOption
• "otlp/trace".EventOption
• "otlp/trace".LifeCycleOption
• "otlp/trace".InstrumentationOption
• "otel/sdk/resource".Option
• "otel/sdk/trace".ParentBasedSamplerOption
• "otel/sdk/trace".ReadOnlySpan
• "otel/sdk/trace".ReadWriteSpan

[punya]

What was the rationale for picking the approach to use for each interface?

Also, I'm happy to work on this if you aren't already planning to do it.

[MrAlias]

The way I made the decision was to prefer switching to a concrete type if possible. If not it became a question of if the interface needs to be implemented outside of the package it is defined in. If it does not and we do not intend for users to implementing the interface add a private method, otherwise we will have to address additions by adding new interfaces.

I'm open to alternatives if people have different opinions.

[MrAlias]

For the trace Span, Tracer, and TracerProvider I am a bit uncertain if we couldn't still add a private method and since we ship a NoOp implementation in the API as well assume any SDK would embed that NoOp implementation. This would ensure forwards compatibility for any SDK authors while still preventing the interface from being implemented directly.

[MrAlias]

@punya I'm happy to assign it to you. I'd want to make sure we have enough consensus on the issue and solution though before starting. @open-telemetry/go-approvers thoughts?