feat: return an error if tbs config fails (#18379)

* 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

* test: validate expected error message

make sure the error in tbs config tests is expected

Author: kruskall

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
 }

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.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) {
 		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.EqualError(t, err, "error processing configuration: invalid sampling.tail config: no default (empty criteria) policy specified accessing 'sampling.tail'")
+		assert.Nil(t, c)
 	})
 }