From 1ba29eaf1c118d5ae1e1270e4c46dc784fbef6d8 Mon Sep 17 00:00:00 2001 From: William Dumont Date: Mon, 18 Nov 2024 16:55:42 +0100 Subject: [PATCH] fix key/pattern logic for the attribute processor (#2124) --- CHANGELOG.md | 2 + .../component/otelcol/config_attraction.go | 46 +++++++++++++++- .../otelcol/config_attraction_test.go | 54 +++++++++++++++++++ .../processor/attributes/attributes.go | 4 ++ .../processor/attributes/attributes_test.go | 17 ++++++ 5 files changed, 121 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69682dd19c..ea9f8dd8ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,8 @@ Main (unreleased) - Fixed issue with reloading configuration and prometheus metrics duplication in `prometheus.write.queue`. (@mattdurham) +- Fixed an issue in the `otelcol.processor.attribute` component where the actions `delete` and `hash` could not be used with the `pattern` argument. (@wildum) + ### Other changes - Change the stability of the `livedebugging` feature from "experimental" to "generally available". (@wildum) diff --git a/internal/component/otelcol/config_attraction.go b/internal/component/otelcol/config_attraction.go index b009397112..b6c8b4082c 100644 --- a/internal/component/otelcol/config_attraction.go +++ b/internal/component/otelcol/config_attraction.go @@ -1,5 +1,16 @@ package otelcol +import ( + "errors" + "fmt" + "strings" +) + +const ( + delete = "delete" + hash = "hash" +) + type AttrActionKeyValueSlice []AttrActionKeyValue func (actions AttrActionKeyValueSlice) Convert() []interface{} { @@ -15,10 +26,27 @@ func (actions AttrActionKeyValueSlice) Convert() []interface{} { return res } +func (actions AttrActionKeyValueSlice) Validate() error { + var validationErrors []error + + for i, action := range actions { + if err := action.validate(); err != nil { + wrappedErr := fmt.Errorf("validation failed for action block number %d: %w", i+1, err) + validationErrors = append(validationErrors, wrappedErr) + } + } + + if len(validationErrors) > 0 { + return errors.Join(validationErrors...) + } + return nil +} + type AttrActionKeyValue struct { // Key specifies the attribute to act upon. - // This is a required field. - Key string `alloy:"key,attr"` + // The actions `delete` and `hash` can use the `pattern`` argument instead of/with the `key` argument. + // The field is required for all other actions. + Key string `alloy:"key,attr,optional"` // Value specifies the value to populate for the key. // The type of the value is inferred from the configuration. @@ -91,3 +119,17 @@ func (args *AttrActionKeyValue) convert() map[string]interface{} { "converted_type": args.ConvertedType, } } + +func (args *AttrActionKeyValue) validate() error { + switch strings.ToLower(args.Action) { + case delete, hash: + if args.Key == "" && args.RegexPattern == "" { + return fmt.Errorf("the action %s requires at least the key argument or the pattern argument to be set", args.Action) + } + default: + if args.Key == "" { + return fmt.Errorf("the action %s requires the key argument to be set", args.Action) + } + } + return nil +} diff --git a/internal/component/otelcol/config_attraction_test.go b/internal/component/otelcol/config_attraction_test.go index 5879bde815..3e6655b03f 100644 --- a/internal/component/otelcol/config_attraction_test.go +++ b/internal/component/otelcol/config_attraction_test.go @@ -1,6 +1,7 @@ package otelcol_test import ( + "strings" "testing" "github.com/grafana/alloy/internal/component/otelcol" @@ -58,3 +59,56 @@ func TestConvertAttrAction(t *testing.T) { result := inputActions.Convert() require.Equal(t, expectedActions, result) } + +func TestValidateAttrAction(t *testing.T) { + inputActions := otelcol.AttrActionKeyValueSlice{ + { + // ok - only key + Action: "insert", + Value: 123, + Key: "attribute1", + }, + { + // not ok - missing key + Action: "insert", + Value: 123, + RegexPattern: "pattern", // pattern is useless here + }, + { + // ok - only key + Action: "delete", + Key: "key", + }, + { + // ok - only pattern + Action: "delete", + RegexPattern: "pattern", + }, + { + // ok - both + Action: "delete", + Key: "key", + RegexPattern: "pattern", + }, + { + // not ok - missing key and pattern + Action: "delete", + }, + { + // ok - only pattern + Action: "hash", + RegexPattern: "pattern", + }, + { + // ok - with uppercase + Action: "HaSH", + RegexPattern: "pattern", + }, + } + + expectedErrors := []string{ + "validation failed for action block number 2: the action insert requires the key argument to be set", + "validation failed for action block number 6: the action delete requires at least the key argument or the pattern argument to be set", + } + require.EqualError(t, inputActions.Validate(), strings.Join(expectedErrors, "\n")) +} diff --git a/internal/component/otelcol/processor/attributes/attributes.go b/internal/component/otelcol/processor/attributes/attributes.go index 48495b3152..4301f77927 100644 --- a/internal/component/otelcol/processor/attributes/attributes.go +++ b/internal/component/otelcol/processor/attributes/attributes.go @@ -55,6 +55,10 @@ func (args *Arguments) SetToDefault() { args.DebugMetrics.SetToDefault() } +func (args *Arguments) Validate() error { + return args.Actions.Validate() +} + // Convert implements processor.Arguments. func (args Arguments) Convert() (otelcomponent.Config, error) { input := make(map[string]interface{}) diff --git a/internal/component/otelcol/processor/attributes/attributes_test.go b/internal/component/otelcol/processor/attributes/attributes_test.go index e2019982cd..6412730700 100644 --- a/internal/component/otelcol/processor/attributes/attributes_test.go +++ b/internal/component/otelcol/processor/attributes/attributes_test.go @@ -134,6 +134,23 @@ func testRunProcessorWithContext(ctx context.Context, t *testing.T, processorCon processortest.TestRunProcessor(prc) } +// Test that the validate function is called. The validation logic for the actions is tested in the otelcol pkg. +func Test_Validate(t *testing.T) { + cfg := ` + action { + value = 111111 + action = "insert" + } + + output { + // no-op: will be overridden by test code. + } + ` + expectedErr := "validation failed for action block number 1: the action insert requires the key argument to be set" + var args attributes.Arguments + require.ErrorContains(t, syntax.Unmarshal([]byte(cfg), &args), expectedErr) +} + func Test_Insert(t *testing.T) { cfg := ` action {