From 01c46bcf0caa93cfd36c7d5cabca2cdb94485b14 Mon Sep 17 00:00:00 2001 From: Ryan Fitzpatrick Date: Wed, 8 Feb 2023 23:20:55 +0000 Subject: [PATCH 1/2] signalfx exporter: Add exclude_properties config option for limiting dimension updates --- .../signalfx-exporter-filter-properties.yaml | 11 ++ exporter/signalfxexporter/README.md | 12 +++ exporter/signalfxexporter/config.go | 4 + exporter/signalfxexporter/config_test.go | 32 ++++++ exporter/signalfxexporter/exporter.go | 1 + exporter/signalfxexporter/exporter_test.go | 100 +++++++++++++++-- .../internal/dimensions/dimclient.go | 59 +++++++--- .../translation/dpfilters/propertyfilter.go | 74 +++++++++++++ .../dpfilters/propertyfilter_test.go | 101 ++++++++++++++++++ .../internal/translation/dpfilters/string.go | 9 ++ .../signalfxexporter/testdata/config.yaml | 11 +- 11 files changed, 393 insertions(+), 21 deletions(-) create mode 100755 .chloggen/signalfx-exporter-filter-properties.yaml create mode 100644 exporter/signalfxexporter/internal/translation/dpfilters/propertyfilter.go create mode 100644 exporter/signalfxexporter/internal/translation/dpfilters/propertyfilter_test.go diff --git a/.chloggen/signalfx-exporter-filter-properties.yaml b/.chloggen/signalfx-exporter-filter-properties.yaml new file mode 100755 index 000000000000..f1775add3184 --- /dev/null +++ b/.chloggen/signalfx-exporter-filter-properties.yaml @@ -0,0 +1,11 @@ +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: signalfxexporter + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Add `exclude_properties` config option to filter dimension update property content + +# One or more tracking issues related to the change +issues: [18464] diff --git a/exporter/signalfxexporter/README.md b/exporter/signalfxexporter/README.md index 41b4e6f7df26..ab1b09cd7cd8 100644 --- a/exporter/signalfxexporter/README.md +++ b/exporter/signalfxexporter/README.md @@ -85,6 +85,18 @@ The following configuration options can also be configured: processor is enabled in the pipeline with one of the cloud provider detectors or environment variable detector setting a unique value to `host.name` attribute within your k8s cluster. And keep `override=true` in resourcedetection config. +- `exclude_properties`: A list of property filters to limit dimension update content. + Property filters can contain any number of the following fields, supporting (negated) + string literals, re2 `/regex/`, and [glob](https://github.com/gobwas/glob) syntax values: + `dimension_name`, `dimension_value`, `property_name`, and `property_value`. For any field + not expressly configured for each filter object, a default catch-all value of `/^.*$/` is used + to allow each specified field to require a match for the filter to take effect: + ```yaml + # will filter all 'k8s.workload.name' properties from 'k8s.pod.uid' dimension updates: + exclude_properties: + - dimension_name: k8s.pod.uid + property_name: k8s.workload.name + ``` - `nonalphanumeric_dimension_chars`: (default = `"_-."`) A string of characters that are allowed to be used as a dimension key in addition to alphanumeric characters. Each nonalphanumeric dimension key character that isn't in this string diff --git a/exporter/signalfxexporter/config.go b/exporter/signalfxexporter/config.go index ff65b529b043..60f122ed0d43 100644 --- a/exporter/signalfxexporter/config.go +++ b/exporter/signalfxexporter/config.go @@ -126,6 +126,10 @@ type Config struct { // See ./translation/default_metrics.go for a list of metrics that are dropped by default. IncludeMetrics []dpfilters.MetricFilter `mapstructure:"include_metrics"` + // ExcludeProperties defines dpfilter.PropertyFilters to prevent inclusion of + // properties to include with dimension updates to the SignalFx backend. + ExcludeProperties []dpfilters.PropertyFilter `mapstructure:"exclude_properties"` + // Correlation configuration for syncing traces service and environment to metrics. Correlation *correlation.Config `mapstructure:"correlation"` diff --git a/exporter/signalfxexporter/config_test.go b/exporter/signalfxexporter/config_test.go index 84ce620879b6..9ac31e2d6ae8 100644 --- a/exporter/signalfxexporter/config_test.go +++ b/exporter/signalfxexporter/config_test.go @@ -52,6 +52,8 @@ func TestLoadConfig(t *testing.T) { seventy := 70 + allRE := mustStringFilter(t, "/^.*$/") + tests := []struct { id component.ID expected *Config @@ -163,6 +165,30 @@ func TestLoadConfig(t *testing.T) { }, }, DeltaTranslationTTL: 3600, + ExcludeProperties: []dpfilters.PropertyFilter{ + { + PropertyName: mustStringFilter(t, "globbed*"), + PropertyValue: allRE, DimensionName: allRE, DimensionValue: allRE, + }, + { + PropertyValue: mustStringFilter(t, "!globbed*value"), + PropertyName: allRE, DimensionName: allRE, DimensionValue: allRE, + }, + { + DimensionName: mustStringFilter(t, "globbed*"), + PropertyName: allRE, PropertyValue: allRE, DimensionValue: allRE, + }, + { + DimensionValue: mustStringFilter(t, "!globbed*value"), + PropertyName: allRE, PropertyValue: allRE, DimensionName: allRE, + }, + { + PropertyName: mustStringFilter(t, "globbed*"), + PropertyValue: mustStringFilter(t, "!globbed*value"), + DimensionName: mustStringFilter(t, "globbed*"), + DimensionValue: mustStringFilter(t, "!globbed*value"), + }, + }, Correlation: &correlation.Config{ HTTPClientSettings: confighttp.HTTPClientSettings{ Endpoint: "", @@ -483,3 +509,9 @@ func TestUnmarshalExcludeMetrics(t *testing.T) { }) } } + +func mustStringFilter(t *testing.T, filter string) *dpfilters.StringFilter { + sf, err := dpfilters.NewStringFilter([]string{filter}) + require.NoError(t, err) + return sf +} diff --git a/exporter/signalfxexporter/exporter.go b/exporter/signalfxexporter/exporter.go index 1da3fbaf07a3..0dfb6e114ec4 100644 --- a/exporter/signalfxexporter/exporter.go +++ b/exporter/signalfxexporter/exporter.go @@ -161,6 +161,7 @@ func (se *signalfxExporter) start(ctx context.Context, host component.Host) (err // to make configurable. PropertiesMaxBuffered: 10000, MetricsConverter: *se.converter, + ExcludeProperties: se.config.ExcludeProperties, }) dimClient.Start() diff --git a/exporter/signalfxexporter/exporter_test.go b/exporter/signalfxexporter/exporter_test.go index 8cdb5b9b305c..a4e58647c2ce 100644 --- a/exporter/signalfxexporter/exporter_test.go +++ b/exporter/signalfxexporter/exporter_test.go @@ -752,8 +752,11 @@ func TestConsumeMetadata(t *testing.T) { name string fields fields args args + excludeProperties []dpfilters.PropertyFilter expectedDimensionKey string expectedDimensionValue string + sendDelay int + shouldNotSendUpdate bool }{ { name: "Test property updates", @@ -769,6 +772,26 @@ func TestConsumeMetadata(t *testing.T) { "tagsToRemove": nil, }, }, + excludeProperties: []dpfilters.PropertyFilter{ + { + DimensionName: mustStringFilter(t, "/^.*$/"), + DimensionValue: mustStringFilter(t, "/^.*$/"), + PropertyName: mustStringFilter(t, "/^property2$/"), + PropertyValue: mustStringFilter(t, "some*value"), + }, + { + DimensionName: mustStringFilter(t, "/^.*$/"), + DimensionValue: mustStringFilter(t, "/^.*$/"), + PropertyName: mustStringFilter(t, "property5"), + PropertyValue: mustStringFilter(t, "/^.*$/"), + }, + { + DimensionName: mustStringFilter(t, "*"), + DimensionValue: mustStringFilter(t, "*"), + PropertyName: mustStringFilter(t, "/^pro[op]erty6$/"), + PropertyValue: mustStringFilter(t, "property*value"), + }, + }, args: args{ []*metadata.MetadataUpdate{ { @@ -777,9 +800,12 @@ func TestConsumeMetadata(t *testing.T) { MetadataDelta: metadata.MetadataDelta{ MetadataToAdd: map[string]string{ "prop.erty1": "val1", + "property5": "added.value", + "property6": "property6.value", }, MetadataToRemove: map[string]string{ "property2": "val2", + "property5": "removed.value", }, MetadataToUpdate: map[string]string{ "prop.erty3": "val33", @@ -805,6 +831,15 @@ func TestConsumeMetadata(t *testing.T) { }, }, }, + excludeProperties: []dpfilters.PropertyFilter{ + { + // confirms tags aren't affected by excludeProperties filters + DimensionName: mustStringFilter(t, "/^.*$/"), + DimensionValue: mustStringFilter(t, "/^.*$/"), + PropertyName: mustStringFilter(t, "/^.*$/"), + PropertyValue: mustStringFilter(t, "/^.*$/"), + }, + }, args: args{ []*metadata.MetadataUpdate{ { @@ -893,6 +928,7 @@ func TestConsumeMetadata(t *testing.T) { }, expectedDimensionKey: "key", expectedDimensionValue: "id", + sendDelay: 1, }, { name: "Test updates on dimensions with nonalphanumeric characters (other than the default allow list)", @@ -931,14 +967,52 @@ func TestConsumeMetadata(t *testing.T) { expectedDimensionKey: "k_e_y", expectedDimensionValue: "id", }, + { + name: "no dimension update for empty properties", + shouldNotSendUpdate: true, + excludeProperties: []dpfilters.PropertyFilter{ + { + DimensionName: mustStringFilter(t, "key"), + DimensionValue: mustStringFilter(t, "/^.*$/"), + PropertyName: mustStringFilter(t, "/^prop\\.erty[13]$/"), + PropertyValue: mustStringFilter(t, "/^.*$/"), + }, + { + DimensionName: mustStringFilter(t, "*"), + DimensionValue: mustStringFilter(t, "id"), + PropertyName: mustStringFilter(t, "property*"), + PropertyValue: mustStringFilter(t, "/^.*$/"), + }, + }, + args: args{ + []*metadata.MetadataUpdate{ + { + ResourceIDKey: "key", + ResourceID: "id", + MetadataDelta: metadata.MetadataDelta{ + MetadataToAdd: map[string]string{ + "prop.erty1": "val1", + "property2": "val2", + "property5": "added.value", + "property6": "property6.value", + }, + MetadataToUpdate: map[string]string{ + "prop.erty3": "val33", + "property4": "val", + }, + }, + }, + }, + }, + }, } for _, tt := range tests { - // Use WaitGroup to ensure the mocked server has encountered - // a request from the exporter. - wg := sync.WaitGroup{} - wg.Add(1) - t.Run(tt.name, func(t *testing.T) { + // Use WaitGroup to ensure the mocked server has encountered + // a request from the exporter. + wg := sync.WaitGroup{} + wg.Add(1) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { b, err := io.ReadAll(r.Body) assert.NoError(t, err) @@ -974,9 +1048,10 @@ func TestConsumeMetadata(t *testing.T) { APIURL: serverURL, LogUpdates: true, Logger: logger, - SendDelay: 1, + SendDelay: tt.sendDelay, PropertiesMaxBuffered: 10, MetricsConverter: *converter, + ExcludeProperties: tt.excludeProperties, }) dimClient.Start() @@ -991,9 +1066,18 @@ func TestConsumeMetadata(t *testing.T) { } err = sme.ConsumeMetadata(tt.args.metadata) + c := make(chan struct{}) + go func() { + defer close(c) + wg.Wait() + }() - // Wait for requests to be handled by the mocked server before assertion. - wg.Wait() + select { + case <-c: + // wait 5ms longer than send delay + case <-time.After(time.Duration(tt.sendDelay)*time.Second + 5*time.Millisecond): + require.True(t, tt.shouldNotSendUpdate, "timeout waiting for response") + } require.NoError(t, err) }) diff --git a/exporter/signalfxexporter/internal/dimensions/dimclient.go b/exporter/signalfxexporter/internal/dimensions/dimclient.go index 95b9b4ca9e99..47d8e6188578 100644 --- a/exporter/signalfxexporter/internal/dimensions/dimclient.go +++ b/exporter/signalfxexporter/internal/dimensions/dimclient.go @@ -33,6 +33,7 @@ import ( "go.uber.org/zap" "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/signalfxexporter/internal/translation" + "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/signalfxexporter/internal/translation/dpfilters" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/sanitize" ) @@ -65,6 +66,8 @@ type DimensionClient struct { logUpdates bool logger *zap.Logger metricsConverter translation.MetricsConverter + // ExcludeProperties will filter DimensionUpdate content to not submit undesired metadata. + ExcludeProperties []dpfilters.PropertyFilter } type queuedDimension struct { @@ -81,6 +84,7 @@ type DimensionClientOptions struct { SendDelay int PropertiesMaxBuffered int MetricsConverter translation.MetricsConverter + ExcludeProperties []dpfilters.PropertyFilter } // NewDimensionClient returns a new client @@ -104,18 +108,19 @@ func NewDimensionClient(ctx context.Context, options DimensionClientOptions) *Di sender := NewReqSender(ctx, client, 20, map[string]string{"client": "dimension"}) return &DimensionClient{ - ctx: ctx, - Token: options.Token, - APIURL: options.APIURL, - sendDelay: time.Duration(options.SendDelay) * time.Second, - delayedSet: make(map[DimensionKey]*DimensionUpdate), - delayedQueue: make(chan *queuedDimension, options.PropertiesMaxBuffered), - requestSender: sender, - client: client, - now: time.Now, - logger: options.Logger, - logUpdates: options.LogUpdates, - metricsConverter: options.MetricsConverter, + ctx: ctx, + Token: options.Token, + APIURL: options.APIURL, + sendDelay: time.Duration(options.SendDelay) * time.Second, + delayedSet: make(map[DimensionKey]*DimensionUpdate), + delayedQueue: make(chan *queuedDimension, options.PropertiesMaxBuffered), + requestSender: sender, + client: client, + now: time.Now, + logger: options.Logger, + logUpdates: options.LogUpdates, + metricsConverter: options.MetricsConverter, + ExcludeProperties: options.ExcludeProperties, } } @@ -127,6 +132,10 @@ func (dc *DimensionClient) Start() { // acceptDimension to be sent to the API. This will return fairly quickly and // won't block. If the buffer is full, the dim update will be dropped. func (dc *DimensionClient) acceptDimension(dimUpdate *DimensionUpdate) error { + if dimUpdate = dc.filterDimensionUpdate(dimUpdate); dimUpdate == nil { + return nil + } + dc.Lock() defer dc.Unlock() @@ -325,3 +334,29 @@ func (dc *DimensionClient) makePatchRequest(dim *DimensionUpdate) (*http.Request return req, nil } + +func (dc *DimensionClient) filterDimensionUpdate(update *DimensionUpdate) *DimensionUpdate { + for _, excludeRule := range dc.ExcludeProperties { + if excludeRule.DimensionName.Matches(update.Name) && excludeRule.DimensionValue.Matches(update.Value) { + for k, v := range update.Properties { + if excludeRule.PropertyName.Matches(k) { + vVal := "" + if v != nil { + vVal = *v + } + if excludeRule.PropertyValue.Matches(vVal) { + delete(update.Properties, k) + } + } + } + } + } + + // Prevent needless dimension updates if all content has been filtered. + // Based on https://github.com/signalfx/signalfx-agent/blob/a10f69ec6b95d7426adaf639773628fa034628b8/pkg/core/propfilters/dimfilter.go#L95 + if len(update.Properties) == 0 && len(update.Tags) == 0 { + return nil + } + + return update +} diff --git a/exporter/signalfxexporter/internal/translation/dpfilters/propertyfilter.go b/exporter/signalfxexporter/internal/translation/dpfilters/propertyfilter.go new file mode 100644 index 000000000000..838e561aaa4e --- /dev/null +++ b/exporter/signalfxexporter/internal/translation/dpfilters/propertyfilter.go @@ -0,0 +1,74 @@ +// Copyright 2021, OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dpfilters // import "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/signalfxexporter/internal/translation/dpfilters" + +import ( + "fmt" + + "go.opentelemetry.io/collector/confmap" +) + +var anyREFilter = func() *StringFilter { + if sf, err := NewStringFilter([]string{"/^.*$/"}); err != nil { + panic(err) + } else { + return sf + } +}() + +// PropertyFilter is a collection of *StringFilter items used in determining if a given property (name and value) +// should be included with a dimension update request. The default values for all fields is the regex +// StringFilter `/^.*$/ to match with any potential value. +// +// Examples: +// Don't send any dimension updates for `k8s.pod.uid` dimension: +// - dimension_name: "k8s.pod.uid" +// Don't send dimension updates for any dimension with a value of `some.value`: +// - dimension_value: "some.value" +// Don't send dimension updates including a `some.property` property for any dimension: +// - property_name: "some.property" +// Don't send dimension updates including a `some.property` property with a "some.value" value for any dimension +// - property_name: "some.property" +// property_value: "some.value" +type PropertyFilter struct { + // PropertyName is the (inverted) literal, regex, or globbed property name/key to not include in dimension updates + PropertyName *StringFilter `mapstructure:"property_name"` + // PropertyValue is the (inverted) literal or globbed property value to not include in dimension updates + PropertyValue *StringFilter `mapstructure:"property_value"` + // DimensionName is the (inverted) literal, regex, or globbed dimension name/key to not target for dimension updates. + // If there are no sub-property filters for its enclosing entry, it will disable dimension updates + // for this dimension name in total. + DimensionName *StringFilter `mapstructure:"dimension_name"` + // PropertyValue is the (inverted) literal, regex, or globbed dimension value to not target with a dimension update + // If there are no sub-property filters for its enclosing entry, it will disable dimension updates + // for this dimension value in total. + DimensionValue *StringFilter `mapstructure:"dimension_value"` +} + +func (pf *PropertyFilter) Unmarshal(in *confmap.Conf) error { + if pf == nil { + return fmt.Errorf("cannot unmarshal nil PropertyFilter") + } + if err := in.Unmarshal(pf, confmap.WithErrorUnused()); err != nil { + return fmt.Errorf("failed unmarshalling PropertyFilter: %w", err) + } + // default values should be /.*/ so that declared fields are what are examined + for _, p := range []**StringFilter{&pf.DimensionName, &pf.DimensionValue, &pf.PropertyName, &pf.PropertyValue} { + if *p == nil { + *p = anyREFilter + } + } + return nil +} diff --git a/exporter/signalfxexporter/internal/translation/dpfilters/propertyfilter_test.go b/exporter/signalfxexporter/internal/translation/dpfilters/propertyfilter_test.go new file mode 100644 index 000000000000..bd0c731c61de --- /dev/null +++ b/exporter/signalfxexporter/internal/translation/dpfilters/propertyfilter_test.go @@ -0,0 +1,101 @@ +// Copyright 2021, OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package dpfilters + +import ( + "testing" + + "github.com/stretchr/testify/require" + "go.opentelemetry.io/collector/confmap" + "gopkg.in/yaml.v3" +) + +func TestPropertyFilterUnmarshaling(t *testing.T) { + for _, test := range []struct { + name string + yaml string + expectedPropertyFilter PropertyFilter + expectedError string + }{ + { + name: "happy path", + yaml: `dimension_name: some.dimension.name +dimension_value: some.dimension.value +property_name: some.property.name +property_value: some.property.value`, + expectedPropertyFilter: PropertyFilter{ + DimensionName: mustStringFilter(t, "some.dimension.name"), + DimensionValue: mustStringFilter(t, "some.dimension.value"), + PropertyName: mustStringFilter(t, "some.property.name"), + PropertyValue: mustStringFilter(t, "some.property.value"), + }, + }, + { + name: "default", + yaml: "", + expectedPropertyFilter: PropertyFilter{ + DimensionName: mustStringFilter(t, "/^.*$/"), + DimensionValue: mustStringFilter(t, "/^.*$/"), + PropertyName: mustStringFilter(t, "/^.*$/"), + PropertyValue: mustStringFilter(t, "/^.*$/"), + }, + }, + { + name: "regexes", + yaml: `dimension_name: /dimension.name/ +dimension_value: '!/dimension.value/' +property_name: /property.name/ +property_value: '!/property.value/'`, + expectedPropertyFilter: PropertyFilter{ + DimensionName: mustStringFilter(t, "/dimension.name/"), + DimensionValue: mustStringFilter(t, "!/dimension.value/"), + PropertyName: mustStringFilter(t, "/property.name/"), + PropertyValue: mustStringFilter(t, "!/property.value/"), + }, + }, + { + name: "invalid regex", + yaml: "dimension_name: '/(?=not.in.re2)/'", + expectedError: "failed unmarshalling PropertyFilter: 1 error(s) decoding:\n\n* error decoding 'dimension_name': error parsing regexp: invalid or unsupported Perl syntax: `(?=`", + }, + { + name: "invalid glob", + yaml: "dimension_value: '*[c-a]'", + expectedError: "failed unmarshalling PropertyFilter: 1 error(s) decoding:\n\n* error decoding 'dimension_value': hi character 'a' should be greater than lo 'c'", + }, + } { + t.Run(test.name, func(t *testing.T) { + var conf map[string]interface{} + err := yaml.Unmarshal([]byte(test.yaml), &conf) + require.NoError(t, err) + + cm := confmap.NewFromStringMap(conf) + pf := &PropertyFilter{} + err = pf.Unmarshal(cm) + if test.expectedError != "" { + require.EqualError(t, err, test.expectedError) + } else { + require.NoError(t, err) + require.Equal(t, test.expectedPropertyFilter, *pf) + } + }) + } +} + +func mustStringFilter(t *testing.T, in string) *StringFilter { + sf, err := NewStringFilter([]string{in}) + require.NoError(t, err) + return sf +} diff --git a/exporter/signalfxexporter/internal/translation/dpfilters/string.go b/exporter/signalfxexporter/internal/translation/dpfilters/string.go index 381f336d11f7..be7cbcf771d4 100644 --- a/exporter/signalfxexporter/internal/translation/dpfilters/string.go +++ b/exporter/signalfxexporter/internal/translation/dpfilters/string.go @@ -101,3 +101,12 @@ func (f *StringFilter) Matches(s string) bool { } return matched } + +func (f *StringFilter) UnmarshalText(in []byte) error { + sf, err := NewStringFilter([]string{string(in)}) + if err != nil { + return err + } + *f = *sf + return nil +} diff --git a/exporter/signalfxexporter/testdata/config.yaml b/exporter/signalfxexporter/testdata/config.yaml index 1d51f35633f8..f61b7c8cb8ed 100644 --- a/exporter/signalfxexporter/testdata/config.yaml +++ b/exporter/signalfxexporter/testdata/config.yaml @@ -60,4 +60,13 @@ signalfx/allsettings: container_name: /^[A-Z][A-Z]$/ include_metrics: - metric_name: metric1 - - metric_names: [metric2, metric3] \ No newline at end of file + - metric_names: [metric2, metric3] + exclude_properties: + - property_name: globbed* + - property_value: '!globbed*value' + - dimension_name: globbed* + - dimension_value: '!globbed*value' + - property_name: globbed* + property_value: '!globbed*value' + dimension_name: globbed* + dimension_value: '!globbed*value' From 5c08e785b89e41cef69a6e1bc2a87ca2498f28fe Mon Sep 17 00:00:00 2001 From: Ryan Fitzpatrick Date: Thu, 9 Feb 2023 15:52:18 +0000 Subject: [PATCH 2/2] set nil StringFilter.Matches() to true --- exporter/signalfxexporter/config_test.go | 8 +---- .../translation/dpfilters/propertyfilter.go | 34 ++----------------- .../dpfilters/propertyfilter_test.go | 14 ++++---- .../internal/translation/dpfilters/string.go | 3 ++ .../translation/dpfilters/string_test.go | 5 +++ 5 files changed, 18 insertions(+), 46 deletions(-) diff --git a/exporter/signalfxexporter/config_test.go b/exporter/signalfxexporter/config_test.go index 9ac31e2d6ae8..9f0d260afde9 100644 --- a/exporter/signalfxexporter/config_test.go +++ b/exporter/signalfxexporter/config_test.go @@ -52,8 +52,6 @@ func TestLoadConfig(t *testing.T) { seventy := 70 - allRE := mustStringFilter(t, "/^.*$/") - tests := []struct { id component.ID expected *Config @@ -167,20 +165,16 @@ func TestLoadConfig(t *testing.T) { DeltaTranslationTTL: 3600, ExcludeProperties: []dpfilters.PropertyFilter{ { - PropertyName: mustStringFilter(t, "globbed*"), - PropertyValue: allRE, DimensionName: allRE, DimensionValue: allRE, + PropertyName: mustStringFilter(t, "globbed*"), }, { PropertyValue: mustStringFilter(t, "!globbed*value"), - PropertyName: allRE, DimensionName: allRE, DimensionValue: allRE, }, { DimensionName: mustStringFilter(t, "globbed*"), - PropertyName: allRE, PropertyValue: allRE, DimensionValue: allRE, }, { DimensionValue: mustStringFilter(t, "!globbed*value"), - PropertyName: allRE, PropertyValue: allRE, DimensionName: allRE, }, { PropertyName: mustStringFilter(t, "globbed*"), diff --git a/exporter/signalfxexporter/internal/translation/dpfilters/propertyfilter.go b/exporter/signalfxexporter/internal/translation/dpfilters/propertyfilter.go index 838e561aaa4e..82ab40403d7e 100644 --- a/exporter/signalfxexporter/internal/translation/dpfilters/propertyfilter.go +++ b/exporter/signalfxexporter/internal/translation/dpfilters/propertyfilter.go @@ -14,23 +14,9 @@ package dpfilters // import "github.com/open-telemetry/opentelemetry-collector-contrib/exporter/signalfxexporter/internal/translation/dpfilters" -import ( - "fmt" - - "go.opentelemetry.io/collector/confmap" -) - -var anyREFilter = func() *StringFilter { - if sf, err := NewStringFilter([]string{"/^.*$/"}); err != nil { - panic(err) - } else { - return sf - } -}() - // PropertyFilter is a collection of *StringFilter items used in determining if a given property (name and value) -// should be included with a dimension update request. The default values for all fields is the regex -// StringFilter `/^.*$/ to match with any potential value. +// should be included with a dimension update request. The default values for all fields is equivalent to the regex +// StringFilter `/^.*$/` to match with any potential value. // // Examples: // Don't send any dimension updates for `k8s.pod.uid` dimension: @@ -56,19 +42,3 @@ type PropertyFilter struct { // for this dimension value in total. DimensionValue *StringFilter `mapstructure:"dimension_value"` } - -func (pf *PropertyFilter) Unmarshal(in *confmap.Conf) error { - if pf == nil { - return fmt.Errorf("cannot unmarshal nil PropertyFilter") - } - if err := in.Unmarshal(pf, confmap.WithErrorUnused()); err != nil { - return fmt.Errorf("failed unmarshalling PropertyFilter: %w", err) - } - // default values should be /.*/ so that declared fields are what are examined - for _, p := range []**StringFilter{&pf.DimensionName, &pf.DimensionValue, &pf.PropertyName, &pf.PropertyValue} { - if *p == nil { - *p = anyREFilter - } - } - return nil -} diff --git a/exporter/signalfxexporter/internal/translation/dpfilters/propertyfilter_test.go b/exporter/signalfxexporter/internal/translation/dpfilters/propertyfilter_test.go index bd0c731c61de..b688a2ef0a5c 100644 --- a/exporter/signalfxexporter/internal/translation/dpfilters/propertyfilter_test.go +++ b/exporter/signalfxexporter/internal/translation/dpfilters/propertyfilter_test.go @@ -46,10 +46,10 @@ property_value: some.property.value`, name: "default", yaml: "", expectedPropertyFilter: PropertyFilter{ - DimensionName: mustStringFilter(t, "/^.*$/"), - DimensionValue: mustStringFilter(t, "/^.*$/"), - PropertyName: mustStringFilter(t, "/^.*$/"), - PropertyValue: mustStringFilter(t, "/^.*$/"), + DimensionName: nil, + DimensionValue: nil, + PropertyName: nil, + PropertyValue: nil, }, }, { @@ -68,12 +68,12 @@ property_value: '!/property.value/'`, { name: "invalid regex", yaml: "dimension_name: '/(?=not.in.re2)/'", - expectedError: "failed unmarshalling PropertyFilter: 1 error(s) decoding:\n\n* error decoding 'dimension_name': error parsing regexp: invalid or unsupported Perl syntax: `(?=`", + expectedError: "1 error(s) decoding:\n\n* error decoding 'dimension_name': error parsing regexp: invalid or unsupported Perl syntax: `(?=`", }, { name: "invalid glob", yaml: "dimension_value: '*[c-a]'", - expectedError: "failed unmarshalling PropertyFilter: 1 error(s) decoding:\n\n* error decoding 'dimension_value': hi character 'a' should be greater than lo 'c'", + expectedError: "1 error(s) decoding:\n\n* error decoding 'dimension_value': hi character 'a' should be greater than lo 'c'", }, } { t.Run(test.name, func(t *testing.T) { @@ -83,7 +83,7 @@ property_value: '!/property.value/'`, cm := confmap.NewFromStringMap(conf) pf := &PropertyFilter{} - err = pf.Unmarshal(cm) + err = cm.Unmarshal(pf, confmap.WithErrorUnused()) if test.expectedError != "" { require.EqualError(t, err, test.expectedError) } else { diff --git a/exporter/signalfxexporter/internal/translation/dpfilters/string.go b/exporter/signalfxexporter/internal/translation/dpfilters/string.go index be7cbcf771d4..4cbdf985d4d4 100644 --- a/exporter/signalfxexporter/internal/translation/dpfilters/string.go +++ b/exporter/signalfxexporter/internal/translation/dpfilters/string.go @@ -77,6 +77,9 @@ func NewStringFilter(items []string) (*StringFilter, error) { // if it is positively matched by a non-glob/regex pattern exactly // and is negated as well. See the unit tests for examples. func (f *StringFilter) Matches(s string) bool { + if f == nil { + return true + } negated, matched := f.staticSet[s] // If a metric is negated and it matched it won't match anything else by // definition. diff --git a/exporter/signalfxexporter/internal/translation/dpfilters/string_test.go b/exporter/signalfxexporter/internal/translation/dpfilters/string_test.go index e239d3bfe087..566f181bf931 100644 --- a/exporter/signalfxexporter/internal/translation/dpfilters/string_test.go +++ b/exporter/signalfxexporter/internal/translation/dpfilters/string_test.go @@ -150,6 +150,11 @@ func TestStringFilter(t *testing.T) { filter: []string{"/!memor*(/"}, shouldError: true, }, + { + filter: nil, + inputs: []string{"cpu.utilization", "memory.utilization"}, + shouldMatch: []bool{false, false}, + }, } for _, test := range tests { t.Run(test.name, func(t *testing.T) {