feat(appender-tracing): propagate span name to logs#3466
feat(appender-tracing): propagate span name to logs#3466SuperFluffy wants to merge 1 commit intoopen-telemetry:mainfrom
Conversation
changelog
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3466 +/- ##
=======================================
- Coverage 83.2% 83.2% -0.1%
=======================================
Files 128 128
Lines 25086 25144 +58
=======================================
+ Hits 20896 20938 +42
- Misses 4190 4206 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| } | ||
| } | ||
| if let Some(name) = current_span_name { | ||
| log_record.add_attribute(Key::new("span.name"), AnyValue::from(name)); |
There was a problem hiding this comment.
"span.name" - Its tricky! There is no established convention, so any name we pick can cause issues. Need to see if we can find any alternate solution.
Also, what happens when nesting of spans is there?
There was a problem hiding this comment.
Also, what happens when nesting of spans is there?
Related to your other comment for the test - this selects the inner-most span, so the span that immediately contains the event. I think that's the most natural span to select?
Alternatively we can follow tracing-subscriber's formatting subscriber which essentially contains a list of all spans.
"span.name"
I agree. None of this is great. Maybe just name?
There was a problem hiding this comment.
Given there is no way we can ensure conflict avoidance, could we avoid making that decision ourselves, and let user make that?
use_span_name("whatever name.user wants") ?
If user does not do the above, then we ignore the span-name. If the do it, then they gives us the attribute name.
WDYT?
| assert_eq!(logs.len(), 1); | ||
| let log = &logs[0]; | ||
|
|
||
| // The span.name should be the current (leaf/innermost) span |
There was a problem hiding this comment.
How do we determine what innermost is what user wants?
There was a problem hiding this comment.
Innermost is the most natural. Alternatively I can see collecting a list of all spans (probably not ideal).
There was a problem hiding this comment.
seems reasonable. Though I can see some users wanting ability to customize it. Probably we can start with this, and based on feedback, allow advanced capabilities.
There was a problem hiding this comment.
could you add this as a TODO
cijothomas
left a comment
cijothomas
left a comment
There was a problem hiding this comment.
Agree we need to provide a way to attach span name, but need to also avoid picking an attribute name ourselves. See
#3466 (comment)
2 participants
Changes
Propagates the tracing span name to OTEL logs.
Similar to the experimental span attributes, the span containing the event is valuable to identify where and what happened.
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial, user-facing changes