From f6d99c5aed5ba0a700edbac8ff32c28814485f99 Mon Sep 17 00:00:00 2001 From: rahulkaukuntla Date: Tue, 25 Feb 2025 10:29:19 -0500 Subject: [PATCH 1/9] pre-incident code --- comp/logs/agent/config/go.mod | 4 +- comp/logs/agent/config/integration_config.go | 61 +++-- comp/logs/agent/config/parser.go | 24 +- comp/logs/agent/config/parser_test.go | 240 ++++++++++++++---- comp/logs/agent/config/processing_rules.go | 2 +- .../inventorychecks_test.go | 2 +- .../container/tailerfactory/file_test.go | 4 +- 7 files changed, 241 insertions(+), 96 deletions(-) diff --git a/comp/logs/agent/config/go.mod b/comp/logs/agent/config/go.mod index 6d0e62f01fd7f..ff0bfd1d6a16d 100644 --- a/comp/logs/agent/config/go.mod +++ b/comp/logs/agent/config/go.mod @@ -11,9 +11,9 @@ require ( github.com/DataDog/datadog-agent/pkg/util/fxutil v0.61.0 github.com/DataDog/datadog-agent/pkg/util/log v0.64.0-devel github.com/DataDog/datadog-agent/pkg/util/pointer v0.61.0 - github.com/DataDog/viper v1.14.0 github.com/stretchr/testify v1.10.0 go.uber.org/fx v1.23.0 + gopkg.in/yaml.v3 v3.0.1 ) require ( @@ -37,6 +37,7 @@ require ( github.com/DataDog/datadog-agent/pkg/util/system/socket v0.61.0 // indirect github.com/DataDog/datadog-agent/pkg/util/winutil v0.61.0 // indirect github.com/DataDog/datadog-agent/pkg/version v0.62.3 // indirect + github.com/DataDog/viper v1.14.0 // indirect github.com/Microsoft/go-winio v0.6.2 // indirect github.com/cihub/seelog v0.0.0-20170130134532-f561c5e57575 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect @@ -72,7 +73,6 @@ require ( golang.org/x/sys v0.30.0 // indirect golang.org/x/text v0.22.0 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect ) // This section was automatically added by 'invoke modules.add-all-replace' command, do not edit manually diff --git a/comp/logs/agent/config/integration_config.go b/comp/logs/agent/config/integration_config.go index ff6f96ecfafb6..6b4718505262c 100644 --- a/comp/logs/agent/config/integration_config.go +++ b/comp/logs/agent/config/integration_config.go @@ -42,22 +42,22 @@ type LogsConfig struct { IntegrationName string Port int // Network - IdleTimeout string `mapstructure:"idle_timeout" json:"idle_timeout"` // Network + IdleTimeout string `mapstructure:"idle_timeout" json:"idle_timeout" yaml:"idle_timeout"` // Network Path string // File, Journald - Encoding string `mapstructure:"encoding" json:"encoding"` // File - ExcludePaths []string `mapstructure:"exclude_paths" json:"exclude_paths"` // File - TailingMode string `mapstructure:"start_position" json:"start_position"` // File + Encoding string `mapstructure:"encoding" json:"encoding" yaml:"encoding"` // File + ExcludePaths []string `mapstructure:"exclude_paths" json:"exclude_paths" yaml:"exclude_paths"` // File + TailingMode string `mapstructure:"start_position" json:"start_position" yaml:"start_position"` // File //nolint:revive // TODO(AML) Fix revive linter - ConfigId string `mapstructure:"config_id" json:"config_id"` // Journald - IncludeSystemUnits []string `mapstructure:"include_units" json:"include_units"` // Journald - ExcludeSystemUnits []string `mapstructure:"exclude_units" json:"exclude_units"` // Journald - IncludeUserUnits []string `mapstructure:"include_user_units" json:"include_user_units"` // Journald - ExcludeUserUnits []string `mapstructure:"exclude_user_units" json:"exclude_user_units"` // Journald - IncludeMatches []string `mapstructure:"include_matches" json:"include_matches"` // Journald - ExcludeMatches []string `mapstructure:"exclude_matches" json:"exclude_matches"` // Journald - ContainerMode bool `mapstructure:"container_mode" json:"container_mode"` // Journald + ConfigId string `mapstructure:"config_id" json:"config_id" yaml:"config_id"` // Journald + IncludeSystemUnits []string `mapstructure:"include_units" json:"include_units" yaml:"include_units"` // Journald + ExcludeSystemUnits []string `mapstructure:"exclude_units" json:"exclude_units" yaml:"exclude_units"` // Journald + IncludeUserUnits []string `mapstructure:"include_user_units" json:"include_user_units" yaml:"include_user_units"` // Journald + ExcludeUserUnits []string `mapstructure:"exclude_user_units" json:"exclude_user_units" yaml:"exclude_user_units"` // Journald + IncludeMatches []string `mapstructure:"include_matches" json:"include_matches" yaml:"include_matches"` // Journald + ExcludeMatches []string `mapstructure:"exclude_matches" json:"exclude_matches" yaml:"exclude_matches"` // Journald + ContainerMode bool `mapstructure:"container_mode" json:"container_mode" yaml:"container_mode"` // Journald Image string // Docker Label string // Docker @@ -67,7 +67,7 @@ type LogsConfig struct { // determine the appropriate tags for the logs. Identifier string // Docker, File - ChannelPath string `mapstructure:"channel_path" json:"channel_path"` // Windows Event + ChannelPath string `mapstructure:"channel_path" json:"channel_path" yaml:"channel_path"` // Windows Event Query string // Windows Event // used as input only by the Channel tailer. @@ -84,14 +84,37 @@ type LogsConfig struct { Service string Source string SourceCategory string - Tags []string - ProcessingRules []*ProcessingRule `mapstructure:"log_processing_rules" json:"log_processing_rules"` + Tags TagsField + ProcessingRules []*ProcessingRule `mapstructure:"log_processing_rules" json:"log_processing_rules" yaml:"log_processing_rules"` // ProcessRawMessage is used to process the raw message instead of only the content part of the message. - ProcessRawMessage *bool `mapstructure:"process_raw_message" json:"process_raw_message"` + ProcessRawMessage *bool `mapstructure:"process_raw_message" json:"process_raw_message" yaml:"process_raw_message"` - AutoMultiLine *bool `mapstructure:"auto_multi_line_detection" json:"auto_multi_line_detection"` - AutoMultiLineSampleSize int `mapstructure:"auto_multi_line_sample_size" json:"auto_multi_line_sample_size"` - AutoMultiLineMatchThreshold float64 `mapstructure:"auto_multi_line_match_threshold" json:"auto_multi_line_match_threshold"` + AutoMultiLine *bool `mapstructure:"auto_multi_line_detection" json:"auto_multi_line_detection" yaml:"auto_multi_line_detection"` + AutoMultiLineSampleSize int `mapstructure:"auto_multi_line_sample_size" json:"auto_multi_line_sample_size" yaml:"auto_multi_line_sample_size"` + AutoMultiLineMatchThreshold float64 `mapstructure:"auto_multi_line_match_threshold" json:"auto_multi_line_match_threshold" yaml:"auto_multi_line_match_threshold"` +} + +// TagsField is a custom type for unmarshalling comma-separated string values or typical yaml fields into a slice of strings. +type TagsField []string + +// UnmarshalYAML is a custom unmarshalling function is needed for string array fields to split comma-separated values. +func (t *TagsField) UnmarshalYAML(unmarshal func(interface{}) error) error { + var str string + if err := unmarshal(&str); err == nil { + // note that we are intentionally avoiding the trimming of any spaces whilst splitting the string + *t = strings.Split(str, ",") + return nil + } + + var raw []interface{} + if err := unmarshal(&raw); err == nil { + for _, item := range raw { + if str, ok := item.(string); ok { + *t = append(*t, str) + } + } + } + return fmt.Errorf("invalid tags format") } // Dump dumps the contents of this struct to a string, for debugging purposes. diff --git a/comp/logs/agent/config/parser.go b/comp/logs/agent/config/parser.go index 9330ca5f8b9a8..19aad3cc4d4ea 100644 --- a/comp/logs/agent/config/parser.go +++ b/comp/logs/agent/config/parser.go @@ -6,13 +6,15 @@ package config import ( - "bytes" "encoding/json" "fmt" - - "github.com/DataDog/viper" + yaml "gopkg.in/yaml.v3" ) +type yamlLogsConfigsWrapper struct { + Logs []*LogsConfig +} + // ParseJSON parses the data formatted in JSON // returns an error if the parsing failed. func ParseJSON(data []byte) ([]*LogsConfig, error) { @@ -24,23 +26,13 @@ func ParseJSON(data []byte) ([]*LogsConfig, error) { return configs, nil } -const yaml = "yaml" -const logsPath = "logs" - // ParseYAML parses the data formatted in YAML, // returns an error if the parsing failed. func ParseYAML(data []byte) ([]*LogsConfig, error) { - var configs []*LogsConfig - var err error - v := viper.New() - v.SetConfigType(yaml) - err = v.ReadConfig(bytes.NewBuffer(data)) + var yamlConfigsWrapper yamlLogsConfigsWrapper + err := yaml.Unmarshal(data, &yamlConfigsWrapper) if err != nil { return nil, fmt.Errorf("could not decode YAML logs config: %v", err) } - err = v.UnmarshalKey(logsPath, &configs) - if err != nil { - return nil, fmt.Errorf("could not parse YAML logs config: %v", err) - } - return configs, nil + return yamlConfigsWrapper.Logs, nil } diff --git a/comp/logs/agent/config/parser_test.go b/comp/logs/agent/config/parser_test.go index cf0f4b49d631a..1114fd5d052a8 100644 --- a/comp/logs/agent/config/parser_test.go +++ b/comp/logs/agent/config/parser_test.go @@ -6,10 +6,10 @@ package config import ( - "strings" - "testing" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "regexp" + "testing" ) func TestParseJSONWithValidFormatShouldSucceed(t *testing.T) { @@ -27,7 +27,7 @@ func TestParseJSONWithValidFormatShouldSucceed(t *testing.T) { config = configs[0] assert.Equal(t, "any_source", config.Source) assert.Equal(t, "any_service", config.Service) - assert.Equal(t, []string{"a", "b:d"}, config.Tags) + assert.EqualValues(t, []string{"a", "b:d"}, config.Tags) configs, err = ParseJSON([]byte(`[{"source":"any_source","service":"any_service","log_processing_rules":[{"type":"multi_line","name":"numbers","pattern":"[0-9]"}]}]`)) assert.Nil(t, err) @@ -55,56 +55,6 @@ func TestParseJSONWithInvalidFormatShouldFail(t *testing.T) { } } -func TestParseYAMLWithValidFormatShouldSucceed(t *testing.T) { - data := []byte(` -logs: - - type: file - path: /var/log/app.log - tags: a, b:c - - type: udp - source: foo - service: bar - - type: docker - log_processing_rules: - - type: include_at_match - name: numbers - pattern: ^[0-9]+$ -`) - - configs, err := ParseYAML(data) - assert.Nil(t, err) - assert.Equal(t, 3, len(configs)) - - var config *LogsConfig - var tag string - var rule *ProcessingRule - - config = configs[0] - assert.Equal(t, FileType, config.Type) - assert.Equal(t, "/var/log/app.log", config.Path) - assert.Equal(t, 2, len(config.Tags)) - - tag = config.Tags[0] - assert.Equal(t, "a", strings.TrimSpace(tag)) - - tag = config.Tags[1] - assert.Equal(t, "b:c", strings.TrimSpace(tag)) - - config = configs[1] - assert.Equal(t, UDPType, config.Type) - assert.Equal(t, "foo", config.Source) - assert.Equal(t, "bar", config.Service) - - config = configs[2] - assert.Equal(t, DockerType, config.Type) - assert.Equal(t, 1, len(config.ProcessingRules)) - - rule = config.ProcessingRules[0] - assert.Equal(t, IncludeAtMatch, rule.Type) - assert.Equal(t, "numbers", rule.Name) - assert.Equal(t, "^[0-9]+$", rule.Pattern) -} - func TestParseYAMLWithInvalidFormatShouldFail(t *testing.T) { invalidFormats := []string{` foo: @@ -120,6 +70,186 @@ foo: for _, format := range invalidFormats { configs, _ := ParseYAML([]byte(format)) - assert.Equal(t, 0, len(configs)) + require.Equal(t, 0, len(configs)) + } +} + +func TestParseYAMLWithValidFormatShouldSucceed(t *testing.T) { + tests := []struct { + name string + yaml []byte + assert func(t *testing.T, configs []*LogsConfig, err error) + }{ + { + name: "Test 0: Parse with file logs and multiple tags", + yaml: []byte(` +logs: + - type: file + path: /var/log/app.log + tags: a, b:c + container_mode: false + auto_multi_line_detection: false + auto_multi_line_match_threshold: 3.0 + port: 8080 +`), + assert: func(t *testing.T, configs []*LogsConfig, err error) { + assert.Nil(t, err) + require.Equal(t, len(configs), 1) + config := configs[0] + assert.Equal(t, "file", config.Type) + assert.Equal(t, "/var/log/app.log", config.Path) + require.Equal(t, len(config.Tags), 2) + assert.Equal(t, "a", config.Tags[0]) + assert.Equal(t, " b:c", config.Tags[1]) + }, + }, + { + name: "Test 0.5: Same as Test 0, but without string separation", + yaml: []byte(` +logs: + - type: file + path: /var/log/app.log + tags: a,b:c + container_mode: false + auto_multi_line_detection: false + auto_multi_line_match_threshold: 3.0 + port: 8080 +`), + assert: func(t *testing.T, configs []*LogsConfig, err error) { + assert.Nil(t, err) + require.Equal(t, len(configs), 1) + config := configs[0] + assert.Equal(t, "file", config.Type) + assert.Equal(t, "/var/log/app.log", config.Path) + require.Equal(t, len(config.Tags), 2) + assert.Equal(t, "a", config.Tags[0]) + assert.Equal(t, "b:c", config.Tags[1]) + }, + }, + { + name: "Test 1: Parse with simple include_at_match processing rule", + yaml: []byte(` +logs: + - type: file + path: /my/test/file.log + service: cardpayment + source: java + log_processing_rules: + - type: include_at_match + name: include_datadoghq_users + pattern: \w+@datadoghq.com +`), + assert: func(t *testing.T, configs []*LogsConfig, err error) { + assert.Nil(t, err) + require.Equal(t, len(configs), 1) + config := configs[0] + assert.Equal(t, "file", config.Type) + assert.Equal(t, "/my/test/file.log", config.Path) + assert.Equal(t, "cardpayment", config.Service) + assert.Equal(t, "java", config.Source) + require.Equal(t, len(config.ProcessingRules), 1) + assert.Equal(t, "include_at_match", config.ProcessingRules[0].Type) + assert.Equal(t, "include_datadoghq_users", config.ProcessingRules[0].Name) + assert.Equal(t, `\w+@datadoghq.com`, config.ProcessingRules[0].Pattern) + _, err = regexp.Compile(config.ProcessingRules[0].Pattern) + assert.Nil(t, err, "Pattern should be a valid regex") + }, + }, + { + name: "Test 2: Parse with another pattern and include_at_match processing rule", + yaml: []byte(` +logs: + - type: file + path: /my/test/file.log + service: cardpayment + source: java + log_processing_rules: + - type: include_at_match + name: include_datadoghq_users + pattern: abc|123 +`), + assert: func(t *testing.T, configs []*LogsConfig, err error) { + assert.Nil(t, err) + require.Equal(t, len(configs), 1) + config := configs[0] + assert.Equal(t, "file", config.Type) + assert.Equal(t, "/my/test/file.log", config.Path) + assert.Equal(t, "cardpayment", config.Service) + assert.Equal(t, "java", config.Source) + require.Equal(t, len(config.ProcessingRules), 1) + assert.Equal(t, "include_at_match", config.ProcessingRules[0].Type) + assert.Equal(t, "include_datadoghq_users", config.ProcessingRules[0].Name) + assert.Equal(t, `abc|123`, config.ProcessingRules[0].Pattern) + _, err = regexp.Compile(config.ProcessingRules[0].Pattern) + assert.Nil(t, err, "Pattern should be a valid regex") + }, + }, + { + name: "Test 3: Parse with multi-line processing rule", + yaml: []byte(` +logs: + - type: file + path: /var/log/pg_log.log + service: database + source: postgresql + log_processing_rules: + - type: multi_line + name: new_log_start_with_date + pattern: \d{4}\-(0?[1-9]|1[012])\-(0?[1-9]|[12][0-9]|3[01]) +`), + assert: func(t *testing.T, configs []*LogsConfig, err error) { + assert.Nil(t, err) + require.Equal(t, len(configs), 1) + config := configs[0] + assert.Equal(t, "file", config.Type) + assert.Equal(t, "/var/log/pg_log.log", config.Path) + assert.Equal(t, "database", config.Service) + assert.Equal(t, "postgresql", config.Source) + require.Equal(t, len(config.ProcessingRules), 1) + assert.Equal(t, "multi_line", config.ProcessingRules[0].Type) + assert.Equal(t, "new_log_start_with_date", config.ProcessingRules[0].Name) + assert.Equal(t, `\d{4}\-(0?[1-9]|1[012])\-(0?[1-9]|[12][0-9]|3[01])`, config.ProcessingRules[0].Pattern) + _, err = regexp.Compile(config.ProcessingRules[0].Pattern) + assert.Nil(t, err, "Pattern should be a valid regex") + }, + }, + { + name: "Test 4: Parse with mask_sequences processing rule", + yaml: []byte(` +logs: + - type: file + path: /my/test/file.log + service: cardpayment + source: java + log_processing_rules: + - type: mask_sequences + name: mask_credit_cards + replace_placeholder: "[masked_credit_card]" + pattern: (?:4[0-9]{12}(?:[0-9]{3})?|[25][1-7][0-9]{14}|6(?:011|5[0-9][0-9])[0-9]{12}|3[47][0-9]{13}|3(?:0[0-5]|[68][0-9])[0-9]{11}|(?:2131|1800|35\d{3})\d{11}) +`), + assert: func(t *testing.T, configs []*LogsConfig, err error) { + assert.Nil(t, err) + require.Equal(t, len(configs), 1) + config := configs[0] + assert.Equal(t, "file", config.Type) + assert.Equal(t, "/my/test/file.log", config.Path) + assert.Equal(t, "cardpayment", config.Service) + assert.Equal(t, "java", config.Source) + require.Equal(t, len(config.ProcessingRules), 1) + assert.Equal(t, "mask_sequences", config.ProcessingRules[0].Type) + assert.Equal(t, "mask_credit_cards", config.ProcessingRules[0].Name) + assert.Equal(t, "[masked_credit_card]", config.ProcessingRules[0].ReplacePlaceholder) + assert.Equal(t, `(?:4[0-9]{12}(?:[0-9]{3})?|[25][1-7][0-9]{14}|6(?:011|5[0-9][0-9])[0-9]{12}|3[47][0-9]{13}|3(?:0[0-5]|[68][0-9])[0-9]{11}|(?:2131|1800|35\d{3})\d{11})`, config.ProcessingRules[0].Pattern) + _, err = regexp.Compile(config.ProcessingRules[0].Pattern) + assert.Nil(t, err, "Pattern should be a valid regex") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + configs, err := ParseYAML(tt.yaml) + tt.assert(t, configs, err) + }) } } diff --git a/comp/logs/agent/config/processing_rules.go b/comp/logs/agent/config/processing_rules.go index 0a5d946a7909f..b42161d0134c0 100644 --- a/comp/logs/agent/config/processing_rules.go +++ b/comp/logs/agent/config/processing_rules.go @@ -23,7 +23,7 @@ const ( type ProcessingRule struct { Type string Name string - ReplacePlaceholder string `mapstructure:"replace_placeholder" json:"replace_placeholder"` + ReplacePlaceholder string `mapstructure:"replace_placeholder" json:"replace_placeholder" yaml:"replace_placeholder"` Pattern string // TODO: should be moved out Regex *regexp.Regexp diff --git a/comp/metadata/inventorychecks/inventorychecksimpl/inventorychecks_test.go b/comp/metadata/inventorychecks/inventorychecksimpl/inventorychecks_test.go index 535833cd53e6c..b981d68f97d3b 100644 --- a/comp/metadata/inventorychecks/inventorychecksimpl/inventorychecks_test.go +++ b/comp/metadata/inventorychecks/inventorychecksimpl/inventorychecks_test.go @@ -249,7 +249,7 @@ func TestGetPayload(t *testing.T) { assert.Equal(t, expectedSourceStatus, actualSource[0]["state"]) assert.Equal(t, "awesome_cache", actualSource[0]["service"]) assert.Equal(t, "redis", actualSource[0]["source"]) - assert.Equal(t, []string{"env:prod"}, actualSource[0]["tags"]) + assert.Equal(t, logConfig.TagsField{"env:prod"}, actualSource[0]["tags"]) } else { assert.Len(t, p.LogsMetadata, 0) } diff --git a/pkg/logs/launchers/container/tailerfactory/file_test.go b/pkg/logs/launchers/container/tailerfactory/file_test.go index 2bb3292a83ad5..c049dccf6afb1 100644 --- a/pkg/logs/launchers/container/tailerfactory/file_test.go +++ b/pkg/logs/launchers/container/tailerfactory/file_test.go @@ -23,7 +23,7 @@ import ( workloadmeta "github.com/DataDog/datadog-agent/comp/core/workloadmeta/def" workloadmetafxmock "github.com/DataDog/datadog-agent/comp/core/workloadmeta/fx-mock" workloadmetamock "github.com/DataDog/datadog-agent/comp/core/workloadmeta/mock" - "github.com/DataDog/datadog-agent/comp/logs/agent/config" + config "github.com/DataDog/datadog-agent/comp/logs/agent/config" configmock "github.com/DataDog/datadog-agent/pkg/config/mock" "github.com/DataDog/datadog-agent/pkg/logs/internal/util/containersorpods" "github.com/DataDog/datadog-agent/pkg/logs/pipeline" @@ -317,7 +317,7 @@ func TestMakeK8sSource(t *testing.T) { require.Equal(t, wildcard, child.Config.Path) require.Equal(t, "src", child.Config.Source) require.Equal(t, "svc", child.Config.Service) - require.Equal(t, []string{"tag!"}, child.Config.Tags) + assert.EqualValues(t, config.TagsField{"tag!"}, child.Config.Tags) require.Equal(t, *child.Config.AutoMultiLine, true) require.Equal(t, child.Config.AutoMultiLineSampleSize, 123) require.Equal(t, child.Config.AutoMultiLineMatchThreshold, 0.123) From 66e1fb83ae39c671b22839e83cd75797fda36447 Mon Sep 17 00:00:00 2001 From: rahulkaukuntla Date: Tue, 25 Feb 2025 11:03:39 -0500 Subject: [PATCH 2/9] testing every parseable struct and including inputs used in e2e tests --- comp/logs/agent/config/integration_config.go | 1 + comp/logs/agent/config/parser_test.go | 156 +++++++++++++++++++ 2 files changed, 157 insertions(+) diff --git a/comp/logs/agent/config/integration_config.go b/comp/logs/agent/config/integration_config.go index 6b4718505262c..cf04c10d05aa2 100644 --- a/comp/logs/agent/config/integration_config.go +++ b/comp/logs/agent/config/integration_config.go @@ -113,6 +113,7 @@ func (t *TagsField) UnmarshalYAML(unmarshal func(interface{}) error) error { *t = append(*t, str) } } + return nil } return fmt.Errorf("invalid tags format") } diff --git a/comp/logs/agent/config/parser_test.go b/comp/logs/agent/config/parser_test.go index 1114fd5d052a8..824b736d38664 100644 --- a/comp/logs/agent/config/parser_test.go +++ b/comp/logs/agent/config/parser_test.go @@ -244,6 +244,162 @@ logs: assert.Nil(t, err, "Pattern should be a valid regex") }, }, + { + name: "Test 5: Parse journald with include_units", + yaml: []byte(` +logs: + - type: journald + path: /var/log/journal/ + source: custom_log + service: random-logger + include_units: + - random-logger.service +`), + assert: func(t *testing.T, configs []*LogsConfig, err error) { + assert.Nil(t, err) + require.Equal(t, len(configs), 1) + config := configs[0] + assert.Equal(t, "journald", config.Type) + assert.Equal(t, "/var/log/journal/", config.Path) + assert.Equal(t, "custom_log", config.Source) + assert.Equal(t, "random-logger", config.Service) + require.Equal(t, len(config.IncludeSystemUnits), 1) + assert.Equal(t, "random-logger.service", config.IncludeSystemUnits[0]) + }, + }, + { + name: "Test 6: Parse journald with exclude_units", + yaml: []byte(` +logs: + - type: journald + source: custom_log + service: no-datadog + exclude_units: + - datadog-agent.service + - datadog-agent-trace.service + - datadog-agent-process.service +`), + assert: func(t *testing.T, configs []*LogsConfig, err error) { + assert.Nil(t, err) + require.Equal(t, len(configs), 1) + config := configs[0] + assert.Equal(t, "journald", config.Type) + assert.Equal(t, "custom_log", config.Source) + assert.Equal(t, "no-datadog", config.Service) + require.Equal(t, len(config.ExcludeSystemUnits), 3) + assert.Equal(t, "datadog-agent.service", config.ExcludeSystemUnits[0]) + assert.Equal(t, "datadog-agent-trace.service", config.ExcludeSystemUnits[1]) + assert.Equal(t, "datadog-agent-process.service", config.ExcludeSystemUnits[2]) + }, + }, + { + name: "Test 7: Parse minimal journald config", + yaml: []byte(` +logs: + - type: journald + service: hello + source: custom_log +`), + assert: func(t *testing.T, configs []*LogsConfig, err error) { + assert.Nil(t, err) + require.Equal(t, len(configs), 1) + config := configs[0] + assert.Equal(t, "journald", config.Type) + assert.Equal(t, "hello", config.Service) + assert.Equal(t, "custom_log", config.Source) + }, + }, + { + name: "Test Comprehensive: All parseable fields in LogsConfig", + yaml: []byte(` +logs: + - type: journald + integrationname: test_integration + port: 8080 + idle_timeout: "30s" + path: /var/log/journal/ + encoding: utf-8 + exclude_paths: + - /var/log/exclude1.log + - /var/log/exclude2.log + start_position: beginning + config_id: test_config_id + include_units: + - systemd-unit.service + exclude_units: + - datadog-agent.service + - datadog-agent-trace.service + - datadog-agent-process.service + include_user_units: + - user-systemd-unit.service + exclude_user_units: + - user-exclude.service + include_matches: + - some_match + exclude_matches: + - some_exclude_match + container_mode: true + image: test_image + label: test_label + name: test_container + identifier: test_identifier + channel_path: /test/channel + query: SELECT * FROM logs + service: test_service + source: test_source + sourcecategory: test_category + tags: + - key:value + - another_key:another_value + log_processing_rules: + - type: include_at_match + name: include_example + pattern: example_pattern + process_raw_message: true + auto_multi_line_detection: true + auto_multi_line_sample_size: 5 + auto_multi_line_match_threshold: 2.5 +`), + assert: func(t *testing.T, configs []*LogsConfig, err error) { + assert.Nil(t, err) + require.Equal(t, len(configs), 1) + config := configs[0] + assert.Equal(t, "journald", config.Type) + assert.Equal(t, "test_integration", config.IntegrationName) + assert.Equal(t, 8080, config.Port) + assert.Equal(t, "30s", config.IdleTimeout) + assert.Equal(t, "/var/log/journal/", config.Path) + assert.Equal(t, "utf-8", config.Encoding) + require.Equal(t, len(config.ExcludePaths), 2) + assert.Equal(t, "beginning", config.TailingMode) + assert.Equal(t, "test_config_id", config.ConfigId) + require.Equal(t, len(config.IncludeSystemUnits), 1) + require.Equal(t, len(config.ExcludeSystemUnits), 3) + require.Equal(t, len(config.IncludeUserUnits), 1) + require.Equal(t, len(config.ExcludeUserUnits), 1) + require.Equal(t, len(config.IncludeMatches), 1) + require.Equal(t, len(config.ExcludeMatches), 1) + assert.Equal(t, true, config.ContainerMode) + assert.Equal(t, "test_image", config.Image) + assert.Equal(t, "test_label", config.Label) + assert.Equal(t, "test_container", config.Name) + assert.Equal(t, "test_identifier", config.Identifier) + assert.Equal(t, "/test/channel", config.ChannelPath) + assert.Equal(t, "SELECT * FROM logs", config.Query) + assert.Equal(t, "test_service", config.Service) + assert.Equal(t, "test_source", config.Source) + assert.Equal(t, "test_category", config.SourceCategory) + require.Equal(t, len(config.Tags), 2) + require.Equal(t, len(config.ProcessingRules), 1) + assert.Equal(t, "include_at_match", config.ProcessingRules[0].Type) + assert.Equal(t, "include_example", config.ProcessingRules[0].Name) + assert.Equal(t, "example_pattern", config.ProcessingRules[0].Pattern) + assert.Equal(t, true, *config.ProcessRawMessage) + assert.Equal(t, true, *config.AutoMultiLine) + assert.Equal(t, 5, config.AutoMultiLineSampleSize) + assert.Equal(t, 2.5, config.AutoMultiLineMatchThreshold) + }, + }, } for _, tt := range tests { From 5767fb84029b94906ff3f141bbcf4d5aacb352ef Mon Sep 17 00:00:00 2001 From: rahulkaukuntla Date: Tue, 25 Feb 2025 11:24:38 -0500 Subject: [PATCH 3/9] lint --- pkg/logs/launchers/container/tailerfactory/file_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/logs/launchers/container/tailerfactory/file_test.go b/pkg/logs/launchers/container/tailerfactory/file_test.go index c049dccf6afb1..41e456a8c0434 100644 --- a/pkg/logs/launchers/container/tailerfactory/file_test.go +++ b/pkg/logs/launchers/container/tailerfactory/file_test.go @@ -14,6 +14,7 @@ import ( "runtime" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/fx" From 7002c49fa55d95a2e74117235a5cea388a3ac91b Mon Sep 17 00:00:00 2001 From: rahulkaukuntla Date: Fri, 28 Feb 2025 13:21:11 -0500 Subject: [PATCH 4/9] handling newlines in []string fields --- comp/logs/agent/config/integration_config.go | 33 ++++++++++---------- comp/logs/agent/config/parser_test.go | 8 ++--- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/comp/logs/agent/config/integration_config.go b/comp/logs/agent/config/integration_config.go index cf04c10d05aa2..5583851b9258d 100644 --- a/comp/logs/agent/config/integration_config.go +++ b/comp/logs/agent/config/integration_config.go @@ -45,19 +45,19 @@ type LogsConfig struct { IdleTimeout string `mapstructure:"idle_timeout" json:"idle_timeout" yaml:"idle_timeout"` // Network Path string // File, Journald - Encoding string `mapstructure:"encoding" json:"encoding" yaml:"encoding"` // File - ExcludePaths []string `mapstructure:"exclude_paths" json:"exclude_paths" yaml:"exclude_paths"` // File - TailingMode string `mapstructure:"start_position" json:"start_position" yaml:"start_position"` // File + Encoding string `mapstructure:"encoding" json:"encoding" yaml:"encoding"` // File + ExcludePaths StringSliceField `mapstructure:"exclude_paths" json:"exclude_paths" yaml:"exclude_paths"` // File + TailingMode string `mapstructure:"start_position" json:"start_position" yaml:"start_position"` // File //nolint:revive // TODO(AML) Fix revive linter - ConfigId string `mapstructure:"config_id" json:"config_id" yaml:"config_id"` // Journald - IncludeSystemUnits []string `mapstructure:"include_units" json:"include_units" yaml:"include_units"` // Journald - ExcludeSystemUnits []string `mapstructure:"exclude_units" json:"exclude_units" yaml:"exclude_units"` // Journald - IncludeUserUnits []string `mapstructure:"include_user_units" json:"include_user_units" yaml:"include_user_units"` // Journald - ExcludeUserUnits []string `mapstructure:"exclude_user_units" json:"exclude_user_units" yaml:"exclude_user_units"` // Journald - IncludeMatches []string `mapstructure:"include_matches" json:"include_matches" yaml:"include_matches"` // Journald - ExcludeMatches []string `mapstructure:"exclude_matches" json:"exclude_matches" yaml:"exclude_matches"` // Journald - ContainerMode bool `mapstructure:"container_mode" json:"container_mode" yaml:"container_mode"` // Journald + ConfigId string `mapstructure:"config_id" json:"config_id" yaml:"config_id"` // Journald + IncludeSystemUnits StringSliceField `mapstructure:"include_units" json:"include_units" yaml:"include_units"` // Journald + ExcludeSystemUnits StringSliceField `mapstructure:"exclude_units" json:"exclude_units" yaml:"exclude_units"` // Journald + IncludeUserUnits StringSliceField `mapstructure:"include_user_units" json:"include_user_units" yaml:"include_user_units"` // Journald + ExcludeUserUnits StringSliceField `mapstructure:"exclude_user_units" json:"exclude_user_units" yaml:"exclude_user_units"` // Journald + IncludeMatches StringSliceField `mapstructure:"include_matches" json:"include_matches" yaml:"include_matches"` // Journald + ExcludeMatches StringSliceField `mapstructure:"exclude_matches" json:"exclude_matches" yaml:"exclude_matches"` // Journald + ContainerMode bool `mapstructure:"container_mode" json:"container_mode" yaml:"container_mode"` // Journald Image string // Docker Label string // Docker @@ -76,7 +76,7 @@ type LogsConfig struct { // ChannelTags are the tags attached to messages on Channel; unlike Tags this can be // modified at runtime (as long as ChannelTagsMutex is held). - ChannelTags []string + ChannelTags StringSliceField // ChannelTagsMutex guards ChannelTags. ChannelTagsMutex sync.Mutex @@ -84,7 +84,7 @@ type LogsConfig struct { Service string Source string SourceCategory string - Tags TagsField + Tags StringSliceField ProcessingRules []*ProcessingRule `mapstructure:"log_processing_rules" json:"log_processing_rules" yaml:"log_processing_rules"` // ProcessRawMessage is used to process the raw message instead of only the content part of the message. ProcessRawMessage *bool `mapstructure:"process_raw_message" json:"process_raw_message" yaml:"process_raw_message"` @@ -94,14 +94,15 @@ type LogsConfig struct { AutoMultiLineMatchThreshold float64 `mapstructure:"auto_multi_line_match_threshold" json:"auto_multi_line_match_threshold" yaml:"auto_multi_line_match_threshold"` } -// TagsField is a custom type for unmarshalling comma-separated string values or typical yaml fields into a slice of strings. -type TagsField []string +// StringSliceField is a custom type for unmarshalling comma-separated string values or typical yaml fields into a slice of strings. +type StringSliceField []string // UnmarshalYAML is a custom unmarshalling function is needed for string array fields to split comma-separated values. -func (t *TagsField) UnmarshalYAML(unmarshal func(interface{}) error) error { +func (t *StringSliceField) UnmarshalYAML(unmarshal func(interface{}) error) error { var str string if err := unmarshal(&str); err == nil { // note that we are intentionally avoiding the trimming of any spaces whilst splitting the string + str = strings.ReplaceAll(str, "\n", "") *t = strings.Split(str, ",") return nil } diff --git a/comp/logs/agent/config/parser_test.go b/comp/logs/agent/config/parser_test.go index 824b736d38664..f686120bb6bda 100644 --- a/comp/logs/agent/config/parser_test.go +++ b/comp/logs/agent/config/parser_test.go @@ -245,7 +245,7 @@ logs: }, }, { - name: "Test 5: Parse journald with include_units", + name: "Test 5: Parse journald with include_units (string with newline)", yaml: []byte(` logs: - type: journald @@ -253,7 +253,7 @@ logs: source: custom_log service: random-logger include_units: - - random-logger.service + random-logger.service `), assert: func(t *testing.T, configs []*LogsConfig, err error) { assert.Nil(t, err) @@ -275,9 +275,7 @@ logs: source: custom_log service: no-datadog exclude_units: - - datadog-agent.service - - datadog-agent-trace.service - - datadog-agent-process.service + datadog-agent.service,datadog-agent-trace.service,datadog-agent-process.service `), assert: func(t *testing.T, configs []*LogsConfig, err error) { assert.Nil(t, err) From dbdece7341a78d34ff9b38ca37f62c014e640a72 Mon Sep 17 00:00:00 2001 From: rahulkaukuntla Date: Fri, 28 Feb 2025 13:23:40 -0500 Subject: [PATCH 5/9] changing name of TagsField to StringSliceField for accuracy --- .../inventorychecks/inventorychecksimpl/inventorychecks_test.go | 2 +- pkg/logs/launchers/container/tailerfactory/file_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/comp/metadata/inventorychecks/inventorychecksimpl/inventorychecks_test.go b/comp/metadata/inventorychecks/inventorychecksimpl/inventorychecks_test.go index b981d68f97d3b..fe205d933504b 100644 --- a/comp/metadata/inventorychecks/inventorychecksimpl/inventorychecks_test.go +++ b/comp/metadata/inventorychecks/inventorychecksimpl/inventorychecks_test.go @@ -249,7 +249,7 @@ func TestGetPayload(t *testing.T) { assert.Equal(t, expectedSourceStatus, actualSource[0]["state"]) assert.Equal(t, "awesome_cache", actualSource[0]["service"]) assert.Equal(t, "redis", actualSource[0]["source"]) - assert.Equal(t, logConfig.TagsField{"env:prod"}, actualSource[0]["tags"]) + assert.Equal(t, logConfig.StringSliceField{"env:prod"}, actualSource[0]["tags"]) } else { assert.Len(t, p.LogsMetadata, 0) } diff --git a/pkg/logs/launchers/container/tailerfactory/file_test.go b/pkg/logs/launchers/container/tailerfactory/file_test.go index 41e456a8c0434..8d9372ae3ca43 100644 --- a/pkg/logs/launchers/container/tailerfactory/file_test.go +++ b/pkg/logs/launchers/container/tailerfactory/file_test.go @@ -318,7 +318,7 @@ func TestMakeK8sSource(t *testing.T) { require.Equal(t, wildcard, child.Config.Path) require.Equal(t, "src", child.Config.Source) require.Equal(t, "svc", child.Config.Service) - assert.EqualValues(t, config.TagsField{"tag!"}, child.Config.Tags) + assert.EqualValues(t, config.StringSliceField{"tag!"}, child.Config.Tags) require.Equal(t, *child.Config.AutoMultiLine, true) require.Equal(t, child.Config.AutoMultiLineSampleSize, 123) require.Equal(t, child.Config.AutoMultiLineMatchThreshold, 0.123) From d24decdb696364d19f5201286b42b38ff9a89046 Mon Sep 17 00:00:00 2001 From: rahulkaukuntla Date: Fri, 28 Feb 2025 14:37:08 -0500 Subject: [PATCH 6/9] fixing test --- pkg/logs/schedulers/channel/scheduler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/logs/schedulers/channel/scheduler_test.go b/pkg/logs/schedulers/channel/scheduler_test.go index fd670d14d19ad..844f6f99b7d7c 100644 --- a/pkg/logs/schedulers/channel/scheduler_test.go +++ b/pkg/logs/schedulers/channel/scheduler_test.go @@ -39,5 +39,5 @@ func TestScheduler(t *testing.T) { require.Equal(t, len(spy.Events), 1) // no change assert.Nil(t, source.Config.Tags) - assert.Equal(t, []string{"foo"}, source.Config.ChannelTags) + assert.Equal(t, config.StringSliceField{"foo"}, source.Config.ChannelTags) } From 69dd42a2c415d587f5ae80e26b595ca9fa93ed99 Mon Sep 17 00:00:00 2001 From: rahulkaukuntla Date: Mon, 3 Mar 2025 13:10:48 -0500 Subject: [PATCH 7/9] resolving merge conflict --- comp/logs/agent/config/go.mod | 1 + 1 file changed, 1 insertion(+) diff --git a/comp/logs/agent/config/go.mod b/comp/logs/agent/config/go.mod index ff0bfd1d6a16d..2cc49c0ca1a35 100644 --- a/comp/logs/agent/config/go.mod +++ b/comp/logs/agent/config/go.mod @@ -11,6 +11,7 @@ require ( github.com/DataDog/datadog-agent/pkg/util/fxutil v0.61.0 github.com/DataDog/datadog-agent/pkg/util/log v0.64.0-devel github.com/DataDog/datadog-agent/pkg/util/pointer v0.61.0 + github.com/DataDog/datadog-agent/pkg/util/scrubber v0.62.3 github.com/stretchr/testify v1.10.0 go.uber.org/fx v1.23.0 gopkg.in/yaml.v3 v3.0.1 From 48d2b507a70bce720aaf7718e1766c7edfe4de6c Mon Sep 17 00:00:00 2001 From: rahulkaukuntla Date: Tue, 4 Mar 2025 14:30:48 -0500 Subject: [PATCH 8/9] addressing comments --- comp/logs/agent/config/integration_config.go | 2 ++ comp/logs/agent/config/parser_test.go | 9 ++++++--- pkg/logs/launchers/container/tailerfactory/file_test.go | 5 ++--- pkg/logs/schedulers/channel/scheduler_test.go | 2 +- 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/comp/logs/agent/config/integration_config.go b/comp/logs/agent/config/integration_config.go index 5583851b9258d..bc953801bca6e 100644 --- a/comp/logs/agent/config/integration_config.go +++ b/comp/logs/agent/config/integration_config.go @@ -112,6 +112,8 @@ func (t *StringSliceField) UnmarshalYAML(unmarshal func(interface{}) error) erro for _, item := range raw { if str, ok := item.(string); ok { *t = append(*t, str) + } else { + return fmt.Errorf("cannot unmarshal %v into a string", item) } } return nil diff --git a/comp/logs/agent/config/parser_test.go b/comp/logs/agent/config/parser_test.go index f686120bb6bda..910b4dda691a0 100644 --- a/comp/logs/agent/config/parser_test.go +++ b/comp/logs/agent/config/parser_test.go @@ -64,12 +64,15 @@ foo: `, ` - type: file path: /var/log/app.log - tags: a, b:c + tags: nil `, ` `} - for _, format := range invalidFormats { - configs, _ := ParseYAML([]byte(format)) + for i, format := range invalidFormats { + configs, err := ParseYAML([]byte(format)) + if i == 1 { + assert.NotNil(t, err) + } require.Equal(t, 0, len(configs)) } } diff --git a/pkg/logs/launchers/container/tailerfactory/file_test.go b/pkg/logs/launchers/container/tailerfactory/file_test.go index 8d9372ae3ca43..d2e29398e5eba 100644 --- a/pkg/logs/launchers/container/tailerfactory/file_test.go +++ b/pkg/logs/launchers/container/tailerfactory/file_test.go @@ -14,7 +14,6 @@ import ( "runtime" "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/fx" @@ -24,7 +23,7 @@ import ( workloadmeta "github.com/DataDog/datadog-agent/comp/core/workloadmeta/def" workloadmetafxmock "github.com/DataDog/datadog-agent/comp/core/workloadmeta/fx-mock" workloadmetamock "github.com/DataDog/datadog-agent/comp/core/workloadmeta/mock" - config "github.com/DataDog/datadog-agent/comp/logs/agent/config" + "github.com/DataDog/datadog-agent/comp/logs/agent/config" configmock "github.com/DataDog/datadog-agent/pkg/config/mock" "github.com/DataDog/datadog-agent/pkg/logs/internal/util/containersorpods" "github.com/DataDog/datadog-agent/pkg/logs/pipeline" @@ -318,7 +317,7 @@ func TestMakeK8sSource(t *testing.T) { require.Equal(t, wildcard, child.Config.Path) require.Equal(t, "src", child.Config.Source) require.Equal(t, "svc", child.Config.Service) - assert.EqualValues(t, config.StringSliceField{"tag!"}, child.Config.Tags) + require.Equal(t, []string{"tag!"}, []string(child.Config.Tags)) require.Equal(t, *child.Config.AutoMultiLine, true) require.Equal(t, child.Config.AutoMultiLineSampleSize, 123) require.Equal(t, child.Config.AutoMultiLineMatchThreshold, 0.123) diff --git a/pkg/logs/schedulers/channel/scheduler_test.go b/pkg/logs/schedulers/channel/scheduler_test.go index 844f6f99b7d7c..7af0d2782f17d 100644 --- a/pkg/logs/schedulers/channel/scheduler_test.go +++ b/pkg/logs/schedulers/channel/scheduler_test.go @@ -39,5 +39,5 @@ func TestScheduler(t *testing.T) { require.Equal(t, len(spy.Events), 1) // no change assert.Nil(t, source.Config.Tags) - assert.Equal(t, config.StringSliceField{"foo"}, source.Config.ChannelTags) + assert.Equal(t, []string{"foo"}, []string(source.Config.ChannelTags)) } From 2d162ec21e80350b27fcc2f79fe859fc45ab0e3e Mon Sep 17 00:00:00 2001 From: rahulkaukuntla Date: Thu, 6 Mar 2025 12:49:12 -0500 Subject: [PATCH 9/9] updating unmarshalling error statement --- comp/logs/agent/config/integration_config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/comp/logs/agent/config/integration_config.go b/comp/logs/agent/config/integration_config.go index bc953801bca6e..e92b68a9e5213 100644 --- a/comp/logs/agent/config/integration_config.go +++ b/comp/logs/agent/config/integration_config.go @@ -118,7 +118,7 @@ func (t *StringSliceField) UnmarshalYAML(unmarshal func(interface{}) error) erro } return nil } - return fmt.Errorf("invalid tags format") + return fmt.Errorf("could not parse YAML config, please double check the yaml files") } // Dump dumps the contents of this struct to a string, for debugging purposes.