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

Azure Event Hub receiver Semantic Convention translator #32486

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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
51 changes: 51 additions & 0 deletions pkg/translator/azure/property_names.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package azure

var mappings = map[string]map[string]string{
TylerHelmuth marked this conversation as resolved.
Show resolved Hide resolved
"common": {},
"FrontDoorAccessLog": {
"trackingReference": "",
ThatRendle marked this conversation as resolved.
Show resolved Hide resolved
"httpMethod": "http.request.method",
"httpVersion": "http.request.version",
ThatRendle marked this conversation as resolved.
Show resolved Hide resolved
"requestUri": "url.full",
"hostName": "server.address",
"requestBytes": "http.request.body.size",
"responseBytes": "http.response.body.size",
"userAgent": "user_agent.original",
"clientIp": "client.address",
"clientPort": "client.port",
"socketIp": "socket.address",
ThatRendle marked this conversation as resolved.
Show resolved Hide resolved
"timeTaken": "http.server.request.duration",
"requestProtocol": "",
"securityProtocol": "",
"securityCipher": "",
"securityCurves": "",
"endpoint": "",
"httpStatusCode": "http.response.status_code",
"pop": "",
"cacheStatus": "",
"matchedRulesSetName": "",
"routeName": "http.route",
"referrer": "http.request.header.referer",
"timeToFirstByte": "",
"errorInfo": "",
ThatRendle marked this conversation as resolved.
Show resolved Hide resolved
"originURL": "",
"originIP": "",
"originName": "",
"result": "",
"sni": "",
},
}

func ResourceLogKeyToSemConvKey(azName string, category string) (string, bool) {
TylerHelmuth marked this conversation as resolved.
Show resolved Hide resolved
if mapping := mappings[category]; mapping != nil {
TylerHelmuth marked this conversation as resolved.
Show resolved Hide resolved
if mapped := mapping[azName]; mapped != "" {
return mapped, true
}
}

if name := mappings["common"][azName]; name != "" {
return name, true
}

return "", false
}
20 changes: 19 additions & 1 deletion pkg/translator/azure/resourcelogs_to_logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,9 +192,11 @@ func extractRawAttributes(log azureLogRecord) map[string]any {
}
attrs[azureOperationName] = log.OperationName
setIf(attrs, azureOperationVersion, log.OperationVersion)

if log.Properties != nil {
attrs[azureProperties] = *log.Properties
copyProperties(log.Category, log.Properties, attrs)
Copy link
Member

@TylerHelmuth TylerHelmuth Apr 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about these changes some more, they end up being a breaking change regardless of whether or not the semconv have approved the attribute names. This change moves the data out of the azure.properties attribute and into new, semantically relevant, fields. The result is the user needs to good look for their data in a new place - a better place, but new nevertheless.

Here is my proposal for how to move these breaking changes forward to give users time to adjust and and time for the semconv to discuss the names. Lets add 2 new feature gates that will control how the data is produced.

  1. A feature gate that, when enabled, will translate the data in the old way and the new way.
  2. A feature gate that, when enabled, will translate the data in only the new way.

If both feature gates are enabled, the first gate will take precedence.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest that this is a new translator, so it's opt-in in the config for this work to come into effect.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new translator (or a new flow in this azure translator) works too. The eventhub receiver will need 2 new formats to help with the transition tho, 1 format that emits only the new data, and 1 format that emits the new data and the old data.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@atoulme @crobert-1 please weigh in as code owners on how you'd like to see these changes integrated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TylerHelmuth I believe that providing a way to only emit old or new is sufficient. I can see an argument for both; but I do agree this could be considered a breaking change.

}

setIf(attrs, azureResultDescription, log.ResultDescription)
setIf(attrs, azureResultSignature, log.ResultSignature)
setIf(attrs, azureResultType, log.ResultType)
Expand All @@ -207,6 +209,22 @@ func extractRawAttributes(log azureLogRecord) map[string]any {
return attrs
}

func copyProperties(category string, properties *any, attrs map[string]any) {
pmap := (*properties).(map[string]any)
var attrsProps map[string]any = nil
for k, v := range pmap {
if otelKey, found := ResourceLogKeyToSemConvKey(k, category); found {
attrs[otelKey] = v
} else {
if attrsProps == nil {
TylerHelmuth marked this conversation as resolved.
Show resolved Hide resolved
attrsProps = map[string]any{}
attrs["azure.properties"] = attrsProps
TylerHelmuth marked this conversation as resolved.
Show resolved Hide resolved
}
attrsProps[k] = v
}
}
}

func setIf(attrs map[string]any, key string, value *string) {
if value != nil && *value != "" {
attrs[key] = *value
Expand Down
Loading