Skip to content

Commit

Permalink
[connector/servicegraph] Extract the getDimensionValue function as …
Browse files Browse the repository at this point in the history
…a common function. (#34908)

**Description:** refactoring getDimensionValue func as util func for
spanmetricsconnector,servicegraphconnector,exceptionconnector common
usage
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

**Link to tracking Issue:** #34627

**Testing:** <Describe what testing was performed and which tests were
added.>

**Documentation:** <Describe the documentation added.>

---------

Signed-off-by: Jared Tan <jian.tan@daocloud.io>
  • Loading branch information
JaredTan95 authored Sep 6, 2024
1 parent abb7604 commit d5595bb
Show file tree
Hide file tree
Showing 19 changed files with 231 additions and 118 deletions.
27 changes: 27 additions & 0 deletions .chloggen/refactoring_dimenssion_func_as_util_func.yaml
Original file line number Diff line number Diff line change
@@ -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: []
27 changes: 12 additions & 15 deletions connector/exceptionsconnector/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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
Expand All @@ -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
}
18 changes: 6 additions & 12 deletions connector/exceptionsconnector/connector_logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 ""
}
11 changes: 6 additions & 5 deletions connector/exceptionsconnector/connector_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -29,7 +30,7 @@ type metricsConnector struct {
config Config

// Additional dimensions to add to metrics.
dimensions []dimension
dimensions []pdatautil.Dimension

keyBuf *bytes.Buffer

Expand Down Expand Up @@ -175,16 +176,16 @@ 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)
dims.PutStr(spanNameKey, span.Name())
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
Expand All @@ -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)
Expand Down
24 changes: 13 additions & 11 deletions connector/exceptionsconnector/connector_metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
10 changes: 6 additions & 4 deletions connector/exceptionsconnector/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)",
Expand All @@ -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},
},
},
} {
Expand Down
5 changes: 4 additions & 1 deletion connector/exceptionsconnector/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
7 changes: 4 additions & 3 deletions connector/servicegraphconnector/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -312,15 +313,15 @@ 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
}
}
}

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
}
Expand Down
3 changes: 3 additions & 0 deletions connector/servicegraphconnector/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
13 changes: 3 additions & 10 deletions connector/servicegraphconnector/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Loading

0 comments on commit d5595bb

Please sign in to comment.