open-telemetry / opentelemetry-collector

OpenTelemetry Collector
https://opentelemetry.io
Apache License 2.0
3.94k stars 1.32k forks source link

[cmd/builder] Disable cgo by default #10040

Open evan-bradley opened 3 weeks ago

evan-bradley commented 3 weeks ago

Description

Disables cgo by default in the builder. I'm proposing this as a breaking change because I doubt it will affect many users, most of whom I expect to be using CGO_ENABLED=0 already, or at a minimum prefer cgo to be disabled. If we would like to have a transition period for this, I can adjust the PR and plan for that.

Link to tracking issue

Fixes #10028

Testing

I can't find a good way to directly test this that doesn't involve either setting up a lot of scaffolding to either create a mock go cli or injects the necessary code into a top-level function. To keep things simple I have just relied on the existing test suite. I had thought of trying to test the output binary, but Go disables cgo if no compiler is detected on the system, so the test wouldn't be reliable across developer machines.

In the future, if we emit a Dockerfile from the builder, we can test that leaving the option as a default runs inside a scratch image.

Documentation

Updated the builder readme.

codecov[bot] commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 70.83333% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 91.55%. Comparing base (1a5da25) to head (a299604). Report is 63 commits behind head on main.

Files Patch % Lines
cmd/builder/internal/builder/main.go 70.83% 2 Missing and 5 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #10040 +/- ## ========================================== - Coverage 91.57% 91.55% -0.02% ========================================== Files 360 360 Lines 16719 16738 +19 ========================================== + Hits 15310 15325 +15 - Misses 1073 1075 +2 - Partials 336 338 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

evan-bradley commented 3 weeks ago
failed to compile the OpenTelemetry Collector distribution: go subcommand failed with args '[build -trimpath -o  ->ldflags=-s -w]': exit status 1, error message: warning: GOPATH set to GOROOT () has no effect
                              go: module cache not found: neither GOMODCACHE nor GOPATH is set
                              go: module cache not found: neither GOMODCACHE nor GOPATH is set

It looks like my workaround didn't work on Windows. If anyone has any ideas, they would be welcome, otherwise I'll spend some time later to dig into this.

TylerHelmuth commented 3 weeks ago

What happens to ocb users who depended on CGO being enabled? Will their builds work but have issues at runtime or will they just fail to build?

evan-bradley commented 3 weeks ago

What happens to ocb users who depended on CGO being enabled? Will their builds work but have issues at runtime or will they just fail to build?

There's a cgo_enabled option that can be set to true to allow enabling cgo. If cgo is left disabled at build time and a component that requires cgo is used, our contributing guidelines suggest printing a warning and defaulting to a no-op.

github-actions[bot] commented 6 days ago

This PR was marked stale due to lack of activity. It will be closed in 14 days.