impl(o11y): add operation name support to Poller interface#5694
impl(o11y): add operation name support to Poller interface#5694haphungw wants to merge 1 commit into
Poller interface#5694Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces an operation_name method to the Poller trait and its various implementations to allow retrieving the name of a long-running operation. The reviewer suggests changing the return type from Option<String> to Option<&str> to avoid unnecessary allocations and clones, making the API more idiomatic and efficient. Additionally, feedback was provided to use as_deref() in implementations and to update test assertions to use string literals for better independence.
1b013bc to
201e963
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5694 +/- ##
==========================================
- Coverage 97.91% 97.88% -0.03%
==========================================
Files 226 226
Lines 55347 55390 +43
==========================================
+ Hits 54191 54219 +28
- Misses 1156 1171 +15 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| /// Returns the name of the operation, if known. | ||
| fn operation_name(&self) -> Option<&str>; |
There was a problem hiding this comment.
This is fine. It is a new public API, but not a big deal. But I just realized that backoff() is also public, and I wonder if we should have not made it so...
There was a problem hiding this comment.
I found an alternative to avoid touching the public API: #5695.
Instead of adding operation_name() to query the inner poller, Tracing<P>::poll now enters the T2 span scope before delegating the poll call. The T2 span is now active in the thread context, and the pollers can record the operation name directly onto it using tracing::Span::current().record().
If you think this works, I can go ahead and close this PR.
Allow tracing decorators to dynamically query LRO operation IDs from underlying poller for telemetry recording.
Because
PollerImplis encapsulated as a generic inside the telemetry decorator, we can't do direct field access (likeself.inner.operation) on the generic type. Declaring this standard method on thePollertrait supports LRO tracing attribute propagation without contaminating public structs.