Skip to content

Commit

Permalink
[exporter/azuremonitor] Fix: Correct Placement of Span Attributes in …
Browse files Browse the repository at this point in the history
…AzureMonitorExporter (open-telemetry#29684)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

This pull request addresses an issue in the Azure Monitor exporter where
span attributes with double and int values were incorrectly added to the
`measurements` field instead of the `properties` field in the
Application Insights schema.

**Changes**

- Modified the AzureMonitorExporter to ensure that span attributes of
type double and int are correctly placed in the properties field.
- Manual testing was conducted to verify that the span attributes appear
correctly in the properties field.

**Link to tracking Issue:** <Issue number if applicable> open-telemetry#29683 

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

- Updated relevant tests to reflect this change and ensure correctness.
  • Loading branch information
rajkumar-rangaraj authored and cparkins committed Jan 10, 2024
1 parent 9904906 commit 147f8ed
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 66 deletions.
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: breaking

# 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

0 comments on commit 147f8ed

Please sign in to comment.