From 2d545e0155141da466626b1fc9785cdc47f9e2ba Mon Sep 17 00:00:00 2001 From: BEN GHORBEL Oussama Date: Mon, 30 Dec 2024 18:13:38 +0100 Subject: [PATCH 1/4] [chore] Ability to provide custom ld and gc flags --- cmd/builder/internal/builder/config.go | 3 ++- cmd/builder/internal/builder/config_test.go | 2 ++ cmd/builder/internal/builder/main.go | 19 +++++++++++++++---- cmd/builder/internal/builder/main_test.go | 10 ++++++++++ cmd/builder/internal/command.go | 4 ++++ 5 files changed, 33 insertions(+), 5 deletions(-) diff --git a/cmd/builder/internal/builder/config.go b/cmd/builder/internal/builder/config.go index cb83375316b..65317d42033 100644 --- a/cmd/builder/internal/builder/config.go +++ b/cmd/builder/internal/builder/config.go @@ -35,6 +35,7 @@ type Config struct { SkipGetModules bool `mapstructure:"-"` SkipStrictVersioning bool `mapstructure:"-"` LDFlags string `mapstructure:"-"` + GCFlags string `mapstructure:"-"` Verbose bool `mapstructure:"-"` Distribution Distribution `mapstructure:"dist"` @@ -71,7 +72,7 @@ type Distribution struct { OutputPath string `mapstructure:"output_path"` Version string `mapstructure:"version"` BuildTags string `mapstructure:"build_tags"` - DebugCompilation bool `mapstructure:"debug_compilation"` + DebugCompilation bool `mapstructure:"debug_compilation"` // TODO depercate? } // Module represents a receiver, exporter, processor or extension for the distribution diff --git a/cmd/builder/internal/builder/config_test.go b/cmd/builder/internal/builder/config_test.go index 14040dd50aa..014569ac659 100644 --- a/cmd/builder/internal/builder/config_test.go +++ b/cmd/builder/internal/builder/config_test.go @@ -179,6 +179,8 @@ func TestNewDefaultConfig(t *testing.T) { require.NoError(t, cfg.Validate()) assert.False(t, cfg.Distribution.DebugCompilation) assert.Empty(t, cfg.Distribution.BuildTags) + assert.Empty(t, cfg.LDFlags) + assert.Empty(t, cfg.GCFlags) } func TestNewBuiltinConfig(t *testing.T) { diff --git a/cmd/builder/internal/builder/main.go b/cmd/builder/internal/builder/main.go index 197d1038613..2f570fc7fc7 100644 --- a/cmd/builder/internal/builder/main.go +++ b/cmd/builder/internal/builder/main.go @@ -109,17 +109,28 @@ func Compile(cfg *Config) error { cfg.Logger.Info("Compiling") - ldflags := "-s -w" + var ldflags = "-s -w" // we strip the symbols by default for smaller binaries + var 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 len(cfg.LDFlags) > 0 { + cfg.Logger.Info("Using custom ldflags", zap.String("ldflags", cfg.LDFlags)) + ldflags = cfg.LDFlags + } + if len(cfg.GCFlags) > 0 { + 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) } diff --git a/cmd/builder/internal/builder/main_test.go b/cmd/builder/internal/builder/main_test.go index 8220e48df85..4b02a02b416 100644 --- a/cmd/builder/internal/builder/main_test.go +++ b/cmd/builder/internal/builder/main_test.go @@ -251,6 +251,16 @@ func TestGenerateAndCompile(t *testing.T) { 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.GCFlags = `all=-N -l` + return cfg + }, + }, { name: "Build Tags Compilation", cfgBuilder: func(t *testing.T) *Config { diff --git a/cmd/builder/internal/command.go b/cmd/builder/internal/command.go index fa1fd235233..c64316fbd2f 100644 --- a/cmd/builder/internal/command.go +++ b/cmd/builder/internal/command.go @@ -27,6 +27,7 @@ const ( skipGetModulesFlag = "skip-get-modules" skipStrictVersioningFlag = "skip-strict-versioning" ldflagsFlag = "ldflags" + gcflagsFlag = "gcflags" distributionOutputPathFlag = "output-path" verboseFlag = "verbose" ) @@ -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") } @@ -147,6 +149,8 @@ func applyFlags(flags *flag.FlagSet, cfg *builder.Config) error { cfg.LDFlags, err = flags.GetString(ldflagsFlag) errs = multierr.Append(errs, err) + cfg.GCFlags, err = flags.GetString(gcflagsFlag) + errs = multierr.Append(errs, err) cfg.Verbose, err = flags.GetBool(verboseFlag) errs = multierr.Append(errs, err) From e1a0d1f91f8a8ebca0b8916199be76ab4edac606 Mon Sep 17 00:00:00 2001 From: BEN GHORBEL Oussama Date: Tue, 31 Dec 2024 16:04:37 +0100 Subject: [PATCH 2/4] [chore] Update README.md --- cmd/builder/README.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/cmd/builder/README.md b/cmd/builder/README.md index bdb97243661..425f8590b62 100644 --- a/cmd/builder/README.md +++ b/cmd/builder/README.md @@ -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: From ee3515348b0034ce9c882c9b90bcff326aea8c2f Mon Sep 17 00:00:00 2001 From: BEN GHORBEL Oussama Date: Tue, 31 Dec 2024 17:17:16 +0100 Subject: [PATCH 3/4] [chore] detect the presence of ld/gc flags. --- cmd/builder/README.md | 4 ++-- cmd/builder/internal/builder/config.go | 2 ++ cmd/builder/internal/builder/config_test.go | 2 ++ cmd/builder/internal/builder/main.go | 4 ++-- cmd/builder/internal/command.go | 17 ++++++++++++----- 5 files changed, 20 insertions(+), 9 deletions(-) diff --git a/cmd/builder/README.md b/cmd/builder/README.md index 425f8590b62..9f63c66b8cf 100644 --- a/cmd/builder/README.md +++ b/cmd/builder/README.md @@ -115,7 +115,7 @@ Use `ocb --help` to learn about which flags are available. 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. +ocb --ldflags="" --config=builder-config.yaml. ``` ### Debugging with Delve @@ -123,7 +123,7 @@ ocb --ldflags=" " --config=builder-config.yaml. 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.` +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: diff --git a/cmd/builder/internal/builder/config.go b/cmd/builder/internal/builder/config.go index 65317d42033..60dc4540b06 100644 --- a/cmd/builder/internal/builder/config.go +++ b/cmd/builder/internal/builder/config.go @@ -35,7 +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"` diff --git a/cmd/builder/internal/builder/config_test.go b/cmd/builder/internal/builder/config_test.go index 014569ac659..8cd02c0107c 100644 --- a/cmd/builder/internal/builder/config_test.go +++ b/cmd/builder/internal/builder/config_test.go @@ -179,7 +179,9 @@ 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) } diff --git a/cmd/builder/internal/builder/main.go b/cmd/builder/internal/builder/main.go index 2f570fc7fc7..746211d37b5 100644 --- a/cmd/builder/internal/builder/main.go +++ b/cmd/builder/internal/builder/main.go @@ -118,11 +118,11 @@ func Compile(cfg *Config) error { ldflags = cfg.LDFlags gcflags = "all=-N -l" } else { - if len(cfg.LDFlags) > 0 { + if cfg.LDSet { cfg.Logger.Info("Using custom ldflags", zap.String("ldflags", cfg.LDFlags)) ldflags = cfg.LDFlags } - if len(cfg.GCFlags) > 0 { + if cfg.GCSet { cfg.Logger.Info("Using custom gcflags", zap.String("gcflags", cfg.GCFlags)) gcflags = cfg.GCFlags } diff --git a/cmd/builder/internal/command.go b/cmd/builder/internal/command.go index c64316fbd2f..aa90b7d66f2 100644 --- a/cmd/builder/internal/command.go +++ b/cmd/builder/internal/command.go @@ -27,7 +27,7 @@ const ( skipGetModulesFlag = "skip-get-modules" skipStrictVersioningFlag = "skip-strict-versioning" ldflagsFlag = "ldflags" - gcflagsFlag = "gcflags" + gcflagsFlag = "gcflags" distributionOutputPathFlag = "output-path" verboseFlag = "verbose" ) @@ -147,10 +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) - cfg.GCFlags, err = flags.GetString(gcflagsFlag) - 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) From 6375e02b27c84ca0ec9f72b6cb3519e98582b072 Mon Sep 17 00:00:00 2001 From: BEN GHORBEL Oussama Date: Tue, 7 Jan 2025 16:55:21 +0100 Subject: [PATCH 4/4] [chore] update files --- cmd/builder/internal/builder/config.go | 4 ++-- cmd/builder/internal/builder/main.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/builder/internal/builder/config.go b/cmd/builder/internal/builder/config.go index 60dc4540b06..d001c06d370 100644 --- a/cmd/builder/internal/builder/config.go +++ b/cmd/builder/internal/builder/config.go @@ -35,9 +35,9 @@ type Config struct { SkipGetModules bool `mapstructure:"-"` SkipStrictVersioning bool `mapstructure:"-"` LDFlags string `mapstructure:"-"` - LDSet bool `mapstructure:"-"` // only used to override LDFlags + LDSet bool `mapstructure:"-"` // only used to override LDFlags GCFlags string `mapstructure:"-"` - GCSet bool `mapstructure:"-"` // only used to override GCFlags + GCSet bool `mapstructure:"-"` // only used to override GCFlags Verbose bool `mapstructure:"-"` Distribution Distribution `mapstructure:"dist"` diff --git a/cmd/builder/internal/builder/main.go b/cmd/builder/internal/builder/main.go index 746211d37b5..ad46d98688b 100644 --- a/cmd/builder/internal/builder/main.go +++ b/cmd/builder/internal/builder/main.go @@ -109,8 +109,8 @@ func Compile(cfg *Config) error { cfg.Logger.Info("Compiling") - var ldflags = "-s -w" // we strip the symbols by default for smaller binaries - var gcflags = "" + 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 { @@ -127,7 +127,7 @@ func Compile(cfg *Config) error { gcflags = cfg.GCFlags } } - + args = append(args, "-ldflags="+ldflags) args = append(args, "-gcflags="+gcflags)