opentelemetry-go-instrumentation icon indicating copy to clipboard operation
opentelemetry-go-instrumentation copied to clipboard

Remove support for Go versions below 1.17

Open RonFed opened this issue 1 year ago • 5 comments

According to our README:

Each major Go release is supported until there are two newer major releases.

So I think we can remove the following parts:

  • Start tracking offsets from 1.17 in structs from the standard library (instead of 1.12 today)
  • Remove all the things related to is_registers_abi This includes both the Go parts and mainly the eBPF parts which can make the code simpler. get_consistent_key can be removed as we'll always use the goroutine pointer as key, get_argument will always be get_argument_by_reg
  • Probably returning an error if we detect a lower version.

RonFed avatar Feb 01 '24 20:02 RonFed

+1 to this, or at least making it clear what go versions we support (mentioned in https://github.com/open-telemetry/opentelemetry-go-instrumentation/issues/636#issuecomment-1925365953)

Start tracking offsets from 1.17 in structs from the standard library (instead of 1.12 today)

This isn't clear to me (that we start at 1.12). At first glance this comment in the offsets generator makes it look like we support all Go versions for instrumentation

Ideally, the automated offsets-generator job should be updated as part of each release to only run against the latest Go version-2. Or, the offsets generator binary itself should do this when it runs.

damemi avatar Feb 05 '24 16:02 damemi

In addition to this as mentioned in https://github.com/open-telemetry/opentelemetry-go-instrumentation/pull/724#issuecomment-1993735802 the tutorial app should be updated because the current one will not work if we don't support <1.17

damemi avatar Mar 18 '24 13:03 damemi

If we could resolve this as late as possible in the beta project we can provide as best of support as possible for the users that need to stay on this for their old version of application.

Ideally, this means all semconv work is done before this.

MrAlias avatar Apr 16 '24 17:04 MrAlias

I think we should not complete this task. Dropping support for instrumentation versions because we want to clean up the code is missing the main value of this project. We want to support as many user use cases as well as we can.

Understandably if there are security or performance issues that come in when supporting old versions of Go we should consider dropping support. But I don't think that is the case here.

MrAlias avatar Jul 03 '24 18:07 MrAlias

SIG meeting notes

  • Go official policy is to support the last two minor versions
  • Go 1.17 is ~5 years old
  • Users with old version are forced to use old version because they use old binaries with that version or there are security blockers to upgrading
    • example: instrumenting Prometheus itself. They use old version of Prometheus that is a binary they have downloaded
    • To get new version of Prometheus would be a bureaucratic process to upgrade
  • You need to make a line somewhere. Indefinite backwards compatibility is not a sustainable strategy
  • Maybe our line is "if you want to use old Go you need to use an old version of this auto-instrumentation"

Action Item:

  • Keep Go 1.17 support until beta
  • Drop support for Go 1.17 prior to stabilizing

MrAlias avatar Jul 09 '24 17:07 MrAlias