test(otlp-common): expand test coverage for OTLP common exporter#2075
test(otlp-common): expand test coverage for OTLP common exporter#2075arjun-rajappa wants to merge 4 commits into
Conversation
Signed-off-by: Arjun Rajappa <arjun.rajappa@ibm.com>
arjun-rajappa
requested review from
ahayworth,
arielvalentin,
dazuma,
ericmustin,
fbogsany,
kaylareopelle,
mwear,
plantfansam,
robbkidd and
robertlaurin
as code owners
|
👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the |
|
Hi @arjun-rajappa, thanks for your patience with my review. Could you take a look at the failing tests? |
|
@kaylareopelle i have split out the new test cases to a seperate PR #2159 to enable this to be more focussed. @arjun-rajappa it would be great if you could avoid mixing in format changes as it just ends up making it more time consuming to review. |
I believe these failures are addressed already in #2074. As these are related to otlp-http. Once the other PR is merged in these should start passing. |
|
I don't think that is the case as our main is passing so a change here is resulting in the failure. Note common is a dependency of http hence why it is possible a change to common could lead to failures in http. |
|
These changes are closely related, as otlp-http relies on methods defined in otlp-common. As a result, changes to otlp-common can impact the tests in otlp-http. If needed, I can add the changes here or rebase this branch entirely on top of #2074. |
Any approach is fine here, as long as it's clear what was brought in and whether we need to adhere to a specific merge order. We can also wait to review this one until #2074 is merged too if that's easier. |
|
Hi @arjun-rajappa, could you merge |
|
Hi @arjun-rajappa, it looks like there are some failures after merging |
Add SPAN_FLAGS_CONTEXT_HAS_IS_REMOTE_MASK flag with sampled bit (1) to all span assertions in trace_exporter_test.rb to align test expectations with actual span flag behavior. - Add flags field to 6 span test assertions - Set flags to SPAN_FLAGS_CONTEXT_HAS_IS_REMOTE_MASK | 1 (sampled) - Ensures test expectations match the span flags set during export Signed-off-by: Arjun Rajappa <arjun.rajappa@ibm.com>
arjun-rajappa
force-pushed
the
add-test-to-otlp-common
branch
from
04514cc to
fb63205
Compare
|
Hi @kaylareopelle , |
|
There is discussion in #2159 about the test cases, with that pr being purely the new tests added here. @arjun-rajappa based on feedback of the tests, it would make things easier if this pr focussed on just the new feature and the corresponding tests of that functionality with general tests added seperately |
3 participants
Closes #1667
Expands test coverage for the OTLP common exporter and adds span flags support per OTLP specification.
Changes
Added build_span_flags method to set remote parent context flags on spans and links
Comprehensive test coverage for all private methods (as_otlp_span, as_otlp_status_code, as_otlp_span_kind, as_otlp_key_value, as_otlp_any_value)
Integration tests for complex spans with multiple resources and scopes
Enhanced error handling tests for encoding issues
Testing
All tests pass. Coverage expanded from basic functionality to comprehensive edge cases.