search

open-telemetry / opentelemetry-specification

Specifications for OpenTelemetry
https://opentelemetry.io
Apache License 2.0
3.76k stars 890 forks source link

Interface stability for implementers and new methods in the spec #1457

Open Oberon00 opened 3 years ago

Oberon00 commented 3 years ago

We need to define our policy for adding new methods to existing interfaces in the specification, especially considering APIs that are intended to be implemented by users, namely "plugin interfaces" (Exporter, Sampler) and API implementation interfaces (Span, TracerProvider, ...). It is clear and already specified that at least for plugin interfaces we must stay ABI-compabible. However, there are several ways to achieve this (other than just never specifying new methods) in different languages (Java default interface implementations, C++ non-pure virtual methods, deriving a new interface in most languages come to mind).

Recently there were some discussions on adding ForceFlush to the SdkTracerProvider class (#1451, #1452) and the Exporter interface (#1454). While it was agreed that the former is unproblematic (SdkTracerProvider being a class), the Exporter is what our versioning spec calls a plugin interface:

SDK Stability

Public portions of SDK packages MUST remain backwards compatible. There are two categories of public features: plugin interfaces and constructors. Examples of plugins include the SpanProcessor, Exporter, and Sampler interfaces. Examples of constructors include configuration objects, environment variables, and SDK builders.

Languages which ship binary artifacts SHOULD offer ABI compatibility for SDK packages.

(I think this is more or less what @tigrannajaryan stated in https://github.com/open-telemetry/opentelemetry-specification/pull/1452#issuecomment-783730643)

In #1454 I remarked about adding ForceFlush to the existing Exporter interface:

In some languages, adding a new method to an interface might be a breaking change for existing implementers. However, there are simple workarounds for that such as adding Flush to an new interface like FlushableExporter extends Exporter. I think it is OK to leave this implementation detail to the language SIGs.

and @bogdandrutu remarked in response:

I think we need to add some text about this, languages should choose what is best for them that allows to extent the API (java8 has default implementation, go can rely o duci typing, etc.) that will allow them to extend the interfaces. And we should not consider breaking change adding a new functionality on an interface. Will file a separate issue.

There however the question if old APIs need to be supported by (API-compatible with) newer SDKs. I think the only thing the spec says about this is the following:

Long Term Support

API support

Major versions of the API MUST be supported for a minimum of three years after the release of the next major API version. API support is defined as follows.

  • ...
  • A version of the SDK which supports the latest minor version of the last major version of the API will continue to be maintained during LTS. Bug and security fixes MUST be backported. Additional feature development is NOT RECOMMENDED.
  • ...
tigrannajaryan commented 3 years ago

IMO the policy should clearly differentiate:

  1. API interfaces which are callable by the end user but are not intended to be implemented by the end user.
  2. SDK interfaces which are callable by the end user but are not intended to be implemented by the end user.
  3. Interfaces which define the contract between API and SDK packages and are intended to be called by API and implemented by SDK, not intended to be called or implemented by the end user.
  4. Interfaces called by SDK which are intended to be implemented by the end user.
Oberon00 commented 3 years ago
  1. API interfaces which are callable by the end user but are not intended to be implemented by the end user.

Which would be all API interfaces defined in the spec, I think?

  1. SDK interfaces which are callable by the end user but are not intended to be implemented by the end user.

I think that is what the spec calls "Constructors" at https://github.com/open-telemetry/opentelemetry-specification/blob/v1.0.1/specification/versioning-and-stability.md#sdk-stability

  1. Interfaces which define the contract between API and SDK packages and are intended to be called by API and implemented by SDK, not intended to be called or implemented by the end user.

I'm not sure there is such a thing. Can you give an example?

  1. Interfaces called by SDK which are intended to be implemented by the end user.

These have the name "Plugin Interface" in the versioning spec.

tigrannajaryan commented 3 years ago
  1. API interfaces which are callable by the end user but are not intended to be implemented by the end user.

Which would be all API interfaces defined in the spec, I think?

Yes, I think so.

  1. Interfaces which define the contract between API and SDK packages and are intended to be called by API and implemented by SDK, not intended to be called or implemented by the end user.

I'm not sure there is such a thing. Can you give an example?

I think you are right. I thought we had something like TracerProviderSdk in the spec, but we don't.

--

The names "Contrsuctors" and "Plugin interface" are not self-explanatory, so I think it is best to clearly articulate the distinction in the PR that addresses this issue.

Oberon00 commented 3 years ago

I think you are right. I thought we had something like TracerProviderSdk in the spec, but we don't.

We don't have the name "TracerProviderSdk", but we do have specification on the SDK's TracerProvider implementation. But this falls under

  1. SDK interfaces which are callable by the end user but are not intended to be implemented by the end user.

as it has methods like Shutdown and ForceFlush.