Skip to content

Commit 24da249

Browse files
authored
[chore] Ability to provide custom ld and gc flags (open-telemetry#11996)
<!--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.-->
1 parent 4db5775 commit 24da249

File tree

7 files changed

+63
-8
lines changed

7 files changed

+63
-8
lines changed

cmd/builder/README.md

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,20 @@ Use `ocb --help` to learn about which flags are available.
110110

111111
## Debug
112112

113-
To keep the debug symbols in the resulting OpenTelemetry Collector binary, set the configuration property `debug_compilation` to true.
113+
### Debug symbols
114+
115+
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:
116+
117+
```console
118+
ocb --ldflags="" --config=builder-config.yaml.
119+
```
120+
121+
### Debugging with Delve
122+
123+
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:
124+
125+
1. Set the configuration property `debug_compilation` to true.
126+
2. Manually override the ldflags and gcflags `ocb --ldflags="" --gcflags="all=-N -l" --config=builder-config.yaml.`
114127

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

cmd/builder/internal/builder/config.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ type Config struct {
3535
SkipGetModules bool `mapstructure:"-"`
3636
SkipStrictVersioning bool `mapstructure:"-"`
3737
LDFlags string `mapstructure:"-"`
38+
LDSet bool `mapstructure:"-"` // only used to override LDFlags
39+
GCFlags string `mapstructure:"-"`
40+
GCSet bool `mapstructure:"-"` // only used to override GCFlags
3841
Verbose bool `mapstructure:"-"`
3942

4043
Distribution Distribution `mapstructure:"dist"`

cmd/builder/internal/builder/config_test.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,10 @@ func TestNewDefaultConfig(t *testing.T) {
179179
require.NoError(t, cfg.Validate())
180180
assert.False(t, cfg.Distribution.DebugCompilation)
181181
assert.Empty(t, cfg.Distribution.BuildTags)
182+
assert.False(t, cfg.LDSet)
183+
assert.Empty(t, cfg.LDFlags)
184+
assert.False(t, cfg.GCSet)
185+
assert.Empty(t, cfg.GCFlags)
182186
}
183187

184188
func TestNewBuiltinConfig(t *testing.T) {

cmd/builder/internal/builder/main.go

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,17 +109,28 @@ func Compile(cfg *Config) error {
109109

110110
cfg.Logger.Info("Compiling")
111111

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

114115
args := []string{"build", "-trimpath", "-o", cfg.Distribution.Name}
115116
if cfg.Distribution.DebugCompilation {
116117
cfg.Logger.Info("Debug compilation is enabled, the debug symbols will be left on the resulting binary")
117118
ldflags = cfg.LDFlags
118-
args = append(args, "-gcflags=all=-N -l")
119-
} else if len(cfg.LDFlags) > 0 {
120-
ldflags += " " + cfg.LDFlags
119+
gcflags = "all=-N -l"
120+
} else {
121+
if cfg.LDSet {
122+
cfg.Logger.Info("Using custom ldflags", zap.String("ldflags", cfg.LDFlags))
123+
ldflags = cfg.LDFlags
124+
}
125+
if cfg.GCSet {
126+
cfg.Logger.Info("Using custom gcflags", zap.String("gcflags", cfg.GCFlags))
127+
gcflags = cfg.GCFlags
128+
}
121129
}
130+
122131
args = append(args, "-ldflags="+ldflags)
132+
args = append(args, "-gcflags="+gcflags)
133+
123134
if cfg.Distribution.BuildTags != "" {
124135
args = append(args, "-tags", cfg.Distribution.BuildTags)
125136
}

cmd/builder/internal/builder/main_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,10 +246,22 @@ func TestGenerateAndCompile(t *testing.T) {
246246
cfg := newTestConfig(t)
247247
cfg.Distribution.OutputPath = t.TempDir()
248248
cfg.Replaces = append(cfg.Replaces, replaces...)
249+
cfg.LDSet = true
249250
cfg.LDFlags = `-X "test.gitVersion=0743dc6c6411272b98494a9b32a63378e84c34da" -X "test.gitTag=local-testing" -X "test.goVersion=go version go1.20.7 darwin/amd64"`
250251
return cfg
251252
},
252253
},
254+
{
255+
name: "GCFlags Compilation",
256+
cfgBuilder: func(t *testing.T) *Config {
257+
cfg := newTestConfig(t)
258+
cfg.Distribution.OutputPath = t.TempDir()
259+
cfg.Replaces = append(cfg.Replaces, replaces...)
260+
cfg.GCSet = true
261+
cfg.GCFlags = `all=-N -l`
262+
return cfg
263+
},
264+
},
253265
{
254266
name: "Build Tags Compilation",
255267
cfgBuilder: func(t *testing.T) *Config {

cmd/builder/internal/command.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ const (
2727
skipGetModulesFlag = "skip-get-modules"
2828
skipStrictVersioningFlag = "skip-strict-versioning"
2929
ldflagsFlag = "ldflags"
30+
gcflagsFlag = "gcflags"
3031
distributionOutputPathFlag = "output-path"
3132
verboseFlag = "verbose"
3233
)
@@ -84,6 +85,7 @@ func initFlags(flags *flag.FlagSet) error {
8485
flags.Bool(skipStrictVersioningFlag, true, "Whether builder should skip strictly checking the calculated versions following dependency resolution")
8586
flags.Bool(verboseFlag, false, "Whether builder should print verbose output (default false)")
8687
flags.String(ldflagsFlag, "", `ldflags to include in the "go build" command`)
88+
flags.String(gcflagsFlag, "", `gcflags to include in the "go build" command`)
8789
flags.String(distributionOutputPathFlag, "", "Where to write the resulting files")
8890
return flags.MarkDeprecated(distributionOutputPathFlag, "use config distribution::output_path")
8991
}
@@ -145,8 +147,17 @@ func applyFlags(flags *flag.FlagSet, cfg *builder.Config) error {
145147
cfg.SkipStrictVersioning, err = flags.GetBool(skipStrictVersioningFlag)
146148
errs = multierr.Append(errs, err)
147149

148-
cfg.LDFlags, err = flags.GetString(ldflagsFlag)
149-
errs = multierr.Append(errs, err)
150+
if flags.Changed(ldflagsFlag) {
151+
cfg.LDSet = true
152+
cfg.LDFlags, err = flags.GetString(ldflagsFlag)
153+
errs = multierr.Append(errs, err)
154+
}
155+
if flags.Changed(gcflagsFlag) {
156+
cfg.GCSet = true
157+
cfg.GCFlags, err = flags.GetString(gcflagsFlag)
158+
errs = multierr.Append(errs, err)
159+
}
160+
150161
cfg.Verbose, err = flags.GetBool(verboseFlag)
151162
errs = multierr.Append(errs, err)
152163

cmd/builder/internal/command_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,14 @@ func TestApplyFlags(t *testing.T) {
6969
},
7070
{
7171
name: "All flag values",
72-
flags: []string{"--skip-generate=true", "--skip-compilation=true", "--skip-get-modules=true", "--skip-strict-versioning=true", "--ldflags=test", "--verbose=true"},
72+
flags: []string{"--skip-generate=true", "--skip-compilation=true", "--skip-get-modules=true", "--skip-strict-versioning=true", "--ldflags=test", "--gcflags=test", "--verbose=true"},
7373
want: &builder.Config{
7474
SkipGenerate: true,
7575
SkipCompilation: true,
7676
SkipGetModules: true,
7777
SkipStrictVersioning: true,
7878
LDFlags: "test",
79+
GCFlags: "test",
7980
Verbose: true,
8081
},
8182
},

0 commit comments

Comments
 (0)