Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[exporter/azuremonitor] Fix: Correct Placement of Span Attributes in AzureMonitorExporter #29684

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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: bug_fix
rajkumar-rangaraj marked this conversation as resolved.
Show resolved Hide resolved

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: azuremonitorexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Fixed an issue where span attributes with double and int values were incorrectly added to the `measurements` field in the Application Insights schema. These attributes are now correctly placed in the `properties` field.

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [29683]

# (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: [user]
72 changes: 23 additions & 49 deletions exporter/azuremonitorexporter/trace_to_envelope.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ func spanToRequestData(span ptrace.Span, incomingSpanType spanType) *contracts.R
data.Name = span.Name()
data.Duration = formatSpanDuration(span)
data.Properties = make(map[string]string)
data.Measurements = make(map[string]float64)
data.ResponseCode, data.Success = getDefaultFormattedSpanStatus(span.Status())

switch incomingSpanType {
Expand All @@ -202,7 +201,7 @@ func spanToRequestData(span ptrace.Span, incomingSpanType spanType) *contracts.R
case messagingSpanType:
fillRequestDataMessaging(span, data)
case unknownSpanType:
copyAttributesWithoutMapping(span.Attributes(), data.Properties, data.Measurements)
copyAttributesWithoutMapping(span.Attributes(), data.Properties)
}

return data
Expand All @@ -218,7 +217,6 @@ func spanToRemoteDependencyData(span ptrace.Span, incomingSpanType spanType) *co
data.ResultCode, data.Success = getDefaultFormattedSpanStatus(span.Status())
data.Duration = formatSpanDuration(span)
data.Properties = make(map[string]string)
data.Measurements = make(map[string]float64)

switch incomingSpanType {
case httpSpanType:
Expand All @@ -230,7 +228,7 @@ func spanToRemoteDependencyData(span ptrace.Span, incomingSpanType spanType) *co
case messagingSpanType:
fillRemoteDependencyDataMessaging(span, data)
case unknownSpanType:
copyAttributesWithoutMapping(span.Attributes(), data.Properties, data.Measurements)
copyAttributesWithoutMapping(span.Attributes(), data.Properties)
}

