From 99f3476b4678070cdd1e5827c99026f3569ac86c Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Thu, 28 Aug 2025 16:11:57 +0200 Subject: [PATCH 1/2] feat: return an error if tbs config fails failure to process the tbs config will log a message and silently fallback to the default config with tbs disabled. This is not ideal and doesn't match the behaviour of other configs which return an error to the caller. Update the tbs config logic to return an error if processing fails Update test to assert error on malformed configs --- internal/beater/config/sampling.go | 19 ++++--------------- internal/beater/config/sampling_test.go | 8 ++++---- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/internal/beater/config/sampling.go b/internal/beater/config/sampling.go index 7bd9f30a8fc..7c1c02caed0 100644 --- a/internal/beater/config/sampling.go +++ b/internal/beater/config/sampling.go @@ -25,7 +25,6 @@ import ( "github.com/dustin/go-humanize" "github.com/elastic/apm-server/internal/elasticsearch" - "github.com/elastic/apm-server/internal/logs" "github.com/elastic/elastic-agent-libs/config" "github.com/elastic/elastic-agent-libs/logp" ) @@ -89,31 +88,21 @@ type TailSamplingPolicy struct { } func (c *TailSamplingConfig) Unpack(in *config.C) error { - var err error - defer func() { - if err != nil { - logger := logp.NewLogger(logs.Config) - logger.Errorf("failed to setup tail sampling: %v", err) - logger.Info("continuing with tail sampling disabled") - *c = TailSamplingConfig(defaultTailSamplingConfig()) - } - }() type tailSamplingConfig TailSamplingConfig cfg := tailSamplingConfig(defaultTailSamplingConfig()) - if err = in.Unpack(&cfg); err != nil { - err = fmt.Errorf("error unpacking config: %w", err) - return nil + if err := in.Unpack(&cfg); err != nil { + return fmt.Errorf("error unpacking sampling.tail config: %w", err) } limit, err := humanize.ParseBytes(cfg.StorageLimit) if err != nil { - return err + return fmt.Errorf("error parsing storage limit: %w", err) } cfg.StorageLimitParsed = limit cfg.Enabled = in.Enabled() *c = TailSamplingConfig(cfg) c.esConfigured = in.HasField("elasticsearch") if validateErr := c.Validate(); validateErr != nil { - err = fmt.Errorf("invalid config: %w", validateErr) + return fmt.Errorf("invalid sampling.tail config: %w", validateErr) } return nil } diff --git a/internal/beater/config/sampling_test.go b/internal/beater/config/sampling_test.go index 854681d24d2..77c6a989cac 100644 --- a/internal/beater/config/sampling_test.go +++ b/internal/beater/config/sampling_test.go @@ -39,8 +39,8 @@ func TestSamplingPoliciesValidation(t *testing.T) { c, err := NewConfig(config.MustNewConfigFrom(map[string]interface{}{ "sampling.tail.enabled": true, }), nil, logptest.NewTestingLogger(t, "")) - assert.NoError(t, err) - assert.False(t, c.Sampling.Tail.Enabled) + assert.Error(t, err) + assert.Nil(t, c) }) t.Run("NoDefaultPolicies", func(t *testing.T) { c, err := NewConfig(config.MustNewConfigFrom(map[string]interface{}{ @@ -49,7 +49,7 @@ func TestSamplingPoliciesValidation(t *testing.T) { "sample_rate": 0.5, }}, }), nil, logptest.NewTestingLogger(t, "")) - assert.NoError(t, err) - assert.False(t, c.Sampling.Tail.Enabled) + assert.Error(t, err) + assert.Nil(t, c) }) } From 82d3299ee3a76b6dd91bb87646b68e475417e383 Mon Sep 17 00:00:00 2001 From: kruskal <99559985+kruskall@users.noreply.github.com> Date: Thu, 11 Sep 2025 20:17:31 +0200 Subject: [PATCH 2/2] test: validate expected error message make sure the error in tbs config tests is expected --- internal/beater/config/sampling_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/beater/config/sampling_test.go b/internal/beater/config/sampling_test.go index 77c6a989cac..e5e84e7a969 100644 --- a/internal/beater/config/sampling_test.go +++ b/internal/beater/config/sampling_test.go @@ -39,7 +39,7 @@ func TestSamplingPoliciesValidation(t *testing.T) { c, err := NewConfig(config.MustNewConfigFrom(map[string]interface{}{ "sampling.tail.enabled": true, }), nil, logptest.NewTestingLogger(t, "")) - assert.Error(t, err) + assert.EqualError(t, err, "error processing configuration: invalid sampling.tail config: no policies specified accessing 'sampling.tail'") assert.Nil(t, c) }) t.Run("NoDefaultPolicies", func(t *testing.T) { @@ -49,7 +49,7 @@ func TestSamplingPoliciesValidation(t *testing.T) { "sample_rate": 0.5, }}, }), nil, logptest.NewTestingLogger(t, "")) - assert.Error(t, err) + assert.EqualError(t, err, "error processing configuration: invalid sampling.tail config: no default (empty criteria) policy specified accessing 'sampling.tail'") assert.Nil(t, c) }) }