Skip to content

Commit

Permalink
[chore] Ability to provide custom ld and gc flags (open-telemetry#11996)
Browse files Browse the repository at this point in the history
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

The primary purpose of this PR is to provide greater flexibility in how
OTEL binaries are built, enabling the inclusion of debugging symbols
when needed, without always stripping them by default.

Currently, debugging symbols are only retained when
debug_compilation=true. However, this approach also disables all
compiler inlining and optimizations (gcflags=all=-N -l) to ensure an
exact match between written and executed code, resulting in a
significant increase in CPU consumption. There are scenarios where we
want binaries with debugging symbols and DWARF information while still
allowing the compiler to optimize and inline. This PR addresses that
need by introducing configurable GCFlags.

`ocb --ldflags="" --gcflags="" --config=builder-config.yaml`

<!-- Issue number if applicable -->
#### Link to tracking issue
Fixes
[#58](open-telemetry/opentelemetry-collector-releases#58)

<!--Describe what testing was performed and which tests were added.-->
#### Testing

Manual 

Override LDflags: 


![image](https://github.com/user-attachments/assets/6bcb0f7b-492a-45fb-a232-9c337afb5f5e)


Override both


![image](https://github.com/user-attachments/assets/00bc6c5f-37f0-438d-b4e1-f7a2d5833ec9)


<!--Describe the documentation added.-->
#### Documentation

README file updated.

-- 

Backward compatibility concerns: 
- As of today, passing cfg.LDFlags will append to LD flags that are by
default to `-s -w`.

Questions: 
- Should we deprecate DebugCompilation property? 
<!--Please delete paragraphs that you did not use before submitting.-->
  • Loading branch information
Dainerx authored Jan 21, 2025
1 parent 4db5775 commit 24da249
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 8 deletions.
15 changes: 14 additions & 1 deletion cmd/builder/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,20 @@ Use `ocb --help` to learn about which flags are available.

## Debug

To keep the debug symbols in the resulting OpenTelemetry Collector binary, set the configuration property `debug_compilation` to true.
### Debug symbols

By default, the LDflags are set to `-s -w`, which strips debugging symbols to produce a smaller OpenTelemetry Collector binary. To retain debugging symbols and DWARF debugging data in the binary, override the LDflags as shown:

```console
ocb --ldflags="" --config=builder-config.yaml.
```

### Debugging with Delve

To ensure the code being executed matches the written code exactly, debugging symbols must be preserved, and compiler inlining and optimizations disabled. You can achieve this in two ways:

1. Set the configuration property `debug_compilation` to true.
2. Manually override the ldflags and gcflags `ocb --ldflags="" --gcflags="all=-N -l" --config=builder-config.yaml.`

Then install `go-delve` and run OpenTelemetry Collector with `dlv` command as the following example:

Expand Down
3 changes: 3 additions & 0 deletions cmd/builder/internal/builder/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ type Config struct {
SkipGetModules bool `mapstructure:"-"`
SkipStrictVersioning bool `mapstructure:"-"`
LDFlags string `mapstructure:"-"`
LDSet bool `mapstructure:"-"` // only used to override LDFlags
GCFlags string `mapstructure:"-"`
GCSet bool `mapstructure:"-"` // only used to override GCFlags
Verbose bool `mapstructure:"-"`

Distribution Distribution `mapstructure:"dist"`
Expand Down
4 changes: 4 additions & 0 deletions cmd/builder/internal/builder/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,10 @@ func TestNewDefaultConfig(t *testing.T) {
require.NoError(t, cfg.Validate())
assert.False(t, cfg.Distribution.DebugCompilation)
assert.Empty(t, cfg.Distribution.BuildTags)
assert.False(t, cfg.LDSet)
assert.Empty(t, cfg.LDFlags)
assert.False(t, cfg.GCSet)
assert.Empty(t, cfg.GCFlags)
}

func TestNewBuiltinConfig(t *testing.T) {
Expand Down
19 changes: 15 additions & 4 deletions cmd/builder/internal/builder/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,28 @@ func Compile(cfg *Config) error {

cfg.Logger.Info("Compiling")

ldflags := "-s -w"
ldflags := "-s -w" // we strip the symbols by default for smaller binaries
gcflags := ""

args := []string{"build", "-trimpath", "-o", cfg.Distribution.Name}
if cfg.Distribution.DebugCompilation {
cfg.Logger.Info("Debug compilation is enabled, the debug symbols will be left on the resulting binary")
ldflags = cfg.LDFlags
args = append(args, "-gcflags=all=-N -l")
} else if len(cfg.LDFlags) > 0 {
ldflags += " " + cfg.LDFlags
gcflags = "all=-N -l"
} else {
if cfg.LDSet {
cfg.Logger.Info("Using custom ldflags", zap.String("ldflags", cfg.LDFlags))
ldflags = cfg.LDFlags
}
if cfg.GCSet {
cfg.Logger.Info("Using custom gcflags", zap.String("gcflags", cfg.GCFlags))
gcflags = cfg.GCFlags
}
}

args = append(args, "-ldflags="+ldflags)
args = append(args, "-gcflags="+gcflags)

if cfg.Distribution.BuildTags != "" {
args = append(args, "-tags", cfg.Distribution.BuildTags)
}
Expand Down
12 changes: 12 additions & 0 deletions cmd/builder/internal/builder/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,22 @@ func TestGenerateAndCompile(t *testing.T) {
cfg := newTestConfig(t)
cfg.Distribution.OutputPath = t.TempDir()
cfg.Replaces = append(cfg.Replaces, replaces...)
cfg.LDSet = true
cfg.LDFlags = `-X "test.gitVersion=0743dc6c6411272b98494a9b32a63378e84c34da" -X "test.gitTag=local-testing" -X "test.goVersion=go version go1.20.7 darwin/amd64"`
return cfg
},
},
{
name: "GCFlags Compilation",
cfgBuilder: func(t *testing.T) *Config {
cfg := newTestConfig(t)
cfg.Distribution.OutputPath = t.TempDir()
cfg.Replaces = append(cfg.Replaces, replaces...)
cfg.GCSet = true
cfg.GCFlags = `all=-N -l`
return cfg
},
},
{
name: "Build Tags Compilation",
cfgBuilder: func(t *testing.T) *Config {
Expand Down
15 changes: 13 additions & 2 deletions cmd/builder/internal/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const (
skipGetModulesFlag = "skip-get-modules"
skipStrictVersioningFlag = "skip-strict-versioning"
ldflagsFlag = "ldflags"
gcflagsFlag = "gcflags"
distributionOutputPathFlag = "output-path"
verboseFlag = "verbose"
)
Expand Down Expand Up @@ -84,6 +85,7 @@ func initFlags(flags *flag.FlagSet) error {
flags.Bool(skipStrictVersioningFlag, true, "Whether builder should skip strictly checking the calculated versions following dependency resolution")
flags.Bool(verboseFlag, false, "Whether builder should print verbose output (default false)")
flags.String(ldflagsFlag, "", `ldflags to include in the "go build" command`)
flags.String(gcflagsFlag, "", `gcflags to include in the "go build" command`)
flags.String(distributionOutputPathFlag, "", "Where to write the resulting files")
return flags.MarkDeprecated(distributionOutputPathFlag, "use config distribution::output_path")
}
Expand Down Expand Up @@ -145,8 +147,17 @@ func applyFlags(flags *flag.FlagSet, cfg *builder.Config) error {
cfg.SkipStrictVersioning, err = flags.GetBool(skipStrictVersioningFlag)
errs = multierr.Append(errs, err)

cfg.LDFlags, err = flags.GetString(ldflagsFlag)
errs = multierr.Append(errs, err)
if flags.Changed(ldflagsFlag) {
cfg.LDSet = true
cfg.LDFlags, err = flags.GetString(ldflagsFlag)
errs = multierr.Append(errs, err)
}
if flags.Changed(gcflagsFlag) {
cfg.GCSet = true
cfg.GCFlags, err = flags.GetString(gcflagsFlag)
errs = multierr.Append(errs, err)
}

cfg.Verbose, err = flags.GetBool(verboseFlag)
errs = multierr.Append(errs, err)

Expand Down
3 changes: 2 additions & 1 deletion cmd/builder/internal/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,14 @@ func TestApplyFlags(t *testing.T) {
},
{
name: "All flag values",
flags: []string{"--skip-generate=true", "--skip-compilation=true", "--skip-get-modules=true", "--skip-strict-versioning=true", "--ldflags=test", "--verbose=true"},
flags: []string{"--skip-generate=true", "--skip-compilation=true", "--skip-get-modules=true", "--skip-strict-versioning=true", "--ldflags=test", "--gcflags=test", "--verbose=true"},
want: &builder.Config{
SkipGenerate: true,
SkipCompilation: true,
SkipGetModules: true,
SkipStrictVersioning: true,
LDFlags: "test",
GCFlags: "test",
Verbose: true,
},
},
Expand Down

0 comments on commit 24da249

Please sign in to comment.