open-telemetry / opentelemetry-go

OpenTelemetry Go API and SDK
https://opentelemetry.io/docs/languages/go
Apache License 2.0
5.25k stars 1.06k forks source link

Document strategy to support experimental methods on stable interfaces #5882

Open dashpole opened 1 week ago

dashpole commented 1 week ago

Forked from https://github.com/open-telemetry/opentelemetry-go/pull/5768, and related to https://github.com/open-telemetry/opentelemetry-specification/issues/4257.

Problem Statement

We should document our policy for how we support for experimental things in the specification. It doesn't mean we will always accept such features, but should describe how it would work. This includes:

This excludes:

Proposed Solution

Decide and document how we are going to handle the above.

dashpole commented 1 week ago

Experimental methods on API (and SDK) interfaces.

The API could be E.g. otel/metric/experimentalmetric and otel/sdk/metric/experimentalmetric. The experimental API would embed the stable API, and also add the experimental method. The SDK would be harder to implement. It would need to include New() functions that implement the experimental API.

Maybe the experimental SDK would coordinate with the stable SDK through functions in an internal folder?

dashpole commented 1 week ago

@codeboten @bogdandrutu

MrAlias commented 1 week ago

Experimental methods on API (and SDK) interfaces.

The API could be E.g. otel/metric/experimentalmetric and otel/sdk/metric/experimentalmetric. The experimental API would embed the stable API, and also add the experimental method. The SDK would be harder to implement. It would need to include New() functions that implement the experimental API.

Maybe the experimental SDK would coordinate with the stable SDK through functions in an internal folder?

This approach means duplicate packages will need to be maintained. It also means users will need to rewrite their code switch between stable/experimental.

Why are branches and forks not a valid place for this type of code?

pellared commented 2 days ago

Why are branches and forks not a valid place for this type of code?

👍 While I hate "long-living branches" (because of maintainability burden), I do not think we have any other user-friendly alternatives.

I would not use build tags as all Go codebases depending on experimental features would have to be build with these build flags. This would be very user-unfriendly.

dashpole commented 2 days ago

Got it. I've added that to the description.

pellared commented 2 days ago
  • Experimental parameters (i.e. new option) for API functions.
  • Experimental parameters (i.e. new option) for SDK functions.
  • Experimental fields for API structs.
  • Experimental fields for SDK structs.

I think you can add this comment to these as well.

mx-psi commented 2 days ago

Why are branches and forks not a valid place for this type of code?

This is not a viable solution if you want to use opentelemetry-go types as part of your library's public API, is it? If that is the case, then

  1. You are forced to use the fork indefinitely if you don't want to make a breaking change
  2. (depending of the use case) You are forced to have some sort of 'bridge' between your fork and opentelemetry-go if you want to use a third library that does not use your fork
mx-psi commented 2 days ago

I would go as far as saying as a user of opentelemetry-go, I would rather have to deal with (properly namespaced) build tags than have to use a fork.

tigrannajaryan commented 2 days ago
  • Experimental parameters (i.e. new option) for API functions.
  • Experimental parameters (i.e. new option) for SDK functions.
  • Experimental fields for API structs.
  • Experimental fields for SDK structs.

I think you can add this comment to these as well.

@pellared are you referring to the approach used by gRPC where they label certain methods "Experimental", e.g. here? This is experimental methods within otherwise a stable package.

mx-psi commented 2 days ago

I think this comment may be relevant in this discussion: https://github.com/open-telemetry/opentelemetry-collector/issues/4832#issuecomment-1039394229 It summarizes ways in which grpc-go's policy has caused harm to the Go ecosystem

pellared commented 2 days ago

You are forced to use the fork indefinitely if you don't want to make a breaking change

@mx-psi, I think only a long-living branch with releases like v1.2.0-exp.1 would be a usable solution. Not a fork. This way applications would depend on experimental versions, would use the same import paths, and build tags would not be necessary.

tigrannajaryan commented 2 days ago

I would not use build tags as all Go codebases depending on experimental features would have to be build with these build flags. This would be very user-unfriendly.

@pellared can you expand on this? Why is this very user-unfriendly?

Also as an alternate to build flags, perhaps we could have runtime feature-flags to enable experimental APIs?

mx-psi commented 2 days ago

You are forced to use the fork indefinitely if you don't want to make a breaking change

@mx-psi, I think only a long-living branch with releases like v1.2.0-exp.1 would be a usable solution. Not a fork. This way applications would depend on experimental versions, would use the same import paths, and build tags would not be necessary.

That would definitely be an improvement. I am not sure how it would work with Go's MVS, but if it can work nicely with it, then that's definitely a solution

pellared commented 2 days ago

I am not sure how it would work with Go's MVS

AFAIK it works like described here: https://semver.org/#spec-item-11

pellared commented 2 days ago

I would not use build tags as all Go codebases depending on experimental features would have to be build with these build flags. This would be very user-unfriendly.

@pellared can you expand on this? Why is this very user-unfriendly?

  1. IDEs usually do not handle build tags nicely. Users would need to add these tags e.g. to gopls config when developing against any software that requires them for building.
  2. Any Go source code that has a direct or indirect dependency on API which requires a build tag would need to add them during compilation (e.g. when running go build, go test).

I have not heard a single good experience of using a custom build tags to control the API surface for a package.

Related issue: https://github.com/golang/go/issues/33389

MrAlias commented 2 days ago
  1. You are forced to use the fork indefinitely if you don't want to make a breaking change

This is no different than having experimental package live in a separate local package.

MrAlias commented 2 days ago

I would not use build tags as all Go codebases depending on experimental features would have to be build with these build flags. This would be very user-unfriendly.

@pellared can you expand on this? Why is this very user-unfriendly?

Also as an alternate to build flags, perhaps we could have runtime feature-flags to enable experimental APIs?

Additionally to what @pellared mentioned, things like the kube API, which have a dependency on OTel, are not able to set build flags in their CI pipeline.

mx-psi commented 1 day ago

You are forced to use the fork indefinitely if you don't want to make a breaking change

@mx-psi, I think only a long-living branch with releases like v1.2.0-exp.1 would be a usable solution. Not a fork. This way applications would depend on experimental versions, would use the same import paths, and build tags would not be necessary.

@pellared Looking at the Go MVS doc that you linked (thanks for the link!) It looks like if a project has two dependencies, using v1.2.0 and v1.2.0-exp.1 respectively, a user would have to force Go to use v1.2.0-exp.1 for their project to be buildable :disappointed:

This is no different than having experimental package live in a separate local package.

@MrAlias Indeed, I think the solution (whenever that is something we can do on Go) should not be that either. It seems like your hands are tied because of the spec and by the fact that most of the API is v1.x, so take my comment more as "if you can avoid a solution with these flaws, please do"