-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[translator/azurelogs] Azure logs fix resource attributes #39571
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
base: main
Are you sure you want to change the base?
[translator/azurelogs] Azure logs fix resource attributes #39571
Conversation
…; remove redundant casting
@@ -114,18 +110,16 @@ func (r ResourceLogsUnmarshaler) UnmarshalLogs(buf []byte) (plog.Logs, error) { | |||
lr.SetSeverityText(log.Level.String()) | |||
} | |||
|
|||
lr.Attributes().PutStr(conventions.AttributeCloudResourceID, log.ResourceID) | |||
lr.Attributes().PutStr(conventions.AttributeCloudProvider, conventions.AttributeCloudProviderAzure) | |||
lr.Attributes().PutStr(conventions.AttributeEventName, eventNameValue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this dropped on purpose or moved somewhere else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was dropped on purpose. It's part of the resource attributes now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@constanca-m I think @atoulme is referring specifically to the AttributeEventName "az.resource.log" which has been removed. The previous values were moved to the attributes but that value was not and was also removed from all of the tests. I think the question is whether it had any value or not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @cparkins for clarification. I thought the az.resource.log
was redundant as we already have category
and every new attribute takes up memory. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@constanca-m sounds good to me.
Description
The issue for this PR was first reported here: #39186.
Currently, the resource logs have no attributes. This means that all records are supposedly the same, which is not true.
This PR adds attributes to the resources.
Link to tracking issue
Relates #39186.
Testing
Unit tests fixed.