Add E2E tests for AWS XRay Exporter with and without XRay ID Generator #1474
Conversation
|
Should rename job |
adot-testbed/app/src/test/java/software/amazon/adot/testbed/TraceIdsWithXRayTests.java
Outdated
Show resolved
Hide resolved
|
|
||
| @Testcontainers(disabledWithoutDocker = true) | ||
| @TestInstance(TestInstance.Lifecycle.PER_CLASS) | ||
| class TraceIdsWithXRayTests { |
There was a problem hiding this comment.
In the spirit of leaving things better than what you found, could you create a base class that has most of the logic for initializing the collector?
There was a problem hiding this comment.
Moved all container initialization to base class.
|
|
||
| @Test | ||
| void testXRayTraceIdSendToXRay() throws Exception { | ||
| List<String> traceIds = createTraces(true); |
There was a problem hiding this comment.
boolean flags for behaviour is a code smell. You could make the trace id generator a static property of this test class and then pass it as parameter to each method. It is more explicit than true and false.
adot-testbed/app/build.gradle.kts
Outdated
| @@ -27,6 +27,16 @@ dependencies { | |||
| testImplementation("com.github.rholder:guava-retrying:2.0.0") | |||
| testImplementation("org.assertj:assertj-core:3.24.2") | |||
| testImplementation("com.fasterxml.jackson.core:jackson-databind:2.13.0") | |||
|
|
|||
| // Trace ID (W3C & XRay) tests with XRay Exporter | |||
| api(platform("io.opentelemetry:opentelemetry-bom:1.30.0")) | |||
There was a problem hiding this comment.
if you use the alpha bom, you don't need to specify the version in any of the opentelemetry dependencies with gid io.opentelemetry
| } | ||
|
|
||
| // Takes a few seconds for traces to appear in XRay | ||
| Thread.sleep(20000); |
There was a problem hiding this comment.
I would prefer to implement a retry so that this code can run as fast as possible. Example: https://github.com/aws-observability/aws-otel-test-framework/blob/terraform/adot-testbed/app/src/test/java/software/amazon/adot/testbed/LogsTests.java#L249
jj22ee
force-pushed
the
trace-id-value-xray-exporter-tests
branch
from
dc5b4a8 to
e4f50aa
Compare
There was a problem hiding this comment.
This PR LGTM and is increasing the coverage of the testbed, which is good.
I would like to just request that this is not merged before ADOT collector v0.36.0 is released. Also I would like to request a follow up pr on the collector repo to change the name of this workflow and perform any other required changes. https://github.com/aws-observability/aws-otel-collector/blob/main/.github/workflows/CI.yml#L711
Description:
https://aws.amazon.com/about-aws/whats-new/2023/10/aws-x-ray-w3c-format-trace-ids-distributed-tracing/
This new feature support by X-Ray means that there should no more need for OTel SDKs to be configured with the XRay ID Generator when using the AWS XRay Exporter. This PR will add E2E tests to validate sending and retrieving traces that are:
Link to tracking Issue:
Testing:
Tested locally and in forked
aws-otel-collectorrepository with a modified workflow.Documentation:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.