From 146022302e04fe248cf59c42ed58f98d614d222e Mon Sep 17 00:00:00 2001 From: Curtis Robert Date: Wed, 2 Mar 2022 15:25:18 -0800 Subject: [PATCH] Changes requested in review - Remove out-dated comments - Change all span/log/metric references to "input data" - Add comments about usage of attributes processor vs. metric transform processor. --- CHANGELOG.md | 4 ++ .../processor/filterconfig/config.go | 35 ++++-------- .../processor/filtermetric/config.go | 15 ++++- .../processor/filtermetric/filtermetric.go | 20 ++----- processor/attributesprocessor/README.md | 56 ++++++++++++------- .../attributesprocessor/attributes_metric.go | 2 +- .../attributes_metric_test.go | 45 ++++++++++----- processor/attributesprocessor/factory.go | 4 +- processor/metricstransformprocessor/README.md | 26 +++++++++ 9 files changed, 130 insertions(+), 77 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7807eef91e84..c952cffdb1ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,10 @@ - `signalfxexporter`: Add validation for `sending_queue` setting (#8026) - `internal/stanza`: Add support for arbitrary attribute types (#8081) - `resourcedetectionprocessor`: Add confighttp.HTTPClientSettings To Resource Detection Config Fixes (#7397) +- `honeycombexporter`: Add validation for `sending_queue` setting (#8113) +- `routingprocessor`: Expand error handling on failure to build exporters (#8125) +- `skywalkingreceiver`: Add new skywalking receiver component folder and structure (#8107) +- `groupbyattrsprocesor`: Allow empty keys, which allows to use the processor for compaction (#7793) ### 🛑 Breaking changes 🛑 diff --git a/internal/coreinternal/processor/filterconfig/config.go b/internal/coreinternal/processor/filterconfig/config.go index a485ff18fc83..d767b101f152 100644 --- a/internal/coreinternal/processor/filterconfig/config.go +++ b/internal/coreinternal/processor/filterconfig/config.go @@ -24,30 +24,30 @@ import ( // by the processor, captured under the 'include' and the second, exclude, to // define what is excluded from the processor. type MatchConfig struct { - // Include specifies the set of span/log/metric properties that must be present in order + // Include specifies the set of input data properties that must be present in order // for this processor to apply to it. - // Note: If `exclude` is specified, the span/log/metric is compared against those + // Note: If `exclude` is specified, the input data is compared against those // properties after the `include` properties. - // This is an optional field. If neither `include` and `exclude` are set, all spans/logs/metrics + // This is an optional field. If neither `include` and `exclude` are set, all input data // are processed. If `include` is set and `exclude` isn't set, then all - // spans/logs/metrics matching the properties in this structure are processed. + // input data matching the properties in this structure are processed. Include *MatchProperties `mapstructure:"include"` - // Exclude specifies when this processor will not be applied to the spans/logs/metrics + // Exclude specifies when this processor will not be applied to the input data // which match the specified properties. // Note: The `exclude` properties are checked after the `include` properties, // if they exist, are checked. // If `include` isn't specified, the `exclude` properties are checked against - // all spans/logs/metrics. - // This is an optional field. If neither `include` and `exclude` are set, all spans/logs/metrics - // are processed. If `exclude` is set and `include` isn't set, then all - // spans/logs/metrics that do not match the properties in this structure are processed. + // all input data. + // This is an optional field. If neither `include` and `exclude` are set, all input data + // is processed. If `exclude` is set and `include` isn't set, then all the + // input data that does not match the properties in this structure are processed. Exclude *MatchProperties `mapstructure:"exclude"` } // MatchProperties specifies the set of properties in a spans/log/metric to match -// against and if the span/log/metric should be included or excluded from the -// processor. At least one of services (spans only), span/log/metric names or +// against and if the input data should be included or excluded from the +// processor. At least one of services (spans only), names or // attributes must be specified. It is supported to have all specified, but // this requires all the properties to match for the inclusion/exclusion to // occur. @@ -107,7 +107,7 @@ type MatchProperties struct { Attributes []Attribute `mapstructure:"attributes"` // Resources specify the list of items to match the resources against. - // A match occurs if the span's resources matches at least one item in this list. + // A match occurs if the data's resources match at least one item in this list. // This is an optional field. Resources []Attribute `mapstructure:"resources"` @@ -115,14 +115,6 @@ type MatchProperties struct { // A match occurs if the span's implementation library matches at least one item in this list. // This is an optional field. Libraries []InstrumentationLibrary `mapstructure:"libraries"` - - // Expressions specifies the list of expr expressions to match metrics against. - // A match occurs if any datapoint in a metric matches at least one expression in this list. - Expressions []string `mapstructure:"expressions"` - - // ResourceAttributes defines a list of possible resource attributes to match metrics against. - // A match occurs if any resource attribute matches all expressions in this given list. - ResourceAttributes []Attribute `mapstructure:"resource_attributes"` } // ValidateForSpans validates properties for spans. @@ -152,9 +144,6 @@ func (mp *MatchProperties) ValidateForLogs() error { return nil } -// ValidateForMetrics validates properties for metrics -// Need to implement method, add comments to new members of MatchProperties - // Attribute specifies the attribute key and optional value to match against. type Attribute struct { // Key specifies the attribute key. diff --git a/internal/coreinternal/processor/filtermetric/config.go b/internal/coreinternal/processor/filtermetric/config.go index 98409833c99d..59a70cb8e34a 100644 --- a/internal/coreinternal/processor/filtermetric/config.go +++ b/internal/coreinternal/processor/filtermetric/config.go @@ -56,7 +56,20 @@ type MatchProperties struct { ResourceAttributes []filterconfig.Attribute `mapstructure:"resource_attributes"` } -// ChecksMetrics returns whether or not the check should iterate through all the metrics +func CreateMatchPropertiesFromDefault(properties *filterconfig.MatchProperties) *MatchProperties { + if properties == nil { + return nil + } + + return &MatchProperties{ + MatchType: MatchType(properties.Config.MatchType), + RegexpConfig: properties.Config.RegexpConfig, + MetricNames: properties.MetricNames, + ResourceAttributes: properties.Resources, + } +} + +// ChecksMetrics returns whether the check should iterate through all the metrics func (mp *MatchProperties) ChecksMetrics() bool { if mp == nil { return false diff --git a/internal/coreinternal/processor/filtermetric/filtermetric.go b/internal/coreinternal/processor/filtermetric/filtermetric.go index 982fc64810d2..bd4967812522 100644 --- a/internal/coreinternal/processor/filtermetric/filtermetric.go +++ b/internal/coreinternal/processor/filtermetric/filtermetric.go @@ -16,28 +16,12 @@ package filtermetric // import "github.com/open-telemetry/opentelemetry-collecto import ( "go.opentelemetry.io/collector/model/pdata" - - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/processor/filterconfig" ) type Matcher interface { MatchMetric(metric pdata.Metric) (bool, error) } -func CreateMatchPropertiesFromDefault(properties *filterconfig.MatchProperties) *MatchProperties { - if properties == nil { - return nil - } - - return &MatchProperties{ - MatchType: MatchType(properties.Config.MatchType), - RegexpConfig: properties.Config.RegexpConfig, - MetricNames: properties.MetricNames, - Expressions: properties.Expressions, - ResourceAttributes: properties.ResourceAttributes, - } -} - // NewMatcher constructs a metric Matcher. If an 'expr' match type is specified, // returns an expr matcher, otherwise a name matcher. func NewMatcher(config *MatchProperties) (Matcher, error) { @@ -51,6 +35,10 @@ func NewMatcher(config *MatchProperties) (Matcher, error) { return newNameMatcher(config) } +// Filters have the ability to include and exclude metrics based on the metric's properties. +// The default is to not skip. If include is defined, the metric must match or it will be skipped. +// If include is not defined but exclude is, metric will be skipped if it matches exclude. Metric +// is included if neither specified. func SkipMetric(include, exclude Matcher, metric pdata.Metric) bool { if include != nil { // A false (or an error) returned in this case means the metric should not be processed. diff --git a/processor/attributesprocessor/README.md b/processor/attributesprocessor/README.md index 8ea52b5bf8e0..1d7f7b8d327f 100644 --- a/processor/attributesprocessor/README.md +++ b/processor/attributesprocessor/README.md @@ -5,17 +5,17 @@ Supported pipeline types: traces, logs, metrics. The attributes processor modifies attributes of a span, log, or metric. Please refer to [config.go](./config.go) for the config spec. -This processor also supports the ability to filter and match spans/logs/metrics to determine +This processor also supports the ability to filter and match input data to determine if they should be [included or excluded](#includeexclude-filtering) for specified actions. It takes a list of actions which are performed in order specified in the config. The supported actions are: -- `insert`: Inserts a new attribute in spans/logs/metrics where the key does not already exist. -- `update`: Updates an attribute in spans/logs/metrics where the key does exist. -- `upsert`: Performs insert or update. Inserts a new attribute in spans/logs/metrics where the - key does not already exist and updates an attribute in spans/logs/metrics where the key +- `insert`: Inserts a new attribute in input data where the key does not already exist. +- `update`: Updates an attribute in input data where the key does exist. +- `upsert`: Performs insert or update. Inserts a new attribute in input data where the + key does not already exist and updates an attribute in input data where the key does exist. -- `delete`: Deletes an attribute from a span/log/metric. +- `delete`: Deletes an attribute from the input data. - `hash`: Hashes (SHA1) an existing attribute value. - `extract`: Extracts values using a regular expression rule from the input key to target keys specified in the rule. If a target key already exists, it will @@ -37,7 +37,7 @@ For the actions `insert`, `update` and `upsert`, # Key specifies the attribute to act upon. - key: action: {insert, update, upsert} - # FromAttribute specifies the attribute from the span/log/metric to use to populate + # FromAttribute specifies the attribute from the input data to use to populate # the value. If the attribute doesn't exist, no action is performed. from_attribute: @@ -116,11 +116,37 @@ processors: Refer to [config.yaml](./testdata/config.yaml) for detailed examples on using the processor. +### Attributes Processor for Metrics vs. Metric Transform Processor + +Regarding metric support, these two processors have overlapping functionality. They can both do simple modifications +of metric attribute key-value pairs. As a general rule the attributes processor has more attribute related +functionality, while the metrics transform processor can do much more data manipulation. The attributes processor +is preferred when the only needed functionality is overlapping, as it natively uses the official OpenTelemetry +data model. However, if the metric transform processor is already in use or its extra functionality is necessary, +there's no need to migrate away from it. + +Shared functionality +* Add attributes +* Update values of attributes + +Attribute processor specific functionality +* delete +* hash +* extract + +Metric transform processor specific functionality +* Rename metrics +* Delete data points +* Toggle data type +* Scale value +* Aggregate across label sets +* Aggregate across label values + ## Include/Exclude Filtering The [attribute processor](README.md) exposes an option to provide a set of properties of a span, log, or metric record to match against to determine -if the span/log/metric should be included or excluded from the processor. To configure +if the input data should be included or excluded from the processor. To configure this option, under `include` and/or `exclude` at least `match_type` and one of the following is required: - For spans, one of `services`, `span_names`, `attributes`, `resources`, or `libraries` must be specified @@ -129,7 +155,7 @@ with a non-empty value for a valid configuration. The `log_names`, `expressions` - For logs, one of `log_names`, `attributes`, `resources`, or `libraries` must be specified with a non-empty value for a valid configuration. The `span_names`, `metric_names`, `expressions`, `resource_attributes`, and `services` fields are invalid. -- For metrics, one of `metric_names`, `expressions`, or `resource_attributes` must be specified +- For metrics, one of `metric_names`, `resources` must be specified with a valid non-empty value for a valid configuration. The `span_names`, `log_names`, and `services` fields are invalid. @@ -161,11 +187,11 @@ attributes: services: [, ..., ] # resources specify an array of items to match the resources against. - # A match occurs if the span/log/metric resources matches at least one of the items. + # A match occurs if the input data resources matches at least one of the items. resources: [, ..., ] # libraries specify an array of items to match the implementation library against. - # A match occurs if the span/log/metric implementation library matches at least one of the items. + # A match occurs if the input data implementation library matches at least one of the items. libraries: [, ..., ] # The span name must match at least one of the items. @@ -180,14 +206,6 @@ attributes: # This is an optional field. metric_names: [, ..., ] - # Expressions are used to match metrics. A match occurs if the expression matches at least - # one data point within a metric. - expressions: [, ..., ] - - # Resource attributes defines a list of possible resource attributes to match metrics against. - # A match occurs if any resource attribute matches all expressions in this given list. - resource_attributes: [, ..., ] - # Attributes specifies the list of attributes to match against. # All of these attributes must match exactly for a match to occur. # This is an optional field. diff --git a/processor/attributesprocessor/attributes_metric.go b/processor/attributesprocessor/attributes_metric.go index 92d404d25c04..fd5fec1155d1 100644 --- a/processor/attributesprocessor/attributes_metric.go +++ b/processor/attributesprocessor/attributes_metric.go @@ -23,7 +23,7 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/processor/filtermetric" ) -type metricAttributesProcessor struct { +type metricAttributesProcessor struct { attrProc *attraction.AttrProc include filtermetric.Matcher exclude filtermetric.Matcher diff --git a/processor/attributesprocessor/attributes_metric_test.go b/processor/attributesprocessor/attributes_metric_test.go index a3748d4a89af..162af6484d10 100644 --- a/processor/attributesprocessor/attributes_metric_test.go +++ b/processor/attributesprocessor/attributes_metric_test.go @@ -1,19 +1,34 @@ +// Copyright The 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 attributesprocessor import ( "context" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/attraction" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/processor/filterconfig" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/processor/filterset" - "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/testdata" - "go.opentelemetry.io/collector/component/componenttest" - "go.opentelemetry.io/collector/consumer/consumertest" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/component/componenttest" + "go.opentelemetry.io/collector/consumer/consumertest" "go.opentelemetry.io/collector/model/pdata" + + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/attraction" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/processor/filterconfig" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/processor/filterset" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/testdata" ) // Common structure for all the Tests @@ -213,7 +228,7 @@ func TestAttributes_FilterMetrics(t *testing.T) { } oCfg.Include = &filterconfig.MatchProperties{ Resources: []filterconfig.Attribute{{Key: "name", Value: "^[^i].*"}}, - Config: *createConfig(filterset.Regexp), + Config: *createConfig(filterset.Regexp), } oCfg.Exclude = &filterconfig.MatchProperties{ Attributes: []filterconfig.Attribute{ @@ -233,7 +248,7 @@ func TestAttributes_FilterMetrics(t *testing.T) { func TestAttributes_FilterMetricsByNameStrict(t *testing.T) { testCases := []metricTestCase{ { - name: "apply", + name: "apply", inputAttributes: map[string]pdata.AttributeValue{}, expectedAttributes: map[string]pdata.AttributeValue{ "attribute1": pdata.NewAttributeValueInt(123), @@ -250,12 +265,12 @@ func TestAttributes_FilterMetricsByNameStrict(t *testing.T) { }, }, { - name: "incorrect_metric_name", + name: "incorrect_metric_name", inputAttributes: map[string]pdata.AttributeValue{}, expectedAttributes: map[string]pdata.AttributeValue{}, }, { - name: "dont_apply", + name: "dont_apply", inputAttributes: map[string]pdata.AttributeValue{}, expectedAttributes: map[string]pdata.AttributeValue{}, }, @@ -296,7 +311,7 @@ func TestAttributes_FilterMetricsByNameStrict(t *testing.T) { func TestAttributes_FilterMetricsByNameRegexp(t *testing.T) { testCases := []metricTestCase{ { - name: "apply_to_metric_with_no_attrs", + name: "apply_to_metric_with_no_attrs", inputAttributes: map[string]pdata.AttributeValue{}, expectedAttributes: map[string]pdata.AttributeValue{ "attribute1": pdata.NewAttributeValueInt(123), @@ -313,12 +328,12 @@ func TestAttributes_FilterMetricsByNameRegexp(t *testing.T) { }, }, { - name: "incorrect_metric_name", + name: "incorrect_metric_name", inputAttributes: map[string]pdata.AttributeValue{}, expectedAttributes: map[string]pdata.AttributeValue{}, }, { - name: "apply_dont_apply", + name: "apply_dont_apply", inputAttributes: map[string]pdata.AttributeValue{}, expectedAttributes: map[string]pdata.AttributeValue{}, }, @@ -418,7 +433,7 @@ func TestMetricAttributes_Hash(t *testing.T) { func BenchmarkAttributes_FilterMetricsByName(b *testing.B) { testCases := []metricTestCase{ { - name: "apply_to_metric_with_no_attrs", + name: "apply_to_metric_with_no_attrs", inputAttributes: map[string]pdata.AttributeValue{}, expectedAttributes: map[string]pdata.AttributeValue{ "attribute1": pdata.NewAttributeValueInt(123), @@ -435,7 +450,7 @@ func BenchmarkAttributes_FilterMetricsByName(b *testing.B) { }, }, { - name: "dont_apply", + name: "dont_apply", inputAttributes: map[string]pdata.AttributeValue{}, expectedAttributes: map[string]pdata.AttributeValue{}, }, diff --git a/processor/attributesprocessor/factory.go b/processor/attributesprocessor/factory.go index ad72fdd21489..671895a2bdac 100644 --- a/processor/attributesprocessor/factory.go +++ b/processor/attributesprocessor/factory.go @@ -123,7 +123,7 @@ func createMetricsProcessor( _ component.ProcessorCreateSettings, cfg config.Processor, nextConsumer consumer.Metrics, - ) (component.MetricsProcessor, error) { +) (component.MetricsProcessor, error) { oCfg := cfg.(*Config) if len(oCfg.Actions) == 0 { @@ -150,4 +150,4 @@ func createMetricsProcessor( nextConsumer, newMetricAttributesProcessor(attrProc, include, exclude).processMetrics, processorhelper.WithCapabilities(processorCapabilities)) -} \ No newline at end of file +} diff --git a/processor/metricstransformprocessor/README.md b/processor/metricstransformprocessor/README.md index 73e21b905e55..e402f0a4ea86 100644 --- a/processor/metricstransformprocessor/README.md +++ b/processor/metricstransformprocessor/README.md @@ -311,3 +311,29 @@ operations: action: group group_resource_labels: {"resouce.type": "container", "source": "kubelet"} ``` + +### Attributes Processor for Metrics vs. Metric Transform Processor + +Regarding metric support, these two processors have overlapping functionality. They can both do simple modifications +of metric attribute key-value pairs. As a general rule the attributes processor has more attribute related +functionality, while the metrics transform processor can do much more data manipulation. The attributes processor +is preferred when the only needed functionality is overlapping, as it natively uses the official OpenTelemetry +data model. However, if the metric transform processor is already in use or its extra functionality is necessary, +there's no need to migrate away from it. + +Shared functionality +* Add attributes +* Update values of attributes + +Attribute processor specific functionality +* delete +* hash +* extract + +Metric transform processor specific functionality +* Rename metrics +* Delete data points +* Toggle data type +* Scale value +* Aggregate across label sets +* Aggregate across label values \ No newline at end of file