test(o11y): add GAPIC LRO tracing logical error integration test#5886
test(o11y): add GAPIC LRO tracing logical error integration test#5886haphungw wants to merge 2 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 within the observability tests module. It adds a new lro_tracing module that verifies tracing spans and attributes for both successful LRO polling and logical LRO failure scenarios. The review feedback suggests enhancing the test assertions by verifying the specific destination ID (operations/wait-2) in the logical error test case for both the LRO Wait and GetOperation spans, and refactoring the test to reuse the find_get_operation_spans helper function to eliminate duplicate span filtering logic.
haphungw
force-pushed
the
fix-lro-tracing-gapic-logical-error
branch
2 times, most recently
from
828819b to
8b00020
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5886 +/- ##
=======================================
Coverage 97.90% 97.90%
=======================================
Files 233 233
Lines 59211 59211
=======================================
Hits 57968 57968
Misses 1243 1243 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
haphungw
force-pushed
the
fix-lro-tracing-gapic-logical-error
branch
4 times, most recently
from
c2728a9 to
eae050b
Compare
| attribute_str(lro_wait_span, "otel.status_code"), | ||
| Some("ERROR") | ||
| ); | ||
| let actual_desc = attribute_str(lro_wait_span, "otel.status_description").unwrap_or_default(); |
There was a problem hiding this comment.
can we add an assertion on error.type here?
There was a problem hiding this comment.
I did miss this in the implementation. The T3 span (GetOperation) already records this, but the T2 span (LRO) currently does not support error.type.
#5895 fixes this. Let's merge this first, and then I'll add the error.type assertions.
| attribute_i64(get_op_span, "gcp.longrunning.status_code"), | ||
| Some(3) | ||
| ); | ||
| assert_eq!( |
There was a problem hiding this comment.
...and here.
It should be human readable and low-cardinality.
haphungw
force-pushed
the
fix-lro-tracing-gapic-logical-error
branch
from
eae050b to
e455a66
Compare
haphungw
force-pushed
the
fix-lro-tracing-gapic-logical-error
branch
from
e455a66 to
1906ce2
Compare
2 participants
Stacked on top of #5885
Adds an integration test validating the tracing behavior of GAPIC LROs that complete with a logical error (i.e. status code 3 / INVALID_ARGUMENT inside the operation body).