-
Notifications
You must be signed in to change notification settings - Fork 506
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
Insert tracing event name into LogRecord::event_name instead of attributes #1928
Insert tracing event name into LogRecord::event_name instead of attributes #1928
Conversation
…entelemetry-rust into tracing-appender-name-metadata
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1928 +/- ##
=======================================
- Coverage 74.9% 74.9% -0.1%
=======================================
Files 122 122
Lines 20298 20308 +10
=======================================
- Hits 15221 15217 -4
- Misses 5077 5091 +14 ☔ View full report in Codecov by Sentry. |
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.
Please add a changelog entry, as this breaking behavior.
Are you planning to modify OTLPExporter to sent this as attribute (with same name as before), so this change will be transparent to the OTLPExporter users?
Added changelog. |
OTLP Should always add it to attributes. Why the condition "add if not already existing"? |
info!("message", name = "my_name")
info!(name: "event_name1", "message", name = "some_other_name") In above scenario, there would be entry for LogRecord::event_name and also with "name" key in attributes. Should we let the OTLP attribute vector have both of these fields? Then we don't need to iterate, just add. |
Thinking again, we can keep the existing behavior - to have two entries with "name" key in the attributes vector in these specific scenarios - which means we can just add without iterating to check. This could be added in this PR itself. |
yes please. (If the user does |
…entelemetry-rust into tracing-appender-name-metadata
…entelemetry-rust into tracing-appender-name-metadata
Make sense. Done, along with updating the changelog. Also triggered integration test to validate. |
Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
Changes
event_name
field was added inLogRecord
as part of #1488, however it was never populated with thename
metadata in tracing appender. Thename
was instead added to the attributes. Ensuring this is now populated properly.LogRecord::event_name
instead of iterating over the attributes.Merge requirement checklist
CHANGELOG.md
files updated for non-trivial, user-facing changes