open-telemetry / opentelemetry-rust

The Rust OpenTelemetry implementation
https://opentelemetry.io
Apache License 2.0
1.81k stars 418 forks source link

Add integration test for OTLP exporters #1327

Open TommyCpp opened 10 months ago

TommyCpp commented 10 months ago

In order to increase our confidence when releasing a new version. We should have integration tests for important exporters(OTLP, jager, etc).

Currently we have limited integration test for jaeger, and CI setup to run integration tests on PRs. But we want to expand/move it to OTLP exporters

hdost commented 10 months ago

Maybe consider using https://docs.rs/testcontainers/latest/testcontainers/ for starting the container. https://github.com/open-telemetry/opentelemetry-java/blob/main/integration-tests/otlp/src/main/java/io/opentelemetry/integrationtest/OtlpExporterIntegrationTest.java This is the example from the Java repo.

cijothomas commented 10 months ago

I also want to share my own experience that, leveraging In-MemoryExporter for intergration tests is another great way of improving confidence in the overall SDK! Yes, it is not true e2e, but does help a lot. Integration test for OTLP (as proposed in this issue), combined with in-memory exporter tests should be a good boost to coverage, and improve our velocity.

https://github.com/open-telemetry/opentelemetry-rust/pull/1305

TommyCpp commented 10 months ago

I stump upon https://github.com/davidB/tracing-opentelemetry-instrumentation-sdk/tree/main/fake-opentelemetry-collector while looking at other issues. Looks like someone already built a fake opentelemetry collector 🤣

hdost commented 7 months ago

Are there any other Tests we want to add? Maybe we can also add a daily/weekly build of the integration test?

TommyCpp commented 7 months ago

Maybe we can also add a daily/weekly build of the integration test?

yeah we could set something up but I want to extend the integration test coverage first

hdost commented 7 months ago

Maybe we can also add a daily/weekly build of the integration test?

yeah we could set something up but I want to extend the integration test coverage first

Which cases do you want to cover maybe we can split it up a bit.

hdost commented 7 months ago

Btw i put this for SDK Stable because it could reveal issues in the underlying SDK implementation.