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

Bump minimum version to go 1.21.0 #32451

Merged
merged 7 commits into from
Apr 19, 2024

Conversation

liustanley
Copy link
Contributor

Description:

We currently use go 1.21 in all go.mod files, this PR changes all go.mod files to include the minor version by using go 1.21.0. It seems that using the minor version is recommended by the Go project: golang/go#62278. One of the dependencies in collector-contrib also uses go 1.21.0, so this will need to be updated eventually anyways: https://github.com/cilium/ebpf/blob/main/go.mod#L3.

Link to tracking Issue:

Testing:

Documentation:

@atoulme
Copy link
Contributor

atoulme commented Apr 16, 2024

Is there a rationale for this change?

@liustanley
Copy link
Contributor Author

Is there a rationale for this change?

The motivation is that we want to import datadog-agent modules to use in collector-contrib, but both repositories have different go versions in their go.mod files. We are making a change in datadog-agent to modify the version to match collector-contrib's version, but datadog-agent requires the minor version due to the EBPF dependency. collector-contrib also has this dependency, and the Go project seems to prefer using the minor version as well.

Co-authored-by: Antoine Toulme <antoine@toulme.name>
@crobert-1
Copy link
Member

Related: #32356

Copy link
Contributor

@atoulme atoulme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@atoulme
Copy link
Contributor

atoulme commented Apr 17, 2024

@mx-psi
Copy link
Member

mx-psi commented Apr 17, 2024

For further context: the Datadog Agent has go 1.21.0 because it depends on cilium/ebpf that uses go 1.21.0 https://github.com/cilium/ebpf/blob/32b95e4a83690a69eb2de5796db313893f71d1d0/go.mod#L3

@liustanley
Copy link
Contributor Author

CI fails with https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/8709569766/job/23889804537?pr=32451 please take a look

I ran make genotelcontribcol with the same version as the CI (go 1.21.9), and it added toolchain go1.21.9 in cmd/otelcontribcol/go.mod. @mx-psi was able to reproduce this on his local machine as well, is this something we need to fix in the CI or can we make this change?

Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried to revert this change upstream on cilium/ebpf/discussions/1438 but it doesn't seem like this is going to change soon.

While this is not well documented, and it is a feature for applications and not libraries, I think this is okay to merge because:

  • The only impact it has on downstream consumers is that we drop support for prereleases of Go 1.21 (so e.g. go1.21rc1 will refuse to build a module with this version). This is mentioned in the 'Go versions' section of https://go.dev/doc/toolchain. This seems okay to me.
  • We will probably end up with this change one way or another, since (i) the 'stable' solution in the ecosystem is for everyone to specify the bugfix version and (ii) go mod init now adds the bugfix version for you on recent versions.

If the upstream discussion changes, we can revert this, or even do so on the bump to Go 1.22 when Go 1.23 is released

@mx-psi mx-psi merged commit 2d6c34d into open-telemetry:main Apr 19, 2024
156 checks passed
@github-actions github-actions bot added this to the next release milestone Apr 19, 2024
rimitchell pushed a commit to rimitchell/opentelemetry-collector-contrib that referenced this pull request May 8, 2024
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

We currently use `go 1.21` in all go.mod files, this PR changes all
go.mod files to include the minor version by using `go 1.21.0`. It seems
that using the minor version is recommended by the Go project:
golang/go#62278. One of the dependencies in
collector-contrib also uses `go 1.21.0`, so this will need to be updated
eventually anyways: https://github.com/cilium/ebpf/blob/main/go.mod#L3.

**Link to tracking Issue:** <Issue number if applicable>

**Testing:** <Describe what testing was performed and which tests were
added.>

**Documentation:** <Describe the documentation added.>

---------

Co-authored-by: Antoine Toulme <antoine@toulme.name>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd/configschema configschema command cmd/githubgen cmd/opampsupervisor cmd/otelcontribcol otelcontribcol command cmd/oteltestbedcol cmd/telemetrygen telemetrygen command confmap/provider/s3provider confmap/provider/secretsmanagerprovider connector/count connector/datadog connector/exceptions connector/failover connector/grafanacloud connector/routing connector/servicegraph connector/spanmetrics examples/demo exporter/alertmanager exporter/alibabacloudlogservice Alibaba components exporter/awscloudwatchlogs awscloudwatchlogs exporter exporter/awsemf awsemf exporter exporter/awskinesis exporter/awss3 exporter/awsxray exporter/azuredataexplorer exporter/azuremonitor exporter/carbon exporter/cassandra exporter/clickhouse exporter/coralogix exporter/datadog Datadog components exporter/dataset exporter/elasticsearch exporter/file exporter/googlecloud exporter/googlecloudpubsub exporter/googlemanagedprometheus Google Managed Prometheus exporter exporter/honeycombmarker exporter/influxdb exporter/instana exporter/kafka exporter/kinetica exporter/loadbalancing exporter/logicmonitor exporter/logzio exporter/loki Loki Exporter exporter/mezmo exporter/opencensus exporter/opensearch exporter/otelarrow exporter/prometheus exporter/prometheusremotewrite exporter/pulsar exporter/rabbitmq exporter/sapm exporter/sentry exporter/signalfx exporter/skywalking exporter/splunkhec exporter/sumologic exporter/syslog exporter/tencentcloudlogservice exporter/zipkin extension/ack extension/asapauth extension/awsproxy extension/basicauth extension/bearertokenauth extension/encoding extension/googleclientauth extension/headerssetter extension/healthcheck Health Check Extension extension/httpforwarder extension/jaegerremotesampling extension/oauth2clientauth extension/observer extension/oidcauth extension/opamp extension/pprof extension/remotetap extension/sigv4auth extension/solarwindsapmsettings extension/storage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants