Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Azure Event Hub receiver Semantic Convention translator #32486
Changes from 2 commits
134db01
0478006
298619a
610340b
87d1cdd
18afa9f
cf37abb
f2b6754
bc6b28b
be58d53
09446db
b25d353
fc758fc
77b4024
a02352f
65e762f
623574b
ce3e549
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
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.
If both feature gates are enabled, the first gate will take precedence.
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'd suggest that this is a new translator, so it's opt-in in the config for this work to come into effect.
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.
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.
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.
@atoulme @crobert-1 please weigh in as code owners on how you'd like to see these changes integrated.
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.
Also @djaglowski
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.
@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.