diff --git a/Docs/Examples/DebugDump/config.yml b/Docs/Examples/DebugDump/config.yml index 281fe9c..88ad966 100644 --- a/Docs/Examples/DebugDump/config.yml +++ b/Docs/Examples/DebugDump/config.yml @@ -12,18 +12,27 @@ # THIS WILL GENERATE A LOT OF DATA, so use it with care. # # If you want to enable filtering and/or PII data, uncomment the -# correpsonding lines and create the additional .yml files. +# corresponding lines below. +# +# You can also use ${file:PATH} to reference an external YAML file, +# e.g.: filter: "${file:/path/to/filter.yml}" +# +# For backwards compatibility, you can also specify a plain file path: +# e.g.: filter: "/path/to/filter.yml" receivers: trace2receiver: socket: "/usr/local//trace2.socket" pipe: "//./pipe/" -# filter: "/usr/local//filter.yml" -# pii: "/usr/local//pii.yml" +# pii: +# include: +# hostname: true +# username: false -# filter: "C:/ProgramData//filter.yml" -# pii: "C:/ProgramData//pii.yml" +# filter: +# defaults: +# ruleset: "dl:verbose" processors: diff --git a/Docs/Examples/ExportToAzureMonitor/config.yml b/Docs/Examples/ExportToAzureMonitor/config.yml index d1b58b5..caddfe3 100644 --- a/Docs/Examples/ExportToAzureMonitor/config.yml +++ b/Docs/Examples/ExportToAzureMonitor/config.yml @@ -11,18 +11,27 @@ # THIS WILL GENERATE A LOT OF DATA, so use it with care. # # If you want to enable filtering and/or PII data, uncomment the -# correpsonding lines and create the additional .yml files. +# corresponding lines below. +# +# You can also use ${file:PATH} to reference an external YAML file, +# e.g.: filter: "${file:/path/to/filter.yml}" +# +# For backwards compatibility, you can also specify a plain file path: +# e.g.: filter: "/path/to/filter.yml" receivers: trace2receiver: socket: "/usr/local//trace2.socket" pipe: "//./pipe/" -# filter: "/usr/local//filter.yml" -# pii: "/usr/local//pii.yml" +# pii: +# include: +# hostname: true +# username: false -# filter: "C:/ProgramData//filter.yml" -# pii: "C:/ProgramData//pii.yml" +# filter: +# defaults: +# ruleset: "dl:verbose" processors: diff --git a/Docs/config-filter-settings.md b/Docs/config-filter-settings.md index d78a63b..3da18f0 100644 --- a/Docs/config-filter-settings.md +++ b/Docs/config-filter-settings.md @@ -1,14 +1,20 @@ # Config Filter Settings -The `filter.yml` file controls how the `trace2receiver` component +The filter settings control how the `trace2receiver` component translates the Trace2 data stream from Git commands into OTEL data structures. This filtering is content- and context-aware and is independent of any statistical filtering performed by later stages in the OTEL Collector pipeline. -The filter settings pathname is set in the +The filter settings are specified inline under the `receivers.trace2receiver.filter` -parameter in the main `config.yml` file. +parameter in the main `config.yml` file. Alternatively, you can use +the `${file:PATH}` syntax to reference an external YAML file. + +For backwards compatibility, you can also specify a plain file path +string (without the `${file:}` wrapper) as the value of the `filter` +field, and the receiver will read and parse the YAML file at that +path. @@ -90,14 +96,15 @@ A ruleset name is essentially an alias for the underlying ruleset file. Using a ruleset name avoids requiring users know how and where the telemetry service is installed. -The `filter.yml` file contains a dictionary to map ruleset names to +The filter settings contain a dictionary to map ruleset names to pathnames: ``` -rulesets: - : - : - ... +filter: + rulesets: + : + : + ... ``` Ruleset files will be loaded when the receiver starts up. @@ -127,14 +134,15 @@ the ruleset "rs:bar". A repo nickname is a simple string without either `dl:` or `rs:` prefix. -The `filter.yml` file contains a dictionary to map nicknames to detail +The filter settings contain a dictionary to map nicknames to detail levels or rulesets: ``` -nicknames: - : | - : | - ... +filter: + nicknames: + : | + : | + ... ``` @@ -160,13 +168,14 @@ or `system` level. $ git config --system trace2.configparams "otel.trace2.*" ``` -The `filter.yml` contains a dictionary to define the spelling of +The filter settings contain a dictionary to define the spelling of these keys: ``` -keynames: - nickname_key: "otel.trace2.nickname" - ruleset_key: "otel.trace2.ruleset" +filter: + keynames: + nickname_key: "otel.trace2.nickname" + ruleset_key: "otel.trace2.ruleset" ``` @@ -199,7 +208,7 @@ $ git -c otel.trace2.nickname=personal status ``` If no nickname is defined or the given repo nickname is not defined in -the `filter.yml` file, the receiver will fall back to the default +the filter settings, the receiver will fall back to the default filter settings. _In the above example, I've suggested "monorepo" and "personal" as @@ -244,8 +253,8 @@ $ cd /path/to/my/repo4 $ git -c otel.trace2.ruleset="dl:summary" status ``` -If the named ruleset or detail level is not defined in the `filter.yml` -file, the receiver will fall back to the default filter settings. +If the named ruleset or detail level is not defined in the filter +settings, the receiver will fall back to the default filter settings. If a Git command sends both a `ruleset_key` and `nickname_key`, the `ruleset_key` wins. (Both key values will be included in the OTEL @@ -298,9 +307,42 @@ This would produce the following in the OTEL process span: ## Filter Settings Syntax Now that all of the concepts have been introduced, we can describe -the complete syntax of the `filter.yml` file. All sections and rows +the complete syntax of the filter settings. All sections and rows are optional. +When the settings are specified inline in the Collector configuration, +they appear under the receiver's `filter` field: + +``` +filter: + keynames: + nickname_key: + ruleset_key: + + nicknames: + : | + : | + ... + + rulesets: + : + : + ... + + defaults: + ruleset: | + + important_events: + - category: + key_prefix: + field_name: + ... +``` + +When the `filter` field references a standalone file using either a +plain path or `${file:PATH}`, the file contains the settings directly +and omits the outer `filter` field: + ``` keynames: nickname_key: @@ -339,19 +381,20 @@ used. In this filter: ``` -keynames: - nickname_key: "otel.trace2.nickname" - ruleset_key: "otel.trace2.ruleset" +filter: + keynames: + nickname_key: "otel.trace2.nickname" + ruleset_key: "otel.trace2.ruleset" -nicknames: - monorepo: "dl:verbose" - personal: "dl:drop" + nicknames: + monorepo: "dl:verbose" + personal: "dl:drop" -rulesets: - "rs:status": "./rulesets/rs-status.yml" + rulesets: + "rs:status": "./rulesets/rs-status.yml" -defaults: - ruleset: "dl:summary" + defaults: + ruleset: "dl:summary" ``` The receiver will watch for the `otel.trace2.nickname` and @@ -371,4 +414,3 @@ use `dl:drop` and not emit any telemetry. All other commands will use the default `dl:summary` and emit command overview telemetry. - diff --git a/Docs/config-pii-settings.md b/Docs/config-pii-settings.md index 9b1ccab..78bc067 100644 --- a/Docs/config-pii-settings.md +++ b/Docs/config-pii-settings.md @@ -1,6 +1,6 @@ # Config PII Settings -The PII Settings file contains privacy-related feature flags for the +The PII settings contain privacy-related feature flags for the `trace2receiver` component. Currently, this includes flags to add user and hostname data that may not be present in the original Trace2 data stream. Later, it may include other flags to redact or not @@ -9,13 +9,31 @@ redact sensitive data found within the Trace2 data stream. NOTE: These flags may add GDPR-sensitive data to the OTEL telemetry data stream. Use them at your own risk. -The PII settings pathname is set in the +The PII settings are specified inline under the `receivers.trace2receiver.pii` -parameter in the main `config.yml` file. +parameter in the main `config.yml` file. Alternatively, you can use +the `${file:PATH}` syntax to reference an external YAML file. -## `pii.yml` Syntax +For backwards compatibility, you can also specify a plain file path +string (without the `${file:}` wrapper) as the value of the `pii` +field, and the receiver will read and parse the YAML file at that +path. -The PII settings file has the following syntax: +## PII Settings Syntax + +When the settings are specified inline in the Collector configuration, +they appear under the receiver's `pii` field: + +``` +pii: + include: + hostname: + username: +``` + +When the `pii` field references a standalone file using either a plain +path or `${file:PATH}`, the file contains the settings directly and +omits the outer `pii` field: ``` include: diff --git a/Docs/configure-custom-collector.md b/Docs/configure-custom-collector.md index 6e4667f..e405010 100644 --- a/Docs/configure-custom-collector.md +++ b/Docs/configure-custom-collector.md @@ -44,13 +44,56 @@ receivers: trace2receiver: socket: pipe: - pii: - filter: - summary: + pii: + + filter: + + summary: + ``` For example: +``` +receivers: + trace2receiver: + socket: "/usr/local/my-collector/trace2.socket" + pipe: "//./pipe/my-collector.pipe" + pii: + include: + hostname: true + username: false + filter: + defaults: + ruleset: "dl:verbose" + summary: + message_patterns: + - prefix: "my_prefix:" + field_name: "myPrefixCount" + region_timers: + - category: "the_category" + label: "objects/foo" + count_field: "fooCount" + time_field: "fooTime" +``` + +If you prefer to keep the `pii`, `filter`, or `summary` configuration +in a separate file, you can use the `${file:PATH}` syntax or specify +a simple file path string: + +``` +receivers: + trace2receiver: + socket: "/usr/local/my-collector/trace2.socket" + pipe: "//./pipe/my-collector.pipe" + pii: "${file:/usr/local/my-collector/pii.yml}" + filter: "${file:/usr/local/my-collector/filter.yml}" + summary: "${file:/usr/local/my-collector/summary.yml}" +``` + +For backwards compatibility, you can also specify a plain file path +without the `${file:}` wrapper: + ``` receivers: trace2receiver: @@ -105,32 +148,40 @@ for details. $ git config --system trace2.eventtarget "//./pipe/my-collector.pipe" ``` -### `` (Optional) +### `pii` (Optional) -The pathname to a `pii.yml` file containing privacy-related feature flags. +Inline PII settings controlling privacy-related feature flags. This is optional. These features are disabled by default. +For backwards compatibility, this field also accepts a simple string +containing a file path to a YAML file with the PII settings. + See [config PII settings](./config-pii-settings.md) for details. -### `` (Optional) +### `filter` (Optional) -The pathname to a `filter.yml` file controlling the verbosity of the +Inline filter settings controlling the verbosity of the generated OTEL telemetry data. This is optional. If omitted, summary-level telemetry will be emitted. +For backwards compatibility, this field also accepts a simple string +containing a file path to a YAML file with the filter settings. + See [config filter settings](./config-filter-settings.md) for details. -### `` (Optional) +### `summary` (Optional) -The pathname to a `summary.yml` file controlling which trace2 events -are aggregated into the `trace2.process.summary` attribute on the OTEL -process span. This is optional. If omitted, no aggregated summary -metrics are emitted. +Inline summary settings controlling which trace2 events are aggregated into +the `trace2.process.summary` attribute on the OTEL process span. +This is optional. If omitted, no aggregated summary metrics are emitted. The summary is emitted at all detail levels (including `dl:summary`), making it useful for surfacing aggregated statistics without requiring verbose telemetry. +For backwards compatibility, this field also accepts a simple string +containing a file path to a YAML file with the summary settings. + See the [summary example](./Examples/summary_example.yml) for a complete example configuration. diff --git a/config.go b/config.go index 7e08bb6..213aa7e 100644 --- a/config.go +++ b/config.go @@ -5,8 +5,19 @@ import ( "path/filepath" "runtime" "strings" + + "github.com/mitchellh/mapstructure" ) +// Note: The `pii`, `filter`, and `summary` fields accept either: +// - inline YAML/JSON configuration (an object/map), or +// - a string containing a file path to a YAML file. +// +// This for backwards compatibility with the original design of the +// config where these were only allowed to be file paths. +// You can continue to use a simple string, or the built-in ${file} +// syntax to specify a file path if inline config is not convenient. + // `Config` represents the complete configuration settings for // an individual receiver declaration from the `config.yaml`. // @@ -45,17 +56,27 @@ type Config struct { // data stream. AllowCommandControlVerbs bool `mapstructure:"enable_commands"` - // Pathname to YML file containing PII settings. - PiiSettingsPath string `mapstructure:"pii"` - piiSettings *PiiSettings - - // Pathname to YML file containing our filter settings. - FilterSettingsPath string `mapstructure:"filter"` - filterSettings *FilterSettings - - // Pathname to YML file containing summary settings. - SummaryPath string `mapstructure:"summary"` - summary *SummarySettings + // PII settings control whether possibly GDPR-sensitive fields + // are included in the telemetry output. + // RawPii is exported because the Collector's reflection-based + // config decoder can only populate exported fields. + RawPii any `mapstructure:"pii"` + // Pii contains the settings resolved from RawPii by Validate. + Pii *PiiSettings `mapstructure:"-"` + + // Filter settings control how the trace2 data is filtered. + // RawFilter is exported because the Collector's reflection-based + // config decoder can only populate exported fields. + RawFilter any `mapstructure:"filter"` + // Filter contains the settings resolved from RawFilter by Validate. + Filter *FilterSettings `mapstructure:"-"` + + // Summary settings control aggregated metrics from trace2 events. + // RawSummary is exported because the Collector's reflection-based + // config decoder can only populate exported fields. + RawSummary any `mapstructure:"summary"` + // Summary contains the settings resolved from RawSummary by Validate. + Summary *SummarySettings `mapstructure:"-"` } // `Validate()` checks if the receiver configuration is valid. @@ -101,23 +122,35 @@ func (cfg *Config) Validate() error { cfg.UnixSocketPath = path } - if len(cfg.PiiSettingsPath) > 0 { - cfg.piiSettings, err = parsePiiFile(cfg.PiiSettingsPath) + if cfg.RawPii != nil { + cfg.Pii, err = resolveAnyField(cfg.RawPii, parsePiiFromBuffer) if err != nil { - return err + return fmt.Errorf("pii: %w", err) } } - if len(cfg.FilterSettingsPath) > 0 { - cfg.filterSettings, err = parseFilterSettings(cfg.FilterSettingsPath) + if cfg.RawFilter != nil { + cfg.Filter, err = resolveAnyField(cfg.RawFilter, parseFilterSettingsFromBuffer) if err != nil { + return fmt.Errorf("filter: %w", err) + } + } + + if cfg.Filter != nil { + if err = cfg.Filter.validate(); err != nil { return err } } - if len(cfg.SummaryPath) > 0 { - cfg.summary, err = parseSummarySettings(cfg.SummaryPath) + if cfg.RawSummary != nil { + cfg.Summary, err = resolveAnyField(cfg.RawSummary, parseSummarySettingsFromBuffer) if err != nil { + return fmt.Errorf("summary: %w", err) + } + } + + if cfg.Summary != nil { + if err = cfg.Summary.validate(); err != nil { return err } } @@ -125,6 +158,40 @@ func (cfg *Config) Validate() error { return nil } +// resolveAnyField handles a config field that may be either: +// - nil: the field was not specified +// - a string: a file path to a YAML file to parse +// - a map: inline configuration to decode into the target struct +func resolveAnyField[T MyYmlFileTypes](raw any, parseFn MyYmlParseBufferFn[T]) (*T, error) { + if raw == nil { + return nil, nil + } + + switch v := raw.(type) { + case string: + if len(v) == 0 { + return nil, nil + } + return parseYmlFile(v, parseFn) + + default: + // Assume it's a map/object from inline configuration. + // Use mapstructure to decode the raw value into the typed struct. + p := new(T) + decoder, err := mapstructure.NewDecoder(&mapstructure.DecoderConfig{ + Result: p, + TagName: "mapstructure", + }) + if err != nil { + return nil, fmt.Errorf("could not create decoder: %w", err) + } + if err = decoder.Decode(v); err != nil { + return nil, fmt.Errorf("could not decode inline config: %w", err) + } + return p, nil + } +} + // Require (the backslash spelling of) `//./pipe/` but allow // `` as an alias for the full spelling. Complain if given a // regular UNC or drive letter pathname. diff --git a/config_test.go b/config_test.go index aff856c..7b002a8 100644 --- a/config_test.go +++ b/config_test.go @@ -1,6 +1,7 @@ package trace2receiver import ( + "fmt" "os" "path/filepath" "runtime" @@ -8,6 +9,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/confmap" + "go.opentelemetry.io/collector/confmap/confmaptest" ) // Test Validate with minimal valid config on Windows @@ -156,68 +159,232 @@ func Test_Config_Validate_RejectDgramUnix(t *testing.T) { assert.Contains(t, err.Error(), "SOCK_DGRAM sockets are not supported") } -// Test Validate with valid PII settings file +// Test Validate with valid PII settings (inline) func Test_Config_Validate_WithValidPiiSettings(t *testing.T) { - // Create a temporary PII settings file + cfg := createMinimalValidConfig() + cfg.Pii = &PiiSettings{ + Include: PiiInclude{ + Hostname: true, + Username: false, + }, + } + + err := cfg.Validate() + assert.NoError(t, err) + assert.NotNil(t, cfg.Pii) +} + +// Test Validate with valid filter settings (inline) +func Test_Config_Validate_WithValidFilterSettings(t *testing.T) { + cfg := createMinimalValidConfig() + cfg.Filter = &FilterSettings{ + Defaults: FilterDefaults{ + RulesetName: "dl:verbose", + }, + } + + err := cfg.Validate() + assert.NoError(t, err) + assert.NotNil(t, cfg.Filter) +} + +// Test Validate with valid summary settings (inline) +func Test_Config_Validate_WithValidSummary(t *testing.T) { + cfg := createMinimalValidConfig() + cfg.Summary = &SummarySettings{ + MessagePatterns: []MessagePatternRule{ + {Prefix: "error:", FieldName: "error_count"}, + {Prefix: "warning:", FieldName: "warning_count"}, + }, + RegionTimers: []RegionTimerRule{ + {Category: "index", Label: "do_read_index", CountField: "index_read_count", TimeField: "index_read_time"}, + }, + } + + err := cfg.Validate() + assert.NoError(t, err) + assert.NotNil(t, cfg.Summary) + assert.Equal(t, 2, len(cfg.Summary.MessagePatterns)) + assert.Equal(t, 1, len(cfg.Summary.RegionTimers)) +} + +// Test Validate with invalid summary settings (empty field_name) +func Test_Config_Validate_WithMalformedSummary(t *testing.T) { + cfg := createMinimalValidConfig() + cfg.Summary = &SummarySettings{ + MessagePatterns: []MessagePatternRule{ + {Prefix: "error:", FieldName: ""}, + }, + } + + err := cfg.Validate() + assert.Error(t, err) + assert.Contains(t, err.Error(), "field_name cannot be empty") +} + +// Test Validate with summary settings with duplicate field names +func Test_Config_Validate_WithDuplicateSummaryFields(t *testing.T) { + cfg := createMinimalValidConfig() + cfg.Summary = &SummarySettings{ + MessagePatterns: []MessagePatternRule{ + {Prefix: "error:", FieldName: "count"}, + {Prefix: "warning:", FieldName: "count"}, + }, + } + + err := cfg.Validate() + assert.Error(t, err) + assert.Contains(t, err.Error(), "duplicate field_name") +} + +// Test Validate with all optional settings valid (inline) +func Test_Config_Validate_WithAllOptionalSettings(t *testing.T) { + inlineConfig := ` +receivers: + trace2receiver: + socket: "/tmp/test.socket" + pipe: "test-pipe" + pii: + include: + hostname: true + filter: + defaults: + ruleset: "dl:summary" + summary: + message_patterns: + - prefix: "error:" + field_name: "error_count" + region_timers: + - category: "index" + label: "do_read_index" + time_field: "index_read_time" +` + + cfg := loadReceiverConfig(t, inlineConfig) + require.NoError(t, cfg.Validate()) + assert.True(t, cfg.Pii.Include.Hostname) + assert.Equal(t, "dl:summary", cfg.Filter.Defaults.RulesetName) + assert.Equal(t, []MessagePatternRule{ + {Prefix: "error:", FieldName: "error_count"}, + }, cfg.Summary.MessagePatterns) + assert.Equal(t, []RegionTimerRule{ + {Category: "index", Label: "do_read_index", TimeField: "index_read_time"}, + }, cfg.Summary.RegionTimers) +} + +func Test_Config_Validate_WithAllFilePaths(t *testing.T) { + tmpDir := t.TempDir() + + piiPath := filepath.Join(tmpDir, "pii.yml") + require.NoError(t, os.WriteFile(piiPath, []byte(` +include: + hostname: true +`), 0644)) + + filterPath := filepath.Join(tmpDir, "filter.yml") + require.NoError(t, os.WriteFile(filterPath, []byte(` +defaults: + ruleset: "dl:verbose" +`), 0644)) + + summaryPath := filepath.Join(tmpDir, "summary.yml") + require.NoError(t, os.WriteFile(summaryPath, []byte(` +message_patterns: + - prefix: "warning:" + field_name: "warning_count" +`), 0644)) + + fileConfig := fmt.Sprintf(` +receivers: + trace2receiver: + socket: "/tmp/test.socket" + pipe: "test-pipe" + pii: %q + filter: %q + summary: %q +`, piiPath, filterPath, summaryPath) + + cfg := loadReceiverConfig(t, fileConfig) + require.NoError(t, cfg.Validate()) + assert.True(t, cfg.Pii.Include.Hostname) + assert.Equal(t, "dl:verbose", cfg.Filter.Defaults.RulesetName) + assert.Equal(t, []MessagePatternRule{ + {Prefix: "warning:", FieldName: "warning_count"}, + }, cfg.Summary.MessagePatterns) +} + +// Test Validate with command control enabled +func Test_Config_Validate_WithCommandControlEnabled(t *testing.T) { + cfg := createMinimalValidConfig() + cfg.AllowCommandControlVerbs = true + + err := cfg.Validate() + assert.NoError(t, err) +} + +// Test Validate with PII settings from file path +func Test_Config_Validate_WithPiiFilePath(t *testing.T) { tmpDir := t.TempDir() piiPath := filepath.Join(tmpDir, "pii.yml") piiContent := ` -pii_filter: - domains: - - pattern: "example.com" - replace: "" +include: + hostname: true + username: false ` err := os.WriteFile(piiPath, []byte(piiContent), 0644) require.NoError(t, err) cfg := createMinimalValidConfig() - cfg.PiiSettingsPath = piiPath + cfg.RawPii = piiPath err = cfg.Validate() assert.NoError(t, err) - assert.NotNil(t, cfg.piiSettings) + assert.NotNil(t, cfg.Pii) + assert.True(t, cfg.Pii.Include.Hostname) + assert.False(t, cfg.Pii.Include.Username) } -// Test Validate with invalid PII settings file -func Test_Config_Validate_WithInvalidPiiSettings(t *testing.T) { +// Test Validate with invalid PII file path +func Test_Config_Validate_WithInvalidPiiFilePath(t *testing.T) { cfg := createMinimalValidConfig() - cfg.PiiSettingsPath = "/nonexistent/pii.yml" + cfg.RawPii = "/nonexistent/pii.yml" err := cfg.Validate() assert.Error(t, err) + assert.Contains(t, err.Error(), "pii:") } -// Test Validate with valid filter settings file -func Test_Config_Validate_WithValidFilterSettings(t *testing.T) { - // Create a temporary filter settings file +// Test Validate with filter settings from file path +func Test_Config_Validate_WithFilterFilePath(t *testing.T) { tmpDir := t.TempDir() filterPath := filepath.Join(tmpDir, "filter.yml") filterContent := ` -default_action: accept +defaults: + ruleset: "dl:verbose" ` err := os.WriteFile(filterPath, []byte(filterContent), 0644) require.NoError(t, err) cfg := createMinimalValidConfig() - cfg.FilterSettingsPath = filterPath + cfg.RawFilter = filterPath err = cfg.Validate() assert.NoError(t, err) - assert.NotNil(t, cfg.filterSettings) + assert.NotNil(t, cfg.Filter) } -// Test Validate with invalid filter settings file -func Test_Config_Validate_WithInvalidFilterSettings(t *testing.T) { +// Test Validate with invalid filter file path +func Test_Config_Validate_WithInvalidFilterFilePath(t *testing.T) { cfg := createMinimalValidConfig() - cfg.FilterSettingsPath = "/nonexistent/filter.yml" + cfg.RawFilter = "/nonexistent/filter.yml" err := cfg.Validate() assert.Error(t, err) + assert.Contains(t, err.Error(), "filter:") } -// Test Validate with valid summary settings file -func Test_Config_Validate_WithValidSummary(t *testing.T) { - // Create a temporary summary settings file +// Test Validate with summary settings from file path +func Test_Config_Validate_WithSummaryFilePath(t *testing.T) { tmpDir := t.TempDir() summaryPath := filepath.Join(tmpDir, "summary.yml") summaryContent := ` @@ -237,27 +404,27 @@ region_timers: require.NoError(t, err) cfg := createMinimalValidConfig() - cfg.SummaryPath = summaryPath + cfg.RawSummary = summaryPath err = cfg.Validate() assert.NoError(t, err) - assert.NotNil(t, cfg.summary) - assert.Equal(t, 2, len(cfg.summary.MessagePatterns)) - assert.Equal(t, 1, len(cfg.summary.RegionTimers)) + assert.NotNil(t, cfg.Summary) + assert.Equal(t, 2, len(cfg.Summary.MessagePatterns)) + assert.Equal(t, 1, len(cfg.Summary.RegionTimers)) } -// Test Validate with invalid summary settings file (nonexistent) -func Test_Config_Validate_WithNonexistentSummary(t *testing.T) { +// Test Validate with invalid summary file path +func Test_Config_Validate_WithInvalidSummaryFilePath(t *testing.T) { cfg := createMinimalValidConfig() - cfg.SummaryPath = "/nonexistent/summary.yml" + cfg.RawSummary = "/nonexistent/summary.yml" err := cfg.Validate() assert.Error(t, err) + assert.Contains(t, err.Error(), "summary:") } -// Test Validate with invalid summary settings file (malformed YAML) -func Test_Config_Validate_WithMalformedSummary(t *testing.T) { - // Create a temporary malformed summary settings file +// Test Validate with malformed summary from file path +func Test_Config_Validate_WithMalformedSummaryFilePath(t *testing.T) { tmpDir := t.TempDir() summaryPath := filepath.Join(tmpDir, "summary.yml") summaryContent := ` @@ -269,93 +436,13 @@ message_patterns: require.NoError(t, err) cfg := createMinimalValidConfig() - cfg.SummaryPath = summaryPath + cfg.RawSummary = summaryPath err = cfg.Validate() assert.Error(t, err) assert.Contains(t, err.Error(), "field_name cannot be empty") } -// Test Validate with summary settings with duplicate field names -func Test_Config_Validate_WithDuplicateSummaryFields(t *testing.T) { - // Create a temporary summary settings file with duplicate fields - tmpDir := t.TempDir() - summaryPath := filepath.Join(tmpDir, "summary.yml") - summaryContent := ` -message_patterns: - - prefix: "error:" - field_name: "count" - - prefix: "warning:" - field_name: "count" -` - err := os.WriteFile(summaryPath, []byte(summaryContent), 0644) - require.NoError(t, err) - - cfg := createMinimalValidConfig() - cfg.SummaryPath = summaryPath - - err = cfg.Validate() - assert.Error(t, err) - assert.Contains(t, err.Error(), "duplicate field_name") -} - -// Test Validate with all optional settings valid -func Test_Config_Validate_WithAllOptionalSettings(t *testing.T) { - // Create temporary files for all settings - tmpDir := t.TempDir() - - piiPath := filepath.Join(tmpDir, "pii.yml") - piiContent := ` -pii_filter: - domains: - - pattern: "example.com" - replace: "" -` - err := os.WriteFile(piiPath, []byte(piiContent), 0644) - require.NoError(t, err) - - filterPath := filepath.Join(tmpDir, "filter.yml") - filterContent := ` -default_action: accept -` - err = os.WriteFile(filterPath, []byte(filterContent), 0644) - require.NoError(t, err) - - summaryPath := filepath.Join(tmpDir, "summary.yml") - summaryContent := ` -message_patterns: - - prefix: "error:" - field_name: "error_count" - -region_timers: - - category: "index" - label: "do_read_index" - time_field: "index_read_time" -` - err = os.WriteFile(summaryPath, []byte(summaryContent), 0644) - require.NoError(t, err) - - cfg := createMinimalValidConfig() - cfg.PiiSettingsPath = piiPath - cfg.FilterSettingsPath = filterPath - cfg.SummaryPath = summaryPath - - err = cfg.Validate() - assert.NoError(t, err) - assert.NotNil(t, cfg.piiSettings) - assert.NotNil(t, cfg.filterSettings) - assert.NotNil(t, cfg.summary) -} - -// Test Validate with command control enabled -func Test_Config_Validate_WithCommandControlEnabled(t *testing.T) { - cfg := createMinimalValidConfig() - cfg.AllowCommandControlVerbs = true - - err := cfg.Validate() - assert.NoError(t, err) -} - // Helper function to create a minimal valid config for the current platform func createMinimalValidConfig() *Config { if runtime.GOOS == "windows" { @@ -367,3 +454,22 @@ func createMinimalValidConfig() *Config { UnixSocketPath: "/tmp/test.socket", } } + +func loadReceiverConfig(t *testing.T, configYAML string) *Config { + t.Helper() + + configPath := filepath.Join(t.TempDir(), "config.yml") + require.NoError(t, os.WriteFile(configPath, []byte(configYAML), 0644)) + + rawConfig, err := confmaptest.LoadConf(configPath) + require.NoError(t, err) + + receiverConfig, err := rawConfig.Sub( + "receivers" + confmap.KeyDelimiter + "trace2receiver", + ) + require.NoError(t, err) + + cfg := createDefaultConfig().(*Config) + require.NoError(t, receiverConfig.Unmarshal(cfg)) + return cfg +} diff --git a/factory.go b/factory.go index de9d521..d4edace 100644 --- a/factory.go +++ b/factory.go @@ -18,10 +18,6 @@ func createDefaultConfig() component.Config { NamedPipePath: "", UnixSocketPath: "", AllowCommandControlVerbs: false, - PiiSettingsPath: "", - piiSettings: nil, - FilterSettingsPath: "", - filterSettings: nil, } } diff --git a/filter_settings.go b/filter_settings.go index 95bf568..06ed1d6 100644 --- a/filter_settings.go +++ b/filter_settings.go @@ -10,10 +10,10 @@ import ( // look for in the Trace2 event stream to help us decide how to // filter data for a particular command. type FilterSettings struct { - Keynames FilterKeynames `mapstructure:"keynames"` - Nicknames FilterNicknames `mapstructure:"nicknames"` - Rulesets FilterRulesets `mapstructure:"rulesets"` - Defaults FilterDefaults `mapstructure:"defaults"` + Keynames FilterKeynames `mapstructure:"keynames"` + Nicknames FilterNicknames `mapstructure:"nicknames"` + Rulesets FilterRulesets `mapstructure:"rulesets"` + Defaults FilterDefaults `mapstructure:"defaults"` ImportantEvents []ImportantEventRule `mapstructure:"important_events"` // The set of custom rulesets defined in YML are each parsed @@ -102,39 +102,8 @@ func parseFilterSettingsFromBuffer(data []byte, path string) (*FilterSettings, e return nil, err } - // After parsing the YML and populating the `mapstructure` fields, we need - // to validate them and/or build internal structures from them. - - // For each custom ruleset [ -> ] in the table (the map[string]string), - // create a peer entry in the internal [ -> ] table and preload - // the various `ruleset.yml` files. - fs.rulesetDefs = make(map[string]*RulesetDefinition) - for k_rs_name, v_rs_path := range fs.Rulesets { - if !strings.HasPrefix(k_rs_name, "rs:") || len(k_rs_name) < 4 || len(v_rs_path) == 0 { - return nil, fmt.Errorf("ruleset has invalid name or pathname'%s':'%s'", k_rs_name, v_rs_path) - } - - fs.rulesetDefs[k_rs_name], err = parseRulesetFile(v_rs_path) - if err != nil { - return nil, err - } - } - - fieldNames := make(map[string]bool) - for i, rule := range fs.ImportantEvents { - if len(rule.Category) == 0 { - return nil, fmt.Errorf("important_events[%d]: category cannot be empty", i) - } - if len(rule.KeyPrefix) == 0 { - return nil, fmt.Errorf("important_events[%d]: key_prefix cannot be empty", i) - } - if len(rule.FieldName) == 0 { - return nil, fmt.Errorf("important_events[%d]: field_name cannot be empty", i) - } - if fieldNames[rule.FieldName] { - return nil, fmt.Errorf("important_events[%d]: duplicate field_name '%s'", i, rule.FieldName) - } - fieldNames[rule.FieldName] = true + if err = fs.validate(); err != nil { + return nil, err } return fs, nil @@ -153,7 +122,7 @@ func apply__important_events(tr2 *trace2Dataset, category string, key string, va return } - fs := tr2.rcvr_base.RcvrConfig.filterSettings + fs := tr2.rcvr_base.RcvrConfig.Filter if fs == nil { return } @@ -166,6 +135,44 @@ func apply__important_events(tr2 *trace2Dataset, category string, key string, va } } +// validate checks the parsed filter settings and builds internal +// structures. For each custom ruleset [ -> ] in the +// table, create a peer entry in the internal [ -> ] +// table and preload the various `ruleset.yml` files. +func (fs *FilterSettings) validate() error { + fs.rulesetDefs = make(map[string]*RulesetDefinition) + for k_rs_name, v_rs_path := range fs.Rulesets { + if !strings.HasPrefix(k_rs_name, "rs:") || len(k_rs_name) < 4 || len(v_rs_path) == 0 { + return fmt.Errorf("ruleset has invalid name or pathname'%s':'%s'", k_rs_name, v_rs_path) + } + + var err error + fs.rulesetDefs[k_rs_name], err = parseRulesetFile(v_rs_path) + if err != nil { + return err + } + } + + fieldNames := make(map[string]bool) + for i, rule := range fs.ImportantEvents { + if len(rule.Category) == 0 { + return fmt.Errorf("important_events[%d]: category cannot be empty", i) + } + if len(rule.KeyPrefix) == 0 { + return fmt.Errorf("important_events[%d]: key_prefix cannot be empty", i) + } + if len(rule.FieldName) == 0 { + return fmt.Errorf("important_events[%d]: field_name cannot be empty", i) + } + if fieldNames[rule.FieldName] { + return fmt.Errorf("important_events[%d]: duplicate field_name '%s'", i, rule.FieldName) + } + fieldNames[rule.FieldName] = true + } + + return nil +} + // Add a ruleset to the filter settings. This is primarily for writing test code. func (fs *FilterSettings) addRuleset(rs_name string, path string, rsdef *RulesetDefinition) { if fs.Rulesets == nil { diff --git a/filter_settings_test.go b/filter_settings_test.go index 68ce188..7a712c2 100644 --- a/filter_settings_test.go +++ b/filter_settings_test.go @@ -363,6 +363,62 @@ important_events: assert.Contains(t, err.Error(), "duplicate field_name") } +func Test_ImportantEvents_InlineValidation(t *testing.T) { + tests := []struct { + name string + rules []ImportantEventRule + errorContains string + }{ + { + name: "valid", + rules: []ImportantEventRule{ + {Category: "network", KeyPrefix: "timeout/", FieldName: "network_timeouts"}, + }, + }, + { + name: "empty category", + rules: []ImportantEventRule{ + {KeyPrefix: "timeout/", FieldName: "network_timeouts"}, + }, + errorContains: "category cannot be empty", + }, + { + name: "empty key prefix", + rules: []ImportantEventRule{ + {Category: "network", FieldName: "network_timeouts"}, + }, + errorContains: "key_prefix cannot be empty", + }, + { + name: "empty field name", + rules: []ImportantEventRule{ + {Category: "network", KeyPrefix: "timeout/"}, + }, + errorContains: "field_name cannot be empty", + }, + { + name: "duplicate field name", + rules: []ImportantEventRule{ + {Category: "network", KeyPrefix: "timeout/", FieldName: "events"}, + {Category: "filesystem", KeyPrefix: "error/", FieldName: "events"}, + }, + errorContains: "duplicate field_name", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fs := &FilterSettings{ImportantEvents: tt.rules} + err := fs.validate() + if tt.errorContains == "" { + assert.NoError(t, err) + } else { + assert.ErrorContains(t, err, tt.errorContains) + } + }) + } +} + // ////////////////////////////////////////////////////////////// func Test_Nil_Nil_FilterSettings(t *testing.T) { diff --git a/go.mod b/go.mod index e2036c6..c667946 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/stretchr/testify v1.11.1 go.opentelemetry.io/collector/component v1.63.0 go.opentelemetry.io/collector/component/componentstatus v0.157.0 + go.opentelemetry.io/collector/confmap v1.63.0 go.opentelemetry.io/collector/consumer v1.63.0 go.opentelemetry.io/collector/pdata v1.63.0 go.opentelemetry.io/collector/receiver v1.63.0 @@ -15,11 +16,19 @@ require ( require ( github.com/cespare/xxhash/v2 v2.3.0 // indirect + github.com/go-viper/mapstructure/v2 v2.5.0 // indirect + github.com/gobwas/glob v0.2.3 // indirect github.com/hashicorp/go-version v1.9.0 // indirect + github.com/knadh/koanf/maps v0.1.2 // indirect + github.com/knadh/koanf/providers/confmap v1.0.0 // indirect + github.com/knadh/koanf/v2 v2.3.5 // indirect + github.com/mitchellh/copystructure v1.2.0 // indirect + github.com/mitchellh/reflectwalk v1.0.2 // indirect go.opentelemetry.io/auto/sdk v1.2.1 // indirect go.opentelemetry.io/collector/featuregate v1.63.0 // indirect go.opentelemetry.io/collector/internal/componentalias v0.157.0 // indirect go.opentelemetry.io/collector/pipeline v1.63.0 // indirect + go.yaml.in/yaml/v3 v3.0.4 // indirect ) require ( diff --git a/go.sum b/go.sum index 370685f..4031c59 100644 --- a/go.sum +++ b/go.sum @@ -8,6 +8,10 @@ github.com/go-logr/logr v1.4.4 h1:tG4xh9yMsRCAiodLVTxyrkzSZ9+o0L1Kg/+cPVcbP/8= github.com/go-logr/logr v1.4.4/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag= github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE= +github.com/go-viper/mapstructure/v2 v2.5.0 h1:vM5IJoUAy3d7zRSVtIwQgBj7BiWtMPfmPEgAXnvj1Ro= +github.com/go-viper/mapstructure/v2 v2.5.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= +github.com/gobwas/glob v0.2.3 h1:A4xDbljILXROh+kObIiy5kIaPYD8e96x1tgBhUI5J+Y= +github.com/gobwas/glob v0.2.3/go.mod h1:d3Ez4x06l9bZtSvzIay5+Yzi0fmZzPgnTbPcKjJAkT8= github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= @@ -15,12 +19,22 @@ github.com/hashicorp/go-version v1.9.0 h1:CeOIz6k+LoN3qX9Z0tyQrPtiB1DFYRPfCIBtaX github.com/hashicorp/go-version v1.9.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo= +github.com/knadh/koanf/maps v0.1.2 h1:RBfmAW5CnZT+PJ1CVc1QSJKf4Xu9kxfQgYVQSu8hpbo= +github.com/knadh/koanf/maps v0.1.2/go.mod h1:npD/QZY3V6ghQDdcQzl1W4ICNVTkohC8E73eI2xW4yI= +github.com/knadh/koanf/providers/confmap v1.0.0 h1:mHKLJTE7iXEys6deO5p6olAiZdG5zwp8Aebir+/EaRE= +github.com/knadh/koanf/providers/confmap v1.0.0/go.mod h1:txHYHiI2hAtF0/0sCmcuol4IDcuQbKTybiB1nOcUo1A= +github.com/knadh/koanf/v2 v2.3.5 h1:2dXJUYaKGm4SGYeoAtBviq9+02JZo/pxQ2ssOd60rJg= +github.com/knadh/koanf/v2 v2.3.5/go.mod h1:gRb40VRAbd4iJMYYD5IxZ6hfuopFcXBpc9bbQpZwo28= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa15WveJJGw= +github.com/mitchellh/copystructure v1.2.0/go.mod h1:qLl+cE2AmVv+CoeAwDPye/v+N2HKCj9FbZEVFJRxO9s= github.com/mitchellh/mapstructure v1.5.1-0.20231216201459-8508981c8b6c h1:cqn374mizHuIWj+OSJCajGr/phAmuMug9qIX3l9CflE= github.com/mitchellh/mapstructure v1.5.1-0.20231216201459-8508981c8b6c/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= +github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zxSIeXaQ= +github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw= github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd h1:TRLaZ9cD/w8PVh93nsPXa1VrQ6jlwL5oN8l14QlcNfg= github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q= @@ -41,6 +55,8 @@ go.opentelemetry.io/collector/component v1.63.0 h1:l98ZCxfCTt/O6dYB0JVKKtewaFLe/ go.opentelemetry.io/collector/component v1.63.0/go.mod h1:yLGMmT7jUiqvuGvkqlfR1CBi0dRkSV67tq22I08ZMPk= go.opentelemetry.io/collector/component/componentstatus v0.157.0 h1:6aARK84axselDP/YGM1cKVPvg6eIyN7Bg9x8x5GzJb4= go.opentelemetry.io/collector/component/componentstatus v0.157.0/go.mod h1:R1nsiV3JluCaffVfjDmglZ0cU3jJUsiaMXgsAZfc670= +go.opentelemetry.io/collector/confmap v1.63.0 h1:1THBabHoQc8t/9r6ztMsghiO1OxDPZpYtn0cuwwsxYI= +go.opentelemetry.io/collector/confmap v1.63.0/go.mod h1:ksJNAmLTiMkBjMYwXFW1MRRfXYnRsHXA0fW+ZGwb/1U= go.opentelemetry.io/collector/consumer v1.63.0 h1:0eOZh0qmDg6HCJaiUqI9FAb4BD4fKJNNO+ygMV/WW0w= go.opentelemetry.io/collector/consumer v1.63.0/go.mod h1:IVhjv4d+PmSf4Ttz/guJFbWJtRM3Ld3nRcZ12gxy6PA= go.opentelemetry.io/collector/consumer/consumertest v0.157.0 h1:zehAVaLn67AWLhhi/LrUY0Tv06XWx5LYCvvmu7xHXHU= diff --git a/important_events_test.go b/important_events_test.go index b89e740..d6e0e0a 100644 --- a/important_events_test.go +++ b/important_events_test.go @@ -22,7 +22,7 @@ func Test_ImportantEvents_Match_Basic(t *testing.T) { } tr2.rcvr_base = &Rcvr_Base{ RcvrConfig: &Config{ - filterSettings: fs, + Filter: fs, }, } @@ -51,7 +51,7 @@ func Test_ImportantEvents_Match_NoMatch(t *testing.T) { } tr2.rcvr_base = &Rcvr_Base{ RcvrConfig: &Config{ - filterSettings: fs, + Filter: fs, }, } @@ -72,7 +72,7 @@ func Test_ImportantEvents_Match_NoConfig(t *testing.T) { } tr2.rcvr_base = &Rcvr_Base{ RcvrConfig: &Config{ - filterSettings: nil, + Filter: nil, }, } @@ -83,7 +83,7 @@ func Test_ImportantEvents_Match_NoConfig(t *testing.T) { // Test important_events values appear in their own attribute at dl:summary func Test_ImportantEvents_EmittedAtSummaryLevel(t *testing.T) { cfg := &Config{ - filterSettings: &FilterSettings{ + Filter: &FilterSettings{ ImportantEvents: []ImportantEventRule{ {Category: "gvfs-helper", KeyPrefix: "error/", FieldName: "gvfs_helper_errors"}, }, @@ -141,7 +141,7 @@ func Test_ImportantEvents_Match_IntValue(t *testing.T) { } tr2.rcvr_base = &Rcvr_Base{ RcvrConfig: &Config{ - filterSettings: fs, + Filter: fs, }, } @@ -157,7 +157,7 @@ func Test_ImportantEvents_Match_IntValue(t *testing.T) { // (nesting > 1 with no matching region). func Test_ImportantEvents_EndToEnd_NestedEvent(t *testing.T) { cfg := &Config{ - filterSettings: &FilterSettings{ + Filter: &FilterSettings{ ImportantEvents: []ImportantEventRule{ {Category: "gvfs-helper", KeyPrefix: "error/", FieldName: "gvfs_helper_errors"}, }, @@ -292,7 +292,7 @@ func extractImportantEventsJSON(t *testing.T, tr2 *trace2Dataset, dl FilterDetai // captured and visible in the OTLP span at dl:summary. func Test_E2E_ImportantEvents_ProcessLevel_AtSummaryDetailLevel(t *testing.T) { cfg := &Config{ - filterSettings: &FilterSettings{ + Filter: &FilterSettings{ ImportantEvents: []ImportantEventRule{ {Category: "gvfs-helper", KeyPrefix: "error/", FieldName: "gvfs_helper_errors"}, }, @@ -325,7 +325,7 @@ func Test_E2E_ImportantEvents_ProcessLevel_AtSummaryDetailLevel(t *testing.T) { // important_events AND the region should have it in its own data. func Test_E2E_ImportantEvents_InsideRegion(t *testing.T) { cfg := &Config{ - filterSettings: &FilterSettings{ + Filter: &FilterSettings{ ImportantEvents: []ImportantEventRule{ {Category: "gvfs-helper", KeyPrefix: "error/", FieldName: "gvfs_helper_errors"}, }, @@ -364,7 +364,7 @@ func Test_E2E_ImportantEvents_InsideRegion(t *testing.T) { // Value should still be captured; region attachment fails silently. func Test_E2E_ImportantEvents_OrphanedNesting(t *testing.T) { cfg := &Config{ - filterSettings: &FilterSettings{ + Filter: &FilterSettings{ ImportantEvents: []ImportantEventRule{ {Category: "gvfs-helper", KeyPrefix: "error/", FieldName: "gvfs_helper_errors"}, }, @@ -395,7 +395,7 @@ func Test_E2E_ImportantEvents_OrphanedNesting(t *testing.T) { // Test: multiple data events matching the same rule accumulate all values. func Test_E2E_ImportantEvents_MultipleValues(t *testing.T) { cfg := &Config{ - filterSettings: &FilterSettings{ + Filter: &FilterSettings{ ImportantEvents: []ImportantEventRule{ {Category: "gvfs-helper", KeyPrefix: "error/", FieldName: "gvfs_helper_errors"}, }, @@ -428,7 +428,7 @@ func Test_E2E_ImportantEvents_MultipleValues(t *testing.T) { // in the important_events. func Test_E2E_ImportantEvents_NonMatchingEventsExcluded(t *testing.T) { cfg := &Config{ - filterSettings: &FilterSettings{ + Filter: &FilterSettings{ ImportantEvents: []ImportantEventRule{ {Category: "gvfs-helper", KeyPrefix: "error/", FieldName: "gvfs_helper_errors"}, }, @@ -458,7 +458,7 @@ func Test_E2E_ImportantEvents_NonMatchingEventsExcluded(t *testing.T) { // Test: integer values (data events can carry int64). func Test_E2E_ImportantEvents_IntegerValue(t *testing.T) { cfg := &Config{ - filterSettings: &FilterSettings{ + Filter: &FilterSettings{ ImportantEvents: []ImportantEventRule{ {Category: "perf", KeyPrefix: "count/", FieldName: "perf_counts"}, }, @@ -488,7 +488,7 @@ func Test_E2E_ImportantEvents_IntegerValue(t *testing.T) { // in the same output without interference. func Test_E2E_ImportantEvents_CoexistsWithOtherRuleTypes(t *testing.T) { cfg := &Config{ - summary: &SummarySettings{ + Summary: &SummarySettings{ MessagePatterns: []MessagePatternRule{ {Prefix: "error:", FieldName: "error_msg_count"}, }, @@ -496,7 +496,7 @@ func Test_E2E_ImportantEvents_CoexistsWithOtherRuleTypes(t *testing.T) { {Category: "gvfs-helper", Label: "fetch", CountField: "fetch_count"}, }, }, - filterSettings: &FilterSettings{ + Filter: &FilterSettings{ ImportantEvents: []ImportantEventRule{ {Category: "gvfs-helper", KeyPrefix: "error/", FieldName: "gvfs_helper_errors"}, }, @@ -539,7 +539,7 @@ func Test_E2E_ImportantEvents_CoexistsWithOtherRuleTypes(t *testing.T) { // Test: captured values appear at ALL detail levels, not just verbose. func Test_E2E_ImportantEvents_AllDetailLevels(t *testing.T) { cfg := &Config{ - filterSettings: &FilterSettings{ + Filter: &FilterSettings{ ImportantEvents: []ImportantEventRule{ {Category: "gvfs-helper", KeyPrefix: "error/", FieldName: "gvfs_helper_errors"}, }, @@ -570,12 +570,12 @@ func Test_E2E_ImportantEvents_AllDetailLevels(t *testing.T) { // create the attribute (no spurious empty arrays). func Test_E2E_ImportantEvents_NoPatternsConfigured(t *testing.T) { cfg := &Config{ - summary: &SummarySettings{ + Summary: &SummarySettings{ MessagePatterns: []MessagePatternRule{ {Prefix: "error:", FieldName: "error_count"}, }, }, - filterSettings: &FilterSettings{}, + Filter: &FilterSettings{}, } events := []string{ @@ -597,8 +597,8 @@ func Test_E2E_ImportantEvents_NoPatternsConfigured(t *testing.T) { // are processed without crashing. func Test_E2E_ImportantEvents_NoSummaryConfig(t *testing.T) { cfg := &Config{ - summary: nil, - filterSettings: &FilterSettings{}, + Summary: nil, + Filter: &FilterSettings{}, } events := []string{ @@ -622,7 +622,7 @@ func Test_E2E_ImportantEvents_NoSummaryConfig(t *testing.T) { // Test: data events on a non-main thread are still captured. func Test_E2E_ImportantEvents_NonMainThread(t *testing.T) { cfg := &Config{ - filterSettings: &FilterSettings{ + Filter: &FilterSettings{ ImportantEvents: []ImportantEventRule{ {Category: "gvfs-helper", KeyPrefix: "error/", FieldName: "gvfs_helper_errors"}, }, @@ -655,7 +655,7 @@ func Test_E2E_ImportantEvents_NonMainThread(t *testing.T) { // stack. The value should still be captured even though region attachment fails. func Test_E2E_ImportantEvents_DeepNesting_PartialRegionStack(t *testing.T) { cfg := &Config{ - filterSettings: &FilterSettings{ + Filter: &FilterSettings{ ImportantEvents: []ImportantEventRule{ {Category: "gvfs-helper", KeyPrefix: "error/", FieldName: "gvfs_helper_errors"}, }, @@ -687,7 +687,7 @@ func Test_E2E_ImportantEvents_DeepNesting_PartialRegionStack(t *testing.T) { // event stream produce independent fields. func Test_E2E_ImportantEvents_MultipleRules(t *testing.T) { cfg := &Config{ - filterSettings: &FilterSettings{ + Filter: &FilterSettings{ ImportantEvents: []ImportantEventRule{ {Category: "gvfs-helper", KeyPrefix: "error/", FieldName: "gvfs_errors"}, {Category: "network", KeyPrefix: "timeout/", FieldName: "network_timeouts"}, diff --git a/platform_unix.go b/platform_unix.go index c93f76e..6400e89 100644 --- a/platform_unix.go +++ b/platform_unix.go @@ -45,13 +45,13 @@ func createTraces(_ context.Context, // possibly the connection from the client process. // Add any requested PII data to `tr2.pii[]`. func (tr2 *trace2Dataset) pii_gather(cfg *Config, conn *net.UnixConn) { - if cfg.piiSettings != nil && cfg.piiSettings.Include.Hostname { + if cfg.Pii != nil && cfg.Pii.Include.Hostname { if h, err := os.Hostname(); err == nil { tr2.pii[string(Trace2PiiHostname)] = h } } - if cfg.piiSettings != nil && cfg.piiSettings.Include.Username { + if cfg.Pii != nil && cfg.Pii.Include.Username { if u, err := getPeerUsername(conn); err == nil { tr2.pii[string(Trace2PiiUsername)] = u } diff --git a/platform_windows.go b/platform_windows.go index 66b9af7..21af014 100644 --- a/platform_windows.go +++ b/platform_windows.go @@ -45,13 +45,13 @@ func createTraces(_ context.Context, // possibly the connection from the client process. // Add any requested PII data to `tr2.pii[]`. func (tr2 *trace2Dataset) pii_gather(cfg *Config) { - if cfg.piiSettings != nil && cfg.piiSettings.Include.Hostname { + if cfg.Pii != nil && cfg.Pii.Include.Hostname { if h, err := os.Hostname(); err == nil { tr2.pii[string(Trace2PiiHostname)] = h } } - if cfg.piiSettings != nil && cfg.piiSettings.Include.Username { + if cfg.Pii != nil && cfg.Pii.Include.Username { // TODO For now, just lookup the current user. This may // or may not be valid when the service is officially // installed. Ideally we should get the user-id of the diff --git a/rcvr_base.go b/rcvr_base.go index 6f21cb7..47e84af 100644 --- a/rcvr_base.go +++ b/rcvr_base.go @@ -35,11 +35,11 @@ func (rcvr_base *Rcvr_Base) Start(unused_ctx context.Context, host component.Hos rcvr_base.Logger.Info("Command verbs are enabled") } - if rcvr_base.RcvrConfig.piiSettings != nil { - if rcvr_base.RcvrConfig.piiSettings.Include.Hostname { + if rcvr_base.RcvrConfig.Pii != nil { + if rcvr_base.RcvrConfig.Pii.Include.Hostname { rcvr_base.Logger.Info("PII: Hostname logging is enabled") } - if rcvr_base.RcvrConfig.piiSettings.Include.Username { + if rcvr_base.RcvrConfig.Pii.Include.Username { rcvr_base.Logger.Info("PII: Username logging is enabled") } } diff --git a/summary.go b/summary.go index bd62ab1..0a2b6d0 100644 --- a/summary.go +++ b/summary.go @@ -108,7 +108,7 @@ func apply__summary_message(tr2 *trace2Dataset, message string) { return } - css := tr2.rcvr_base.RcvrConfig.summary + css := tr2.rcvr_base.RcvrConfig.Summary if css == nil { return } @@ -134,7 +134,7 @@ func apply__summary_region(tr2 *trace2Dataset, region *TrRegion) { return } - css := tr2.rcvr_base.RcvrConfig.summary + css := tr2.rcvr_base.RcvrConfig.Summary if css == nil { return } diff --git a/summary_settings.go b/summary_settings.go index 8f7ef12..9b68241 100644 --- a/summary_settings.go +++ b/summary_settings.go @@ -60,19 +60,29 @@ func parseSummarySettingsFromBuffer(data []byte, path string) (*SummarySettings, return nil, err } + if err = css.validate(); err != nil { + return nil, err + } + + return css, nil +} + +// validate checks the parsed summary settings for errors such as +// empty required fields and duplicate field names. +func (css *SummarySettings) validate() error { // Track all field names to detect duplicates fieldNames := make(map[string]bool) // Validate message pattern rules for i, rule := range css.MessagePatterns { if len(rule.Prefix) == 0 { - return nil, fmt.Errorf("message_patterns[%d]: prefix cannot be empty", i) + return fmt.Errorf("message_patterns[%d]: prefix cannot be empty", i) } if len(rule.FieldName) == 0 { - return nil, fmt.Errorf("message_patterns[%d]: field_name cannot be empty", i) + return fmt.Errorf("message_patterns[%d]: field_name cannot be empty", i) } if fieldNames[rule.FieldName] { - return nil, fmt.Errorf("message_patterns[%d]: duplicate field_name '%s'", i, rule.FieldName) + return fmt.Errorf("message_patterns[%d]: duplicate field_name '%s'", i, rule.FieldName) } fieldNames[rule.FieldName] = true } @@ -80,29 +90,29 @@ func parseSummarySettingsFromBuffer(data []byte, path string) (*SummarySettings, // Validate region timer rules for i, rule := range css.RegionTimers { if len(rule.Category) == 0 { - return nil, fmt.Errorf("region_timers[%d]: category cannot be empty", i) + return fmt.Errorf("region_timers[%d]: category cannot be empty", i) } if len(rule.Label) == 0 { - return nil, fmt.Errorf("region_timers[%d]: label cannot be empty", i) + return fmt.Errorf("region_timers[%d]: label cannot be empty", i) } if len(rule.CountField) == 0 && len(rule.TimeField) == 0 { - return nil, fmt.Errorf("region_timers[%d]: at least one of count_field or time_field must be specified", i) + return fmt.Errorf("region_timers[%d]: at least one of count_field or time_field must be specified", i) } if len(rule.CountField) > 0 { if fieldNames[rule.CountField] { - return nil, fmt.Errorf("region_timers[%d]: duplicate field_name '%s'", i, rule.CountField) + return fmt.Errorf("region_timers[%d]: duplicate field_name '%s'", i, rule.CountField) } fieldNames[rule.CountField] = true } if len(rule.TimeField) > 0 { if fieldNames[rule.TimeField] { - return nil, fmt.Errorf("region_timers[%d]: duplicate field_name '%s'", i, rule.TimeField) + return fmt.Errorf("region_timers[%d]: duplicate field_name '%s'", i, rule.TimeField) } fieldNames[rule.TimeField] = true } } - return css, nil + return nil } diff --git a/summary_test.go b/summary_test.go index 44c1e34..ad2e99d 100644 --- a/summary_test.go +++ b/summary_test.go @@ -347,7 +347,7 @@ func Test_MessagePatternMatching_Basic(t *testing.T) { } tr2.rcvr_base = &Rcvr_Base{ RcvrConfig: &Config{ - summary: css, + Summary: css, }, } @@ -380,7 +380,7 @@ func Test_MessagePatternMatching_MultipleMatches(t *testing.T) { } tr2.rcvr_base = &Rcvr_Base{ RcvrConfig: &Config{ - summary: css, + Summary: css, }, } @@ -403,7 +403,7 @@ func Test_MessagePatternMatching_NoConfig(t *testing.T) { } tr2.rcvr_base = &Rcvr_Base{ RcvrConfig: &Config{ - summary: nil, + Summary: nil, }, } @@ -432,7 +432,7 @@ func Test_RegionTimerAggregation_Basic(t *testing.T) { } tr2.rcvr_base = &Rcvr_Base{ RcvrConfig: &Config{ - summary: css, + Summary: css, }, } @@ -482,7 +482,7 @@ func Test_RegionTimerAggregation_CountOnly(t *testing.T) { } tr2.rcvr_base = &Rcvr_Base{ RcvrConfig: &Config{ - summary: css, + Summary: css, }, } @@ -522,7 +522,7 @@ func Test_RegionTimerAggregation_TimeOnly(t *testing.T) { } tr2.rcvr_base = &Rcvr_Base{ RcvrConfig: &Config{ - summary: css, + Summary: css, }, } @@ -545,12 +545,12 @@ func Test_RegionTimerAggregation_TimeOnly(t *testing.T) { func Test_Summary_EmittedAtSummaryLevel(t *testing.T) { // Create a minimal config with summary cfg := &Config{ - summary: &SummarySettings{ + Summary: &SummarySettings{ MessagePatterns: []MessagePatternRule{ {Prefix: "test_msg:", FieldName: "msgCount"}, }, }, - filterSettings: &FilterSettings{}, + Filter: &FilterSettings{}, } rcvr := &Rcvr_Base{ @@ -615,7 +615,7 @@ func Test_RegionTimerAggregation_NoMatch(t *testing.T) { } tr2.rcvr_base = &Rcvr_Base{ RcvrConfig: &Config{ - summary: css, + Summary: css, }, } diff --git a/trace2dataset.go b/trace2dataset.go index 499faff..b6b0dd8 100644 --- a/trace2dataset.go +++ b/trace2dataset.go @@ -273,10 +273,10 @@ func NewTrace2Dataset(rcvr_base *Rcvr_Base) *trace2Dataset { if rcvr_base != nil && rcvr_base.RcvrConfig != nil { cfg := rcvr_base.RcvrConfig - if cfg.summary != nil { - tr2.process.summary = configuredSummary(cfg.summary) + if cfg.Summary != nil { + tr2.process.summary = configuredSummary(cfg.Summary) } - if cfg.filterSettings != nil && len(cfg.filterSettings.ImportantEvents) > 0 { + if cfg.Filter != nil && len(cfg.Filter.ImportantEvents) > 0 { tr2.process.importantEvents = make(map[string][]interface{}) } } @@ -521,7 +521,7 @@ func (tr2 *trace2Dataset) exportTraces() { } dl, dl_debug := computeDetailLevel( - tr2.rcvr_base.RcvrConfig.filterSettings, + tr2.rcvr_base.RcvrConfig.Filter, tr2.process.paramSetValues, tr2.process.qualifiedNames) @@ -532,8 +532,8 @@ func (tr2 *trace2Dataset) exportTraces() { } var keynames FilterKeynames - if tr2.rcvr_base.RcvrConfig.filterSettings != nil { - keynames = tr2.rcvr_base.RcvrConfig.filterSettings.Keynames + if tr2.rcvr_base.RcvrConfig.Filter != nil { + keynames = tr2.rcvr_base.RcvrConfig.Filter.Keynames } traces := tr2.ToTraces(dl, keynames)