opentelemetry-collector icon indicating copy to clipboard operation
opentelemetry-collector copied to clipboard

[cmd/builder] Disable cgo by default

Open evan-bradley opened this issue 1 year ago • 5 comments

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.

evan-bradley avatar Apr 29 '24 17:04 evan-bradley

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
@@            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.

codecov[bot] avatar Apr 29 '24 18:04 codecov[bot]

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.

evan-bradley avatar Apr 29 '24 18:04 evan-bradley

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?

TylerHelmuth avatar Apr 29 '24 23:04 TylerHelmuth

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.

evan-bradley avatar Apr 30 '24 15:04 evan-bradley

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

github-actions[bot] avatar May 15 '24 03:05 github-actions[bot]

Closed as inactive. Feel free to reopen if this PR is still being worked on.

github-actions[bot] avatar Jun 01 '24 03:06 github-actions[bot]