-
Notifications
You must be signed in to change notification settings - Fork 144
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
Support for publishing resource attirbutes in fluent logger. #503
Support for publishing resource attirbutes in fluent logger. #503
Conversation
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.
I don’t think that’s the case. The change is covered by my CLA, as I am raising the PR. Just for a background - This exporter is maintained by @ThomsonTan and me, and it’s used internally within Microsoft. @bnavarma is one of the users of this within MS :) |
Ok then. |
@@ -99,6 +100,93 @@ void inline PopulateAttribute( | |||
} | |||
} | |||
|
|||
void inline PopulateOwnedAttribute( |
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.
nit: wrap this function PopulateOwnedAttribute
in the new experimental macro, or we'll get an unused function for build without enabling the experimental flag?
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.
Looks good to me with a suggestion.
This PR incorporate changes from #500, as @bnavarma is experiencing issues with CLA signing. The changes are relevant and are gated behind a feature flag."