Interface stability documentation (open-telemetry#2012)

* Interface stability documentation

* Update versioning policy

* Update CONTRIBUTING

* Document how to extend immutable interfaces

* Markdown format VERSIONING changes

Author: MrAlias

CONTRIBUTING.md

@@ -380,14 +380,77 @@ func NewDog(name string, o ...DogOption) Dog    {…}
 func NewBird(name string, o ...BirdOption) Bird {…}
 ```
 
-### Interface Type
+### Interfaces
 
 To allow other developers to better comprehend the code, it is important
 to ensure it is sufficiently documented. One simple measure that contributes
 to this aim is self-documenting by naming method parameters. Therefore,
 where appropriate, methods of every exported interface type should have
 their parameters appropriately named.
 
+#### Interface Stability
+
+All exported stable interfaces that include the following warning in their
+doumentation are allowed to be extended with additional methods.
+
+> Warning: methods may be added to this interface in minor releases.
+
+Otherwise, stable interfaces MUST NOT be modified.
+
+If new functionality is needed for an interface that cannot be changed it MUST
+be added by including an additional interface. That added interface can be a
+simple interface for the specific functionality that you want to add or it can
+be a super-set of the original interface. For example, if you wanted to a
+`Close` method to the `Exporter` interface:
+
+```go
+type Exporter interface {
+	Export()
+}
+```
+
+A new interface, `Closer`, can be added:
+
+```go
+type Closer interface {
+	Close()
+}
+```
+
+Code that is passed the `Exporter` interface can now check to see if the passed
+value also satisfies the new interface. E.g.
+
+```go
+func caller(e Exporter) {
+	/* ... */
+	if c, ok := e.(Closer); ok {
+		c.Close()
+	}
+	/* ... */
+}
+```
+
+Alternatively, a new type that is the super-set of an `Exporter` can be created.
+
+```go
+type ClosingExporter struct {
+	Exporter
+	Close()
+}
+```
+
+This new type can be used similar to the simple interface above in that a
+passed `Exporter` type can be asserted to satisfy the `ClosingExporter` type
+and the `Close` method called.
+
+This super-set approach can be useful if there is explicit behavior that needs
+to be coupled with the original type and passed as a unified type to a new
+function, but, because of this coupling, it also limits the applicability of
+the added functionality. If there exist other interfaces where this
+functionality should be added, each one will need their own super-set
+interfaces and will duplicate the pattern. For this reason, the simple targeted
+interface that defines the specific functionality should be preferred.
+
 ## Approvers and Maintainers
 
 Approvers:

VERSIONING.md

@@ -12,7 +12,14 @@ is designed so the following goals can be achieved.
   * [Semantic import
     versioning](https://github.com/golang/go/wiki/Modules#semantic-import-versioning)
     will be used.
-    * Versions will comply with [semver 2.0](https://semver.org/spec/v2.0.0.html).
+    * Versions will comply with [semver
+      2.0](https://semver.org/spec/v2.0.0.html) with the following exceptions.
+      * New methods may be added to exported API interfaces. All exported
+        interfaces that fall within this exception will include the following
+        paragraph in their public documentation.
+
+        > Warning: methods may be added to this interface in minor releases.
+
     * If a module is version `v2` or higher, the major version of the module
       must be included as a `/vN` at the end of the module paths used in
       `go.mod` files (e.g., `module go.opentelemetry.io/otel/v2`, `require

error_handler.go

@@ -16,7 +16,12 @@ package otel // import "go.opentelemetry.io/otel"
 
 // ErrorHandler handles irremediable events.
 type ErrorHandler interface {
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
 	// Handle handles any error deemed irremediable by an OpenTelemetry
 	// component.
 	Handle(error)
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
 }

exporters/otlp/otlptrace/clients.go

@@ -24,10 +24,16 @@ import (
 // transformation of data into wire format, and the transmission of that
 // data to the collector.
 type Client interface {
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
 	// Start should establish connection(s) to endpoint(s). It is
 	// called just once by the exporter, so the implementation
 	// does not need to worry about idempotence and locking.
 	Start(ctx context.Context) error
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
 	// Stop should close the connections. The function is called
 	// only once by the exporter, so the implementation does not
 	// need to worry about idempotence, but it may be called
@@ -36,8 +42,13 @@ type Client interface {
 	// synchronization point - after the function returns, the
 	// process of closing connections is assumed to be finished.
 	Stop(ctx context.Context) error
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
 	// UploadTraces should transform the passed traces to the wire
 	// format and send it to the collector. May be called
 	// concurrently.
 	UploadTraces(ctx context.Context, protoSpans []*tracepb.ResourceSpans) error
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
 }

propagation/propagation.go

@@ -21,12 +21,23 @@ import (
 
 // TextMapCarrier is the storage medium used by a TextMapPropagator.
 type TextMapCarrier interface {
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
 	// Get returns the value associated with the passed key.
 	Get(key string) string
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
 	// Set stores the key-value pair.
 	Set(key string, value string)
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
 	// Keys lists the keys stored in this carrier.
 	Keys() []string
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
 }
 
 // HeaderCarrier adapts http.Header to satisfy the TextMapCarrier interface.
@@ -54,12 +65,23 @@ func (hc HeaderCarrier) Keys() []string {
 // TextMapPropagator propagates cross-cutting concerns as key-value text
 // pairs within a carrier that travels in-band across process boundaries.
 type TextMapPropagator interface {
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
 	// Inject set cross-cutting concerns from the Context into the carrier.
 	Inject(ctx context.Context, carrier TextMapCarrier)
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
 	// Extract reads cross-cutting concerns from the carrier into a Context.
 	Extract(ctx context.Context, carrier TextMapCarrier) context.Context
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
 	// Fields returns the keys who's values are set with Inject.
 	Fields() []string
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
 }
 
 type compositeTextMapPropagator []TextMapPropagator

sdk/resource/auto.go

@@ -29,12 +29,17 @@ var (
 
 // Detector detects OpenTelemetry resource information
 type Detector interface {
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
 	// Detect returns an initialized Resource based on gathered information.
 	// If the source information to construct a Resource contains invalid
 	// values, a Resource is returned with the valid parts of the source
 	// information used for initialization along with an appropriately
 	// wrapped ErrPartialResource error.
 	Detect(ctx context.Context) (*Resource, error)
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
 }
 
 // Detect calls all input detectors sequentially and merges each result with the previous one.

sdk/trace/id_generator.go

@@ -26,8 +26,18 @@ import (
 
 // IDGenerator allows custom generators for TraceID and SpanID.
 type IDGenerator interface {
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
+	// NewIDs returns a new trace and span ID.
 	NewIDs(ctx context.Context) (trace.TraceID, trace.SpanID)
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
+	// NewSpanID returns a ID for a new span in the trace with traceID.
 	NewSpanID(ctx context.Context, traceID trace.TraceID) trace.SpanID
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
 }
 
 type randomIDGenerator struct {

sdk/trace/sampling.go

@@ -25,8 +25,19 @@ import (
 
 // Sampler decides whether a trace should be sampled and exported.
 type Sampler interface {
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
+	// ShouldSample returns a SamplingResult based on a decision made from the
+	// passed parameters.
 	ShouldSample(parameters SamplingParameters) SamplingResult
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
+	// Description returns information describing the Sampler.
 	Description() string
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
 }
 
 // SamplingParameters contains the values passed to a Sampler.

sdk/trace/span.go

@@ -34,6 +34,8 @@ import (
 // ReadOnlySpan allows reading information from the data structure underlying a
 // trace.Span. It is used in places where reading information from a span is
 // necessary but changing the span isn't necessary or allowed.
+//
+// Warning: methods may be added to this interface in minor releases.
 type ReadOnlySpan interface {
 	// Name returns the name of the span.
 	Name() string
@@ -88,6 +90,8 @@ type ReadOnlySpan interface {
 // This interface exposes the union of the methods of trace.Span (which is a
 // "write-only" span) and ReadOnlySpan. New methods for writing or reading span
 // information should be added under trace.Span or ReadOnlySpan, respectively.
+//
+// Warning: methods may be added to this interface in minor releases.
 type ReadWriteSpan interface {
 	trace.Span
 	ReadOnlySpan

sdk/trace/span_exporter.go

@@ -19,6 +19,9 @@ import "context"
 // SpanExporter handles the delivery of spans to external receivers. This is
 // the final component in the trace export pipeline.
 type SpanExporter interface {
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
 	// ExportSpans exports a batch of spans.
 	//
 	// This function is called synchronously, so there is no concurrency
@@ -31,9 +34,14 @@ type SpanExporter interface {
 	// returned by this function are considered unrecoverable and will be
 	// reported to a configured error Handler.
 	ExportSpans(ctx context.Context, spans []ReadOnlySpan) error
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
 	// Shutdown notifies the exporter of a pending halt to operations. The
 	// exporter is expected to preform any cleanup or synchronization it
 	// requires while honoring all timeouts and cancellations contained in
 	// the passed context.
 	Shutdown(ctx context.Context) error
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
 }

sdk/trace/span_processor.go

@@ -24,13 +24,20 @@ import (
 // and end of a Span's lifecycle, and are called in the order they are
 // registered.
 type SpanProcessor interface {
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
+
 	// OnStart is called when a span is started. It is called synchronously
 	// and should not block.
 	OnStart(parent context.Context, s ReadWriteSpan)
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
 
 	// OnEnd is called when span is finished. It is called synchronously and
 	// hence not block.
 	OnEnd(s ReadOnlySpan)
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
 
 	// Shutdown is called when the SDK shuts down. Any cleanup or release of
 	// resources held by the processor should be done in this call.
@@ -41,12 +48,16 @@ type SpanProcessor interface {
 	// All timeouts and cancellations contained in ctx must be honored, this
 	// should not block indefinitely.
 	Shutdown(ctx context.Context) error
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
 
 	// ForceFlush exports all ended spans to the configured Exporter that have not yet
 	// been exported.  It should only be called when absolutely necessary, such as when
 	// using a FaaS provider that may suspend the process after an invocation, but before
 	// the Processor can export the completed spans.
 	ForceFlush(ctx context.Context) error
+	// DO NOT CHANGE: any modification will not be backwards compatible and
+	// must never be done outside of a new major release.
 }
 
 type spanProcessorState struct {

trace/trace.go

@@ -337,6 +337,8 @@ func (sc SpanContext) MarshalJSON() ([]byte, error) {
 // and timed operation of a workflow that is traced. A Tracer is used to
 // create a Span and it is then up to the operation the Span represents to
 // properly end the Span when the operation itself ends.
+//
+// Warning: methods may be added to this interface in minor releases.
 type Span interface {
 	// End completes the Span. The Span is considered complete and ready to be
 	// delivered through the rest of the telemetry pipeline after this method
@@ -478,6 +480,8 @@ func (sk SpanKind) String() string {
 }
 
 // Tracer is the creator of Spans.
+//
+// Warning: methods may be added to this interface in minor releases.
 type Tracer interface {
 	// Start creates a span and a context.Context containing the newly-created span.
 	//
@@ -496,6 +500,8 @@ type Tracer interface {
 }
 
 // TracerProvider provides access to instrumentation Tracers.
+//
+// Warning: methods may be added to this interface in minor releases.
 type TracerProvider interface {
 	// Tracer creates an implementation of the Tracer interface.
 	// The instrumentationName must be the name of the library providing