From c9d0c11d2852d4b05f8ac5727c0ba5d0246d2b00 Mon Sep 17 00:00:00 2001 From: Sam DeHaan Date: Thu, 12 Dec 2024 13:49:47 -0500 Subject: [PATCH] fix: Prevent the constant logging of cgroup errors on non-linux machines (#2264) * Prevent the constant logging of cgroup errors on non-linux machines * Update changelog * Document AUTOMEMLIMIT_EXPERIMENT * Add test --- CHANGELOG.md | 2 ++ .../reference/cli/environment-variables.md | 2 ++ go.mod | 2 +- go.sum | 4 +-- .../automemlimit_cgroups_unsupported.go | 25 ++++++++++++++++ internal/alloycli/automemlimit_linux.go | 12 ++++++++ .../alloycli/automemlimit_nonlinux_test.go | 30 +++++++++++++++++++ internal/alloycli/cmd_run.go | 6 ++-- 8 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 internal/alloycli/automemlimit_cgroups_unsupported.go create mode 100644 internal/alloycli/automemlimit_linux.go create mode 100644 internal/alloycli/automemlimit_nonlinux_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 733bc72dc3..9da7c5266b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,8 @@ Main (unreleased) ### Bugfixes +- Fixed issue with automemlimit logging bad messages and trying to access cgroup on non-linux builds (@dehaansa) + - Fixed issue with reloading configuration and prometheus metrics duplication in `prometheus.write.queue`. (@mattdurham) - Updated `prometheus.write.queue` to fix issue with TTL comparing different scales of time. (@mattdurham) diff --git a/docs/sources/reference/cli/environment-variables.md b/docs/sources/reference/cli/environment-variables.md index a0633188bc..c5eefcdc07 100644 --- a/docs/sources/reference/cli/environment-variables.md +++ b/docs/sources/reference/cli/environment-variables.md @@ -20,6 +20,7 @@ The following environment variables are supported: * `PPROF_BLOCK_PROFILING_RATE` * `GOMEMLIMIT` * `AUTOMEMLIMIT` +* `AUTOMEMLIMIT_EXPERIMENT` * `GOGC` * `GOMAXPROCS` * `GOTRACEBACK` @@ -80,6 +81,7 @@ For example, if you want to keep memory usage below `10GiB`, use `GOMEMLIMIT=9Gi The `GOMEMLIMIT` environment variable is either automatically set to 90% of an available `cgroup` value using the [`automemlimit`][automemlimit] module, or you can explicitly set the `GOMEMLIMIT` environment variable before you run {{< param "PRODUCT_NAME" >}}. You can also change the 90% ratio by setting the `AUTOMEMLIMIT` environment variable to a float value between `0` and `1.0`. No changes occur if the limit can't be determined and you didn't explicitly define a `GOMEMLIMIT` value. +The `AUTOMEMLIMIT_EXPERIMENT` variable can be set to `system` to use the [`automemlimit`][automemlimit] module's System provider, which sets `GOMEMLIMIT` based on the same ratio applied to the total system memory. As `cgroup` is a Linux specific concept, this is the only way to use the `automemlimit` module to automatically set `GOMEMLIMIT` on non-Linux OSes. ## GOGC diff --git a/go.mod b/go.mod index 4f1e607e2a..0c662da968 100644 --- a/go.mod +++ b/go.mod @@ -10,7 +10,7 @@ require ( github.com/Azure/go-autorest/autorest v0.11.29 github.com/DATA-DOG/go-sqlmock v1.5.2 github.com/IBM/sarama v1.43.3 - github.com/KimMachineGun/automemlimit v0.6.0 + github.com/KimMachineGun/automemlimit v0.6.1 github.com/Lusitaniae/apache_exporter v0.11.1-0.20220518131644-f9522724dab4 github.com/Masterminds/sprig/v3 v3.2.3 github.com/PuerkitoBio/rehttp v1.4.0 diff --git a/go.sum b/go.sum index f5ea529ff8..77ad0b47c0 100644 --- a/go.sum +++ b/go.sum @@ -917,8 +917,8 @@ github.com/IBM/sarama v1.43.3/go.mod h1:FVIRaLrhK3Cla/9FfRF5X9Zua2KpS3SYIXxhac1H github.com/Jeffail/gabs v1.1.0/go.mod h1:6xMvQMK4k33lb7GUUpaAPh6nKMmemQeg5d4gn7/bOXc= github.com/JohnCGriffin/overflow v0.0.0-20211019200055-46fa312c352c h1:RGWPOewvKIROun94nF7v2cua9qP+thov/7M50KEoeSU= github.com/JohnCGriffin/overflow v0.0.0-20211019200055-46fa312c352c/go.mod h1:X0CRv0ky0k6m906ixxpzmDRLvX58TFUKS2eePweuyxk= -github.com/KimMachineGun/automemlimit v0.6.0 h1:p/BXkH+K40Hax+PuWWPQ478hPjsp9h1CPDhLlA3Z37E= -github.com/KimMachineGun/automemlimit v0.6.0/go.mod h1:T7xYht7B8r6AG/AqFcUdc7fzd2bIdBKmepfP2S1svPY= +github.com/KimMachineGun/automemlimit v0.6.1 h1:ILa9j1onAAMadBsyyUJv5cack8Y1WT26yLj/V+ulKp8= +github.com/KimMachineGun/automemlimit v0.6.1/go.mod h1:T7xYht7B8r6AG/AqFcUdc7fzd2bIdBKmepfP2S1svPY= github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible/go.mod h1:r7JcOSlj0wfOMncg0iLm8Leh48TZaKVeNIfJntJ2wa0= github.com/Lusitaniae/apache_exporter v0.11.1-0.20220518131644-f9522724dab4 h1:UEm6tMvLH2t2honcWG0q+h0qr/60++9cuh/fy7hLyMc= github.com/Lusitaniae/apache_exporter v0.11.1-0.20220518131644-f9522724dab4/go.mod h1:IfUbHkoXypdKnVGHWQ3DLRRRZzIU/JIExQAd9xgxRfw= diff --git a/internal/alloycli/automemlimit_cgroups_unsupported.go b/internal/alloycli/automemlimit_cgroups_unsupported.go new file mode 100644 index 0000000000..e61cfddbd2 --- /dev/null +++ b/internal/alloycli/automemlimit_cgroups_unsupported.go @@ -0,0 +1,25 @@ +//go:build !linux +// +build !linux + +package alloycli + +import ( + "log/slog" + "os" + "slices" + "strings" + + "github.com/KimMachineGun/automemlimit/memlimit" + "github.com/grafana/alloy/internal/runtime/logging" +) + +func applyAutoMemLimit(l *logging.Logger) { + // For non-linux builds without cgroups, memlimit will always report an error. + // However, if the system experiment is requested, we can use the system memory limit provider. + // This logic is similar to https://github.com/KimMachineGun/automemlimit/blob/main/memlimit/experiment.go + if v, ok := os.LookupEnv("AUTOMEMLIMIT_EXPERIMENT"); ok { + if slices.Contains(strings.Split(v, ","), "system") { + memlimit.SetGoMemLimitWithOpts(memlimit.WithProvider(memlimit.FromSystem), memlimit.WithLogger(slog.New(l.Handler()))) + } + } +} diff --git a/internal/alloycli/automemlimit_linux.go b/internal/alloycli/automemlimit_linux.go new file mode 100644 index 0000000000..148c160a46 --- /dev/null +++ b/internal/alloycli/automemlimit_linux.go @@ -0,0 +1,12 @@ +package alloycli + +import ( + "log/slog" + + "github.com/KimMachineGun/automemlimit/memlimit" + "github.com/grafana/alloy/internal/runtime/logging" +) + +func applyAutoMemLimit(l *logging.Logger) { + memlimit.SetGoMemLimitWithOpts(memlimit.WithLogger(slog.New(l.Handler()))) +} diff --git a/internal/alloycli/automemlimit_nonlinux_test.go b/internal/alloycli/automemlimit_nonlinux_test.go new file mode 100644 index 0000000000..6b6fefaaf9 --- /dev/null +++ b/internal/alloycli/automemlimit_nonlinux_test.go @@ -0,0 +1,30 @@ +//go:build !linux +// +build !linux + +package alloycli + +import ( + "bytes" + "log/slog" + "testing" + + "github.com/KimMachineGun/automemlimit/memlimit" + "github.com/grafana/alloy/internal/runtime/logging" + "github.com/stretchr/testify/require" +) + +func TestNoMemlimitErrorLogs(t *testing.T) { + buffer := bytes.NewBuffer(nil) + + l, err := logging.New(buffer, logging.DefaultOptions) + require.NoError(t, err) + + applyAutoMemLimit(l) + + require.Equal(t, "", buffer.String()) + + // Linux behavior, to confirm error is logged + memlimit.SetGoMemLimitWithOpts(memlimit.WithLogger(slog.New(l.Handler()))) + + require.Contains(t, buffer.String(), "cgroups is not supported on this system") +} diff --git a/internal/alloycli/cmd_run.go b/internal/alloycli/cmd_run.go index 43f6768255..7ef521ed19 100644 --- a/internal/alloycli/cmd_run.go +++ b/internal/alloycli/cmd_run.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "io/fs" - "log/slog" "os" "os/signal" "path/filepath" @@ -16,7 +15,6 @@ import ( "syscall" "time" - "github.com/KimMachineGun/automemlimit/memlimit" "github.com/fatih/color" "github.com/go-kit/log" "github.com/grafana/ckit/advertise" @@ -221,8 +219,8 @@ func (fr *alloyRun) Run(cmd *cobra.Command, configPath string) error { level.Info(l).Log("boringcrypto enabled", boringcrypto.Enabled) // Set the memory limit, this will honor GOMEMLIMIT if set - // If there is a cgroup will follow that - memlimit.SetGoMemLimitWithOpts(memlimit.WithLogger(slog.New(l.Handler()))) + // If there is a cgroup on linux it will use that + applyAutoMemLimit(l) // Enable the profiling. setMutexBlockProfiling(l)