open-telemetry / opentelemetry-collector

OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
4.46k stars 1.47k forks source link

Add policy for experimental Go API #9209

Open mx-psi opened 10 months ago

mx-psi commented 10 months ago

When working on #8935, one of the discussion points was about experimental symbols https://github.com/open-telemetry/opentelemetry-collector/pull/8935#discussion_r1396237074, which we decided to postpone. My proposal is to rely on separate Go modules as much as we can, and fall back to Go build tags whenever that is not feasible.

Prior work

For previous issues that touch on this topic see #4705 and #4832

Proposed solution

  1. Create a new module set[^1] called experimental following the versioning schema v0.0.x. Parts of the codebase we deem very unstable will be on separate modules under this module set. Symbols in these modules may not follow our usual deprecation guidelines (e.g. API may be removed without notice and we may remove the module after deprecating).
  2. For symbols that need to be added to an existing stable or beta module, add them under new build tags. These build tags:
    • Should have a common namespace (e.g. all should start with otel_collector_)
    • Should be independent (enabling any subset of available build tags should produce code that can be built and works)
    • Code using experimental symbols under this build tag should produce a working build when the build tag is not present (with possible missing functionality) or produce a meaningful error message (e.g. panic with a meaningful message if the build tag is not present).

[^1]: Edit: That is, a new entry under the module-sets key on versions.yaml

For (2), we may need to explore how this interacts with the builder (e.g. specify build tags used by components on metadata.yaml and have the builder pick these up in some way).

Rationale

yurishkuro commented 10 months ago

my 2c:

dmitryax commented 10 months ago

Some experimental APIs might be depending on the existing stable API. If we want to move them to experimental modules, we'd have to deal with some tricky inter-dependancy issues and unnecessary complicate the modules layout. For such use cases, build tags seems like a pretty good solution. I hope there should be a way to tweek IDEs to work better with them, I haven't tried it myself though.

For experimental APIs that can be easily introduced as separate modules, why do we need to move them under experimental/ instead of just starting with 0.x version?

The problem with the experimental/ prefix is that users have to make changes to migrate to the stable version once the API is mature while using the build tags or 0.x version doesn't require any changes.

mx-psi commented 10 months ago

@yurishkuro @dmitryax I am not proposing a new experimental/ folder, what I am proposing is a new module set: a new entry under module-sets on the versions.yaml file that has a versioning schema 0.0.x instead of 0.x.y.

[From @yurishkuro] If you don't want people to use the module, don't publish it. If you want people to use it but also to reserve the ability to make breaking changes - use semver. What does experimental/ add on top of that? Note that "experimental" as a comment is different because it can be placed inside a stable module whose semver rules are different. [From @dmitryax] For experimental APIs that can be easily introduced as separate modules, why do we need to move them under experimental/ instead of just starting with 0.x version?

Our beta modules have some stability guarantees, both in terms of what we document but also on what we are in practice doing (we are conservative with making breaking changes even if we can). An experimental module would be one where 'anything goes': things may break from one version to the other and we may give up on the module entirely.

personally when I think of build tags I can only remember the pain they cause, very poor usability especially inside an IDE

I agree that build tags have usability issues, I think the alternative (the 'just a comment' approach) causes much more pain as exemplified by grpc-go. Still, they are usable if your IDE is configured correctly.

[From @dmitryax] The problem with the experimental/ prefix is that users have to make changes to migrate to the stable version once the API is mature while using the build tags or 0.x version doesn't require any changes.

Given that I am not proposing using a different folder I think this is no longer a concern, right?

dmitryax commented 10 months ago

Our beta modules have some stability guarantees, both in terms of what we document but also on what we are in practice doing (we are conservative with making breaking changes even if we can). An experimental module would be one where 'anything goes': things may break from one version to the other and we may give up on the module entirely.

Sorry for the misunderstanding. Having another experimental module for this purpose makes sense to me 👍

Given that I am not proposing using a different folder I think this is no longer a concern, right?

Right.

mx-psi commented 5 months ago

I am going to remove this from Collector v1, I think we will need to solve this eventually, but we don't have to fix this for Collector 1.0