From d5595bbfaa6199962b86cb5841b656a08d77f194 Mon Sep 17 00:00:00 2001 From: Jared Tan Date: Fri, 6 Sep 2024 20:58:47 +0800 Subject: [PATCH] [connector/servicegraph] Extract the `getDimensionValue` function as a common function. (#34908) **Description:** refactoring getDimensionValue func as util func for spanmetricsconnector,servicegraphconnector,exceptionconnector common usage **Link to tracking Issue:** #34627 **Testing:** **Documentation:** --------- Signed-off-by: Jared Tan --- ...actoring_dimenssion_func_as_util_func.yaml | 27 ++++++++ connector/exceptionsconnector/connector.go | 27 ++++---- .../exceptionsconnector/connector_logs.go | 18 ++---- .../exceptionsconnector/connector_metrics.go | 11 ++-- .../connector_metrics_test.go | 24 +++---- connector/exceptionsconnector/factory_test.go | 10 +-- connector/exceptionsconnector/go.mod | 5 +- connector/servicegraphconnector/connector.go | 7 ++- connector/servicegraphconnector/go.mod | 3 + connector/servicegraphconnector/util.go | 13 +--- connector/spanmetricsconnector/connector.go | 50 ++++----------- .../spanmetricsconnector/connector_test.go | 23 +++---- .../spanmetricsconnector/factory_test.go | 10 +-- connector/spanmetricsconnector/go.mod | 3 + connector/sumconnector/go.mod | 8 +-- .../integrationtest/go.mod | 3 + internal/pdatautil/attributes.go | 41 ++++++++++++ internal/pdatautil/attributes_test.go | 63 +++++++++++++++++++ testbed/go.mod | 3 + 19 files changed, 231 insertions(+), 118 deletions(-) create mode 100644 .chloggen/refactoring_dimenssion_func_as_util_func.yaml create mode 100644 internal/pdatautil/attributes.go create mode 100644 internal/pdatautil/attributes_test.go diff --git a/.chloggen/refactoring_dimenssion_func_as_util_func.yaml b/.chloggen/refactoring_dimenssion_func_as_util_func.yaml new file mode 100644 index 000000000000..4fda833ffb8a --- /dev/null +++ b/.chloggen/refactoring_dimenssion_func_as_util_func.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# 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: exceptionsconnector,servicegraphconnector,spanmetricsconnector + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Extract the `getDimensionValue` function as a common function. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [34627] + +# (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: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] \ No newline at end of file diff --git a/connector/exceptionsconnector/connector.go b/connector/exceptionsconnector/connector.go index a815d7255f3b..517848cf50e9 100644 --- a/connector/exceptionsconnector/connector.go +++ b/connector/exceptionsconnector/connector.go @@ -6,6 +6,8 @@ package exceptionsconnector // import "github.com/open-telemetry/opentelemetry-c import ( "go.opentelemetry.io/collector/pdata/pcommon" conventions "go.opentelemetry.io/collector/semconv/v1.18.0" + + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil" ) const ( @@ -20,21 +22,16 @@ const ( eventNameExc = "exception" // OpenTelemetry non-standard constant. ) -type dimension struct { - name string - value *pcommon.Value -} - -func newDimensions(cfgDims []Dimension) []dimension { +func newDimensions(cfgDims []Dimension) []pdatautil.Dimension { if len(cfgDims) == 0 { return nil } - dims := make([]dimension, len(cfgDims)) + dims := make([]pdatautil.Dimension, len(cfgDims)) for i := range cfgDims { - dims[i].name = cfgDims[i].Name + dims[i].Name = cfgDims[i].Name if cfgDims[i].Default != nil { val := pcommon.NewValueStr(*cfgDims[i].Default) - dims[i].value = &val + dims[i].Value = &val } } return dims @@ -47,21 +44,21 @@ func newDimensions(cfgDims []Dimension) []dimension { // // The ok flag indicates if a dimension value was fetched in order to differentiate // an empty string value from a state where no value was found. -func getDimensionValue(d dimension, spanAttrs pcommon.Map, eventAttrs pcommon.Map, resourceAttr pcommon.Map) (v pcommon.Value, ok bool) { +func getDimensionValue(d pdatautil.Dimension, spanAttrs pcommon.Map, eventAttrs pcommon.Map, resourceAttr pcommon.Map) (v pcommon.Value, ok bool) { // The more specific span attribute should take precedence. - if attr, exists := spanAttrs.Get(d.name); exists { + if attr, exists := spanAttrs.Get(d.Name); exists { return attr, true } - if attr, exists := eventAttrs.Get(d.name); exists { + if attr, exists := eventAttrs.Get(d.Name); exists { return attr, true } // falling back to searching in resource attributes - if attr, exists := resourceAttr.Get(d.name); exists { + if attr, exists := resourceAttr.Get(d.Name); exists { return attr, true } // Set the default if configured, otherwise this metric will have no value set for the dimension. - if d.value != nil { - return *d.value, true + if d.Value != nil { + return *d.Value, true } return v, ok } diff --git a/connector/exceptionsconnector/connector_logs.go b/connector/exceptionsconnector/connector_logs.go index 5fa2e6b2c8f5..7126980e8e4c 100644 --- a/connector/exceptionsconnector/connector_logs.go +++ b/connector/exceptionsconnector/connector_logs.go @@ -15,13 +15,14 @@ import ( "go.uber.org/zap" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/traceutil" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil" ) type logsConnector struct { config Config // Additional dimensions to add to logs. - dimensions []dimension + dimensions []pdatautil.Dimension logsConsumer consumer.Logs component.StartFunc @@ -113,20 +114,13 @@ func (c *logsConnector) attrToLogRecord(sl plog.ScopeLogs, serviceName string, s // Add configured dimension attributes to the log record. for _, d := range c.dimensions { - if v, ok := getDimensionValue(d, spanAttrs, eventAttrs, resourceAttrs); ok { - logRecord.Attributes().PutStr(d.name, v.Str()) + if v, ok := pdatautil.GetDimensionValue(d, spanAttrs, eventAttrs, resourceAttrs); ok { + logRecord.Attributes().PutStr(d.Name, v.Str()) } } // Add stacktrace to the log record. - logRecord.Attributes().PutStr(exceptionStacktraceKey, getValue(eventAttrs, exceptionStacktraceKey)) + attrVal, _ := pdatautil.GetAttributeValue(exceptionStacktraceKey, eventAttrs) + logRecord.Attributes().PutStr(exceptionStacktraceKey, attrVal) return logRecord } - -// getValue returns the value of the attribute with the given key. -func getValue(attr pcommon.Map, key string) string { - if attrVal, ok := attr.Get(key); ok { - return attrVal.Str() - } - return "" -} diff --git a/connector/exceptionsconnector/connector_metrics.go b/connector/exceptionsconnector/connector_metrics.go index bf0f4a60c30b..a53d4f70c3ce 100644 --- a/connector/exceptionsconnector/connector_metrics.go +++ b/connector/exceptionsconnector/connector_metrics.go @@ -18,6 +18,7 @@ import ( "go.uber.org/zap" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/traceutil" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil" ) const ( @@ -29,7 +30,7 @@ type metricsConnector struct { config Config // Additional dimensions to add to metrics. - dimensions []dimension + dimensions []pdatautil.Dimension keyBuf *bytes.Buffer @@ -175,7 +176,7 @@ func (c *metricsConnector) addExemplar(exc *exception, traceID pcommon.TraceID, e.SetDoubleValue(float64(exc.count)) } -func buildDimensionKVs(dimensions []dimension, serviceName string, span ptrace.Span, eventAttrs pcommon.Map, resourceAttrs pcommon.Map) pcommon.Map { +func buildDimensionKVs(dimensions []pdatautil.Dimension, serviceName string, span ptrace.Span, eventAttrs pcommon.Map, resourceAttrs pcommon.Map) pcommon.Map { dims := pcommon.NewMap() dims.EnsureCapacity(3 + len(dimensions)) dims.PutStr(serviceNameKey, serviceName) @@ -183,8 +184,8 @@ func buildDimensionKVs(dimensions []dimension, serviceName string, span ptrace.S dims.PutStr(spanKindKey, traceutil.SpanKindStr(span.Kind())) dims.PutStr(statusCodeKey, traceutil.StatusCodeStr(span.Status().Code())) for _, d := range dimensions { - if v, ok := getDimensionValue(d, span.Attributes(), eventAttrs, resourceAttrs); ok { - v.CopyTo(dims.PutEmpty(d.name)) + if v, ok := pdatautil.GetDimensionValue(d, span.Attributes(), eventAttrs, resourceAttrs); ok { + v.CopyTo(dims.PutEmpty(d.Name)) } } return dims @@ -195,7 +196,7 @@ func buildDimensionKVs(dimensions []dimension, serviceName string, span ptrace.S // or resource attributes. If the dimension exists in both, the span's attributes, being the most specific, takes precedence. // // The metric key is a simple concatenation of dimension values, delimited by a null character. -func buildKey(dest *bytes.Buffer, serviceName string, span ptrace.Span, optionalDims []dimension, eventAttrs pcommon.Map, resourceAttrs pcommon.Map) { +func buildKey(dest *bytes.Buffer, serviceName string, span ptrace.Span, optionalDims []pdatautil.Dimension, eventAttrs pcommon.Map, resourceAttrs pcommon.Map) { concatDimensionValue(dest, serviceName, false) concatDimensionValue(dest, span.Name(), true) concatDimensionValue(dest, traceutil.SpanKindStr(span.Kind()), true) diff --git a/connector/exceptionsconnector/connector_metrics_test.go b/connector/exceptionsconnector/connector_metrics_test.go index 0537d0157422..797619d575e6 100644 --- a/connector/exceptionsconnector/connector_metrics_test.go +++ b/connector/exceptionsconnector/connector_metrics_test.go @@ -20,6 +20,8 @@ import ( "go.uber.org/zap" "go.uber.org/zap/zaptest" "google.golang.org/grpc/metadata" + + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil" ) // metricID represents the minimum attributes that uniquely identifies a metric in our tests. @@ -277,7 +279,7 @@ func TestBuildKeyWithDimensions(t *testing.T) { defaultFoo := pcommon.NewValueStr("bar") for _, tc := range []struct { name string - optionalDims []dimension + optionalDims []pdatautil.Dimension resourceAttrMap map[string]any spanAttrMap map[string]any wantKey string @@ -288,22 +290,22 @@ func TestBuildKeyWithDimensions(t *testing.T) { }, { name: "neither span nor resource contains key, dim provides default", - optionalDims: []dimension{ - {name: "foo", value: &defaultFoo}, + optionalDims: []pdatautil.Dimension{ + {Name: "foo", Value: &defaultFoo}, }, wantKey: "ab\u0000c\u0000SPAN_KIND_UNSPECIFIED\u0000STATUS_CODE_UNSET\u0000bar", }, { name: "neither span nor resource contains key, dim provides no default", - optionalDims: []dimension{ - {name: "foo"}, + optionalDims: []pdatautil.Dimension{ + {Name: "foo"}, }, wantKey: "ab\u0000c\u0000SPAN_KIND_UNSPECIFIED\u0000STATUS_CODE_UNSET", }, { name: "span attribute contains dimension", - optionalDims: []dimension{ - {name: "foo"}, + optionalDims: []pdatautil.Dimension{ + {Name: "foo"}, }, spanAttrMap: map[string]any{ "foo": 99, @@ -312,8 +314,8 @@ func TestBuildKeyWithDimensions(t *testing.T) { }, { name: "resource attribute contains dimension", - optionalDims: []dimension{ - {name: "foo"}, + optionalDims: []pdatautil.Dimension{ + {Name: "foo"}, }, resourceAttrMap: map[string]any{ "foo": 99, @@ -322,8 +324,8 @@ func TestBuildKeyWithDimensions(t *testing.T) { }, { name: "both span and resource attribute contains dimension, should prefer span attribute", - optionalDims: []dimension{ - {name: "foo"}, + optionalDims: []pdatautil.Dimension{ + {Name: "foo"}, }, spanAttrMap: map[string]any{ "foo": 100, diff --git a/connector/exceptionsconnector/factory_test.go b/connector/exceptionsconnector/factory_test.go index f485f9c19b69..f39b009d4b46 100644 --- a/connector/exceptionsconnector/factory_test.go +++ b/connector/exceptionsconnector/factory_test.go @@ -11,6 +11,8 @@ import ( "go.opentelemetry.io/collector/connector/connectortest" "go.opentelemetry.io/collector/consumer/consumertest" "go.opentelemetry.io/collector/pdata/pcommon" + + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil" ) func TestNewConnector(t *testing.T) { @@ -19,7 +21,7 @@ func TestNewConnector(t *testing.T) { for _, tc := range []struct { name string dimensions []Dimension - wantDimensions []dimension + wantDimensions []pdatautil.Dimension }{ { name: "simplest config (use defaults)", @@ -30,9 +32,9 @@ func TestNewConnector(t *testing.T) { {Name: "http.method", Default: &defaultMethod}, {Name: "http.status_code"}, }, - wantDimensions: []dimension{ - {name: "http.method", value: &defaultMethodValue}, - {"http.status_code", nil}, + wantDimensions: []pdatautil.Dimension{ + {Name: "http.method", Value: &defaultMethodValue}, + {Name: "http.status_code", Value: nil}, }, }, } { diff --git a/connector/exceptionsconnector/go.mod b/connector/exceptionsconnector/go.mod index 062c2074da18..63c4e69f4306 100644 --- a/connector/exceptionsconnector/go.mod +++ b/connector/exceptionsconnector/go.mod @@ -4,6 +4,7 @@ go 1.22.0 require ( github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal v0.108.0 + github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil v0.108.0 github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden v0.108.0 github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest v0.108.0 github.com/stretchr/testify v1.9.0 @@ -67,8 +68,10 @@ require ( replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal => ../../internal/coreinternal -replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil => ../../pkg/pdatautil +replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil => ../../internal/pdatautil replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest => ../../pkg/pdatatest replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ../../pkg/golden + +replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil => ../../pkg/pdatautil diff --git a/connector/servicegraphconnector/connector.go b/connector/servicegraphconnector/connector.go index 587daa1e6e2e..bef5d42d00fc 100644 --- a/connector/servicegraphconnector/connector.go +++ b/connector/servicegraphconnector/connector.go @@ -24,6 +24,7 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/connector/servicegraphconnector/internal/metadata" "github.com/open-telemetry/opentelemetry-collector-contrib/connector/servicegraphconnector/internal/store" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil" ) const ( @@ -264,7 +265,7 @@ func (p *serviceGraphConnector) aggregateMetrics(ctx context.Context, td ptrace. // A database request will only have one span, we don't wait for the server // span but just copy details from the client span - if dbName, ok := findAttributeValue(p.config.DatabaseNameAttribute, rAttributes, span.Attributes()); ok { + if dbName, ok := pdatautil.GetAttributeValue(p.config.DatabaseNameAttribute, rAttributes, span.Attributes()); ok { e.ConnectionType = store.Database e.ServerService = dbName e.ServerLatencySec = spanDuration(span) @@ -312,7 +313,7 @@ func (p *serviceGraphConnector) aggregateMetrics(ctx context.Context, td ptrace. func (p *serviceGraphConnector) upsertDimensions(kind string, m map[string]string, resourceAttr pcommon.Map, spanAttr pcommon.Map) { for _, dim := range p.config.Dimensions { - if v, ok := findAttributeValue(dim, resourceAttr, spanAttr); ok { + if v, ok := pdatautil.GetAttributeValue(dim, resourceAttr, spanAttr); ok { m[kind+"_"+dim] = v } } @@ -320,7 +321,7 @@ func (p *serviceGraphConnector) upsertDimensions(kind string, m map[string]strin func (p *serviceGraphConnector) upsertPeerAttributes(m []string, peers map[string]string, spanAttr pcommon.Map) { for _, s := range m { - if v, ok := findAttributeValue(s, spanAttr); ok { + if v, ok := pdatautil.GetAttributeValue(s, spanAttr); ok { peers[s] = v break } diff --git a/connector/servicegraphconnector/go.mod b/connector/servicegraphconnector/go.mod index b8f8b9905e53..9b7f7e02cd1f 100644 --- a/connector/servicegraphconnector/go.mod +++ b/connector/servicegraphconnector/go.mod @@ -3,6 +3,7 @@ module github.com/open-telemetry/opentelemetry-collector-contrib/connector/servi go 1.22.0 require ( + github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil v0.108.0 github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden v0.108.0 github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest v0.108.0 github.com/stretchr/testify v1.9.0 @@ -123,4 +124,6 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest => ../../pkg/pdatatest +replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil => ../../internal/pdatautil + replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil => ../../pkg/pdatautil diff --git a/connector/servicegraphconnector/util.go b/connector/servicegraphconnector/util.go index fc447e9546de..25c0bea082aa 100644 --- a/connector/servicegraphconnector/util.go +++ b/connector/servicegraphconnector/util.go @@ -6,17 +6,10 @@ package servicegraphconnector // import "github.com/open-telemetry/opentelemetry import ( "go.opentelemetry.io/collector/pdata/pcommon" semconv "go.opentelemetry.io/collector/semconv/v1.9.0" -) -func findAttributeValue(key string, attributes ...pcommon.Map) (string, bool) { - for _, attr := range attributes { - if v, ok := attr.Get(key); ok { - return v.AsString(), true - } - } - return "", false -} + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil" +) func findServiceName(attributes pcommon.Map) (string, bool) { - return findAttributeValue(semconv.AttributeServiceName, attributes) + return pdatautil.GetAttributeValue(semconv.AttributeServiceName, attributes) } diff --git a/connector/spanmetricsconnector/connector.go b/connector/spanmetricsconnector/connector.go index c3f8c897aa9e..89ed6e15b5f5 100644 --- a/connector/spanmetricsconnector/connector.go +++ b/connector/spanmetricsconnector/connector.go @@ -23,6 +23,7 @@ import ( "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/cache" "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metrics" "github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal/traceutil" + utilattri "github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil" ) @@ -51,7 +52,7 @@ type connectorImp struct { metricsConsumer consumer.Metrics // Additional dimensions to add to metrics. - dimensions []dimension + dimensions []utilattri.Dimension resourceMetrics *cache.Cache[resourceKey, *resourceMetrics] @@ -71,7 +72,7 @@ type connectorImp struct { shutdownOnce sync.Once // Event dimensions to add to the events metric. - eDimensions []dimension + eDimensions []utilattri.Dimension events EventsConfig @@ -90,21 +91,16 @@ type resourceMetrics struct { lastSeen time.Time } -type dimension struct { - name string - value *pcommon.Value -} - -func newDimensions(cfgDims []Dimension) []dimension { +func newDimensions(cfgDims []Dimension) []utilattri.Dimension { if len(cfgDims) == 0 { return nil } - dims := make([]dimension, len(cfgDims)) + dims := make([]utilattri.Dimension, len(cfgDims)) for i := range cfgDims { - dims[i].name = cfgDims[i].Name + dims[i].Name = cfgDims[i].Name if cfgDims[i].Default != nil { val := pcommon.NewValueStr(*cfgDims[i].Default) - dims[i].value = &val + dims[i].Value = &val } } return dims @@ -511,7 +507,7 @@ func contains(elements []string, value string) bool { return false } -func (p *connectorImp) buildAttributes(serviceName string, span ptrace.Span, resourceAttrs pcommon.Map, dimensions []dimension) pcommon.Map { +func (p *connectorImp) buildAttributes(serviceName string, span ptrace.Span, resourceAttrs pcommon.Map, dimensions []utilattri.Dimension) pcommon.Map { attr := pcommon.NewMap() attr.EnsureCapacity(4 + len(dimensions)) if !contains(p.config.ExcludeDimensions, serviceNameKey) { @@ -527,8 +523,8 @@ func (p *connectorImp) buildAttributes(serviceName string, span ptrace.Span, res attr.PutStr(statusCodeKey, traceutil.StatusCodeStr(span.Status().Code())) } for _, d := range dimensions { - if v, ok := getDimensionValue(d, span.Attributes(), resourceAttrs); ok { - v.CopyTo(attr.PutEmpty(d.name)) + if v, ok := utilattri.GetDimensionValue(d, span.Attributes(), resourceAttrs); ok { + v.CopyTo(attr.PutEmpty(d.Name)) } } return attr @@ -546,7 +542,7 @@ func concatDimensionValue(dest *bytes.Buffer, value string, prefixSep bool) { // or resource/event attributes. If the dimension exists in both, the span's attributes, being the most specific, takes precedence. // // 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, resourceOrEventAttrs pcommon.Map) metrics.Key { +func (p *connectorImp) buildKey(serviceName string, span ptrace.Span, optionalDims []utilattri.Dimension, resourceOrEventAttrs pcommon.Map) metrics.Key { p.keyBuf.Reset() if !contains(p.config.ExcludeDimensions, serviceNameKey) { concatDimensionValue(p.keyBuf, serviceName, false) @@ -562,7 +558,7 @@ func (p *connectorImp) buildKey(serviceName string, span ptrace.Span, optionalDi } for _, d := range optionalDims { - if v, ok := getDimensionValue(d, span.Attributes(), resourceOrEventAttrs); ok { + if v, ok := utilattri.GetDimensionValue(d, span.Attributes(), resourceOrEventAttrs); ok { concatDimensionValue(p.keyBuf, v.AsString(), true) } } @@ -570,28 +566,6 @@ func (p *connectorImp) buildKey(serviceName string, span ptrace.Span, optionalDi return metrics.Key(p.keyBuf.String()) } -// getDimensionValue gets the dimension value for the given configured dimension. -// It searches through the span's attributes first, being the more specific; -// falling back to searching in resource attributes if it can't be found in the span. -// Finally, falls back to the configured default value if provided. -// -// The ok flag indicates if a dimension value was fetched in order to differentiate -// an empty string value from a state where no value was found. -func getDimensionValue(d dimension, spanAttr pcommon.Map, resourceAttr pcommon.Map) (v pcommon.Value, ok bool) { - // The more specific span attribute should take precedence. - if attr, exists := spanAttr.Get(d.name); exists { - return attr, true - } - if attr, exists := resourceAttr.Get(d.name); exists { - return attr, true - } - // Set the default if configured, otherwise this metric will have no value set for the dimension. - if d.value != nil { - return *d.value, true - } - return v, ok -} - // buildMetricName builds the namespace prefix for the metric name. func buildMetricName(namespace string, name string) string { if namespace != "" { diff --git a/connector/spanmetricsconnector/connector_test.go b/connector/spanmetricsconnector/connector_test.go index 2a2cf445455d..4d1789c898af 100644 --- a/connector/spanmetricsconnector/connector_test.go +++ b/connector/spanmetricsconnector/connector_test.go @@ -30,6 +30,7 @@ import ( "google.golang.org/grpc/metadata" "github.com/open-telemetry/opentelemetry-collector-contrib/connector/spanmetricsconnector/internal/metrics" + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil" ) const ( @@ -550,7 +551,7 @@ func TestBuildKeyWithDimensions(t *testing.T) { defaultFoo := pcommon.NewValueStr("bar") for _, tc := range []struct { name string - optionalDims []dimension + optionalDims []pdatautil.Dimension resourceAttrMap map[string]any spanAttrMap map[string]any wantKey string @@ -561,22 +562,22 @@ func TestBuildKeyWithDimensions(t *testing.T) { }, { name: "neither span nor resource contains key, dim provides default", - optionalDims: []dimension{ - {name: "foo", value: &defaultFoo}, + optionalDims: []pdatautil.Dimension{ + {Name: "foo", Value: &defaultFoo}, }, wantKey: "ab\u0000c\u0000SPAN_KIND_UNSPECIFIED\u0000STATUS_CODE_UNSET\u0000bar", }, { name: "neither span nor resource contains key, dim provides no default", - optionalDims: []dimension{ - {name: "foo"}, + optionalDims: []pdatautil.Dimension{ + {Name: "foo"}, }, wantKey: "ab\u0000c\u0000SPAN_KIND_UNSPECIFIED\u0000STATUS_CODE_UNSET", }, { name: "span attribute contains dimension", - optionalDims: []dimension{ - {name: "foo"}, + optionalDims: []pdatautil.Dimension{ + {Name: "foo"}, }, spanAttrMap: map[string]any{ "foo": 99, @@ -585,8 +586,8 @@ func TestBuildKeyWithDimensions(t *testing.T) { }, { name: "resource attribute contains dimension", - optionalDims: []dimension{ - {name: "foo"}, + optionalDims: []pdatautil.Dimension{ + {Name: "foo"}, }, resourceAttrMap: map[string]any{ "foo": 99, @@ -595,8 +596,8 @@ func TestBuildKeyWithDimensions(t *testing.T) { }, { name: "both span and resource attribute contains dimension, should prefer span attribute", - optionalDims: []dimension{ - {name: "foo"}, + optionalDims: []pdatautil.Dimension{ + {Name: "foo"}, }, spanAttrMap: map[string]any{ "foo": 100, diff --git a/connector/spanmetricsconnector/factory_test.go b/connector/spanmetricsconnector/factory_test.go index b57799760cbf..75aaf79d60c7 100644 --- a/connector/spanmetricsconnector/factory_test.go +++ b/connector/spanmetricsconnector/factory_test.go @@ -12,6 +12,8 @@ import ( "go.opentelemetry.io/collector/connector/connectortest" "go.opentelemetry.io/collector/consumer/consumertest" "go.opentelemetry.io/collector/pdata/pcommon" + + "github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil" ) func TestNewConnector(t *testing.T) { @@ -22,7 +24,7 @@ func TestNewConnector(t *testing.T) { durationHistogramBuckets []time.Duration dimensions []Dimension wantDurationHistogramBuckets []float64 - wantDimensions []dimension + wantDimensions []pdatautil.Dimension }{ { name: "simplest config (use defaults)", @@ -34,9 +36,9 @@ func TestNewConnector(t *testing.T) { {Name: "http.method", Default: &defaultMethod}, {Name: "http.status_code"}, }, - wantDimensions: []dimension{ - {name: "http.method", value: &defaultMethodValue}, - {"http.status_code", nil}, + wantDimensions: []pdatautil.Dimension{ + {Name: "http.method", Value: &defaultMethodValue}, + {Name: "http.status_code", Value: nil}, }, }, } { diff --git a/connector/spanmetricsconnector/go.mod b/connector/spanmetricsconnector/go.mod index 7075796c7ea7..03e281399329 100644 --- a/connector/spanmetricsconnector/go.mod +++ b/connector/spanmetricsconnector/go.mod @@ -8,6 +8,7 @@ require ( github.com/jonboulle/clockwork v0.4.0 github.com/lightstep/go-expohisto v1.0.0 github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal v0.108.0 + github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil v0.108.0 github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil v0.108.0 github.com/stretchr/testify v1.9.0 go.opentelemetry.io/collector/component v0.108.2-0.20240904075637-48b11ba1c5f8 @@ -81,3 +82,5 @@ retract ( replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest => ../../pkg/pdatatest replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ../../pkg/golden + +replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil => ../../internal/pdatautil diff --git a/connector/sumconnector/go.mod b/connector/sumconnector/go.mod index 7760a3bbae13..1223dc33d593 100644 --- a/connector/sumconnector/go.mod +++ b/connector/sumconnector/go.mod @@ -74,10 +74,6 @@ require ( gopkg.in/yaml.v3 v3.0.1 // indirect ) -replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil => ../../pkg/pdatautil - -replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest => ../../pkg/pdatatest - replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal => ../../internal/coreinternal replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/filter => ../../internal/filter @@ -85,3 +81,7 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/filte replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl => ../../pkg/ottl replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden => ../../pkg/golden + +replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil => ../../pkg/pdatautil + +replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatatest => ../../pkg/pdatatest diff --git a/exporter/elasticsearchexporter/integrationtest/go.mod b/exporter/elasticsearchexporter/integrationtest/go.mod index d4b1b17ab9cd..d72cf9fe176f 100644 --- a/exporter/elasticsearchexporter/integrationtest/go.mod +++ b/exporter/elasticsearchexporter/integrationtest/go.mod @@ -93,6 +93,7 @@ require ( github.com/open-telemetry/opentelemetry-collector-contrib/exporter/syslogexporter v0.108.0 // indirect github.com/open-telemetry/opentelemetry-collector-contrib/exporter/zipkinexporter v0.108.0 // indirect github.com/open-telemetry/opentelemetry-collector-contrib/internal/coreinternal v0.108.0 // indirect + github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil v0.108.0 // indirect github.com/open-telemetry/opentelemetry-collector-contrib/pkg/golden v0.108.0 // indirect github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl v0.108.0 // indirect github.com/open-telemetry/opentelemetry-collector-contrib/pkg/pdatautil v0.108.0 // indirect @@ -291,3 +292,5 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl => .. replace github.com/open-telemetry/opentelemetry-collector-contrib/connector/routingconnector => ../../../connector/routingconnector replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/exp/metrics => ../../../internal/exp/metrics + +replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil => ../../../internal/pdatautil diff --git a/internal/pdatautil/attributes.go b/internal/pdatautil/attributes.go new file mode 100644 index 000000000000..6963e93d8c6f --- /dev/null +++ b/internal/pdatautil/attributes.go @@ -0,0 +1,41 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package pdatautil // import "github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil" + +import "go.opentelemetry.io/collector/pdata/pcommon" + +type Dimension struct { + Name string + Value *pcommon.Value +} + +// GetDimensionValue gets the Dimension Value for the given configured Dimension. +// It iterates over multiple attributes until a value is found. +// The order comes first, the higher the priority. +// Finally, falls back to the configured default value if provided. +// +// The ok flag indicates if a Dimension Value was fetched in order to differentiate +// an empty string value from a state where no value was found. +func GetDimensionValue(d Dimension, attributes ...pcommon.Map) (v pcommon.Value, ok bool) { + for _, attrs := range attributes { + if attr, exists := attrs.Get(d.Name); exists { + return attr, true + } + } + // Set the default if configured, otherwise this metric will have no Value set for the Dimension. + if d.Value != nil { + return *d.Value, true + } + return v, ok +} + +// GetAttributeValue look up value from the given attributes for the specified key, and if not found, return empty string. +func GetAttributeValue(key string, attributes ...pcommon.Map) (string, bool) { + for _, attr := range attributes { + if v, ok := attr.Get(key); ok { + return v.AsString(), true + } + } + return "", false +} diff --git a/internal/pdatautil/attributes_test.go b/internal/pdatautil/attributes_test.go new file mode 100644 index 000000000000..7fea4472aa0e --- /dev/null +++ b/internal/pdatautil/attributes_test.go @@ -0,0 +1,63 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package pdatautil + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "go.opentelemetry.io/collector/pdata/pcommon" +) + +func TestGetDimensionValue(t *testing.T) { + resourceattris := pcommon.NewMap() + resourceattris.PutStr("service.name", "mock-service-name") + + spanattris := pcommon.NewMap() + spanattris.PutStr("span.name", "mock-span-name") + + otherattris := pcommon.NewMap() + otherattris.PutStr("a", "b") + otherattris.PutStr("foo", "bar") + + defaultFoo := pcommon.NewValueStr("bar") + + tests := []struct { + name string + dimension Dimension + attributes []pcommon.Map + wantDimensionVal string + }{ + { + name: "success get dimension value", + dimension: Dimension{Name: "foo"}, + attributes: []pcommon.Map{resourceattris, spanattris, otherattris}, + wantDimensionVal: "bar", + }, + { + name: "not found and get default dimension provided value", + dimension: Dimension{ + Name: "foo", + Value: &defaultFoo, + }, + attributes: []pcommon.Map{resourceattris, spanattris}, + wantDimensionVal: "bar", + }, + { + name: "not found and get default get empty value", + dimension: Dimension{ + Name: "foo", + }, + attributes: []pcommon.Map{resourceattris, spanattris}, + wantDimensionVal: "", + }, + } + + for _, tc := range tests { + val, ok := GetDimensionValue(tc.dimension, tc.attributes...) + if ok { + assert.Equal(t, tc.wantDimensionVal, val.AsString()) + } + } +} diff --git a/testbed/go.mod b/testbed/go.mod index 441dba35b211..998c9dcd6530 100644 --- a/testbed/go.mod +++ b/testbed/go.mod @@ -203,6 +203,7 @@ require ( github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f // indirect github.com/open-telemetry/opentelemetry-collector-contrib/extension/ackextension v0.108.0 // indirect github.com/open-telemetry/opentelemetry-collector-contrib/internal/exp/metrics v0.108.0 // indirect + github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil v0.108.0 // indirect github.com/open-telemetry/opentelemetry-collector-contrib/internal/sharedcomponent v0.108.0 // indirect github.com/open-telemetry/opentelemetry-collector-contrib/pkg/batchperresourceattr v0.108.0 // indirect github.com/open-telemetry/opentelemetry-collector-contrib/pkg/experimentalmetricmetadata v0.108.0 // indirect @@ -413,3 +414,5 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/extension/acke replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl => ../pkg/ottl replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/exp/metrics => ../internal/exp/metrics + +replace github.com/open-telemetry/opentelemetry-collector-contrib/internal/pdatautil => ../internal/pdatautil