feat(o11y): propagate LRO destination ID to child spans#5871
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces tracking and recording of the destination ID for long-running operations (LRO) within LroRecorder by adding a thread-safe destination_id field, implementing helper methods, updating the polling attributes macro, and adding corresponding tests. The review feedback recommends simplifying the implementation by replacing std::sync::RwLock with std::sync::Mutex for the destination_id field, which aligns with the repository's style guide to avoid unnecessary complexity.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5871 +/- ##
=======================================
Coverage 97.89% 97.89%
=======================================
Files 227 227
Lines 57995 57995
=======================================
Hits 56774 56774
Misses 1221 1221 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
haphungw
force-pushed
the
feat-lro-propagate-resource-id
branch
from
0eb02c1 to
c9a339a
Compare
There was a problem hiding this comment.
I'm not sure its as immutable anymore as it mutates the destination ID.
There was a problem hiding this comment.
fixed the comment.
As for the immutability, when I was planning this in the first place, I forgot that the destination ID is only available after the first response comes back. Only then can we propagate it to the clones of the recorder. So it can't be strictly immutable anymore, but the ID is only set once.
| pub struct LroRecorder { | ||
| span: Span, | ||
| attempt_count: Option<u32>, | ||
| destination_id: std::sync::Arc<std::sync::Mutex<Option<String>>>, |
There was a problem hiding this comment.
can this be a oncelock to simplify reads (I feel like we did this somewhere before?)
There was a problem hiding this comment.
you're right, fixed!
haphungw
force-pushed
the
feat-lro-propagate-resource-id
branch
from
c9a339a to
b40da83
Compare
2 participants
Introduce stateful propagation of the LRO destination ID (resource ID/operation name) across LRO polling attempts inside
google-cloud-lro.