Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document strategy to support experimental methods on stable interfaces #5882

Open
dashpole opened this issue Oct 10, 2024 · 20 comments
Open
Labels
enhancement New feature or request

Comments

@dashpole
Copy link
Contributor

dashpole commented Oct 10, 2024

Forked from #5768, and related to open-telemetry/opentelemetry-specification#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:

  • Experimental behavior with no API artifacts (e.g. exemplar collection, service.instance.id auto-generation).
    • Today, we do this through /internal/x directories, and OTEL_GO_X_FOO environment variables.
  • Experimental methods on SDK-only interfaces (e.g. span processor).
  • Experimental methods on API (and SDK) interfaces.
    • This will be done using forks or long-lived branches on the repository.
  • 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.

This excludes:

  • Experimental signals. It is assumed that these will be created as new, unstable modules while they are under development.

Proposed Solution

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

@dashpole dashpole added the enhancement New feature or request label Oct 10, 2024
@dashpole
Copy link
Contributor Author

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
Copy link
Contributor Author

@codeboten @bogdandrutu

@MrAlias
Copy link
Contributor

MrAlias commented Oct 12, 2024

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
Copy link
Member

pellared commented Oct 16, 2024

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
Copy link
Contributor Author

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

@pellared
Copy link
Member

  • 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
Copy link
Member

mx-psi commented Oct 16, 2024

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
Copy link
Member

mx-psi commented Oct 16, 2024

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
Copy link
Member

  • 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
Copy link
Member

mx-psi commented Oct 16, 2024

I think this comment may be relevant in this discussion: open-telemetry/opentelemetry-collector#4832 (comment) It summarizes ways in which grpc-go's policy has caused harm to the Go ecosystem

@pellared
Copy link
Member

pellared commented Oct 16, 2024

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
Copy link
Member

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
Copy link
Member

mx-psi commented Oct 16, 2024

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
Copy link
Member

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
Copy link
Member

pellared commented Oct 16, 2024

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: golang/go#33389

@MrAlias
Copy link
Contributor

MrAlias commented Oct 16, 2024

  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
Copy link
Contributor

MrAlias commented Oct 16, 2024

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.

@pellared pellared changed the title Document strategy to support experimental methods on stable interfaces. Document strategy to support experimental methods on stable interfaces Oct 17, 2024
@mx-psi
Copy link
Member

mx-psi commented Oct 17, 2024

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 😞

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"

@pellared
Copy link
Member

pellared commented Oct 22, 2024

Related Slack thread showcasing user-facing problems with we add new methods to interfaces in API packages: https://cloud-native.slack.com/archives/C01NPAXACKT/p1729262962637489

Actually, a fork and usage of replace in go.mod maybe the best what we can provide...
I think this is also the most popular pattern.

@dashpole
Copy link
Contributor Author

My notes from the Spec SIG meeting:

Some ideas that were thrown around during the meeting:

  1. Branch on the otel-go for each feature
  2. Single long-lived branch on otel-go for all experimental features
  3. PRs to the OTel-go repo

It seemed like others on the call weren't as happy with (3). JS and Java both seem to have a single artifact with all of the experimental features. But that would put us in a position of needing to maintain + rebase a branch with patches each release.

Both (1) and (3) can result in stale feature implementations, but it isn't clear to me if feature branches need to be maintained (with regular releases to update dependencies, etc). I'm inclined to treat that as a nice-to-have (people shouldn't use these features in production).

Documentation for (2) would be simpler, since instructions for using the long-lived branch would always be the same. Documentation for (1) or (3) would need to be updated with the list of branches/PRs when new ones are added or removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants