diff --git a/processor/spanmetricsprocessor/processor.go b/processor/spanmetricsprocessor/processor.go index fa5a4b85a732..7627d4e1e4b8 100644 --- a/processor/spanmetricsprocessor/processor.go +++ b/processor/spanmetricsprocessor/processor.go @@ -33,10 +33,11 @@ import ( ) const ( - serviceNameKey = conventions.AttributeServiceName - operationKey = "operation" // is there a constant we can refer to? - spanKindKey = tracetranslator.TagSpanKind - statusCodeKey = tracetranslator.TagStatusCode + serviceNameKey = conventions.AttributeServiceName + operationKey = "operation" // is there a constant we can refer to? + spanKindKey = tracetranslator.TagSpanKind + statusCodeKey = tracetranslator.TagStatusCode + metricKeySeparator = string(byte(0)) ) var ( @@ -76,15 +77,9 @@ type processorImp struct { latencyBucketCounts map[metricKey][]uint64 latencyBounds []float64 - // Builds the unique identifier for a metric: an ordered concatenation the metric's dimension values. - metricKeyBuilder strings.Builder - // A cache of dimension key-value maps keyed by a unique identifier formed by a concatenation of its values: // e.g. { "foo/barOK": { "serviceName": "foo", "operation": "/bar", "status_code": "OK" }} metricKeyToDimensions map[metricKey]dimKV - - // A reusable buffer for storing intermediate dimension key-values as they are added to the metric key. - dimensionsBuffer map[string]string } func newProcessor(logger *zap.Logger, config configmodels.Exporter, nextConsumer consumer.TracesConsumer) *processorImp { @@ -114,7 +109,6 @@ func newProcessor(logger *zap.Logger, config configmodels.Exporter, nextConsumer latencyBucketCounts: make(map[metricKey][]uint64), nextConsumer: nextConsumer, dimensions: pConfig.Dimensions, - dimensionsBuffer: make(map[string]string), metricKeyToDimensions: make(map[metricKey]dimKV), } } @@ -296,7 +290,8 @@ func (p *processorImp) aggregateMetricsForSpan(serviceName string, span pdata.Sp index := sort.SearchFloat64s(p.latencyBounds, latencyInMilliseconds) p.lock.Lock() - key := p.buildKey(serviceName, span) + key := buildKey(serviceName, span, p.dimensions) + p.cache(serviceName, span, key) p.updateCallMetrics(key) p.updateLatencyMetrics(key, latencyInMilliseconds, index) p.lock.Unlock() @@ -317,34 +312,49 @@ func (p *processorImp) updateLatencyMetrics(key metricKey, latency float64, inde p.latencyBucketCounts[key][index]++ } -func (p *processorImp) addDimension(key, value string) { +func buildDimensionKVs(serviceName string, span pdata.Span, optionalDims []Dimension) dimKV { + dims := make(dimKV) + dims[serviceNameKey] = serviceName + dims[operationKey] = span.Name() + dims[spanKindKey] = span.Kind().String() + dims[statusCodeKey] = span.Status().Code().String() + spanAttr := span.Attributes() + var value string + for _, d := range optionalDims { + // Set the default if configured, otherwise this metric will have no value set for the dimension. + if d.Default != nil { + value = *d.Default + } + if attr, ok := spanAttr.Get(d.Name); ok { + value = tracetranslator.AttributeValueToString(attr, false) + } + dims[d.Name] = value + } + return dims +} + +func concatDimensionValue(metricKeyBuilder *strings.Builder, value string, prefixSep bool) { // It's worth noting that from pprof benchmarks, WriteString is the most expensive operation of this processor. // Specifically, the need to grow the underlying []byte slice to make room for the appended string. - p.metricKeyBuilder.WriteString(value) - p.dimensionsBuffer[key] = value + if prefixSep { + metricKeyBuilder.WriteString(metricKeySeparator) + } + metricKeyBuilder.WriteString(value) } // buildKey builds the metric key from the service name and span metadata such as operation, kind, status_code and // any additional dimensions the user has configured. // The metric key is a simple concatenation of dimension values. -func (p *processorImp) buildKey(serviceName string, span pdata.Span) metricKey { - // Reset back to a clean state. - defer func() { - // Compiler will optimize this map clearing operation. - // https://github.com/golang/go/blob/master/doc/go1.11.html#L447 - for k := range p.dimensionsBuffer { - delete(p.dimensionsBuffer, k) - } - p.metricKeyBuilder.Reset() - }() +func buildKey(serviceName string, span pdata.Span, optionalDims []Dimension) metricKey { + var metricKeyBuilder strings.Builder + concatDimensionValue(&metricKeyBuilder, serviceName, false) + concatDimensionValue(&metricKeyBuilder, span.Name(), true) + concatDimensionValue(&metricKeyBuilder, span.Kind().String(), true) + concatDimensionValue(&metricKeyBuilder, span.Status().Code().String(), true) - p.addDimension(serviceNameKey, serviceName) - p.addDimension(operationKey, span.Name()) - p.addDimension(spanKindKey, span.Kind().String()) - p.addDimension(statusCodeKey, span.Status().Code().String()) spanAttr := span.Attributes() var value string - for _, d := range p.dimensions { + for _, d := range optionalDims { // Set the default if configured, otherwise this metric will have no value set for the dimension. if d.Default != nil { value = *d.Default @@ -352,28 +362,18 @@ func (p *processorImp) buildKey(serviceName string, span pdata.Span) metricKey { if attr, ok := spanAttr.Get(d.Name); ok { value = tracetranslator.AttributeValueToString(attr, false) } - p.addDimension(d.Name, value) + concatDimensionValue(&metricKeyBuilder, value, true) } - metricKey := metricKey(p.metricKeyBuilder.String()) - - p.cache(metricKey) - return metricKey + k := metricKey(metricKeyBuilder.String()) + return k } -// cache caches the dimension key-value map stored in the dimensionsBuffer for the given metricKey. +// cache the dimension key-value map for the metricKey if there is a cache miss. // This enables a lookup of the dimension key-value map when constructing the metric like so: // LabelsMap().InitFromMap(p.metricKeyToDimensions[key]) -func (p *processorImp) cache(metricKey metricKey) { - if _, ok := p.metricKeyToDimensions[metricKey]; ok { - return - } - // Create new map to copy the buffer contents. - targetMap := make(dimKV) - - // Copy from the original map to the target map - for key, value := range p.dimensionsBuffer { - targetMap[key] = value +func (p *processorImp) cache(serviceName string, span pdata.Span, k metricKey) { + if _, ok := p.metricKeyToDimensions[k]; !ok { + p.metricKeyToDimensions[k] = buildDimensionKVs(serviceName, span, p.dimensions) } - p.metricKeyToDimensions[metricKey] = targetMap } diff --git a/processor/spanmetricsprocessor/processor_test.go b/processor/spanmetricsprocessor/processor_test.go index ba24bd8d36c3..4b93878b2bd1 100644 --- a/processor/spanmetricsprocessor/processor_test.go +++ b/processor/spanmetricsprocessor/processor_test.go @@ -282,7 +282,6 @@ func newProcessorImp(mexp *mocks.MetricsExporter, tcon *mocks.TracesConsumer, de {nullAttrName, defaultNullValue}, }, metricKeyToDimensions: make(map[metricKey]dimKV), - dimensionsBuffer: make(map[string]string), } } @@ -481,3 +480,15 @@ func newOTLPExporters(t *testing.T) (*otlpexporter.Config, component.MetricsExpo require.NoError(t, err) return otlpConfig, mexp, texp } + +func TestBuildKey(t *testing.T) { + span0 := pdata.NewSpan() + span0.SetName("c") + k0 := buildKey("ab", span0, nil) + + span1 := pdata.NewSpan() + span1.SetName("bc") + k1 := buildKey("a", span1, nil) + + assert.NotEqual(t, k0, k1) +}