test(o11y): add Discovery LRO tracing error integration tests#5892
test(o11y): add Discovery LRO tracing error integration tests#5892haphungw wants to merge 5 commits into
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 integration tests for Long-Running Operation (LRO) tracing under tests/o11y, covering both Showcase (GAPIC) and Discovery-based client pollers across success, logical error, transient error, and permanent error scenarios. Feedback on the changes includes removing redundant inner #[cfg(google_cloud_unstable_tracing)] attributes since the entire file is already gated with that configuration, and addressing a discrepancy where the LRO Wait span status remains UNSET instead of ERROR during logical errors in Discovery LROs.
| #[cfg(google_cloud_unstable_tracing)] | ||
| google_cloud_lro::record_polling_attributes!(&span); |
There was a problem hiding this comment.
The inner #[cfg(google_cloud_unstable_tracing)] attribute is redundant because the entire file is already gated with #![cfg(google_cloud_unstable_tracing)] at the top (line 15). Removing these inner attributes simplifies the code.
This also applies to lines 607, 706, 719, and 820.
| #[cfg(google_cloud_unstable_tracing)] | |
| google_cloud_lro::record_polling_attributes!(&span); | |
| google_cloud_lro::record_polling_attributes!(&span); |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5892 +/- ##
=======================================
Coverage 97.89% 97.90%
=======================================
Files 233 233
Lines 59202 59202
=======================================
+ Hits 57958 57961 +3
+ Misses 1244 1241 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
haphungw
force-pushed
the
fix-lro-tracing-discovery-errors
branch
2 times, most recently
from
bc2a064 to
89f0be9
Compare
| _ => None, | ||
| }; | ||
| if let Some((recorder, status)) = op_details { | ||
| recorder.span().record("otel.status_code", "ERROR"); |
There was a problem hiding this comment.
Do we have a constant anywhere for "ERROR" or "otel.status_code"?
|
|
||
| let query = move |name: String| async move { | ||
| let span = tracing::info_span!( | ||
| "client_request", |
There was a problem hiding this comment.
can we call the actual client_request macro here?
|
|
||
| let spans = TestLayer::capture(guard); | ||
|
|
||
| // 1. T2 span "LRO Wait" should be UNSET/None (because until_done returns Ok) |
There was a problem hiding this comment.
| // 1. T2 span "LRO Wait" should be UNSET/None (because until_done returns Ok) | |
| // 1. T2 span "LRO Wait" should be ERROR because the operation failed. |
Add error.type attribute to the LRO Wait span and populate it from the status code if the long-running operation fails with a status code. This allows observability tools to track error rates by error type for long-running operations.
haphungw
force-pushed
the
fix-lro-tracing-discovery-errors
branch
from
89f0be9 to
9981700
Compare
haphungw
force-pushed
the
fix-lro-tracing-discovery-errors
branch
from
9981700 to
0d587a2
Compare
2 participants
Add tracing integration tests for Discovery LRO error paths.
Stacked on top of #5891