return data
Expand All @@ -240,9 +238,8 @@ func spanToRemoteDependencyData(span ptrace.Span, incomingSpanType spanType) *co
func spanEventToExceptionData(spanEvent ptrace.SpanEvent) *contracts.ExceptionData {
data := contracts.NewExceptionData()
data.Properties = make(map[string]string)
data.Measurements = make(map[string]float64)

attrs := copyAndExtractExceptionAttributes(spanEvent.Attributes(), data.Properties, data.Measurements)
attrs := copyAndExtractExceptionAttributes(spanEvent.Attributes(), data.Properties)

details := contracts.NewExceptionDetails()
details.TypeName = attrs.ExceptionType
Expand All @@ -262,7 +259,7 @@ func spanEventToMessageData(spanEvent ptrace.SpanEvent) *contracts.MessageData {
data.Message = spanEvent.Name()
data.Properties = make(map[string]string)

copyAttributesWithoutMapping(spanEvent.Attributes(), data.Properties, nil)
copyAttributesWithoutMapping(spanEvent.Attributes(), data.Properties)
return data
}

Expand All @@ -274,7 +271,7 @@ func getFormattedHTTPStatusValues(statusCode int64) (statusAsString string, succ
// Maps HTTP Server Span to AppInsights RequestData
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md#semantic-conventions-for-http-spans
func fillRequestDataHTTP(span ptrace.Span, data *contracts.RequestData) {
attrs := copyAndExtractHTTPAttributes(span.Attributes(), data.Properties, data.Measurements)
attrs := copyAndExtractHTTPAttributes(span.Attributes(), data.Properties)

if attrs.HTTPStatusCode != 0 {
data.ResponseCode, data.Success = getFormattedHTTPStatusValues(attrs.HTTPStatusCode)
Expand Down Expand Up @@ -361,7 +358,7 @@ func fillRequestDataHTTP(span ptrace.Span, data *contracts.RequestData) {
// Maps HTTP Client Span to AppInsights RemoteDependencyData
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md
func fillRemoteDependencyDataHTTP(span ptrace.Span, data *contracts.RemoteDependencyData) {
attrs := copyAndExtractHTTPAttributes(span.Attributes(), data.Properties, data.Measurements)
attrs := copyAndExtractHTTPAttributes(span.Attributes(), data.Properties)

data.Type = "HTTP"
if attrs.HTTPStatusCode != 0 {
Expand Down Expand Up @@ -449,7 +446,7 @@ func fillRemoteDependencyDataHTTP(span ptrace.Span, data *contracts.RemoteDepend
// Maps RPC Server Span to AppInsights RequestData
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/rpc.md
func fillRequestDataRPC(span ptrace.Span, data *contracts.RequestData) {
attrs := copyAndExtractRPCAttributes(span.Attributes(), data.Properties, data.Measurements)
attrs := copyAndExtractRPCAttributes(span.Attributes(), data.Properties)

data.ResponseCode = getRPCStatusCodeAsString(attrs)

Expand All @@ -475,7 +472,7 @@ func fillRequestDataRPC(span ptrace.Span, data *contracts.RequestData) {
// Maps RPC Client Span to AppInsights RemoteDependencyData
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/rpc.md
func fillRemoteDependencyDataRPC(span ptrace.Span, data *contracts.RemoteDependencyData) {
attrs := copyAndExtractRPCAttributes(span.Attributes(), data.Properties, data.Measurements)
attrs := copyAndExtractRPCAttributes(span.Attributes(), data.Properties)

data.ResultCode = getRPCStatusCodeAsString(attrs)

Expand All @@ -501,7 +498,7 @@ func getRPCStatusCodeAsString(rpcAttributes *RPCAttributes) (statusCodeAsString
// Maps Database Client Span to AppInsights RemoteDependencyData
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/database.md
func fillRemoteDependencyDataDatabase(span ptrace.Span, data *contracts.RemoteDependencyData) {
attrs := copyAndExtractDatabaseAttributes(span.Attributes(), data.Properties, data.Measurements)
attrs := copyAndExtractDatabaseAttributes(span.Attributes(), data.Properties)

data.Type = attrs.DBSystem

Expand All @@ -519,7 +516,7 @@ func fillRemoteDependencyDataDatabase(span ptrace.Span, data *contracts.RemoteDe
// Maps Messaging Consumer/Server Span to AppInsights RequestData
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/messaging.md
func fillRequestDataMessaging(span ptrace.Span, data *contracts.RequestData) {
attrs := copyAndExtractMessagingAttributes(span.Attributes(), data.Properties, data.Measurements)
attrs := copyAndExtractMessagingAttributes(span.Attributes(), data.Properties)

// TODO Understand how to map attributes to RequestData fields
if attrs.MessagingURL != "" {
Expand All @@ -534,7 +531,7 @@ func fillRequestDataMessaging(span ptrace.Span, data *contracts.RequestData) {
// Maps Messaging Producer/Client Span to AppInsights RemoteDependencyData
// https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/messaging.md
func fillRemoteDependencyDataMessaging(span ptrace.Span, data *contracts.RemoteDependencyData) {
attrs := copyAndExtractMessagingAttributes(span.Attributes(), data.Properties, data.Measurements)
attrs := copyAndExtractMessagingAttributes(span.Attributes(), data.Properties)

// TODO Understand how to map attributes to RemoteDependencyData fields
data.Data = attrs.MessagingURL
Expand All @@ -553,11 +550,10 @@ func fillRemoteDependencyDataMessaging(span ptrace.Span, data *contracts.RemoteD
func copyAndMapAttributes(
attributeMap pcommon.Map,
properties map[string]string,
measurements map[string]float64,
mappingFunc func(k string, v pcommon.Value)) {

attributeMap.Range(func(k string, v pcommon.Value) bool {
setAttributeValueAsPropertyOrMeasurement(k, v, properties, measurements)
setAttributeValueAsProperty(k, v, properties)
if mappingFunc != nil {
mappingFunc(k, v)
}
Expand All @@ -568,23 +564,20 @@ func copyAndMapAttributes(
// Copies all attributes to either properties or measurements without any kind of mapping to a known set of attributes
func copyAttributesWithoutMapping(
attributeMap pcommon.Map,
properties map[string]string,
measurements map[string]float64) {
properties map[string]string) {

copyAndMapAttributes(attributeMap, properties, measurements, nil)
copyAndMapAttributes(attributeMap, properties, nil)
}

// Attribute extraction logic for HTTP Span attributes
func copyAndExtractHTTPAttributes(
attributeMap pcommon.Map,
properties map[string]string,
measurements map[string]float64) *HTTPAttributes {
properties map[string]string) *HTTPAttributes {

attrs := &HTTPAttributes{}
copyAndMapAttributes(
attributeMap,
properties,
measurements,
func(k string, v pcommon.Value) { attrs.MapAttribute(k, v) })

return attrs
Expand All @@ -593,14 +586,12 @@ func copyAndExtractHTTPAttributes(
// Attribute extraction logic for RPC Span attributes
func copyAndExtractRPCAttributes(
attributeMap pcommon.Map,
properties map[string]string,
measurements map[string]float64) *RPCAttributes {
properties map[string]string) *RPCAttributes {

attrs := &RPCAttributes{}
copyAndMapAttributes(
attributeMap,
properties,
measurements,
func(k string, v pcommon.Value) { attrs.MapAttribute(k, v) })

return attrs
Expand All @@ -609,14 +600,12 @@ func copyAndExtractRPCAttributes(
// Attribute extraction logic for Database Span attributes
func copyAndExtractDatabaseAttributes(
attributeMap pcommon.Map,
properties map[string]string,
measurements map[string]float64) *DatabaseAttributes {
properties map[string]string) *DatabaseAttributes {

attrs := &DatabaseAttributes{}
copyAndMapAttributes(
attributeMap,
properties,
measurements,
func(k string, v pcommon.Value) { attrs.MapAttribute(k, v) })

return attrs
Expand All @@ -625,14 +614,12 @@ func copyAndExtractDatabaseAttributes(
// Attribute extraction logic for Messaging Span attributes
func copyAndExtractMessagingAttributes(
attributeMap pcommon.Map,
properties map[string]string,
measurements map[string]float64) *MessagingAttributes {
properties map[string]string) *MessagingAttributes {

attrs := &MessagingAttributes{}
copyAndMapAttributes(
attributeMap,
properties,
measurements,
func(k string, v pcommon.Value) { attrs.MapAttribute(k, v) })

return attrs
Expand All @@ -641,14 +628,12 @@ func copyAndExtractMessagingAttributes(
// Attribute extraction logic for Span event exception attributes
func copyAndExtractExceptionAttributes(
attributeMap pcommon.Map,
properties map[string]string,
measurements map[string]float64) *ExceptionAttributes {
properties map[string]string) *ExceptionAttributes {

attrs := &ExceptionAttributes{}
copyAndMapAttributes(
attributeMap,
properties,
measurements,
func(k string, v pcommon.Value) { attrs.MapAttribute(k, v) })

return attrs
Expand Down Expand Up @@ -715,13 +700,10 @@ func writeFormattedPeerAddressFromNetworkAttributes(networkAttributes *NetworkAt
}
}

// Sets the attribute value as a property or measurement.
// Int and floats go to measurements if measurements is not nil, otherwise everything goes to properties.
func setAttributeValueAsPropertyOrMeasurement(
func setAttributeValueAsProperty(
key string,
attributeValue pcommon.Value,
properties map[string]string,
measurements map[string]float64) {
properties map[string]string) {

switch attributeValue.Type() {
case pcommon.ValueTypeBool:
Expand All @@ -731,18 +713,10 @@ func setAttributeValueAsPropertyOrMeasurement(
properties[key] = attributeValue.Str()

case pcommon.ValueTypeInt:
if measurements == nil {
properties[key] = strconv.FormatInt(attributeValue.Int(), 10)
} else {
measurements[key] = float64(attributeValue.Int())
}
properties[key] = strconv.FormatInt(attributeValue.Int(), 10)

case pcommon.ValueTypeDouble:
if measurements == nil {
properties[key] = strconv.FormatFloat(attributeValue.Double(), 'f', -1, 64)
} else {
measurements[key] = attributeValue.Double()
}
properties[key] = strconv.FormatFloat(attributeValue.Double(), 'f', -1, 64)
}
}

Expand Down
28 changes: 11 additions & 17 deletions exporter/azuremonitorexporter/trace_to_envelope_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ func commonRequestDataValidations(
span ptrace.Span,
data *contracts.RequestData) {

assertAttributesCopiedToPropertiesOrMeasurements(t, span.Attributes(), data.Properties, data.Measurements)
assertAttributesCopiedToProperties(t, span.Attributes(), data.Properties)
assert.Equal(t, defaultSpanIDAsHex, data.Id)
assert.Equal(t, defaultSpanDuration, data.Duration)

Expand All @@ -616,7 +616,7 @@ func commonRemoteDependencyDataValidations(
span ptrace.Span,
data *contracts.RemoteDependencyData) {

assertAttributesCopiedToPropertiesOrMeasurements(t, span.Attributes(), data.Properties, data.Measurements)
assertAttributesCopiedToProperties(t, span.Attributes(), data.Properties)
assert.Equal(t, defaultSpanIDAsHex, data.Id)
assert.Equal(t, defaultSpanDuration, data.Duration)
}
Expand Down Expand Up @@ -711,35 +711,29 @@ func defaultInternalRemoteDependencyDataValidations(
span ptrace.Span,
data *contracts.RemoteDependencyData) {

assertAttributesCopiedToPropertiesOrMeasurements(t, span.Attributes(), data.Properties, data.Measurements)
assertAttributesCopiedToProperties(t, span.Attributes(), data.Properties)
assert.Equal(t, "InProc", data.Type)
}

// Verifies that all attributes are copies to either the properties or measurements maps of the envelope's data element
func assertAttributesCopiedToPropertiesOrMeasurements(
// Verifies that all attributes are copies to either the properties maps of the envelope's data element
func assertAttributesCopiedToProperties(
t *testing.T,
attributeMap pcommon.Map,
properties map[string]string,
measurements map[string]float64) {
properties map[string]string) {

attributeMap.Range(func(k string, v pcommon.Value) bool {
p, exists := properties[k]
assert.True(t, exists)

switch v.Type() {
case pcommon.ValueTypeStr:
p, exists := properties[k]
assert.True(t, exists)
assert.Equal(t, v.Str(), p)
case pcommon.ValueTypeBool:
p, exists := properties[k]
assert.True(t, exists)
assert.Equal(t, strconv.FormatBool(v.Bool()), p)
case pcommon.ValueTypeInt:
m, exists := measurements[k]
assert.True(t, exists)
assert.Equal(t, float64(v.Int()), m)
assert.Equal(t, strconv.FormatInt(v.Int(), 10), p)
case pcommon.ValueTypeDouble:
m, exists := measurements[k]
assert.True(t, exists)
assert.Equal(t, v.Double(), m)
assert.Equal(t, strconv.FormatFloat(v.Double(), 'f', -1, 64), p)
}
return true
})
Expand Down