From 8b4262fd9eb045c67312868bae497f72a3a4e317 Mon Sep 17 00:00:00 2001 From: Arseniy Antonov Date: Fri, 2 Jun 2023 19:31:12 +0300 Subject: [PATCH 01/11] [connector/spanmetricsconnector] Added disabling options for histogram and sum metrics Added disabling options for histogram and sum metrics. In some cases users wants to optionally disable part of metrics cause the can generate too many data or just not needed. --- connector/spanmetricsconnector/README.md | 3 + connector/spanmetricsconnector/config.go | 4 +- connector/spanmetricsconnector/connector.go | 62 +++++++++++++------ .../spanmetricsconnector/connector_test.go | 9 +++ connector/spanmetricsconnector/factory.go | 2 +- connector/spanmetricsconnector/go.mod | 1 + connector/spanmetricsconnector/go.sum | 2 + 7 files changed, 62 insertions(+), 21 deletions(-) diff --git a/connector/spanmetricsconnector/README.md b/connector/spanmetricsconnector/README.md index 0a02be62c815..ba8c4e29fc91 100644 --- a/connector/spanmetricsconnector/README.md +++ b/connector/spanmetricsconnector/README.md @@ -73,6 +73,7 @@ The following settings can be optionally configured: - `histogram` (default: `explicit`): Use to configure the type of histogram to record calculated from spans duration measurements. Must be either `explicit` or `exponential`. + - `disable` (default: `false`): Use to disable histogram metrics at all. - `unit` (default: `ms`): The time unit for recording duration measurements. calculated from spans duration measurements. One of either: `ms` or `s`. - `explicit`: @@ -88,6 +89,7 @@ The following settings can be optionally configured: If the `name`d attribute is missing in the span, the optional provided `default` is used. If no `default` is provided, this dimension will be **omitted** from the metric. +- `exclude_dimensions`: the list of dimensions to be excluded from default. Use to exclude unneeded data from metrics. - `dimensions_cache_size` (default: `1000`): the size of cache for storing Dimensions to improve collectors memory usage. Must be a positive number. - `aggregation_temporality` (default: `AGGREGATION_TEMPORALITY_CUMULATIVE`): Defines the aggregation temporality of the generated metrics. One of either `AGGREGATION_TEMPORALITY_CUMULATIVE` or `AGGREGATION_TEMPORALITY_DELTA`. @@ -118,6 +120,7 @@ connectors: - name: http.method default: GET - name: http.status_code + exclude_dimensions: ['status.code'] dimensions_cache_size: 1000 aggregation_temporality: "AGGREGATION_TEMPORALITY_CUMULATIVE" metrics_flush_interval: 15s diff --git a/connector/spanmetricsconnector/config.go b/connector/spanmetricsconnector/config.go index bd03b63847bb..b19edbdef04a 100644 --- a/connector/spanmetricsconnector/config.go +++ b/connector/spanmetricsconnector/config.go @@ -38,7 +38,8 @@ type Config struct { // - status.code // The dimensions will be fetched from the span's attributes. Examples of some conventionally used attributes: // https://github.com/open-telemetry/opentelemetry-collector/blob/main/model/semconv/opentelemetry.go. - Dimensions []Dimension `mapstructure:"dimensions"` + Dimensions []Dimension `mapstructure:"dimensions"` + ExcludeDimensions []string `mapstructure:"exclude_dimensions"` // DimensionsCacheSize defines the size of cache for storing Dimensions, which helps to avoid cache memory growing // indefinitely over the lifetime of the collector. @@ -57,6 +58,7 @@ type Config struct { } type HistogramConfig struct { + Disable bool `mapstructure:"disable"` Unit metrics.Unit `mapstructure:"unit"` Exponential *ExponentialHistogramConfig `mapstructure:"exponential"` Explicit *ExplicitHistogramConfig `mapstructure:"explicit"` diff --git a/connector/spanmetricsconnector/connector.go b/connector/spanmetricsconnector/connector.go index 99f08eaf158f..e091937d5af8 100644 --- a/connector/spanmetricsconnector/connector.go +++ b/connector/spanmetricsconnector/connector.go @@ -117,6 +117,9 @@ func newConnector(logger *zap.Logger, config component.Config, ticker *clock.Tic } func initHistogramMetrics(cfg Config) metrics.HistogramMetrics { + if cfg.Histogram.Disable { + return nil + } if cfg.Histogram.Exponential != nil { maxSize := structure.DefaultMaxSize if cfg.Histogram.Exponential.MaxSize != 0 { @@ -235,12 +238,13 @@ func (p *connectorImp) buildMetrics() pmetric.Metrics { metric := sm.Metrics().AppendEmpty() metric.SetName(buildMetricName(p.config.Namespace, metricNameCalls)) sums.BuildMetrics(metric, p.startTimestamp, p.config.GetAggregationTemporality()) - - histograms := rawMetrics.histograms - metric = sm.Metrics().AppendEmpty() - metric.SetName(buildMetricName(p.config.Namespace, metricNameDuration)) - metric.SetUnit(p.config.Histogram.Unit.String()) - histograms.BuildMetrics(metric, p.startTimestamp, p.config.GetAggregationTemporality()) + if !p.config.Histogram.Disable { + histograms := rawMetrics.histograms + metric = sm.Metrics().AppendEmpty() + metric.SetName(buildMetricName(p.config.Namespace, metricNameDuration)) + metric.SetUnit(p.config.Histogram.Unit.String()) + histograms.BuildMetrics(metric, p.startTimestamp, p.config.GetAggregationTemporality()) + } } return m @@ -255,8 +259,10 @@ func (p *connectorImp) resetState() { p.metricKeyToDimensions.RemoveEvictedItems() // Exemplars are only relevant to this batch of traces, so must be cleared within the lock - for _, m := range p.resourceMetrics { - m.histograms.Reset(true) + if !p.config.Histogram.Disable { + for _, m := range p.resourceMetrics { + m.histograms.Reset(true) + } } } } @@ -302,14 +308,14 @@ func (p *connectorImp) aggregateMetrics(traces ptrace.Traces) { attributes = p.buildAttributes(serviceName, span, resourceAttr) p.metricKeyToDimensions.Add(key, attributes) } - - // aggregate histogram metrics - h := histograms.GetOrCreate(key, attributes) - h.Observe(duration) - if !span.TraceID().IsEmpty() { - h.AddExemplar(span.TraceID(), span.SpanID(), duration) + if !p.config.Histogram.Disable { + // aggregate histogram metrics + h := histograms.GetOrCreate(key, attributes) + h.Observe(duration) + if !span.TraceID().IsEmpty() { + h.AddExemplar(span.TraceID(), span.SpanID(), duration) + } } - // aggregate sums metrics s := sums.GetOrCreate(key, attributes) s.Add(1) @@ -334,13 +340,31 @@ func (p *connectorImp) getOrCreateResourceMetrics(attr pcommon.Map) *resourceMet return v } +// contains checks if string slice contains a string value +func contains(elements []string, value string) bool { + for _, element := range elements { + if value == element { + return true + } + } + return false +} + func (p *connectorImp) buildAttributes(serviceName string, span ptrace.Span, resourceAttrs pcommon.Map) pcommon.Map { attr := pcommon.NewMap() attr.EnsureCapacity(4 + len(p.dimensions)) - attr.PutStr(serviceNameKey, serviceName) - attr.PutStr(spanNameKey, span.Name()) - attr.PutStr(spanKindKey, traceutil.SpanKindStr(span.Kind())) - attr.PutStr(statusCodeKey, traceutil.StatusCodeStr(span.Status().Code())) + if !contains(p.config.ExcludeDimensions, serviceNameKey) { + attr.PutStr(serviceNameKey, serviceName) + } + if !contains(p.config.ExcludeDimensions, spanNameKey) { + attr.PutStr(spanNameKey, span.Name()) + } + if !contains(p.config.ExcludeDimensions, spanKindKey) { + attr.PutStr(spanKindKey, traceutil.SpanKindStr(span.Kind())) + } + if !contains(p.config.ExcludeDimensions, statusCodeKey) { + attr.PutStr(statusCodeKey, traceutil.StatusCodeStr(span.Status().Code())) + } for _, d := range p.dimensions { if v, ok := getDimensionValue(d, span.Attributes(), resourceAttrs); ok { v.CopyTo(attr.PutEmpty(d.name)) diff --git a/connector/spanmetricsconnector/connector_test.go b/connector/spanmetricsconnector/connector_test.go index f4473d290bb6..d100110cf910 100644 --- a/connector/spanmetricsconnector/connector_test.go +++ b/connector/spanmetricsconnector/connector_test.go @@ -1000,6 +1000,15 @@ func TestConnector_initHistogramMetrics(t *testing.T) { config: Config{}, want: metrics.NewExplicitHistogramMetrics(defaultHistogramBucketsMs), }, + { + name: "Disable histogram", + config: Config{ + Histogram: HistogramConfig{ + Disable: true, + }, + }, + want: nil, + }, { name: "initialize explicit histogram with default bounds (ms)", config: Config{ diff --git a/connector/spanmetricsconnector/factory.go b/connector/spanmetricsconnector/factory.go index 26ec56ccc671..792b47935b7e 100644 --- a/connector/spanmetricsconnector/factory.go +++ b/connector/spanmetricsconnector/factory.go @@ -31,7 +31,7 @@ func createDefaultConfig() component.Config { AggregationTemporality: "AGGREGATION_TEMPORALITY_CUMULATIVE", DimensionsCacheSize: defaultDimensionsCacheSize, MetricsFlushInterval: 15 * time.Second, - Histogram: HistogramConfig{Unit: defaultUnit}, + Histogram: HistogramConfig{Disable: false, Unit: defaultUnit}, } } diff --git a/connector/spanmetricsconnector/go.mod b/connector/spanmetricsconnector/go.mod index a70ad359e63b..4231e9affc21 100644 --- a/connector/spanmetricsconnector/go.mod +++ b/connector/spanmetricsconnector/go.mod @@ -35,6 +35,7 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/stretchr/objx v0.5.0 // indirect + go.opentelemetry.io/collector/exporter v0.78.2 // indirect go.opentelemetry.io/collector/featuregate v1.0.0-rcv0012.0.20230525165144-87dd85a6c034 // indirect go.opentelemetry.io/otel v1.16.0 // indirect go.opentelemetry.io/otel/metric v1.16.0 // indirect diff --git a/connector/spanmetricsconnector/go.sum b/connector/spanmetricsconnector/go.sum index a3851fecb3ff..68bf3a83dacf 100644 --- a/connector/spanmetricsconnector/go.sum +++ b/connector/spanmetricsconnector/go.sum @@ -273,6 +273,8 @@ go.opentelemetry.io/collector/confmap v0.78.3-0.20230525165144-87dd85a6c034 h1:Z go.opentelemetry.io/collector/confmap v0.78.3-0.20230525165144-87dd85a6c034/go.mod h1:nudYcQJDF5zmAd2GKe5PetMF8/Lg1Mc/EjAKQBXHdyg= go.opentelemetry.io/collector/consumer v0.78.3-0.20230525165144-87dd85a6c034 h1:IKetmMO8glVRQy8b96UU7gVYRriBrlDtOCAZOqgoVpo= go.opentelemetry.io/collector/consumer v0.78.3-0.20230525165144-87dd85a6c034/go.mod h1:5fS5WBqYH7gJPKyNxAZ+/sym7WC6W5/gLxuyFd5vzWw= +go.opentelemetry.io/collector/exporter v0.78.2 h1:77HMkPFXAxys+WWpVdmezq2+LZpgTVaBLFecYOiqFH8= +go.opentelemetry.io/collector/exporter v0.78.2/go.mod h1:X+J2XrNPFLq9P6mTJojmX2BCcZvQdTxfAaGrrTiTBfc= go.opentelemetry.io/collector/featuregate v1.0.0-rcv0012.0.20230525165144-87dd85a6c034 h1:DWwGIvimhygvFih+yYQwaR4TV4yHD2RCTPDRhFQCv7w= go.opentelemetry.io/collector/featuregate v1.0.0-rcv0012.0.20230525165144-87dd85a6c034/go.mod h1:/kVAsGUCyJXIDSgHftCN63QiwAEVHRLX2Kh/S+dqgHY= go.opentelemetry.io/collector/pdata v1.0.0-rcv0012.0.20230525165144-87dd85a6c034 h1:nURnFEE9rRWA7Za7wcxQifrJ9QH/eaEyUDJFRWlfCyk= From 188b88412025838fd53a96a0e4e5259c7914b90e Mon Sep 17 00:00:00 2001 From: Arseniy Antonov Date: Mon, 5 Jun 2023 18:25:11 +0300 Subject: [PATCH 02/11] [GH-16344] spanmetricsconnector added changelog --- .../spanmetricsconnector_disable_options.yaml | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100755 .chloggen/spanmetricsconnector_disable_options.yaml diff --git a/.chloggen/spanmetricsconnector_disable_options.yaml b/.chloggen/spanmetricsconnector_disable_options.yaml new file mode 100755 index 000000000000..fcb7cfb60af2 --- /dev/null +++ b/.chloggen/spanmetricsconnector_disable_options.yaml @@ -0,0 +1,20 @@ +# Use this changelog template to create an entry for release notes. +# If your change doesn't affect end users, such as a test fix or a tooling change, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. + +# 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: spanmetricsconnector + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "Added disabling options for histogram and sum metrics. In some cases users wants to optionally disable part of metrics cause they can generate too many data or just not needed." + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [16344] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: From 69ac20d58edc55f14c365aeea86fc686a7add389 Mon Sep 17 00:00:00 2001 From: Arseniy Antonov Date: Wed, 7 Jun 2023 18:23:04 +0300 Subject: [PATCH 03/11] [GH-16344] spanmetricsconnector added tests and fixed spelling after review --- connector/spanmetricsconnector/README.md | 4 +-- connector/spanmetricsconnector/connector.go | 16 +++++++++--- .../spanmetricsconnector/connector_test.go | 26 +++++++++++++++++++ 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/connector/spanmetricsconnector/README.md b/connector/spanmetricsconnector/README.md index 13146d52990a..84c5da08ca49 100644 --- a/connector/spanmetricsconnector/README.md +++ b/connector/spanmetricsconnector/README.md @@ -75,7 +75,7 @@ The following settings can be optionally configured: - `histogram` (default: `explicit`): Use to configure the type of histogram to record calculated from spans duration measurements. Must be either `explicit` or `exponential`. - - `disable` (default: `false`): Use to disable histogram metrics at all. + - `disable` (default: `false`): Disable all histogram metrics. - `unit` (default: `ms`): The time unit for recording duration measurements. calculated from spans duration measurements. One of either: `ms` or `s`. - `explicit`: @@ -91,7 +91,7 @@ The following settings can be optionally configured: If the `name`d attribute is missing in the span, the optional provided `default` is used. If no `default` is provided, this dimension will be **omitted** from the metric. -- `exclude_dimensions`: the list of dimensions to be excluded from default. Use to exclude unneeded data from metrics. +- `exclude_dimensions`: the list of dimensions to be excluded from the default set of dimensions. Use to exclude unneeded data from metrics. - `dimensions_cache_size` (default: `1000`): the size of cache for storing Dimensions to improve collectors memory usage. Must be a positive number. - `aggregation_temporality` (default: `AGGREGATION_TEMPORALITY_CUMULATIVE`): Defines the aggregation temporality of the generated metrics. One of either `AGGREGATION_TEMPORALITY_CUMULATIVE` or `AGGREGATION_TEMPORALITY_DELTA`. diff --git a/connector/spanmetricsconnector/connector.go b/connector/spanmetricsconnector/connector.go index e091937d5af8..e65ad3668d27 100644 --- a/connector/spanmetricsconnector/connector.go +++ b/connector/spanmetricsconnector/connector.go @@ -387,10 +387,18 @@ func concatDimensionValue(dest *bytes.Buffer, value string, prefixSep bool) { // The metric key is a simple concatenation of dimension values, delimited by a null character. func (p *connectorImp) buildKey(serviceName string, span ptrace.Span, optionalDims []dimension, resourceAttrs pcommon.Map) metrics.Key { p.keyBuf.Reset() - concatDimensionValue(p.keyBuf, serviceName, false) - concatDimensionValue(p.keyBuf, span.Name(), true) - concatDimensionValue(p.keyBuf, traceutil.SpanKindStr(span.Kind()), true) - concatDimensionValue(p.keyBuf, traceutil.StatusCodeStr(span.Status().Code()), true) + if !contains(p.config.ExcludeDimensions, serviceNameKey) { + concatDimensionValue(p.keyBuf, serviceName, false) + } + if !contains(p.config.ExcludeDimensions, spanNameKey) { + concatDimensionValue(p.keyBuf, span.Name(), true) + } + if !contains(p.config.ExcludeDimensions, spanKindKey) { + concatDimensionValue(p.keyBuf, traceutil.SpanKindStr(span.Kind()), true) + } + if !contains(p.config.ExcludeDimensions, statusCodeKey) { + concatDimensionValue(p.keyBuf, traceutil.StatusCodeStr(span.Status().Code()), true) + } for _, d := range optionalDims { if v, ok := getDimensionValue(d, span.Attributes(), resourceAttrs); ok { diff --git a/connector/spanmetricsconnector/connector_test.go b/connector/spanmetricsconnector/connector_test.go index d100110cf910..6f91150334d8 100644 --- a/connector/spanmetricsconnector/connector_test.go +++ b/connector/spanmetricsconnector/connector_test.go @@ -390,6 +390,32 @@ func TestBuildKeySameServiceNameCharSequence(t *testing.T) { assert.Equal(t, metrics.Key("a\u0000bc\u0000SPAN_KIND_UNSPECIFIED\u0000STATUS_CODE_UNSET"), k1) } +func TestBuildKeyExcludeDimensionsAll(t *testing.T) { + factory := NewFactory() + cfg := factory.CreateDefaultConfig().(*Config) + cfg.ExcludeDimensions = []string{"span.kind", "service.name", "span.name", "status.code"} + c, err := newConnector(zaptest.NewLogger(t), cfg, nil) + require.NoError(t, err) + + span0 := ptrace.NewSpan() + span0.SetName("spanName") + k0 := c.buildKey("serviceName", span0, nil, pcommon.NewMap()) + assert.Equal(t, metrics.Key(""), k0) +} + +func TestBuildKeyExcludeWrongDimensions(t *testing.T) { + factory := NewFactory() + cfg := factory.CreateDefaultConfig().(*Config) + cfg.ExcludeDimensions = []string{"span.kind", "service.name.wrong.name", "span.name", "status.code"} + c, err := newConnector(zaptest.NewLogger(t), cfg, nil) + require.NoError(t, err) + + span0 := ptrace.NewSpan() + span0.SetName("spanName") + k0 := c.buildKey("serviceName", span0, nil, pcommon.NewMap()) + assert.Equal(t, metrics.Key("serviceName"), k0) +} + func TestBuildKeyWithDimensions(t *testing.T) { factory := NewFactory() cfg := factory.CreateDefaultConfig().(*Config) From de7f08bb6cf6ab9925d0013b6e69bec8e357b247 Mon Sep 17 00:00:00 2001 From: Arseniy Antonov Date: Sat, 10 Jun 2023 17:02:59 +0300 Subject: [PATCH 04/11] [GH-16344] spanmetricsconnector added tests by review --- .../spanmetricsconnector/connector_test.go | 81 ++++++++++++++++++- 1 file changed, 80 insertions(+), 1 deletion(-) diff --git a/connector/spanmetricsconnector/connector_test.go b/connector/spanmetricsconnector/connector_test.go index 6f91150334d8..1a7b2b043a38 100644 --- a/connector/spanmetricsconnector/connector_test.go +++ b/connector/spanmetricsconnector/connector_test.go @@ -75,6 +75,24 @@ type span struct { spanID [8]byte } +// verifyDisabledHistogram expects that histograms are disabled. +func verifyDisabledHistogram(t testing.TB, input pmetric.Metrics) bool { + for i := 0; i < input.ResourceMetrics().Len(); i++ { + rm := input.ResourceMetrics().At(i) + ilm := rm.ScopeMetrics() + // Checking all metrics, naming notice: ilmC/mC - C here is for Counter. + for ilmC := 0; ilmC < ilm.Len(); ilmC++ { + m := ilm.At(ilmC).Metrics() + for mC := 0; mC < m.Len(); mC++ { + metric := m.At(mC) + assert.NotEqual(t, metric.Type(), pmetric.MetricTypeExponentialHistogram) + assert.NotEqual(t, metric.Type(), pmetric.MetricTypeHistogram) + } + } + } + return true +} + // verifyConsumeMetricsInputCumulative expects one accumulation of metrics, and marked as cumulative func verifyConsumeMetricsInputCumulative(t testing.TB, input pmetric.Metrics) bool { return verifyConsumeMetricsInput(t, input, pmetric.AggregationTemporalityCumulative, 1) @@ -370,6 +388,12 @@ func exponentialHistogramsConfig() HistogramConfig { }, } } +func disabledHistogramsConfig() HistogramConfig { + return HistogramConfig{ + Unit: defaultUnit, + Disable: true, + } +} func TestBuildKeySameServiceNameCharSequence(t *testing.T) { factory := NewFactory() @@ -627,6 +651,14 @@ func TestConsumeTraces(t *testing.T) { verifier func(t testing.TB, input pmetric.Metrics) bool traces []ptrace.Traces }{ + // disabling histogram + { + name: "Test histogram metrics are disabled", + aggregationTemporality: cumulative, + histogramConfig: disabledHistogramsConfig, + verifier: verifyDisabledHistogram, + traces: []ptrace.Traces{buildSampleTrace()}, + }, // exponential buckets histogram { name: "Test single consumption, three spans (Cumulative), using exp. histogram", @@ -795,10 +827,57 @@ func BenchmarkConnectorConsumeTraces(b *testing.B) { } } -func newConnectorImp(t *testing.T, mcon consumer.Metrics, defaultNullValue *string, histogramConfig func() HistogramConfig, temporality string, logger *zap.Logger, ticker *clock.Ticker) *connectorImp { +func TestExcludeDimensionsConsumeTraces(t *testing.T) { + mcon := &mocks.MetricsConsumer{} + mcon.On("ConsumeMetrics", mock.Anything, mock.Anything).Return(nil) + excludeDimensions := []string{"span.kind", "span.name", "totallyWrongNameDoesNotAffectAnything"} + p := newConnectorImp(t, mcon, stringp("defaultNullValue"), explicitHistogramsConfig, cumulative, zaptest.NewLogger(t), nil, excludeDimensions...) + traces := buildSampleTrace() + + // Test + ctx := metadata.NewIncomingContext(context.Background(), nil) + + p.ConsumeTraces(ctx, traces) + metrics := p.buildMetrics() + + for i := 0; i < metrics.ResourceMetrics().Len(); i++ { + rm := metrics.ResourceMetrics().At(i) + ilm := rm.ScopeMetrics() + // Checking all metrics, naming notice: ilmC/mC - C here is for Counter. + for ilmC := 0; ilmC < ilm.Len(); ilmC++ { + m := ilm.At(ilmC).Metrics() + for mC := 0; mC < m.Len(); mC++ { + metric := m.At(mC) + // We check only sum and histogram metrics here, because for now only they are present in this module. + + if metric.Type() == pmetric.MetricTypeExponentialHistogram || metric.Type() == pmetric.MetricTypeHistogram { + dp := metric.Histogram().DataPoints() + for dpi := 0; dpi < dp.Len(); dpi++ { + for attributeKey, _ := range dp.At(dpi).Attributes().AsRaw() { + assert.NotContains(t, excludeDimensions, attributeKey) + } + + } + } else { + dp := metric.Sum().DataPoints() + for dpi := 0; dpi < dp.Len(); dpi++ { + for attributeKey, _ := range dp.At(dpi).Attributes().AsRaw() { + assert.NotContains(t, excludeDimensions, attributeKey) + } + } + } + + } + } + } + +} + +func newConnectorImp(t *testing.T, mcon consumer.Metrics, defaultNullValue *string, histogramConfig func() HistogramConfig, temporality string, logger *zap.Logger, ticker *clock.Ticker, excludedDimensions ...string) *connectorImp { cfg := &Config{ AggregationTemporality: temporality, Histogram: histogramConfig(), + ExcludeDimensions: excludedDimensions, DimensionsCacheSize: DimensionsCacheSize, Dimensions: []Dimension{ // Set nil defaults to force a lookup for the attribute in the span. From 60bbb97c3c4e9c66acaf722bbe8c2efeeec12618 Mon Sep 17 00:00:00 2001 From: Arseniy Antonov Date: Thu, 15 Jun 2023 16:10:04 +0300 Subject: [PATCH 05/11] [GH-16344] spanmetricsconnector fixed linter errors --- connector/spanmetricsconnector/connector_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/connector/spanmetricsconnector/connector_test.go b/connector/spanmetricsconnector/connector_test.go index 1a7b2b043a38..a5c2468a6448 100644 --- a/connector/spanmetricsconnector/connector_test.go +++ b/connector/spanmetricsconnector/connector_test.go @@ -837,7 +837,8 @@ func TestExcludeDimensionsConsumeTraces(t *testing.T) { // Test ctx := metadata.NewIncomingContext(context.Background(), nil) - p.ConsumeTraces(ctx, traces) + err := p.ConsumeTraces(ctx, traces) + require.NoError(t, err) metrics := p.buildMetrics() for i := 0; i < metrics.ResourceMetrics().Len(); i++ { @@ -853,7 +854,7 @@ func TestExcludeDimensionsConsumeTraces(t *testing.T) { if metric.Type() == pmetric.MetricTypeExponentialHistogram || metric.Type() == pmetric.MetricTypeHistogram { dp := metric.Histogram().DataPoints() for dpi := 0; dpi < dp.Len(); dpi++ { - for attributeKey, _ := range dp.At(dpi).Attributes().AsRaw() { + for attributeKey := range dp.At(dpi).Attributes().AsRaw() { assert.NotContains(t, excludeDimensions, attributeKey) } @@ -861,7 +862,7 @@ func TestExcludeDimensionsConsumeTraces(t *testing.T) { } else { dp := metric.Sum().DataPoints() for dpi := 0; dpi < dp.Len(); dpi++ { - for attributeKey, _ := range dp.At(dpi).Attributes().AsRaw() { + for attributeKey := range dp.At(dpi).Attributes().AsRaw() { assert.NotContains(t, excludeDimensions, attributeKey) } } From 09a91da66c5495108859be9af3019dc2d9852e11 Mon Sep 17 00:00:00 2001 From: Arseniy Antonov Date: Sun, 18 Jun 2023 14:38:00 +0300 Subject: [PATCH 06/11] [GH-16344] spanmetricsconnector fixed by review suggestiongs --- .../spanmetricsconnector/connector_test.go | 55 ++++++++++++------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/connector/spanmetricsconnector/connector_test.go b/connector/spanmetricsconnector/connector_test.go index a5c2468a6448..69fd182ff800 100644 --- a/connector/spanmetricsconnector/connector_test.go +++ b/connector/spanmetricsconnector/connector_test.go @@ -79,14 +79,14 @@ type span struct { func verifyDisabledHistogram(t testing.TB, input pmetric.Metrics) bool { for i := 0; i < input.ResourceMetrics().Len(); i++ { rm := input.ResourceMetrics().At(i) - ilm := rm.ScopeMetrics() - // Checking all metrics, naming notice: ilmC/mC - C here is for Counter. - for ilmC := 0; ilmC < ilm.Len(); ilmC++ { - m := ilm.At(ilmC).Metrics() + ism := rm.ScopeMetrics() + // Checking all metrics, naming notice: ismC/mC - C here is for Counter. + for ismC := 0; ismC < ism.Len(); ismC++ { + m := ism.At(ismC).Metrics() for mC := 0; mC < m.Len(); mC++ { metric := m.At(mC) - assert.NotEqual(t, metric.Type(), pmetric.MetricTypeExponentialHistogram) - assert.NotEqual(t, metric.Type(), pmetric.MetricTypeHistogram) + assert.NotEqual(t, pmetric.MetricTypeExponentialHistogram, metric.Type()) + assert.NotEqual(t, pmetric.MetricTypeHistogram, metric.Type()) } } } @@ -388,6 +388,7 @@ func exponentialHistogramsConfig() HistogramConfig { }, } } + func disabledHistogramsConfig() HistogramConfig { return HistogramConfig{ Unit: defaultUnit, @@ -843,29 +844,45 @@ func TestExcludeDimensionsConsumeTraces(t *testing.T) { for i := 0; i < metrics.ResourceMetrics().Len(); i++ { rm := metrics.ResourceMetrics().At(i) - ilm := rm.ScopeMetrics() + ism := rm.ScopeMetrics() // Checking all metrics, naming notice: ilmC/mC - C here is for Counter. - for ilmC := 0; ilmC < ilm.Len(); ilmC++ { - m := ilm.At(ilmC).Metrics() + for ilmC := 0; ilmC < ism.Len(); ilmC++ { + m := ism.At(ilmC).Metrics() for mC := 0; mC < m.Len(); mC++ { metric := m.At(mC) // We check only sum and histogram metrics here, because for now only they are present in this module. - if metric.Type() == pmetric.MetricTypeExponentialHistogram || metric.Type() == pmetric.MetricTypeHistogram { - dp := metric.Histogram().DataPoints() - for dpi := 0; dpi < dp.Len(); dpi++ { - for attributeKey := range dp.At(dpi).Attributes().AsRaw() { - assert.NotContains(t, excludeDimensions, attributeKey) + switch metric.Type() { + case pmetric.MetricTypeExponentialHistogram: + { + dp := metric.Histogram().DataPoints() + for dpi := 0; dpi < dp.Len(); dpi++ { + for attributeKey := range dp.At(dpi).Attributes().AsRaw() { + assert.NotContains(t, excludeDimensions, attributeKey) + } + } + } + case pmetric.MetricTypeHistogram: + { + dp := metric.Histogram().DataPoints() + for dpi := 0; dpi < dp.Len(); dpi++ { + for attributeKey := range dp.At(dpi).Attributes().AsRaw() { + assert.NotContains(t, excludeDimensions, attributeKey) + } + } } - } else { - dp := metric.Sum().DataPoints() - for dpi := 0; dpi < dp.Len(); dpi++ { - for attributeKey := range dp.At(dpi).Attributes().AsRaw() { - assert.NotContains(t, excludeDimensions, attributeKey) + default: + { + dp := metric.Sum().DataPoints() + for dpi := 0; dpi < dp.Len(); dpi++ { + for attributeKey := range dp.At(dpi).Attributes().AsRaw() { + assert.NotContains(t, excludeDimensions, attributeKey) + } } } + } } From 2fa7e5744f756e9fea581011d4bd91e9619751a8 Mon Sep 17 00:00:00 2001 From: Arseniy Antonov Date: Mon, 19 Jun 2023 13:51:27 +0300 Subject: [PATCH 07/11] [GH-16344] spanmetricsconnector fixed by review suggestiongs --- connector/spanmetricsconnector/connector_test.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/connector/spanmetricsconnector/connector_test.go b/connector/spanmetricsconnector/connector_test.go index 69fd182ff800..181816c27b43 100644 --- a/connector/spanmetricsconnector/connector_test.go +++ b/connector/spanmetricsconnector/connector_test.go @@ -853,17 +853,7 @@ func TestExcludeDimensionsConsumeTraces(t *testing.T) { // We check only sum and histogram metrics here, because for now only they are present in this module. switch metric.Type() { - case pmetric.MetricTypeExponentialHistogram: - { - dp := metric.Histogram().DataPoints() - for dpi := 0; dpi < dp.Len(); dpi++ { - for attributeKey := range dp.At(dpi).Attributes().AsRaw() { - assert.NotContains(t, excludeDimensions, attributeKey) - } - - } - } - case pmetric.MetricTypeHistogram: + case pmetric.MetricTypeExponentialHistogram, pmetric.MetricTypeHistogram: { dp := metric.Histogram().DataPoints() for dpi := 0; dpi < dp.Len(); dpi++ { From ac3cf1e1210da5952e6176410d9f9d3ce155ddc8 Mon Sep 17 00:00:00 2001 From: Arseniy Antonov Date: Mon, 19 Jun 2023 14:53:12 +0300 Subject: [PATCH 08/11] [GH-16344] spanmetricsconnector added eraly return --- connector/spanmetricsconnector/connector.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/connector/spanmetricsconnector/connector.go b/connector/spanmetricsconnector/connector.go index e65ad3668d27..bef6266a2435 100644 --- a/connector/spanmetricsconnector/connector.go +++ b/connector/spanmetricsconnector/connector.go @@ -259,11 +259,13 @@ func (p *connectorImp) resetState() { p.metricKeyToDimensions.RemoveEvictedItems() // Exemplars are only relevant to this batch of traces, so must be cleared within the lock - if !p.config.Histogram.Disable { - for _, m := range p.resourceMetrics { - m.histograms.Reset(true) - } + if p.config.Histogram.Disable { + return } + for _, m := range p.resourceMetrics { + m.histograms.Reset(true) + } + } } From d6054d5bd9023579879f9644b00456c68807e384 Mon Sep 17 00:00:00 2001 From: Laser Date: Wed, 28 Jun 2023 10:19:11 +0300 Subject: [PATCH 09/11] Update .chloggen/spanmetricsconnector_disable_options.yaml Co-authored-by: Antoine Toulme --- .chloggen/spanmetricsconnector_disable_options.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/spanmetricsconnector_disable_options.yaml b/.chloggen/spanmetricsconnector_disable_options.yaml index fcb7cfb60af2..21f69c058f8f 100755 --- a/.chloggen/spanmetricsconnector_disable_options.yaml +++ b/.chloggen/spanmetricsconnector_disable_options.yaml @@ -9,7 +9,7 @@ change_type: enhancement component: spanmetricsconnector # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). -note: "Added disabling options for histogram and sum metrics. In some cases users wants to optionally disable part of metrics cause they can generate too many data or just not needed." +note: "Added disabling options for histogram and sum metrics. In some cases users want to optionally disable part of metrics because they generate too much data or are not needed." # Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. issues: [16344] From efdc33d834eae9baa8bcada8d9e09d8fd1f7385e Mon Sep 17 00:00:00 2001 From: Laser Date: Wed, 28 Jun 2023 11:09:39 +0300 Subject: [PATCH 10/11] Update spanmetricsconnector_disable_options.yaml Fixed desctiption by reviwew --- .chloggen/spanmetricsconnector_disable_options.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.chloggen/spanmetricsconnector_disable_options.yaml b/.chloggen/spanmetricsconnector_disable_options.yaml index 21f69c058f8f..15f6f4af1438 100755 --- a/.chloggen/spanmetricsconnector_disable_options.yaml +++ b/.chloggen/spanmetricsconnector_disable_options.yaml @@ -9,7 +9,7 @@ change_type: enhancement component: spanmetricsconnector # A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). -note: "Added disabling options for histogram and sum metrics. In some cases users want to optionally disable part of metrics because they generate too much data or are not needed." +note: "Added disabling options for histogram metrics and option to exclude default labels from all metrics. In some cases users want to optionally disable part of metrics because they generate too much data or are not needed." # Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. issues: [16344] From 39ad151077ab21f1cdcc6c09243234cbb0a94a0f Mon Sep 17 00:00:00 2001 From: Arseniy Antonov Date: Wed, 28 Jun 2023 16:29:37 +0300 Subject: [PATCH 11/11] Main merged and fixed linter --- connector/spanmetricsconnector/connector_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/connector/spanmetricsconnector/connector_test.go b/connector/spanmetricsconnector/connector_test.go index 181816c27b43..4146c63aa126 100644 --- a/connector/spanmetricsconnector/connector_test.go +++ b/connector/spanmetricsconnector/connector_test.go @@ -863,7 +863,7 @@ func TestExcludeDimensionsConsumeTraces(t *testing.T) { } } - default: + case pmetric.MetricTypeEmpty, pmetric.MetricTypeGauge, pmetric.MetricTypeSum, pmetric.MetricTypeSummary: { dp := metric.Sum().DataPoints() for dpi := 0; dpi < dp.Len(); dpi++ {