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

Publish deprecation packages for all previously unused/disabled paths #2328

Open
lizthegrey opened this issue Oct 26, 2021 · 14 comments
Open
Labels
enhancement New feature or request

Comments

@lizthegrey
Copy link
Member

Problem Statement

Users trying to upgrade go.opentelemetry.io/otel see messages saying modules can't be found under paths, or see their dependencies stuck on the last version to contain that directory.

Proposed Solution

Use https://play-with-go.dev/retract-module-versions_go116_en/ to publish packages retracting themselves and previous versions.

@lizthegrey lizthegrey added the enhancement New feature or request label Oct 26, 2021
@MrAlias
Copy link
Contributor

MrAlias commented Nov 11, 2021

We need to determine the best way to "resurrect" the old modules. Do we roll back to the commit they were dropped on, branch, and tag a release specific for that module that retracts all the previous ones?

It might also make sense to not just retract all previous modules, but maybe make a release that deprecates the packages and points users to the new replacement if was exists. I.E. the old exporter/* modules could be deprecated in favor of the exporters modules.

@MadVikingGod
Copy link
Contributor

So I dug into this a bit. I don't think that a // Deprecated comment will not work in this case, because the problem is with version selection, which happens before we see any Deprecated comments.

For retracted commits to work there needs to be three things, go version 1.16 or later, a release that has the mod with the retract, and it to be tagged appropriately. So for modules we have removed we should roll back to the previous version and update the mod with the appropriate tag.

I compiled a list of orphaned versions that I will attach at the end.

For example. We no longer have an exporters/otlp, and we haven't sense v0.20.0. For us to retract it we need to publish a v0.21.0 (or later) that has that mod with something like:

retract (
    [v0.0.1 v0.20.0] // Moved to exporters/otlptrace
    v0.21.0 // retract version Moved to exporters/otlptrace
)

This is blocked by #2412

Old version:

  • bridge/opencensus/examples/bridge/v0.14.0
  • example/basic/v0.15.0
  • example/grpc/v0.10.0
  • example/http-stackdriver/v0.2.3
  • example/http/v0.10.0
  • example/prom-collector/v0.20.0
  • exporter/metric/prometheus/v0.2.1
  • exporter/trace/jaeger/v0.2.1
  • exporter/trace/stackdriver/v0.2.1
  • exporter/trace/stdout/v0.1.2
  • exporters/metric/prometheus/v0.21.0
  • exporters/otlp/v0.20.0
  • exporters/stdout/v0.20.0
  • exporters/trace/jaeger/v0.20.0
  • exporters/trace/jaeger/v1.0.0-RC1
  • exporters/trace/stackdriver/v0.2.3
  • exporters/trace/zipkin/v0.20.0
  • exporters/trace/zipkin/v1.0.0-RC1
  • oteltest/v0.20.0
  • oteltest/v1.0.0-RC3
  • schema/v0.0.1

@MadVikingGod
Copy link
Contributor

MadVikingGod commented Feb 28, 2022

There are three ways to notify users not to use some sort of published code within the go ecosystem:

  1. Deprecating a module. This is done with // Deprecate: some note here above the module in the go.mod. This will apply to all versions of a module if @latest has this comment.
  2. Retracting a module. This is done via the retract directive in a go.mod. This will only affect the versions listed in the '@latestversion. This has the additional effect of hiding those versions as upgrade targets in thego list -m -u`
  3. Deprecating a portion of a package. This only works on types and functions within a package, and its net effect is to hide in the docs and cause errors with some 3rd party vet tools.

Note: None of these solutions will give a warning when building. Only 1, and 2 will give a warning if you try to list upgrades or go get a deprecated/retracted version.

There are two different problems that need addressing,

  1. Mods that have been removed. I detailed this above, I think publishing a version of each that is patch version bump with a deprecate for each can give users a hint of where these packages may have gone.
  2. Breaking changes made within a package (for example the latest is the Metrics API). These are harder to detect, as they don't have an external version bump, but they do cause friction in some cases. If we want to solve this long term I think we need to build/find something to alert us when we do break the API and retract those versions going forward.

@MadVikingGod
Copy link
Contributor

I have experimented a bit with this and created an experimental repo: https://github.com/MadVikingGod/mod-experiment

@MadVikingGod
Copy link
Contributor

I think this is related to #2328, and #3046.

@MrAlias
Copy link
Contributor

MrAlias commented Aug 3, 2022

I think this is related to #2328, and #3046.

#2328 is this issue, did you mean another issue?

@MadVikingGod
Copy link
Contributor

Yes. #2938

@MadVikingGod
Copy link
Contributor

I will go and try and find orphaned tags again so the list is up to date. But here is my working plan:

  1. Create a branch called deprecate/v### from the tag that it represents, e.g. deprecate/v0.20.0 from tag v0.20.0.
  2. Create a PR that includes a module deprecation message for all affected modules. e.g. example/prom-collector, exporters/otlp, exporters/stdout, exporters/trace/jaeger, exporters/trace/zipkin, and oteltest.
  3. When that is merged tag it with a +1 patch version. e.g. exporters/otlp/v0.20.1 etc.
  4. If this doesn't resolve the issue repeat 2 and 3 but with a retract statement.

@MadVikingGod
Copy link
Contributor

MadVikingGod commented Aug 3, 2022

Not many versions have changed (kind of expected). I've only added internal/metric and sdk/export/metric, also removed schema.

bridge/opencensus/exam

  • bridge/opencensus/eples/bridge/v0.14.0
  • example/basic/v0.15.0
  • example/grpc/v0.10.0
  • example/jaeger/v1.16.0
  • example/http-stackdriver/v0.2.3
  • example/http/v0.10.0
  • example/prom-collector/v0.20.0
  • exporter/metric/prometheus/v0.2.1
  • exporter/trace/jaeger/v0.2.1
  • exporter/trace/stackdriver/v0.2.1
  • exporter/trace/stdout/v0.1.2
  • exporters/metric/prometheus/v0.21.0
  • exporters/otlp/v0.20.0
  • exporters/stdout/v0.20.0
  • exporters/trace/jaeger/v0.20.0
  • exporters/trace/jaeger/v1.0.0-RC1
  • exporters/trace/stackdriver/v0.2.3
  • exporters/trace/zipkin/v0.20.0
  • exporters/trace/zipkin/v1.0.0-RC1
  • oteltest/v0.20.0
  • oteltest/v1.0.0-RC3
  • internal/metric v0.27.0
  • sdk/export/metric v0.28.0

@MadVikingGod
Copy link
Contributor

@MrAlias and @Aneurysm9 Does this sound like a safe plan of action?

@MrAlias
Copy link
Contributor

MrAlias commented Aug 3, 2022

  1. Create a branch called deprecate/v### from the tag that it represents, e.g. deprecate/v0.20.0 from tag v0.20.0.

Will you deprecate all the above packages at once? Or will this branch only contain one package deprecation?

I am in favor of starting with a single package deprecation to ensure a correct workflow. Doing batches after that sounds good to me.

  1. Create a PR that includes a module deprecation message for all affected modules. e.g. example/prom-collector, exporters/otlp, exporters/stdout, exporters/trace/jaeger, exporters/trace/zipkin, and oteltest.

A deprecation notice needs to be added to the module (go.mod) but also the package documentation.

  1. If this doesn't resolve the issue repeat 2 and 3 but with a retract statement.

I did some (somewhat incomplete) testing on this deprecation process and did not find the retract statement to be useful.

Adding a complete retraction of all the module versions results in the retraction being ignored. The Go tooling looks to take the approach that it will still resolve some module version if you ask to use it. It will pick a retracted version if those are the only options.

Based on the linked testing above, I expect the deprecation notice is going to be what we want.

@MadVikingGod
Copy link
Contributor

I was planning on doing multiple packages at once grouped by the version number. So there would only be one commit for v0.20.1 that deprecated 6 packages. I was going to start with that one in particular because it was referenced in #3046, and if the reporter is still around, I have a test case to work with.

A deprecation notice needs to be added to the module (go.mod) but also the package documentation.

That is possible, and the exact message can be discussed in the PR.

I also did a bit of testing as referenced earlier. I found that the go tooling didn't consider a deprecation comment at any point, only 3rd party tools used it. This might have changed from 1.16 when I tested.

@thethan
Copy link

thethan commented Sep 14, 2022

That is possible, and the exact message can be discussed in the PR.

@MadVikingGod I know we are not the original reporters of #3046 but I know I saw the same error and could help with testing it as I am using bazel and seeing the same issue as the reporter.

@MrAlias
Copy link
Contributor

MrAlias commented Mar 14, 2024

go.opentelemetry.io/otel/label needs to be addressed in this as well: #2000 (comment)

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

4 participants