trace: changing interfaces will break backwards compability

[katiehockman]

The documentation for several of the interfaces have an addendum stating:

Warning: methods may be added to this interface in minor releases.

This is not a backwards compatible change to make per the language requirements, and any new methods added to an interface would be a breaking change for anyone who is implementing it. See the "Working With Interfaces" section of https://go.dev/blog/module-compatibility for more details about why this is a breaking change and what should be done instead should future changes be necessary.

Given that this package is a v1 API, it should be considered stable, and these kinds of breaking changes can no longer be expected in minor releases (since it's not a v0 API anymore), and those lines in the documentation should be removed.

[MrAlias]

This is a know difference between this projects versioning and stability policy and the Go language. It is something inherited from the OpenTelemetry project itself. It is not something we can change.

[katiehockman]

Thanks for the quick response @MrAlias!

Doesn't this go against the API stability for the Open Telemetry project? Namely:

An existing API call MAY be extended without incrementing the major version number if the particular language allows to do it in a backward-compatible manner.

Go wouldn't allow these kinds of changes in a backwards-compatible manner, so I thought it would go against the project's policy.

I'm raising this because I'm concerned as a potential user of this API. If I decided to implement a Tracer using the API and interface currently provided, then many of my project's users will be immediately broken if this package extends the API in a minor release.

[MrAlias]

I'm raising this because I'm concerned as a potential user of this API. If I decided to implement a Tracer using the API and interface currently provided, then many of my project's users will be immediately broken if this package extends the API in a minor release.

Indeed. That is a valid concern to have and we recommend if you plan to go this route you are fully aware of this possible breaking behavior when you upgrade your implementations.

[katiehockman]

It is something inherited from the OpenTelemetry project itself.

Could you please link to where this is documented? I was looking at the API spec for the project and didn't see this requirement, but I might be looking in the wrong place. I was also looking at this issue, but I couldn't find a link in there. If there is a problem with the OpenTelemetry project's specification that is forcing opentelemetry-go to follow this practice, then I'd like to file a bug against that spec to try to make this library safer to use.

[jmacd]

I support @katiehockman's interpretation that

An existing API call MAY be extended without incrementing the major version number if the particular language allows to do it in a backward-compatible manner.

is meant to prevent extending APIs in languages where there would be a breaking change. AFAIK all the changes in the trace spec since it was declared stable have gone through this sort of review by each SIG -- and those changes have been SHOULD requirements to allow SIGs that would be forced into a breaking change to opt out.

Admittedly, this is debated here too and still unresolved: open-telemetry/opentelemetry-specification#1457

[katiehockman]

those changes have been SHOULD requirements to allow SIGs that would be forced into a breaking change to opt out.

To be clear, this library wouldn't necessarily be required to opt out. They could just bump the major version when the change is made, which would still allow for backwards compatibility. But yes in the worst case they could just opt out of certain features if they don't feel they can bump the major version.

[jmacd]

@katiehockman Correct--I should have said something like "allowed to choose not to implement an optional feature as opposed to a breaking change or a major version number change".

[katiehockman]

@MrAlias Do you feel that this issue could be re-opened, given the discussion currently being had, or is this a firm rule that this library has chosen to take? If it's something in the spec that's forcing this then I would like to file an issue against the spec to better support situations like this.
Thank you!

[MrAlias]

@MrAlias Do you feel that this issue could be re-opened, given the discussion currently being had, or is this a firm rule that this library has chosen to take? If it's something in the spec that's forcing this then I would like to file an issue against the spec to better support situations like this. Thank you!

It is a firm rule of this project, one we have adopted into the reference versioning policy.

For that policy to change the specification will need to update their policy to ensure no stable interface is extended.

[mx-psi]

This was discussed on the OpenTelemetry Go SIG meeting on October 13th. My understanding from that recording is that there is no consensus among OTel Go maintainers about this; I think the issue should be reopened to signal that this is an open question. @MrAlias, could you reopen the issue?

It would also be good if OTel Go maintainers can link to the wording on the latest specification release that supports this 'unstable interfaces' policy. #1714 is not very explicit about what spec section is the one that motivated the decision, and things have changed a lot since the issue was written early last year. It's difficult to participate in the conversation without knowing what the decision is precisely based on.

[bogdandrutu]

It would also be good if OTel Go maintainers can link to the wording on the latest specification release that supports this 'unstable interfaces' policy. #1714 is not very explicit about what spec section is the one that motivated the decision, and things have changed a lot since the issue was written early last year. It's difficult to participate in the conversation without knowing what the decision is precisely based on.

I would not call them "unstable" interfaces. The fact that golang has this limitation can be work around by implementations the same way we do between API and SDK. Mainly as a third party implementation you can release a version of the implementation for every API version and ask your users to consume your SDK and API with the same version as we do in this repo (this is what users MUST do for API/SDK to work if we add funcs to the interface anyway). We may argue if the authors of the trace/metrics API did the right thing, but I feel that is a bit too late to do.

This is a problem in any decoupling between client - server / api - implementation / etc. If your client/api is newer than server/implementation you may have functionality that are not available on the other side (unless you decide to stop the world and never add new functionality, even bumping the version does not help, since the server may not support that new version).

[MrAlias]

For those who would like this project to not extend API interfaces, please make sure your voice is heard in the specification issue to add a new method to the Span interface: open-telemetry/opentelemetry-specification#3186.

If it is merged as is, it will likely result in that interface having a new method added to it.

[katiehockman]

If it is merged as is, it will likely result in that interface having a new method added to it.

One thing that I'm not yet understanding is why the Go OTel API can't bump the version from v1 to v2 when they need to extend the API/interface. @MrAlias can you help me understand why this isn't possible?

By filing this issue, I by no means meant to insinuate that the spec can never grow, and that the API can never change in the future. In fact, I encourage that when it makes sense and improves the user experience.
The reason I raised this issue is because the documentation says that the code will not maintain backwards compatibility, and that the interfaces will change whenever the spec changes. The interface can certainly change, but it has to be associated with a major version bump.

Guidelines provided by the Go language specification:

You must update to a major version when changes you’re making in a potential new version can’t guarantee backward compatibility for the module’s users. For example, you’ll make this change if you change your module’s public API such that it breaks client code using previous versions of the module.

It sounds like we're all in agreement that extending the interface will break client code that's using previous versions of the module. So the specification requires migration to go.opentelemetry.io/otel/trace/v2 when the interface must change. Then go.opentelemetry.io/otel/trace/v3 the next time, and so on.
[Example]

That page does call out that bumping the version can cause churn, but that the churn is necessary to maintain a healthy Go ecosystem, and in this case, a healthy OTel ecosystem as well.

[mx-psi]

I filed #4160 to ask for guidance on how to deal with these interfaces.

[pellared]

One thing that I'm not yet understanding is why the Go OTel API can't bump the version from v1 to v2 when they need to extend the API/interface

@katiehockman PTAL #3968. I am not merging it for a few days so that you can provide feedback.

[apparentlymart]

Hi all,

In other codebases that have this problem of needing to grow interfaces over time I've seen a pattern that is a little clunky but workable:

Each interface that might grow is documented as such and the library provides a zero-sized named type (e.g. an empty struct type) which implements all of the interface methods as some kind of default "do nothing" implementation, or as returning an explicit "not implemented" error.

Implementers are then required (by documented contract only) to use a struct type and embed the library's empty implementation into that struct type. That means that the empty implementation methods will "show through" any methods that the downstream implementer doesn't include, and so you can extend the interface and the empty implementation together to avoid compile-time errors.

In situations where the empty implementation returns some kind of "not implemented" signal, the code that calls that interface should presumably tolerate that result and fall back to doing something else instead, under the assumption that the implementation was written for an earlier version of the interface.

The main use of this that comes to my mind immediately is in the generated interfaces for gRPC services described in a protocol buffers schema. Similar to OpenTelemetry, the Go implementation is trying to represent language-agnostic API in a Go-appropriate way, and need to have some way to add new methods to a client or server interface if the schema evolves to include new RPC functions in future. They do that by generating an empty implementation where all methods return the gRPC "unimplemented" error, which turns those situations into runtime errors rather than compile-time errors.

I've seen it in some other Go libraries too but I don't have other references readily to hand.

[pellared]

@apparentlymart if I understand you correctly this is how we have designed most of the metrics interfaces: https://pkg.go.dev/go.opentelemetry.io/otel/metric

All interfaces in this API embed a corresponding interface from go.opentelemetry.io/otel/metric/embedded. If an author wants the default behavior of their implementations to be a compilation failure, signaling to their users they need to update to the latest version of that implementation, they need to embed the corresponding interface from go.opentelemetry.io/otel/metric/embedded in their implementation.

Do I understand that the proposal is to make something similar for trace API? If so then I think we already have an issue for it here: #4160

[pellared]

Closing per #4620 (comment)